diff --git a/app/helpers/system_note_helper.rb b/app/helpers/system_note_helper.rb index 1d8b657025cd3d1e5e53f5c8c09b7c4f7a4ad6df..f2e1d158c2d499c11126ee34cbb86c3d9a179270 100644 --- a/app/helpers/system_note_helper.rb +++ b/app/helpers/system_note_helper.rb @@ -40,7 +40,9 @@ module SystemNoteHelper 'new_alert_added' => 'warning', 'severity' => 'information-o', 'cloned' => 'documents', - 'issue_type' => 'pencil-square' + 'issue_type' => 'pencil-square', + 'attention_requested' => 'user', + 'attention_request_removed' => 'user' }.freeze def system_note_icon_name(note) diff --git a/app/models/system_note_metadata.rb b/app/models/system_note_metadata.rb index 749b9dce97ccda86288838bb278a94869bd78d93..7b13109dbc4450bb507ba8f1fe03e75d254519d3 100644 --- a/app/models/system_note_metadata.rb +++ b/app/models/system_note_metadata.rb @@ -24,6 +24,7 @@ class SystemNoteMetadata < ApplicationRecord opened closed merged duplicate locked unlocked outdated reviewer tag due_date pinned_embed cherry_pick health_status approved unapproved status alert_issue_added relate unrelate new_alert_added severity + attention_requested attention_request_removed ].freeze validates :note, presence: true, unless: :importing? diff --git a/app/services/merge_requests/toggle_attention_requested_service.rb b/app/services/merge_requests/toggle_attention_requested_service.rb index 4e36ae065bbaa7a4630e1128aa299953de3b45f3..fd24e87454c5c066c2636ffff99f92aa03f759cd 100644 --- a/app/services/merge_requests/toggle_attention_requested_service.rb +++ b/app/services/merge_requests/toggle_attention_requested_service.rb @@ -19,7 +19,10 @@ def execute update_state(assignee) if reviewer&.attention_requested? || assignee&.attention_requested? + create_attention_request_note notity_user + else + create_remove_attention_request_note end success @@ -35,6 +38,14 @@ def notity_user todo_service.create_attention_requested_todo(merge_request, current_user, user) end + def create_attention_request_note + SystemNoteService.request_attention(merge_request, merge_request.project, current_user, user) + end + + def create_remove_attention_request_note + SystemNoteService.remove_attention_request(merge_request, merge_request.project, current_user, user) + end + def assignee merge_request.find_assignee(user) end diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index e98dfb872fe72731c47e1202a39b47cc4cba6b80..0d13c73d49d66cc854f82fcaf5037e0f38336d9b 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -115,6 +115,14 @@ def change_status(noteable, project, author, status, source = nil) ::SystemNotes::IssuablesService.new(noteable: noteable, project: project, author: author).change_status(status, source) end + def request_attention(noteable, project, author, user) + ::SystemNotes::IssuablesService.new(noteable: noteable, project: project, author: author).request_attention(user) + end + + def remove_attention_request(noteable, project, author, user) + ::SystemNotes::IssuablesService.new(noteable: noteable, project: project, author: author).remove_attention_request(user) + end + # Called when 'merge when pipeline succeeds' is executed def merge_when_pipeline_succeeds(noteable, project, author, sha) ::SystemNotes::MergeRequestsService.new(noteable: noteable, project: project, author: author).merge_when_pipeline_succeeds(sha) diff --git a/app/services/system_notes/issuables_service.rb b/app/services/system_notes/issuables_service.rb index 92540f957c8cfa66a20f5d019db318bfadd18402..d33dcd65589b82a49d4f27ae88d3648cb5631640 100644 --- a/app/services/system_notes/issuables_service.rb +++ b/app/services/system_notes/issuables_service.rb @@ -323,23 +323,34 @@ def cross_reference_exists?(mentioned_in) existing_mentions_for(mentioned_in, noteable, notes).exists? end - # Called when a Noteable has been marked as a duplicate of another Issue + # Called when a user's attention has been requested for a Notable # - # canonical_issue - Issue that this is a duplicate of + # user - User's whos attention has been requested # # Example Note text: # - # "marked this issue as a duplicate of #1234" - # - # "marked this issue as a duplicate of other_project#5678" + # "requested attention from @eli.wisoky" # # Returns the created Note object - def mark_duplicate_issue(canonical_issue) - body = "marked this issue as a duplicate of #{canonical_issue.to_reference(project)}" + def request_attention(user) + body = "requested attention from #{user.to_reference}" - issue_activity_counter.track_issue_marked_as_duplicate_action(author: author) if noteable.is_a?(Issue) + create_note(NoteSummary.new(noteable, project, author, body, action: 'attention_requested')) + end - create_note(NoteSummary.new(noteable, project, author, body, action: 'duplicate')) + # Called when a user's attention request has been removed for a Notable + # + # user - User's whos attention request has been removed + # + # Example Note text: + # + # "removed attention request from @eli.wisoky" + # + # Returns the created Note object + def remove_attention_request(user) + body = "removed attention request from #{user.to_reference}" + + create_note(NoteSummary.new(noteable, project, author, body, action: 'attention_request_removed')) end # Called when a Noteable has been marked as the canonical Issue of a duplicate @@ -358,6 +369,25 @@ def mark_canonical_issue_of_duplicate(duplicate_issue) create_note(NoteSummary.new(noteable, project, author, body, action: 'duplicate')) end + # Called when a Noteable has been marked as a duplicate of another Issue + # + # canonical_issue - Issue that this is a duplicate of + # + # Example Note text: + # + # "marked this issue as a duplicate of #1234" + # + # "marked this issue as a duplicate of other_project#5678" + # + # Returns the created Note object + def mark_duplicate_issue(canonical_issue) + body = "marked this issue as a duplicate of #{canonical_issue.to_reference(project)}" + + issue_activity_counter.track_issue_marked_as_duplicate_action(author: author) if noteable.is_a?(Issue) + + create_note(NoteSummary.new(noteable, project, author, body, action: 'duplicate')) + end + def add_email_participants(body) create_note(NoteSummary.new(noteable, project, author, body)) end diff --git a/spec/services/merge_requests/toggle_attention_requested_service_spec.rb b/spec/services/merge_requests/toggle_attention_requested_service_spec.rb index e2455a71eef0215c0b1a823a71ff4460d700068a..e5ba7bcefaee1d706eb007523cc8b71a5b7854bc 100644 --- a/spec/services/merge_requests/toggle_attention_requested_service_spec.rb +++ b/spec/services/merge_requests/toggle_attention_requested_service_spec.rb @@ -19,6 +19,8 @@ allow(NotificationService).to receive(:new) { notification_service } allow(service).to receive(:todo_service).and_return(todo_service) allow(service).to receive(:notification_service).and_return(notification_service) + allow(SystemNoteService).to receive(:request_attention) + allow(SystemNoteService).to receive(:remove_attention_request) project.add_developer(current_user) project.add_developer(user) @@ -93,6 +95,12 @@ service.execute end + + it 'creates a request attention system note' do + expect(SystemNoteService).to receive(:request_attention).with(merge_request, merge_request.project, current_user, assignee_user) + + service.execute + end end context 'assignee is the same as reviewer' do @@ -132,6 +140,12 @@ service.execute end + + it 'creates a remove attention request system note' do + expect(SystemNoteService).to receive(:remove_attention_request).with(merge_request, merge_request.project, current_user, user) + + service.execute + end end end end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 11c6dbe92e7f8c2966056ab046f488a656a856e1..3ec2c71b20c131d1710c7d94db963eeece13fd0a 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -146,6 +146,30 @@ end end + describe '.request_attention' do + let(:user) { double } + + it 'calls IssuableService' do + expect_next_instance_of(::SystemNotes::IssuablesService) do |service| + expect(service).to receive(:request_attention).with(user) + end + + described_class.request_attention(noteable, project, author, user) + end + end + + describe '.remove_attention_request' do + let(:user) { double } + + it 'calls IssuableService' do + expect_next_instance_of(::SystemNotes::IssuablesService) do |service| + expect(service).to receive(:remove_attention_request).with(user) + end + + described_class.remove_attention_request(noteable, project, author, user) + end + end + describe '.merge_when_pipeline_succeeds' do it 'calls MergeRequestsService' do sha = double diff --git a/spec/services/system_notes/issuables_service_spec.rb b/spec/services/system_notes/issuables_service_spec.rb index 43760e296bc2e02e99de5ae1a0bdbb586b5fdb4f..7e53e66303b81261f02869eeea1e850e8c1a90ae 100644 --- a/spec/services/system_notes/issuables_service_spec.rb +++ b/spec/services/system_notes/issuables_service_spec.rb @@ -199,6 +199,42 @@ def build_note(old_reviewers, new_reviewers) end end + describe '#request_attention' do + subject { service.request_attention(user) } + + let(:user) { create(:user) } + + it_behaves_like 'a system note' do + let(:action) { 'attention_requested' } + end + + context 'when attention requested' do + it_behaves_like 'a note with overridable created_at' + + it 'sets the note text' do + expect(subject.note).to eq "requested attention from @#{user.username}" + end + end + end + + describe '#remove_attention_request' do + subject { service.remove_attention_request(user) } + + let(:user) { create(:user) } + + it_behaves_like 'a system note' do + let(:action) { 'attention_request_removed' } + end + + context 'when attention request is removed' do + it_behaves_like 'a note with overridable created_at' + + it 'sets the note text' do + expect(subject.note).to eq "removed attention request from @#{user.username}" + end + end + end + describe '#change_title' do let(:noteable) { create(:issue, project: project, title: 'Lorem ipsum') }