From 56ef4ec415733f91a9ed0968091796daf98d146d Mon Sep 17 00:00:00 2001 From: Robert May Date: Tue, 11 Aug 2020 15:04:15 +0100 Subject: [PATCH] Update commented_by implementation Next iteration of adding `commented_by` to the API. This commit relocates where it's implemented, preparing to fetch the real data. --- ee/app/models/approval_wrapped_rule.rb | 6 ++++++ .../ee/api/entities/merge_request_approval_setting_rule.rb | 1 + ee/lib/ee/api/entities/merge_request_approval_state_rule.rb | 2 -- ee/spec/models/approval_wrapped_rule_spec.rb | 6 ++++++ ee/spec/requests/api/merge_request_approvals_spec.rb | 2 +- 5 files changed, 14 insertions(+), 3 deletions(-) diff --git a/ee/app/models/approval_wrapped_rule.rb b/ee/app/models/approval_wrapped_rule.rb index ddf32e0e842185..357f38f1ee1d6a 100644 --- a/ee/app/models/approval_wrapped_rule.rb +++ b/ee/app/models/approval_wrapped_rule.rb @@ -74,6 +74,12 @@ def approved_approvers end end + def commented_approvers + strong_memoize(:commented_approvers) do + [] + end + end + def approved? strong_memoize(:approved) do approvals_left <= 0 || unactioned_approvers.size <= 0 diff --git a/ee/lib/ee/api/entities/merge_request_approval_setting_rule.rb b/ee/lib/ee/api/entities/merge_request_approval_setting_rule.rb index 95ed3a68c1205f..bc6cffa00a6faa 100644 --- a/ee/lib/ee/api/entities/merge_request_approval_setting_rule.rb +++ b/ee/lib/ee/api/entities/merge_request_approval_setting_rule.rb @@ -9,6 +9,7 @@ module Entities # To be removed in https://gitlab.com/gitlab-org/gitlab/issues/13574. class MergeRequestApprovalSettingRule < MergeRequestApprovalStateRule expose :approvers, using: ::API::Entities::UserBasic, override: true + expose :commented_approvers, as: :commented_by, using: ::API::Entities::UserBasic end end end diff --git a/ee/lib/ee/api/entities/merge_request_approval_state_rule.rb b/ee/lib/ee/api/entities/merge_request_approval_state_rule.rb index 7dde02d2bfaa1e..053a5ca3688d18 100644 --- a/ee/lib/ee/api/entities/merge_request_approval_state_rule.rb +++ b/ee/lib/ee/api/entities/merge_request_approval_state_rule.rb @@ -7,8 +7,6 @@ class MergeRequestApprovalStateRule < MergeRequestApprovalRule expose :code_owner expose :approved_approvers, as: :approved_by, using: ::API::Entities::UserBasic expose :approved?, as: :approved - - expose :approved_approvers, as: :commented_by, using: ::API::Entities::UserBasic end end end diff --git a/ee/spec/models/approval_wrapped_rule_spec.rb b/ee/spec/models/approval_wrapped_rule_spec.rb index d10c18fa61e9cb..cfaa9df90a74e4 100644 --- a/ee/spec/models/approval_wrapped_rule_spec.rb +++ b/ee/spec/models/approval_wrapped_rule_spec.rb @@ -138,6 +138,12 @@ end end + describe "#commented_approvers" do + it "returns an array" do + expect(subject.commented_approvers).to be_an(Array) + end + end + describe '#unactioned_approvers' do context 'when some approvers has not approved yet' do before do diff --git a/ee/spec/requests/api/merge_request_approvals_spec.rb b/ee/spec/requests/api/merge_request_approvals_spec.rb index 6aeb0298687c71..49998010f10676 100644 --- a/ee/spec/requests/api/merge_request_approvals_spec.rb +++ b/ee/spec/requests/api/merge_request_approvals_spec.rb @@ -174,7 +174,7 @@ expect(rule_response['name']).to eq('foo') expect(rule_response['approvers'][0]['username']).to eq(approver.username) expect(rule_response['approved_by'][0]['username']).to eq(approver.username) - expect(rule_response['commented_by'][0]['username']).to eq(approver.username) + expect(rule_response['commented_by']).to eq([]) expect(rule_response['source_rule']).to be_nil expect(rule_response['section']).to be_nil end -- GitLab