Commit 1383ea53 authored by Patrick Bajao's avatar Patrick Bajao

Merge branch 'ph/attentionRequiredPropertyInApi' into 'master'

Adds the attention required property to the sidebar API

See merge request gitlab-org/gitlab!73279
parents b8b55015 0ca396b7
...@@ -142,8 +142,8 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -142,8 +142,8 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
cache_context = [ cache_context = [
params[:serializer], params[:serializer],
current_user&.cache_key, current_user&.cache_key,
@merge_request.assignees.map(&:cache_key), @merge_request.merge_request_assignees.map(&:cache_key),
@merge_request.reviewers.map(&:cache_key) @merge_request.merge_request_reviewers.map(&:cache_key)
] ]
render_cached(@merge_request, render_cached(@merge_request,
......
...@@ -1942,6 +1942,10 @@ class MergeRequest < ApplicationRecord ...@@ -1942,6 +1942,10 @@ class MergeRequest < ApplicationRecord
end end
end end
def attention_required_enabled?
Feature.enabled?(:mr_attention_requests, project, default_enabled: :yaml)
end
private private
def set_draft_status def set_draft_status
......
...@@ -9,4 +9,8 @@ class MergeRequestAssignee < ApplicationRecord ...@@ -9,4 +9,8 @@ class MergeRequestAssignee < ApplicationRecord
validates :assignee, uniqueness: { scope: :merge_request_id } validates :assignee, uniqueness: { scope: :merge_request_id }
scope :in_projects, ->(project_ids) { joins(:merge_request).where(merge_requests: { target_project_id: project_ids }) } scope :in_projects, ->(project_ids) { joins(:merge_request).where(merge_requests: { target_project_id: project_ids }) }
def cache_key
[model_name.cache_key, id, state, assignee.cache_key]
end
end end
...@@ -5,4 +5,8 @@ class MergeRequestReviewer < ApplicationRecord ...@@ -5,4 +5,8 @@ class MergeRequestReviewer < ApplicationRecord
belongs_to :merge_request belongs_to :merge_request
belongs_to :reviewer, class_name: 'User', foreign_key: :user_id, inverse_of: :merge_request_reviewers belongs_to :reviewer, class_name: 'User', foreign_key: :user_id, inverse_of: :merge_request_reviewers
def cache_key
[model_name.cache_key, id, state, reviewer.cache_key]
end
end end
...@@ -2,10 +2,10 @@ ...@@ -2,10 +2,10 @@
class MergeRequestSidebarExtrasEntity < IssuableSidebarExtrasEntity class MergeRequestSidebarExtrasEntity < IssuableSidebarExtrasEntity
expose :assignees do |merge_request, options| expose :assignees do |merge_request, options|
MergeRequestUserEntity.represent(merge_request.assignees, options.merge(merge_request: merge_request)) MergeRequestUserEntity.represent(merge_request.assignees, options.merge(merge_request: merge_request, type: :assignees))
end end
expose :reviewers do |merge_request, options| expose :reviewers do |merge_request, options|
MergeRequestUserEntity.represent(merge_request.reviewers, options.merge(merge_request: merge_request)) MergeRequestUserEntity.represent(merge_request.reviewers, options.merge(merge_request: merge_request, type: :reviewers))
end end
end end
...@@ -16,10 +16,12 @@ class MergeRequestUserEntity < ::API::Entities::UserBasic ...@@ -16,10 +16,12 @@ class MergeRequestUserEntity < ::API::Entities::UserBasic
request.current_user&.can?(:update_merge_request, options[:merge_request]) request.current_user&.can?(:update_merge_request, options[:merge_request])
end end
expose :reviewed, if: satisfies(:present?, :allows_reviewers?) do |reviewer, options| expose :reviewed, if: satisfies(:present?, :allows_reviewers?) do |user, options|
reviewer = options[:merge_request].find_reviewer(reviewer) find_reviewer_or_assignee(user, options)&.reviewed?
end
reviewer&.reviewed? expose :attention_required, if: satisfies(:present?, :allows_reviewers?, :attention_required_enabled?) do |user, options|
find_reviewer_or_assignee(user, options)&.attention_required?
end end
expose :approved, if: satisfies(:present?) do |user, options| expose :approved, if: satisfies(:present?) do |user, options|
...@@ -27,6 +29,16 @@ class MergeRequestUserEntity < ::API::Entities::UserBasic ...@@ -27,6 +29,16 @@ class MergeRequestUserEntity < ::API::Entities::UserBasic
# makes one query per merge request, whereas #approved_by? makes one per user # makes one query per merge request, whereas #approved_by? makes one per user
options[:merge_request].approvals.any? { |app| app.user_id == user.id } options[:merge_request].approvals.any? { |app| app.user_id == user.id }
end end
private
def find_reviewer_or_assignee(user, options)
if options[:type] == :reviewers
options[:merge_request].find_reviewer(user)
else
options[:merge_request].find_assignee(user)
end
end
end end
MergeRequestUserEntity.prepend_mod_with('MergeRequestUserEntity') MergeRequestUserEntity.prepend_mod_with('MergeRequestUserEntity')
...@@ -4,7 +4,7 @@ require 'spec_helper' ...@@ -4,7 +4,7 @@ require 'spec_helper'
RSpec.describe MergeRequestUserEntity do RSpec.describe MergeRequestUserEntity do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:merge_request) { create(:merge_request) } let_it_be(:merge_request) { create(:merge_request, assignees: [user]) }
let(:request) { EntityRequest.new(project: merge_request.target_project, current_user: user) } let(:request) { EntityRequest.new(project: merge_request.target_project, current_user: user) }
...@@ -18,7 +18,8 @@ RSpec.describe MergeRequestUserEntity do ...@@ -18,7 +18,8 @@ RSpec.describe MergeRequestUserEntity do
it 'exposes needed attributes' do it 'exposes needed attributes' do
is_expected.to include( is_expected.to include(
:id, :name, :username, :state, :avatar_url, :web_url, :id, :name, :username, :state, :avatar_url, :web_url,
:can_merge, :can_update_merge_request, :reviewed, :approved :can_merge, :can_update_merge_request, :reviewed, :approved,
:attention_required
) )
end end
...@@ -56,6 +57,10 @@ RSpec.describe MergeRequestUserEntity do ...@@ -56,6 +57,10 @@ RSpec.describe MergeRequestUserEntity do
end end
end end
context 'attention_required' do
it { is_expected.to include(attention_required: true ) }
end
describe 'performance' do describe 'performance' do
let_it_be(:user_a) { create(:user) } let_it_be(:user_a) { create(:user) }
let_it_be(:user_b) { create(:user) } let_it_be(:user_b) { create(:user) }
......
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