Commit 5ee571ed authored by allison.browne's avatar allison.browne

Fix alert assignment and assignement todo bugs

Pass current_user, not assignee to ToDoService function.
Guard creation of todo's for users who can not read alerts.
Check for nil current_user to send permission error
parent 27ae8d37
...@@ -35,7 +35,11 @@ module AlertManagement ...@@ -35,7 +35,11 @@ module AlertManagement
attr_reader :alert, :current_user, :params attr_reader :alert, :current_user, :params
def allowed? 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 end
def todo_service def todo_service
...@@ -80,9 +84,10 @@ module AlertManagement ...@@ -80,9 +84,10 @@ module AlertManagement
end end
def assign_todo 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 end
def add_assignee_system_note(old_assignees) def add_assignee_system_note(old_assignees)
......
...@@ -20,6 +20,15 @@ describe AlertManagement::Alerts::UpdateService do ...@@ -20,6 +20,15 @@ describe AlertManagement::Alerts::UpdateService do
describe '#execute' do describe '#execute' do
subject(:response) { service.execute } 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 context 'when user does not have permission to update alerts' do
let(:current_user) { user_without_permissions } let(:current_user) { user_without_permissions }
...@@ -81,6 +90,37 @@ describe AlertManagement::Alerts::UpdateService do ...@@ -81,6 +90,37 @@ describe AlertManagement::Alerts::UpdateService do
expect { response }.to change { Todo.where(user: user_with_permissions).count }.by(1) expect { response }.to change { Todo.where(user: user_with_permissions).count }.by(1)
end 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 context 'with multiple users included' do
let(:params) { { assignees: [user_with_permissions, user_without_permissions] } } let(:params) { { assignees: [user_with_permissions, user_without_permissions] } }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment