Fix ScenarioUser coupling bug

app/models/scenario_user.rb

@@ -16,7 +16,7 @@ class ScenarioUser < ApplicationRecord
   def couple_existing_user
     return unless user_email.present? && user_id.blank?
 
-    couple_to(User.find_by(id: user_id))
+    couple_to(User.find_by(user_email: user_email))
   end
 
   # Couples the record to an existing User.

spec/models/scenario_user_spec.rb

@@ -43,6 +43,60 @@
     end
   end
 
+  describe '#couple_existing_user' do
+    context 'when a user with the email exists in the system' do
+      let(:existing_user) { create(:user, user_email: '[email protected]') }
+      let(:scenario_user) do
+        build(:scenario_user, scenario: scenario, user_email: '[email protected]', user_id: nil)
+      end
+
+      before { existing_user }
+
+      it 'couples the scenario_user to the existing user on create' do
+        scenario_user.save!
+        expect(scenario_user.user_id).to eq(existing_user.id)
+      end
+
+      it 'removes the user_email after coupling' do
+        scenario_user.save!
+        expect(scenario_user.user_email).to be_nil
+      end
+    end
+
+    context 'when no user with the email exists' do
+      let(:scenario_user) do
+        build(:scenario_user, scenario: scenario, user_email: '[email protected]', user_id: nil)
+      end
+
+      it 'keeps the user_email' do
+        scenario_user.save!
+        expect(scenario_user.user_email).to eq('[email protected]')
+      end
+
+      it 'does not set a user_id' do
+        scenario_user.save!
+        expect(scenario_user.user_id).to be_nil
+      end
+    end
+
+    context 'when user_id is already set' do
+      let(:user) { create(:user) }
+      let(:scenario_user) do
+        build(:scenario_user, scenario: scenario, user_id: user.id, user_email: nil)
+      end
+
+      it 'does not change the user_id' do
+        scenario_user.save!
+        expect(scenario_user.user_id).to eq(user.id)
+      end
+
+      it 'keeps user_email nil' do
+        scenario_user.save!
+        expect(scenario_user.user_email).to be_nil
+      end
+    end
+  end
+
   it 'allows updating the role if not the last scenario owner' do
     # The first user added will automatically become the scenario owner
     scenario.user = create(:user)

spec/requests/api/v3/scenario_users_controller_spec.rb

@@ -42,7 +42,7 @@
     let(:json) { JSON.parse(response.body) }
 
     context 'with proper params' do
-      let(:user_viewer) { create(:user, email: '[email protected]') }
+      let(:user_viewer) { create(:user, user_email: '[email protected]') }
 
       before do
         user_viewer
@@ -64,14 +64,26 @@
 
       it 'adds the given users to the scenario' do
         expect(json).to contain_exactly(
-          a_hash_including('user_id' => nil, 'user_email' => '[email protected]',
+          a_hash_including('user_id' => user_viewer.id, 'user_email' => nil,
             'role' => 'scenario_viewer', 'role_id' => 1, 'scenario_id' => scenario.id),
           a_hash_including('user_id' => nil, 'user_email' => '[email protected]',
             'role' => 'scenario_collaborator', 'role_id' => 2, 'scenario_id' => scenario.id),
           a_hash_including('user_id' => nil, 'user_email' => '[email protected]',
             'role' => 'scenario_owner', 'role_id' => 3, 'scenario_id' => scenario.id)
         )
       end
+
+      it 'couples the existing user automatically' do
+        viewer_scenario_user = scenario.scenario_users.find_by(user_id: user_viewer.id)
+        expect(viewer_scenario_user).to be_present
+        expect(viewer_scenario_user.user_email).to be_nil
+      end
+
+      it 'keeps pending users as email-only' do
+        collaborator = scenario.scenario_users.find_by(user_email: '[email protected]')
+        expect(collaborator).to be_present
+        expect(collaborator.user_id).to be_nil
+      end
     end
 
     context 'with missing role information' do
@@ -132,7 +144,7 @@
     end
 
     context 'when the user was already present on the scenario' do
-      let(:user_viewer) { create(:user, email: '[email protected]') }
+      let(:user_viewer) { create(:user, user_email: '[email protected]') }
 
       before do
         create(
@@ -143,7 +155,7 @@
           headers: access_token_header(user, :delete),
           params: {
             scenario_users: [
-              { user_id: user_viewer.id, role: 'scenario_viewer' }
+              { user_email: '[email protected]', role: 'scenario_viewer' }
             ]
           }
       end
@@ -153,7 +165,7 @@
       end
 
       it 'returns an error' do
-        expect(json['errors']['']).to include('base')
+        expect(json['errors']['[email protected]']).to include('duplicate')
       end
     end
   end