From eaef3237b57a30dbb897301212f516990207cf43 Mon Sep 17 00:00:00 2001 From: Robert May Date: Tue, 11 Aug 2020 16:47:02 +0100 Subject: [PATCH 1/5] Add data to commented_by Adds a simple lookup for note authors who are approvers for an MR. --- app/models/concerns/issuable.rb | 2 ++ ee/app/models/approval_wrapped_rule.rb | 2 +- ee/spec/models/approval_wrapped_rule_spec.rb | 13 +++++++++++++ spec/models/concerns/issuable_spec.rb | 1 + 4 files changed, 17 insertions(+), 1 deletion(-) diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index fe51bdc2486121..2a3eefe1de6214 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -61,6 +61,8 @@ def award_emojis_loaded? end end + has_many :authors, through: :notes + has_many :label_links, as: :target, dependent: :destroy, inverse_of: :target # rubocop:disable Cop/ActiveRecordDependent has_many :labels, through: :label_links has_many :todos, as: :target, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent diff --git a/ee/app/models/approval_wrapped_rule.rb b/ee/app/models/approval_wrapped_rule.rb index 357f38f1ee1d6a..dc8c91e19de706 100644 --- a/ee/app/models/approval_wrapped_rule.rb +++ b/ee/app/models/approval_wrapped_rule.rb @@ -76,7 +76,7 @@ def approved_approvers def commented_approvers strong_memoize(:commented_approvers) do - [] + merge_request.authors & approvers end end diff --git a/ee/spec/models/approval_wrapped_rule_spec.rb b/ee/spec/models/approval_wrapped_rule_spec.rb index cfaa9df90a74e4..f321a732cb6abe 100644 --- a/ee/spec/models/approval_wrapped_rule_spec.rb +++ b/ee/spec/models/approval_wrapped_rule_spec.rb @@ -139,9 +139,22 @@ end describe "#commented_approvers" do + let(:rule) { create(:approval_project_rule, project: merge_request.project, approvals_required: approvals_required) } + + before do + rule.users = [approver1, approver3] + end + it "returns an array" do expect(subject.commented_approvers).to be_an(Array) end + + it "returns an array of approvers who have commented" do + create(:note, project: merge_request.project, noteable: merge_request, author: approver1) + + expect(subject.commented_approvers).to include(approver1) + expect(subject.commented_approvers).not_to include(approver3) + end end describe '#unactioned_approvers' do diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index c943dd57f05269..51288c0988c327 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -17,6 +17,7 @@ it { is_expected.to have_many(:notes).dependent(:destroy) } it { is_expected.to have_many(:todos).dependent(:destroy) } it { is_expected.to have_many(:labels) } + it { is_expected.to have_many(:authors).through(:notes) } context 'Notes' do let!(:note) { create(:note, noteable: issue, project: issue.project) } -- GitLab From 705952ad8ed9c5f2ebf4c616ab79976cd8bd9ebb Mon Sep 17 00:00:00 2001 From: Robert May Date: Tue, 11 Aug 2020 16:54:01 +0100 Subject: [PATCH 2/5] Add changelog entry --- ee/changelogs/unreleased/commented-by-3.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 ee/changelogs/unreleased/commented-by-3.yml diff --git a/ee/changelogs/unreleased/commented-by-3.yml b/ee/changelogs/unreleased/commented-by-3.yml new file mode 100644 index 00000000000000..f7000dcd731e26 --- /dev/null +++ b/ee/changelogs/unreleased/commented-by-3.yml @@ -0,0 +1,5 @@ +--- +title: Add commented_by to approval rules +merge_request: 39237 +author: +type: added -- GitLab From e8bf2dbcd2bf936341fb7a85e1b9215800698b57 Mon Sep 17 00:00:00 2001 From: Robert May Date: Tue, 11 Aug 2020 18:22:12 +0100 Subject: [PATCH 3/5] Rename association for clarity Also adds info to import/export all_models.yml --- app/models/concerns/issuable.rb | 2 +- ee/app/models/approval_wrapped_rule.rb | 2 +- spec/lib/gitlab/import_export/all_models.yml | 2 ++ spec/models/concerns/issuable_spec.rb | 2 +- 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 2a3eefe1de6214..387c4e561d2eb5 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -61,7 +61,7 @@ def award_emojis_loaded? end end - has_many :authors, through: :notes + has_many :note_authors, through: :notes, source: :author has_many :label_links, as: :target, dependent: :destroy, inverse_of: :target # rubocop:disable Cop/ActiveRecordDependent has_many :labels, through: :label_links diff --git a/ee/app/models/approval_wrapped_rule.rb b/ee/app/models/approval_wrapped_rule.rb index dc8c91e19de706..0bbf3ef09f255b 100644 --- a/ee/app/models/approval_wrapped_rule.rb +++ b/ee/app/models/approval_wrapped_rule.rb @@ -76,7 +76,7 @@ def approved_approvers def commented_approvers strong_memoize(:commented_approvers) do - merge_request.authors & approvers + merge_request.note_authors & approvers end end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index bc51e8747d4021..d69f389051fd78 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -47,6 +47,7 @@ issues: - alert_management_alert - status_page_published_incident - namespace +- note_authors events: - author - project @@ -167,6 +168,7 @@ merge_requests: - deployments - user_mentions - system_note_metadata +- note_authors external_pull_requests: - project merge_request_diff: diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 51288c0988c327..0824b5c7834575 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -17,7 +17,7 @@ it { is_expected.to have_many(:notes).dependent(:destroy) } it { is_expected.to have_many(:todos).dependent(:destroy) } it { is_expected.to have_many(:labels) } - it { is_expected.to have_many(:authors).through(:notes) } + it { is_expected.to have_many(:note_authors).through(:notes) } context 'Notes' do let!(:note) { create(:note, noteable: issue, project: issue.project) } -- GitLab From d6fc575ff7485e28a8913b79add72d9de3c142e6 Mon Sep 17 00:00:00 2001 From: Robert May Date: Tue, 11 Aug 2020 19:28:31 +0100 Subject: [PATCH 4/5] Add missing relation --- spec/lib/gitlab/import_export/all_models.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index d69f389051fd78..73ba89c4a960e4 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -673,6 +673,7 @@ epic: - events - resource_label_events - user_mentions +- note_authors epic_issue: - epic - issue -- GitLab From e9d3a0e2c99d42ca2b3c559f3605c08a541d0c35 Mon Sep 17 00:00:00 2001 From: Robert May Date: Wed, 12 Aug 2020 11:22:46 +0100 Subject: [PATCH 5/5] Tweaks to association and specs --- app/models/concerns/issuable.rb | 2 +- ee/changelogs/unreleased/commented-by-3.yml | 2 +- ee/spec/models/approval_wrapped_rule_spec.rb | 9 ++------- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 387c4e561d2eb5..dd5aedbb760b7d 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -61,7 +61,7 @@ def award_emojis_loaded? end end - has_many :note_authors, through: :notes, source: :author + has_many :note_authors, -> { distinct }, through: :notes, source: :author has_many :label_links, as: :target, dependent: :destroy, inverse_of: :target # rubocop:disable Cop/ActiveRecordDependent has_many :labels, through: :label_links diff --git a/ee/changelogs/unreleased/commented-by-3.yml b/ee/changelogs/unreleased/commented-by-3.yml index f7000dcd731e26..e5d3e1f1446929 100644 --- a/ee/changelogs/unreleased/commented-by-3.yml +++ b/ee/changelogs/unreleased/commented-by-3.yml @@ -1,5 +1,5 @@ --- -title: Add commented_by to approval rules +title: Add commented_by to approval_settings endpoint merge_request: 39237 author: type: added diff --git a/ee/spec/models/approval_wrapped_rule_spec.rb b/ee/spec/models/approval_wrapped_rule_spec.rb index f321a732cb6abe..25a9eaa92479f3 100644 --- a/ee/spec/models/approval_wrapped_rule_spec.rb +++ b/ee/spec/models/approval_wrapped_rule_spec.rb @@ -139,11 +139,7 @@ end describe "#commented_approvers" do - let(:rule) { create(:approval_project_rule, project: merge_request.project, approvals_required: approvals_required) } - - before do - rule.users = [approver1, approver3] - end + let(:rule) { create(:approval_project_rule, project: merge_request.project, approvals_required: approvals_required, users: [approver1, approver3]) } it "returns an array" do expect(subject.commented_approvers).to be_an(Array) @@ -152,8 +148,7 @@ it "returns an array of approvers who have commented" do create(:note, project: merge_request.project, noteable: merge_request, author: approver1) - expect(subject.commented_approvers).to include(approver1) - expect(subject.commented_approvers).not_to include(approver3) + expect(subject.commented_approvers).to eq([approver1]) end end -- GitLab