From f817e3513c888e844847ac2862ed8c2258c4586f Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Sat, 27 Sep 2025 14:26:08 +0200 Subject: [PATCH] Fix some tests --- .../family_invitations_controller.rb | 1 + app/services/families/invite.rb | 32 ++++++++--- app/views/family_invitations/index.html.erb | 57 +++++++++++++++++++ spec/requests/family_invitations_spec.rb | 53 ++++++++--------- 4 files changed, 110 insertions(+), 33 deletions(-) create mode 100644 app/views/family_invitations/index.html.erb diff --git a/app/controllers/family_invitations_controller.rb b/app/controllers/family_invitations_controller.rb index 4e663d2b..b6d5a716 100644 --- a/app/controllers/family_invitations_controller.rb +++ b/app/controllers/family_invitations_controller.rb @@ -49,6 +49,7 @@ class FamilyInvitationsController < ApplicationController ) if service.call + current_user.reload redirect_to family_path(current_user.family), notice: 'Welcome to the family!' else redirect_to root_path, alert: service.error_message || 'Unable to accept invitation' diff --git a/app/services/families/invite.rb b/app/services/families/invite.rb index c5926d84..b0634a36 100644 --- a/app/services/families/invite.rb +++ b/app/services/families/invite.rb @@ -4,7 +4,7 @@ module Families class Invite include ActiveModel::Validations - attr_reader :family, :email, :invited_by, :invitation + attr_reader :family, :email, :invited_by, :invitation, :errors validates :email, presence: true, format: { with: URI::MailTo::EMAIL_REGEXP } @@ -12,6 +12,7 @@ module Families @family = family @email = email.downcase.strip @invited_by = invited_by + @errors = {} end def call @@ -25,21 +26,38 @@ module Families end true - rescue ActiveRecord::RecordInvalid + rescue ActiveRecord::RecordInvalid => e + @errors[:base] = e.message false end + def error_message + return errors.values.first if errors.any? + return validation_error_message unless valid? + + 'Failed to send invitation' + end + private + def validation_error_message + errors.full_messages.first || 'Invalid invitation data' + end + def invite_sendable? - return false unless invited_by.family_owner? - return false if family.members.count >= Family::MAX_MEMBERS - return false if user_already_in_family? - return false if pending_invitation_exists? + return add_error_and_false(:invited_by, 'You must be a family owner to send invitations') unless invited_by.family_owner? + return add_error_and_false(:family, 'Family is full') if family.members.count >= Family::MAX_MEMBERS + return add_error_and_false(:email, 'User is already in a family') if user_already_in_family? + return add_error_and_false(:email, 'Invitation already sent to this email') if pending_invitation_exists? true end + def add_error_and_false(attribute, message) + @errors[attribute] = message + false + end + def user_already_in_family? User.joins(:family_membership) .where(email: email) @@ -71,4 +89,4 @@ module Families ) end end -end +end \ No newline at end of file diff --git a/app/views/family_invitations/index.html.erb b/app/views/family_invitations/index.html.erb new file mode 100644 index 00000000..1c7082af --- /dev/null +++ b/app/views/family_invitations/index.html.erb @@ -0,0 +1,57 @@ +
+
+
+
+

+ <%= t('family_invitations.index.title', default: 'Family Invitations') %> +

+ <%= link_to family_path(@family), + class: "bg-gray-600 hover:bg-gray-700 text-white px-4 py-2 rounded-md font-medium transition-colors duration-200" do %> + <%= t('family_invitations.index.back_to_family', default: 'Back to Family') %> + <% end %> +
+ + <% if @pending_invitations.any? %> +
+ <% @pending_invitations.each do |invitation| %> +
+
+
<%= invitation.email %>
+
+ <%= t('family_invitations.index.invited_on', default: 'Invited') %> + <%= invitation.created_at.strftime('%B %d, %Y') %> +
+
+ <%= t('family_invitations.index.expires_on', default: 'Expires') %> + <%= invitation.expires_at.strftime('%B %d, %Y at %I:%M %p') %> +
+
+ +
+ <%= link_to public_invitation_path(invitation.token), + class: "text-blue-600 hover:text-blue-800 text-sm font-medium" do %> + <%= t('family_invitations.index.view_invitation', default: 'View') %> + <% end %> + + <% if policy(@family).manage_invitations? %> + <%= link_to family_invitation_path(@family, invitation), + method: :delete, + confirm: t('family_invitations.index.cancel_confirm', default: 'Are you sure you want to cancel this invitation?'), + class: "text-red-600 hover:text-red-800 text-sm font-medium" do %> + <%= t('family_invitations.index.cancel', default: 'Cancel') %> + <% end %> + <% end %> +
+
+ <% end %> +
+ <% else %> +
+

+ <%= t('family_invitations.index.no_invitations', default: 'No pending invitations') %> +

+
+ <% end %> +
+
+
\ No newline at end of file diff --git a/spec/requests/family_invitations_spec.rb b/spec/requests/family_invitations_spec.rb index 217800a2..3712129c 100644 --- a/spec/requests/family_invitations_spec.rb +++ b/spec/requests/family_invitations_spec.rb @@ -120,18 +120,19 @@ RSpec.describe 'Family Invitations', type: :request do post "/families/#{family.id}/invitations", params: duplicate_params expect(response).to redirect_to(family_path(family)) follow_redirect! - expect(response.body).to include('Failed to send invitation') + expect(response.body).to include('Invitation already sent to this email') end end context 'when user is not the owner' do before { membership.update!(role: :member) } - it 'returns forbidden' do + it 'redirects due to authorization failure' do post "/families/#{family.id}/invitations", params: { family_invitation: { email: 'test@example.com' } } - expect(response).to have_http_status(:forbidden) + expect(response).to have_http_status(:see_other) + expect(flash[:alert]).to include('not authorized') end end @@ -162,27 +163,28 @@ RSpec.describe 'Family Invitations', type: :request do describe 'POST /families/:family_id/invitations/:id/accept' do let(:invitee) { create(:user) } + let(:invitee_invitation) { create(:family_invitation, family: family, invited_by: user, email: invitee.email) } context 'with valid invitation and user' do before { sign_in invitee } it 'accepts the invitation' do expect do - post "/families/#{family.id}/invitations/#{invitation.id}/accept" + post "/families/#{family.id}/invitations/#{invitee_invitation.token}/accept" end.to change { invitee.reload.family }.from(nil).to(family) end it 'redirects with success message' do - post "/families/#{family.id}/invitations/#{invitation.id}/accept" + post "/families/#{family.id}/invitations/#{invitee_invitation.token}/accept" expect(response).to redirect_to(family_path(family)) follow_redirect! expect(response.body).to include('Welcome to the family!') end it 'marks invitation as accepted' do - post "/families/#{family.id}/invitations/#{invitation.id}/accept" - invitation.reload - expect(invitation.status).to eq('accepted') + post "/families/#{family.id}/invitations/#{invitee_invitation.token}/accept" + invitee_invitation.reload + expect(invitee_invitation.status).to eq('accepted') end end @@ -196,41 +198,39 @@ RSpec.describe 'Family Invitations', type: :request do it 'does not accept the invitation' do expect do - post "/families/#{family.id}/invitations/#{invitation.id}/accept" + post "/families/#{family.id}/invitations/#{invitee_invitation.token}/accept" end.not_to(change { invitee.reload.family }) end it 'redirects with error message' do - post "/families/#{family.id}/invitations/#{invitation.id}/accept" + post "/families/#{family.id}/invitations/#{invitee_invitation.token}/accept" expect(response).to redirect_to(root_path) - follow_redirect! - expect(response.body).to include('You must leave your current family') + expect(flash[:alert]).to include('You must leave your current family before joining a new one') end end context 'when invitation is expired' do before do - invitation.update!(expires_at: 1.day.ago) + invitee_invitation.update!(expires_at: 1.day.ago) sign_in invitee end it 'does not accept the invitation' do expect do - post "/families/#{family.id}/invitations/#{invitation.id}/accept" + post "/families/#{family.id}/invitations/#{invitee_invitation.token}/accept" end.not_to(change { invitee.reload.family }) end it 'redirects with error message' do - post "/families/#{family.id}/invitations/#{invitation.id}/accept" + post "/families/#{family.id}/invitations/#{invitee_invitation.token}/accept" expect(response).to redirect_to(root_path) - follow_redirect! - expect(response.body).to include('expired') + expect(flash[:alert]).to include('This invitation is no longer valid or has expired') end end context 'when not authenticated' do it 'redirects to login' do - post "/families/#{family.id}/invitations/#{invitation.id}/accept" + post "/families/#{family.id}/invitations/#{invitee_invitation.token}/accept" expect(response).to redirect_to(new_user_session_path) end end @@ -240,14 +240,14 @@ RSpec.describe 'Family Invitations', type: :request do before { sign_in user } it 'cancels the invitation' do - delete "/families/#{family.id}/invitations/#{invitation.id}" + delete "/families/#{family.id}/invitations/#{invitation.token}" invitation.reload expect(invitation.status).to eq('cancelled') expect(response).to redirect_to(family_path(family)) end it 'redirects with success message' do - delete "/families/#{family.id}/invitations/#{invitation.id}" + delete "/families/#{family.id}/invitations/#{invitation.token}" expect(response).to redirect_to(family_path(family)) follow_redirect! expect(response.body).to include('Invitation cancelled') @@ -256,9 +256,10 @@ RSpec.describe 'Family Invitations', type: :request do context 'when user is not the owner' do before { membership.update!(role: :member) } - it 'returns forbidden' do - delete "/families/#{family.id}/invitations/#{invitation.id}" - expect(response).to have_http_status(:forbidden) + it 'redirects due to authorization failure' do + delete "/families/#{family.id}/invitations/#{invitation.token}" + expect(response).to have_http_status(:see_other) + expect(flash[:alert]).to include('not authorized') end end @@ -268,7 +269,7 @@ RSpec.describe 'Family Invitations', type: :request do before { sign_in outsider } it 'redirects to families index' do - delete "/families/#{family.id}/invitations/#{invitation.id}" + delete "/families/#{family.id}/invitations/#{invitation.token}" expect(response).to redirect_to(families_path) end end @@ -277,7 +278,7 @@ RSpec.describe 'Family Invitations', type: :request do before { sign_out user } it 'redirects to login' do - delete "/families/#{family.id}/invitations/#{invitation.id}" + delete "/families/#{family.id}/invitations/#{invitation.token}" expect(response).to redirect_to(new_user_session_path) end end @@ -304,7 +305,7 @@ RSpec.describe 'Family Invitations', type: :request do # 3. Invitee accepts invitation sign_in invitee - post "/families/#{family.id}/invitations/#{created_invitation.id}/accept" + post "/families/#{family.id}/invitations/#{created_invitation.token}/accept" expect(response).to redirect_to(family_path(family)) # 4. Verify invitee is now in family