Merge branch 'todo-bug-fix' into 'master'

Fix Alert Todo bug by passing current_user instead of assignee

See merge request gitlab-org/gitlab!34642

app/services/alert_management/alerts/update_service.rb

@@ -35,7 +35,11 @@ module AlertManagement
       attr_reader :alert, :current_user, :params
 
       def allowed?
-        current_user.can?(:update_alert_management_alert, alert)
+        current_user&.can?(:update_alert_management_alert, alert)
+      end
+
+      def assignee_todo_allowed?
+        assignee&.can?(:read_alert_management_alert, alert)
       end
 
       def todo_service
@@ -80,9 +84,10 @@ module AlertManagement
       end
 
       def assign_todo
-        return unless assignee
+        # Remove check in follow-up issue https://gitlab.com/gitlab-org/gitlab/-/issues/222672
+        return unless assignee_todo_allowed?
 
-        todo_service.assign_alert(alert, assignee)
+        todo_service.assign_alert(alert, current_user)
       end
 
       def add_assignee_system_note(old_assignees)

spec/services/alert_management/alerts/update_service_spec.rb

@@ -20,6 +20,15 @@ describe AlertManagement::Alerts::UpdateService do
   describe '#execute' do
     subject(:response) { service.execute }
 
+    context 'when the current_user is nil' do
+      let(:current_user) { nil }
+
+      it 'results in an error' do
+        expect(response).to be_error
+        expect(response.message).to eq('You have no permissions')
+      end
+    end
+
     context 'when user does not have permission to update alerts' do
       let(:current_user) { user_without_permissions }
 
@@ -81,6 +90,37 @@ describe AlertManagement::Alerts::UpdateService do
         expect { response }.to change { Todo.where(user: user_with_permissions).count }.by(1)
       end
 
+      context 'when current user is not the assignee' do
+        let(:assignee_user) { create(:user) }
+        let(:params) { { assignees: [assignee_user] } }
+
+        it 'skips adding todo for assignee without permission to read alert' do
+          expect { response }.not_to change(Todo, :count)
+        end
+
+        context 'when assignee has read permission' do
+          before do
+            project.add_developer(assignee_user)
+          end
+
+          it 'adds a todo' do
+            response
+
+            expect(Todo.first.author).to eq(current_user)
+          end
+        end
+
+        context 'when current_user is nil' do
+          let(:current_user) { nil }
+
+          it 'skips adding todo if current_user is nil' do
+            project.add_developer(assignee_user)
+
+            expect { response }.not_to change(Todo, :count)
+          end
+        end
+      end
+
       context 'with multiple users included' do
         let(:params) { { assignees: [user_with_permissions, user_without_permissions] } }