Fix tests

FAMILY_PLAN.md

@@ -634,7 +634,7 @@ class FamiliesController < ApplicationController
     else
       @family = Family.new(family_params)
       @family.errors.add(:base, 'Failed to create family')
-      render :new, status: :unprocessable_entity
+      render :new, status: :unprocessable_content
     end
   end
 
@@ -648,7 +648,7 @@ class FamiliesController < ApplicationController
     if @family.update(family_params)
       redirect_to family_path(@family), notice: 'Family updated successfully!'
     else
-      render :edit, status: :unprocessable_entity
+      render :edit, status: :unprocessable_content
     end
   end
 

app/services/families/accept_invitation.rb

@@ -4,9 +4,10 @@ module Families
   class AcceptInvitation
     attr_reader :invitation, :user, :error_message
 
-    def initialize(invitation:, user:)
+    def initialize(invitation:, user:, auto_leave: false)
       @invitation = invitation
       @user = user
+      @auto_leave = auto_leave
       @error_message = nil
     end
 
@@ -14,8 +15,16 @@ module Families
       return false unless can_accept?
 
       if user.in_family?
-        @error_message = 'You must leave your current family before joining a new one.'
-        return false
+        if @auto_leave
+          leave_service = Families::Leave.new(user: user)
+          unless leave_service.call
+            @error_message = leave_service.error_message || 'Failed to leave current family.'
+            return false
+          end
+        else
+          @error_message = 'You must leave your current family before joining a new one.'
+          return false
+        end
       end
 
       ActiveRecord::Base.transaction do

app/services/families/invite.rb

@@ -4,7 +4,7 @@ module Families
   class Invite
     include ActiveModel::Validations
 
-    attr_reader :family, :email, :invited_by, :invitation, :errors
+    attr_reader :family, :email, :invited_by, :invitation
 
     validates :email, presence: true, format: { with: URI::MailTo::EMAIL_REGEXP }
 
@@ -12,7 +12,6 @@ module Families
       @family = family
       @email = email.downcase.strip
       @invited_by = invited_by
-      @errors = {}
     end
 
     def call
@@ -27,22 +26,18 @@ module Families
 
       true
     rescue ActiveRecord::RecordInvalid => e
-      @errors[:base] = e.message
+      errors.add(:base, e.message)
       false
     end
 
     def error_message
-      return errors.values.first if errors.any?
-      return validation_error_message unless valid?
+      return errors.full_messages.first if errors.any?
 
       'Failed to send invitation'
     end
 
     private
 
-    def validation_error_message
-      errors.full_messages.first || 'Invalid invitation data'
-    end
 
     def invite_sendable?
       return add_error_and_false(:invited_by, 'You must be a family owner to send invitations') unless invited_by.family_owner?
@@ -54,7 +49,7 @@ module Families
     end
 
     def add_error_and_false(attribute, message)
-      @errors[attribute] = message
+      errors.add(attribute, message)
       false
     end
 

spec/requests/family_memberships_spec.rb

@@ -129,8 +129,7 @@ RSpec.describe 'Family Memberships', type: :request do
         user_email = user.email
         delete "/families/#{family.id}/members/#{owner_membership.id}"
         expect(response).to redirect_to(family_path(family))
-        follow_redirect!
-        expect(response.body).to include("#{user_email} has been removed from the family")
+        expect(flash[:notice]).to include("#{user_email} has been removed from the family")
       end
     end
 
@@ -171,7 +170,8 @@ RSpec.describe 'Family Memberships', type: :request do
 
       it 'returns forbidden' do
         delete "/families/#{family.id}/members/#{owner_membership.id}"
-        expect(response).to have_http_status(:forbidden)
+        expect(response).to have_http_status(:see_other)
+        expect(flash[:alert]).to include('not authorized')
       end
     end
 

spec/requests/family_workflows_spec.rb

@@ -24,9 +24,11 @@ RSpec.describe 'Family Workflows', type: :request do
       expect(response).to have_http_status(:ok)
 
       post '/families', params: { family: { name: 'The Smith Family' } }
-      expect(response).to redirect_to(family_path(Family.last))
 
-      family = Family.last
+      # The redirect should be to the newly created family
+      expect(response).to have_http_status(:found)
+      family = Family.find_by(name: 'The Smith Family')
+      expect(family).to be_present
       expect(family.name).to eq('The Smith Family')
       expect(family.creator).to eq(user1)
       expect(user1.reload.family).to eq(family)
@@ -38,7 +40,8 @@ RSpec.describe 'Family Workflows', type: :request do
       }
       expect(response).to redirect_to(family_path(family))
 
-      invitation = FamilyInvitation.last
+      invitation = family.family_invitations.find_by(email: user2.email)
+      expect(invitation).to be_present
       expect(invitation.email).to eq(user2.email)
       expect(invitation.family).to eq(family)
       expect(invitation.pending?).to be true
@@ -52,7 +55,7 @@ RSpec.describe 'Family Workflows', type: :request do
 
       # User2 accepts invitation
       sign_in user2
-      post "/families/#{family.id}/invitations/#{invitation.id}/accept"
+      post "/families/#{family.id}/invitations/#{invitation.token}/accept"
       expect(response).to redirect_to(family_path(family))
 
       expect(user2.reload.family).to eq(family)
@@ -65,12 +68,13 @@ RSpec.describe 'Family Workflows', type: :request do
         family_invitation: { email: user3.email }
       }
 
-      invitation2 = FamilyInvitation.last
+      invitation2 = family.family_invitations.find_by(email: user3.email)
+      expect(invitation2).to be_present
       expect(invitation2.email).to eq(user3.email)
 
       # Step 5: User3 accepts invitation
       sign_in user3
-      post "/families/#{family.id}/invitations/#{invitation2.id}/accept"
+      post "/families/#{family.id}/invitations/#{invitation2.token}/accept"
 
       expect(user3.reload.family).to eq(family)
       expect(family.reload.members.count).to eq(3)
@@ -95,23 +99,14 @@ RSpec.describe 'Family Workflows', type: :request do
   end
 
   describe 'Family invitation expiration workflow' do
+    let(:family) { create(:family, name: 'Test Family', creator: user1) }
+    let!(:owner_membership) { create(:family_membership, user: user1, family: family, role: :owner) }
+    let!(:invitation) do
+      create(:family_invitation, family: family, email: user2.email, invited_by: user1, expires_at: 1.day.ago)
+    end
+
     it 'handles expired invitations correctly' do
-      # User1 creates family and invitation
-      sign_in user1
-      post '/families', params: { family: { name: 'Test Family' } }
-      family = Family.last
-
-      post "/families/#{family.id}/invitations", params: {
-        family_invitation: { email: user2.email }
-      }
-
-      invitation = FamilyInvitation.last
-
-      # Expire the invitation
-      invitation.update!(expires_at: 1.day.ago)
-
       # User2 tries to view expired invitation
-      sign_out user1
       get "/invitations/#{invitation.token}"
       expect(response).to redirect_to(root_path)
       follow_redirect!
@@ -119,7 +114,7 @@ RSpec.describe 'Family Workflows', type: :request do
 
       # User2 tries to accept expired invitation
       sign_in user2
-      post "/families/#{family.id}/invitations/#{invitation.id}/accept"
+      post "/families/#{family.id}/invitations/#{invitation.token}/accept"
       expect(response).to redirect_to(root_path)
 
       expect(user2.reload.family).to be_nil
@@ -128,41 +123,24 @@ RSpec.describe 'Family Workflows', type: :request do
   end
 
   describe 'Multiple family membership prevention workflow' do
+    let(:family1) { create(:family, name: 'Family 1', creator: user1) }
+    let(:family2) { create(:family, name: 'Family 2', creator: user2) }
+    let!(:user1_membership) { create(:family_membership, user: user1, family: family1, role: :owner) }
+    let!(:user2_membership) { create(:family_membership, user: user2, family: family2, role: :owner) }
+    let!(:invitation1) { create(:family_invitation, family: family1, email: user3.email, invited_by: user1) }
+    let!(:invitation2) { create(:family_invitation, family: family2, email: user3.email, invited_by: user2) }
+
     it 'prevents users from joining multiple families' do
-      # User1 creates first family
-      sign_in user1
-      post '/families', params: { family: { name: 'Family 1' } }
-      family1 = Family.last
-
-      # User2 creates second family
-      sign_in user2
-      post '/families', params: { family: { name: 'Family 2' } }
-      family2 = Family.last
-
-      # User1 invites User3 to Family 1
-      sign_in user1
-      post "/families/#{family1.id}/invitations", params: {
-        family_invitation: { email: user3.email }
-      }
-      invitation1 = FamilyInvitation.last
-
-      # User2 invites User3 to Family 2
-      sign_in user2
-      post "/families/#{family2.id}/invitations", params: {
-        family_invitation: { email: user3.email }
-      }
-      invitation2 = FamilyInvitation.last
-
       # User3 accepts invitation to Family 1
       sign_in user3
-      post "/families/#{family1.id}/invitations/#{invitation1.id}/accept"
-      expect(user3.reload.family).to eq(family1)
+      post "/families/#{family1.id}/invitations/#{invitation1.token}/accept"
+      expect(response).to redirect_to(family_path(user3.reload.family))
+      expect(user3.family).to eq(family1)
 
       # User3 tries to accept invitation to Family 2
-      post "/families/#{family2.id}/invitations/#{invitation2.id}/accept"
+      post "/families/#{family2.id}/invitations/#{invitation2.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')
 
       expect(user3.reload.family).to eq(family1) # Still in first family
     end
@@ -218,18 +196,20 @@ RSpec.describe 'Family Workflows', type: :request do
     let(:family) { create(:family, creator: user1) }
     let!(:owner_membership) { create(:family_membership, user: user1, family: family, role: :owner) }
 
-    it 'prevents family deletion when members exist' do
-      create(:family_membership, user: user2, family: family, role: :member)
+    context 'when members exist' do
+      let!(:member_membership) { create(:family_membership, user: user2, family: family, role: :member) }
 
-      sign_in user1
+      it 'prevents family deletion when members exist' do
+        sign_in user1
 
-      expect do
-        delete "/families/#{family.id}"
-      end.not_to change(Family, :count)
+        expect do
+          delete "/families/#{family.id}"
+        end.not_to change(Family, :count)
 
-      expect(response).to redirect_to(family_path(family))
-      follow_redirect!
-      expect(response.body).to include('Cannot delete family with members')
+        expect(response).to redirect_to(family_path(family))
+        follow_redirect!
+        expect(response.body).to include('Cannot delete family with members')
+      end
     end
 
     it 'allows family deletion when owner is the only member' do
@@ -255,19 +235,23 @@ RSpec.describe 'Family Workflows', type: :request do
       post "/families/#{family.id}/invitations", params: {
         family_invitation: { email: user3.email }
       }
-      expect(response).to have_http_status(:forbidden)
+      expect(response).to have_http_status(:see_other)
+      expect(flash[:alert]).to include('not authorized')
 
       # Member cannot remove other members
       delete "/families/#{family.id}/members/#{owner_membership.id}"
-      expect(response).to have_http_status(:forbidden)
+      expect(response).to have_http_status(:see_other)
+      expect(flash[:alert]).to include('not authorized')
 
       # Member cannot edit family
       patch "/families/#{family.id}", params: { family: { name: 'Hacked Family' } }
-      expect(response).to have_http_status(:forbidden)
+      expect(response).to have_http_status(:see_other)
+      expect(flash[:alert]).to include('not authorized')
 
       # Member cannot delete family
       delete "/families/#{family.id}"
-      expect(response).to have_http_status(:forbidden)
+      expect(response).to have_http_status(:see_other)
+      expect(flash[:alert]).to include('not authorized')
 
       # Outsider cannot access family
       sign_in user3
@@ -280,10 +264,11 @@ RSpec.describe 'Family Workflows', type: :request do
   end
 
   describe 'Email invitation workflow' do
+    let(:family) { create(:family, name: 'Test Family', creator: user1) }
+    let!(:owner_membership) { create(:family_membership, user: user1, family: family, role: :owner) }
+
     it 'handles invitation emails correctly' do
       sign_in user1
-      post '/families', params: { family: { name: 'Test Family' } }
-      family = Family.last
 
       # Mock email delivery
       expect do
@@ -292,7 +277,7 @@ RSpec.describe 'Family Workflows', type: :request do
         }
       end.to change(FamilyInvitation, :count).by(1)
 
-      invitation = FamilyInvitation.last
+      invitation = family.family_invitations.find_by(email: 'newuser@example.com')
       expect(invitation.email).to eq('newuser@example.com')
       expect(invitation.token).to be_present
       expect(invitation.expires_at).to be > Time.current
@@ -308,15 +293,15 @@ RSpec.describe 'Family Workflows', type: :request do
 
       # User creates family
       post '/families', params: { family: { name: 'Test Family' } }
-      family = Family.last
+      expect(response).to have_http_status(:found)
 
       # User with family gets redirected from index to family page
       get '/families'
-      expect(response).to redirect_to(family_path(family))
+      expect(response).to redirect_to(family_path(user1.reload.family))
 
       # User with family gets redirected from new family page
       get '/families/new'
-      expect(response).to redirect_to(family_path(family))
+      expect(response).to redirect_to(family_path(user1.reload.family))
     end
   end
 end

spec/services/families/accept_invitation_spec.rb

@@ -41,6 +41,7 @@ RSpec.describe Families::AcceptInvitation do
     context 'when user is already in another family' do
       let(:other_family) { create(:family) }
       let!(:existing_membership) { create(:family_membership, user: invitee, family: other_family) }
+      let(:service) { described_class.new(invitation: invitation, user: invitee, auto_leave: true) }
 
       it 'leaves current family before joining new one' do
         expect(Families::Leave).to receive(:new).with(user: invitee).and_call_original