Merge branch 'ph/324854/attentionRequestedFilter' into 'master'

Added attention requested filter

See merge request gitlab-org/gitlab!74800

app/assets/javascripts/filtered_search/add_extra_tokens_for_merge_requests.js

@@ -13,6 +13,21 @@ export default (IssuableTokenKeys, disableTargetBranchFilter = false) => {
   IssuableTokenKeys.tokenKeys.splice(2, 0, reviewerToken);
   IssuableTokenKeys.tokenKeysWithAlternative.splice(2, 0, reviewerToken);
 
+  if (window.gon?.features?.mrAttentionRequests) {
+    const attentionRequestedToken = {
+      formattedKey: __('Attention'),
+      key: 'attention',
+      type: 'string',
+      param: '',
+      symbol: '@',
+      icon: 'user',
+      tag: '@attention',
+      hideNotEqual: true,
+    };
+    IssuableTokenKeys.tokenKeys.splice(2, 0, attentionRequestedToken);
+    IssuableTokenKeys.tokenKeysWithAlternative.splice(2, 0, attentionRequestedToken);
+  }
+
   const draftToken = {
     token: {
       formattedKey: __('Draft'),

app/assets/javascripts/filtered_search/available_dropdown_mappings.js

@@ -77,6 +77,11 @@ export default class AvailableDropdownMappings {
         gl: DropdownUser,
         element: this.container.querySelector('#js-dropdown-reviewer'),
       },
+      attention: {
+        reference: null,
+        gl: DropdownUser,
+        element: this.container.getElementById('js-dropdown-attention-requested'),
+      },
       'approved-by': {
         reference: null,
         gl: DropdownUser,

app/assets/javascripts/filtered_search/constants.js

-export const USER_TOKEN_TYPES = ['author', 'assignee', 'approved-by', 'reviewer'];
+export const USER_TOKEN_TYPES = ['author', 'assignee', 'approved-by', 'reviewer', 'attention'];
 
 export const DROPDOWN_TYPE = {
   hint: 'hint',

app/controllers/dashboard_controller.rb

@@ -20,6 +20,10 @@ class DashboardController < Dashboard::ApplicationController
 
   urgency :low, [:merge_requests]
 
+  before_action only: [:merge_requests] do
+    push_frontend_feature_flag(:mr_attention_requests, default_enabled: :yaml)
+  end
+
   def activity
     respond_to do |format|
       format.html

app/controllers/projects/merge_requests_controller.rb

@@ -30,6 +30,10 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   before_action :set_issuables_index, only: [:index]
   before_action :authenticate_user!, only: [:assign_related_issues]
   before_action :check_user_can_push_to_source_branch!, only: [:rebase]
+  before_action only: [:index, :show] do
+    push_frontend_feature_flag(:mr_attention_requests, project, default_enabled: :yaml)
+  end
+
   before_action only: [:show] do
     push_frontend_feature_flag(:file_identifier_hash)
     push_frontend_feature_flag(:merge_request_widget_graphql, @project, default_enabled: :yaml)
@@ -40,7 +44,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     push_frontend_feature_flag(:improved_emoji_picker, project, default_enabled: :yaml)
     push_frontend_feature_flag(:diffs_virtual_scrolling, project, default_enabled: :yaml)
     push_frontend_feature_flag(:restructured_mr_widget, project, default_enabled: :yaml)
-    push_frontend_feature_flag(:mr_attention_requests, project, default_enabled: :yaml)
     push_frontend_feature_flag(:refactor_mr_widgets_extensions, @project, default_enabled: :yaml)
     push_frontend_feature_flag(:rebase_without_ci_ui, @project, default_enabled: :yaml)
     push_frontend_feature_flag(:rearrange_pipelines_table, project, default_enabled: :yaml)

app/finders/merge_requests_finder.rb

@@ -44,7 +44,8 @@ class MergeRequestsFinder < IssuableFinder
       :reviewer_id,
       :reviewer_username,
       :target_branch,
-      :wip
+      :wip,
+      :attention
     ]
   end
 
@@ -69,6 +70,7 @@ class MergeRequestsFinder < IssuableFinder
     items = by_approvals(items)
     items = by_deployments(items)
     items = by_reviewer(items)
+    items = by_attention(items)
 
     by_source_project_id(items)
   end
@@ -218,6 +220,12 @@ class MergeRequestsFinder < IssuableFinder
     end
   end
 
+  def by_attention(items)
+    return items unless params.attention?
+
+    items.attention(params.attention)
+  end
+
   def parse_datetime(input)
     # To work around http://www.ruby-lang.org/en/news/2021/11/15/date-parsing-method-regexp-dos-cve-2021-41817/
     DateTime.parse(input.byteslice(0, 128)) if input

app/finders/merge_requests_finder/params.rb

@@ -21,5 +21,11 @@ class MergeRequestsFinder
         end
       end
     end
+
+    def attention
+      strong_memoize(:attention) do
+        User.find_by_username(params[:attention])
+      end
+    end
   end
 end

app/models/merge_request.rb

@@ -406,6 +406,17 @@ class MergeRequest < ApplicationRecord
     )
   end
 
+  scope :attention, ->(user) do
+    # rubocop: disable Gitlab/Union
+    union = Gitlab::SQL::Union.new([
+      MergeRequestReviewer.select(:merge_request_id).where(user_id: user.id, state: MergeRequestReviewer.states[:attention_requested]),
+      MergeRequestAssignee.select(:merge_request_id).where(user_id: user.id, state: MergeRequestAssignee.states[:attention_requested])
+    ])
+    # rubocop: enable Gitlab/Union
+
+    with(Gitlab::SQL::CTE.new(:reviewers_and_assignees, union).to_arel).where('merge_requests.id in (select merge_request_id from reviewers_and_assignees)')
+  end
+
   def self.total_time_to_merge
     join_metrics
       .merge(MergeRequest::Metrics.with_valid_time_to_merge)

app/views/shared/issuable/_search_bar.html.haml

@@ -107,6 +107,16 @@
                     = render 'shared/issuable/user_dropdown_item',
                       user: User.new(username: '{{username}}', name: '{{name}}'),
                       avatar: { lazy: true, url: '{{avatar_url}}' }
+                - if Feature.enabled?(:mr_attention_requests, default_enabled: :yaml)
+                  #js-dropdown-attention-requested.filtered-search-input-dropdown-menu.dropdown-menu
+                    - if current_user
+                      %ul{ data: { dropdown: true } }
+                        = render 'shared/issuable/user_dropdown_item',
+                          user: current_user
+                    %ul.filter-dropdown{ data: { dynamic: true, dropdown: true } }
+                      = render 'shared/issuable/user_dropdown_item',
+                        user: User.new(username: '{{username}}', name: '{{name}}'),
+                        avatar: { lazy: true, url: '{{avatar_url}}' }
                 = render_if_exists 'shared/issuable/approver_dropdown'
                 = render_if_exists 'shared/issuable/approved_by_dropdown'
                 #js-dropdown-milestone.filtered-search-input-dropdown-menu.dropdown-menu

db/post_migrate/20220127112243_add_index_to_merge_request_assignees_state.rb

+# frozen_string_literal: true
+
+class AddIndexToMergeRequestAssigneesState < Gitlab::Database::Migration[1.0]
+  disable_ddl_transaction!
+
+  INDEX_NAME = 'index_on_merge_request_assignees_state'
+
+  def up
+    add_concurrent_index :merge_request_assignees, :state, where: 'state = 2', name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :merge_request_assignees, INDEX_NAME
+  end
+end

db/post_migrate/20220127112412_add_index_to_merge_request_reviewers_state.rb

+# frozen_string_literal: true
+
+class AddIndexToMergeRequestReviewersState < Gitlab::Database::Migration[1.0]
+  disable_ddl_transaction!
+
+  INDEX_NAME = 'index_on_merge_request_reviewers_state'
+
+  def up
+    add_concurrent_index :merge_request_reviewers, :state, where: 'state = 2', name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :merge_request_reviewers, INDEX_NAME
+  end
+end

db/schema_migrations/20220127112243

+7707b9bcdcd7ec28af31b90355905eb8971c12a90c4334b0ae214e45fac9645a
\ No newline at end of file

db/schema_migrations/20220127112412

+350409be3f491b61a1d757dbd839b48d088339883e8e019d00ad90e6adc350e6
\ No newline at end of file

db/structure.sql

@@ -27129,6 +27129,10 @@ CREATE UNIQUE INDEX index_on_instance_statistics_recorded_at_and_identifier ON a
 
 CREATE INDEX index_on_label_links_all_columns ON label_links USING btree (target_id, label_id, target_type);
 
+CREATE INDEX index_on_merge_request_assignees_state ON merge_request_assignees USING btree (state) WHERE (state = 2);
+
+CREATE INDEX index_on_merge_request_reviewers_state ON merge_request_reviewers USING btree (state) WHERE (state = 2);
+
 CREATE INDEX index_on_merge_requests_for_latest_diffs ON merge_requests USING btree (target_project_id) INCLUDE (id, latest_merge_request_diff_id);
 
 COMMENT ON INDEX index_on_merge_requests_for_latest_diffs IS 'Index used to efficiently obtain the oldest merge request for a commit SHA';

locale/gitlab.pot

@@ -4999,6 +4999,9 @@ msgstr[1] ""
 msgid "Attaching the file failed."
 msgstr ""
 
+msgid "Attention"
+msgstr ""
+
 msgid "Audit Events"
 msgstr ""
 

spec/finders/merge_requests_finder/params_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe MergeRequestsFinder::Params do
+  let(:user) { create(:user) }
+
+  subject { described_class.new(params, user, MergeRequest) }
+
+  describe 'attention' do
+    context 'attention param exists' do
+      let(:params) { { attention: user.username } }
+
+      it { expect(subject.attention).to eq(user) }
+    end
+
+    context 'attention param does not exist' do
+      let(:params) { { attention: nil } }
+
+      it { expect(subject.attention).to eq(nil) }
+    end
+  end
+end

spec/finders/merge_requests_finder_spec.rb

@@ -408,6 +408,22 @@ RSpec.describe MergeRequestsFinder do
         end
       end
 
+      context 'attention' do
+        subject { described_class.new(user, params).execute }
+
+        before do
+          reviewer = merge_request1.find_reviewer(user2)
+          reviewer.update!(state: :reviewed)
+        end
+
+        context 'by username' do
+          let(:params) { { attention: user2.username } }
+          let(:expected_mr) { [merge_request2, merge_request3] }
+
+          it { is_expected.to contain_exactly(*expected_mr) }
+        end
+      end
+
       context 'reviewer filtering' do
         subject { described_class.new(user, params).execute }
 

spec/models/merge_request_spec.rb

@@ -144,6 +144,20 @@ RSpec.describe MergeRequest, factory_default: :keep do
       end
     end
 
+    describe '.attention' do
+      let_it_be(:merge_request5) { create(:merge_request, :unique_branches, assignees: [user2])}
+      let_it_be(:merge_request6) { create(:merge_request, :unique_branches, assignees: [user2])}
+
+      before do
+        assignee = merge_request6.find_assignee(user2)
+        assignee.update!(state: :reviewed)
+      end
+
+      it 'returns MRs that have any attention requests' do
+        expect(described_class.attention(user2)).to eq([merge_request2, merge_request5])
+      end
+    end
+
     describe '.drafts' do
       it 'returns MRs where draft == true' do
         expect(described_class.drafts).to eq([merge_request4])