From f898f3aab0b874b81d509ddedac455b0dff3cf94 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Sat, 4 Oct 2025 20:31:36 +0200 Subject: [PATCH] Fix tests --- .../svg/icons/lucide/outline/chevron-left.svg | 1 + .../icons/lucide/outline/chevron-right.svg | 1 + .../family_invitations_controller.rb | 20 +++++++++------- app/services/families/leave.rb | 24 ++++++++++++++----- app/views/families/edit.html.erb | 2 +- app/views/families/show.html.erb | 5 ++-- app/views/map/index.html.erb | 4 ++-- spec/requests/users_spec.rb | 12 ++++++---- spec/services/families/leave_spec.rb | 24 ++++++++++++++----- 9 files changed, 61 insertions(+), 32 deletions(-) create mode 100644 app/assets/svg/icons/lucide/outline/chevron-left.svg create mode 100644 app/assets/svg/icons/lucide/outline/chevron-right.svg diff --git a/app/assets/svg/icons/lucide/outline/chevron-left.svg b/app/assets/svg/icons/lucide/outline/chevron-left.svg new file mode 100644 index 00000000..47bdb982 --- /dev/null +++ b/app/assets/svg/icons/lucide/outline/chevron-left.svg @@ -0,0 +1 @@ + diff --git a/app/assets/svg/icons/lucide/outline/chevron-right.svg b/app/assets/svg/icons/lucide/outline/chevron-right.svg new file mode 100644 index 00000000..4c0ff5ee --- /dev/null +++ b/app/assets/svg/icons/lucide/outline/chevron-right.svg @@ -0,0 +1 @@ + diff --git a/app/controllers/family_invitations_controller.rb b/app/controllers/family_invitations_controller.rb index 80634dc4..65193987 100644 --- a/app/controllers/family_invitations_controller.rb +++ b/app/controllers/family_invitations_controller.rb @@ -78,17 +78,19 @@ class FamilyInvitationsController < ApplicationController def destroy authorize @family, :manage_invitations? - if @invitation.update(status: :cancelled) + begin + if @invitation.update(status: :cancelled) + redirect_to family_path(@family), + notice: 'Invitation cancelled' + else + redirect_to family_path(@family), + alert: 'Failed to cancel invitation. Please try again' + end + rescue StandardError => e + Rails.logger.error "Error cancelling family invitation: #{e.message}" redirect_to family_path(@family), - notice: 'Invitation cancelled' - else - redirect_to family_path(@family), - alert: 'Failed to cancel invitation. Please try again' + alert: 'An unexpected error occurred while cancelling the invitation' end - rescue StandardError => e - Rails.logger.error "Error cancelling family invitation: #{e.message}" - redirect_to family_path(@family), - alert: 'An unexpected error occurred while cancelling the invitation' end private diff --git a/app/services/families/leave.rb b/app/services/families/leave.rb index 59b53eda..7adcb8c6 100644 --- a/app/services/families/leave.rb +++ b/app/services/families/leave.rb @@ -12,10 +12,14 @@ module Families def call return false unless validate_can_leave + # Store family info before removing membership + @family_name = user.family.name + @family_owner = user.family.owner + ActiveRecord::Base.transaction do handle_ownership_transfer if user.family_owner? remove_membership - send_notification + send_notifications end true @@ -68,18 +72,26 @@ module Families user.family_membership.destroy! end - def send_notification + def send_notifications return unless defined?(Notification) + # Notify the user who left Notification.create!( user: user, kind: :info, title: 'Left Family', - content: "You've left the family" + content: "You've left the family \"#{@family_name}\"" + ) + + # Notify the family owner + return unless @family_owner&.persisted? + + Notification.create!( + user: @family_owner, + kind: :info, + title: 'Family Member Left', + content: "#{user.email} has left the family \"#{@family_name}\"" ) - rescue StandardError => e - # Don't fail the entire operation if notification fails - Rails.logger.warn "Failed to send family leave notification: #{e.message}" end def handle_record_invalid_error(error) diff --git a/app/views/families/edit.html.erb b/app/views/families/edit.html.erb index dbd28303..e9f6e4ff 100644 --- a/app/views/families/edit.html.erb +++ b/app/views/families/edit.html.erb @@ -88,7 +88,7 @@ method: :delete, data: { confirm: 'Are you sure you want to delete this family? This action cannot be undone.', turbo_confirm: 'Are you sure you want to delete this family? This action cannot be undone.' }, class: "btn btn-outline btn-error" do %> - <%= icon 'trash-2', class: "inline-block w-4 h-4 mr-2 -mt-1" %> + <%= icon 'trash-2', class: "inline-block w-4" %> Delete Family <% end %> <% end %> diff --git a/app/views/families/show.html.erb b/app/views/families/show.html.erb index 408085d5..a47c1e9a 100644 --- a/app/views/families/show.html.erb +++ b/app/views/families/show.html.erb @@ -17,8 +17,7 @@ <% if policy(@family).update? %> <%= link_to edit_family_path(@family), class: "btn btn-outline btn-info" do %> - <%= icon 'square-pen', class: "inline-block w-4 h-4 mr-2 -mt-1" %> - <%= t('families.show.edit', default: 'Edit') %> + <%= icon 'square-pen', class: "inline-block w-4" %><%= t('families.show.edit', default: 'Edit') %> <% end %> <% end %> @@ -36,7 +35,7 @@ method: :delete, data: { confirm: 'Are you sure you want to delete this family? This action cannot be undone.', turbo_confirm: 'Are you sure you want to delete this family? This action cannot be undone.' }, class: "btn btn-outline btn-error" do %> - <%= icon 'trash-2', class: "inline-block w-4 h-4 mr-2 -mt-1" %> + <%= icon 'trash-2', class: "inline-block w-4" %> Delete <% end %> <% end %> diff --git a/app/views/map/index.html.erb b/app/views/map/index.html.erb index b28d5b95..3f7e3046 100644 --- a/app/views/map/index.html.erb +++ b/app/views/map/index.html.erb @@ -9,7 +9,7 @@
<%= link_to map_path(start_at: @start_at - 1.day, end_at: @end_at - 1.day, import_id: params[:import_id]), class: "btn border border-base-300 hover:btn-ghost w-full" do %> - ◀️ + <%= icon 'chevron-left' %> <% end %>
@@ -30,7 +30,7 @@
<%= link_to map_path(start_at: @start_at + 1.day, end_at: @end_at + 1.day, import_id: params[:import_id]), class: "btn border border-base-300 hover:btn-ghost w-full" do %> - ▶️ + <%= icon 'chevron-right' %> <% end %>
diff --git a/spec/requests/users_spec.rb b/spec/requests/users_spec.rb index 8c0bcdf5..219c4d4d 100644 --- a/spec/requests/users_spec.rb +++ b/spec/requests/users_spec.rb @@ -11,19 +11,21 @@ RSpec.describe 'Users', type: :request do describe 'GET /users/sign_up' do context 'when self-hosted' do before do - stub_const('SELF_HOSTED', true) + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with('SELF_HOSTED').and_return('true') end - it 'returns http success' do + it 'redirects to root path' do get '/users/sign_up' - expect(response).to have_http_status(:not_found) + expect(response).to redirect_to(root_path) + expect(flash[:alert]).to include('Registration is not available') end end context 'when not self-hosted' do before do - stub_const('SELF_HOSTED', false) - Rails.application.reload_routes! + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:[]).with('SELF_HOSTED').and_return(nil) end it 'returns http success' do diff --git a/spec/services/families/leave_spec.rb b/spec/services/families/leave_spec.rb index fe016bfb..49344785 100644 --- a/spec/services/families/leave_spec.rb +++ b/spec/services/families/leave_spec.rb @@ -10,20 +10,32 @@ RSpec.describe Families::Leave do describe '#call' do context 'when user is a member (not owner)' do let(:member) { create(:user) } - let!(:membership) { create(:family_membership, user: member, family: family, role: :member) } + let!(:owner_membership) { create(:family_membership, user: user, family: family, role: :owner) } + let!(:member_membership) { create(:family_membership, user: member, family: family, role: :member) } let(:service) { described_class.new(user: member) } it 'removes the membership' do - expect { service.call }.to change(FamilyMembership, :count).by(-1) + result = service.call + expect(result).to be_truthy, "Expected service to succeed but got error: #{service.error_message}" + expect(FamilyMembership.count).to eq(1) # Only owner should remain expect(member.reload.family_membership).to be_nil end - it 'sends notification' do - expect { service.call }.to change(Notification, :count).by(1) + it 'sends notification to member who left' do + expect { service.call }.to change(Notification, :count).by(2) - notification = member.notifications.last + member_notification = member.notifications.last + expect(member_notification.title).to eq('Left Family') + expect(member_notification.content).to include(family.name) + end - expect(notification.title).to eq('Left Family') + it 'sends notification to family owner' do + service.call + + owner_notification = user.notifications.last + expect(owner_notification.title).to eq('Family Member Left') + expect(owner_notification.content).to include(member.email) + expect(owner_notification.content).to include(family.name) end it 'returns true' do