Merge branch 'aa-sort-merge-requests-by-merged_date' into 'master'

Sort merge request by merge date

See merge request gitlab-org/gitlab!64720

app/finders/concerns/merged_at_filter.rb

@@ -10,7 +10,7 @@ module MergedAtFilter
     mr_metrics_scope = mr_metrics_scope.merged_after(merged_after) if merged_after.present?
     mr_metrics_scope = mr_metrics_scope.merged_before(merged_before) if merged_before.present?
 
-    join_metrics(items, mr_metrics_scope)
+    items.join_metrics.merge(mr_metrics_scope)
   end
 
   def merged_after
@@ -20,22 +20,4 @@ module MergedAtFilter
   def merged_before
     params[:merged_before]
   end
-
-  # rubocop: disable CodeReuse/ActiveRecord
-  #
-  # This join optimizes merged_at queries when the finder is invoked for a project by moving
-  # the target_project_id condition from merge_requests table to merge_request_metrics table.
-  def join_metrics(items, mr_metrics_scope)
-    scope = if project_id = items.where_values_hash["target_project_id"]
-              # removing the original merge_requests.target_project_id condition
-              items = items.unscope(where: :target_project_id)
-              # adding the target_project_id condition to merge_request_metrics
-              items.join_metrics(project_id)
-            else
-              items.join_metrics
-            end
-
-    scope.merge(mr_metrics_scope)
-  end
-  # rubocop: enable CodeReuse/ActiveRecord
 end

app/helpers/sorting_helper.rb

@@ -26,6 +26,9 @@ module SortingHelper
       sort_value_recently_updated  => sort_title_recently_updated,
       sort_value_popularity        => sort_title_popularity,
       sort_value_priority          => sort_title_priority,
+      sort_value_merged_date       => sort_title_merged_date,
+      sort_value_merged_recently   => sort_title_merged_recently,
+      sort_value_merged_earlier    => sort_title_merged_earlier,
       sort_value_upvotes           => sort_title_upvotes,
       sort_value_contacted_date    => sort_title_contacted_date,
       sort_value_relative_position => sort_title_relative_position,
@@ -178,6 +181,7 @@ module SortingHelper
       sort_value_oldest_updated => sort_value_recently_updated,
       sort_value_milestone_later => sort_value_milestone,
       sort_value_due_date_later => sort_value_due_date,
+      sort_value_merged_recently => sort_value_merged_date,
       sort_value_least_popular => sort_value_popularity
     }
   end
@@ -190,6 +194,8 @@ module SortingHelper
       sort_value_milestone => sort_value_milestone_later,
       sort_value_due_date => sort_value_due_date_later,
       sort_value_due_date_soon => sort_value_due_date_later,
+      sort_value_merged_date => sort_value_merged_recently,
+      sort_value_merged_earlier => sort_value_merged_recently,
       sort_value_popularity => sort_value_least_popular,
       sort_value_most_popular => sort_value_least_popular
     }.merge(issuable_sort_option_overrides)
@@ -210,7 +216,7 @@ module SortingHelper
 
   def sort_direction_icon(sort_value)
     case sort_value
-    when sort_value_milestone, sort_value_due_date, /_asc\z/
+    when sort_value_milestone, sort_value_due_date, sort_value_merged_date, /_asc\z/
       'sort-lowest'
     else
       'sort-highest'

app/helpers/sorting_titles_values_helper.rb

@@ -26,6 +26,18 @@ module SortingTitlesValuesHelper
     s_('SortOptions|Label priority')
   end
 
+  def sort_title_merged_date
+    s_('SortOptions|Merged date')
+  end
+
+  def sort_title_merged_recently
+    s_('SortOptions|Merged recently')
+  end
+
+  def sort_title_merged_earlier
+    s_('SortOptions|Merged earlier')
+  end
+
   def sort_title_largest_group
     s_('SortOptions|Largest group')
   end
@@ -175,6 +187,18 @@ module SortingTitlesValuesHelper
     'label_priority'
   end
 
+  def sort_value_merged_date
+    'merged_at'
+  end
+
+  def sort_value_merged_recently
+    'merged_at_desc'
+  end
+
+  def sort_value_merged_earlier
+    'merged_at_asc'
+  end
+
   def sort_value_largest_group
     'storage_size_desc'
   end

app/models/merge_request.rb

@@ -300,6 +300,11 @@ class MergeRequest < ApplicationRecord
 
     query = joins(:metrics)
 
+    if !target_project_id && self.where_values_hash["target_project_id"]
+      target_project_id = self.where_values_hash["target_project_id"]
+      query = query.unscope(where: :target_project_id)
+    end
+
     project_condition = if target_project_id
                           MergeRequest::Metrics.arel_table[:target_project_id].eq(target_project_id)
                         else

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

 - sort_value = @sort
 - sort_title = issuable_sort_option_title(sort_value)
 - viewing_issues = controller.controller_name == 'issues' || controller.action_name == 'issues'
+- viewing_merge_requests = controller.controller_name == 'merge_requests'
 
 .dropdown.inline.gl-ml-3.issue-sort-dropdown
   .btn-group{ role: 'group' }
@@ -17,6 +18,7 @@
           = sortable_item(sort_title_due_date,          page_filter_path(sort: sort_value_due_date),          sort_title) if viewing_issues
           = sortable_item(sort_title_popularity,        page_filter_path(sort: sort_value_popularity),        sort_title)
           = sortable_item(sort_title_label_priority,    page_filter_path(sort: sort_value_label_priority),    sort_title)
+          = sortable_item(sort_title_merged_date,       page_filter_path(sort: sort_value_merged_date),       sort_title) if viewing_merge_requests
           = sortable_item(sort_title_relative_position, page_filter_path(sort: sort_value_relative_position), sort_title) if viewing_issues
           = render_if_exists('shared/ee/issuable/sort_dropdown', viewing_issues: viewing_issues, sort_title: sort_title)
     = issuable_sort_direction_button(sort_value)

locale/gitlab.pot

@@ -30665,6 +30665,15 @@ msgstr ""
 msgid "SortOptions|Manual"
 msgstr ""
 
+msgid "SortOptions|Merged date"
+msgstr ""
+
+msgid "SortOptions|Merged earlier"
+msgstr ""
+
+msgid "SortOptions|Merged recently"
+msgstr ""
+
 msgid "SortOptions|Milestone due date"
 msgstr ""
 

spec/features/merge_requests/user_lists_merge_requests_spec.rb

@@ -23,7 +23,9 @@ RSpec.describe 'Merge requests > User lists merge requests' do
                   milestone: create(:milestone, project: project, due_date: '2013-12-11'),
                   created_at: 1.minute.ago,
                   updated_at: 1.minute.ago)
-    create(:merge_request,
+    @fix.metrics.update_column(:merged_at, 10.seconds.ago)
+
+    @markdown = create(:merge_request,
            title: 'markdown',
            source_project: project,
            source_branch: 'markdown',
@@ -32,12 +34,15 @@ RSpec.describe 'Merge requests > User lists merge requests' do
            milestone: create(:milestone, project: project, due_date: '2013-12-12'),
            created_at: 2.minutes.ago,
            updated_at: 2.minutes.ago)
-    create(:merge_request,
+    @markdown.metrics.update_column(:merged_at, 50.seconds.ago)
+
+    @merge_test = create(:merge_request,
            title: 'merge-test',
            source_project: project,
            source_branch: 'merge-test',
            created_at: 3.minutes.ago,
            updated_at: 10.seconds.ago)
+    @merge_test.metrics.update_column(:merged_at, 10.seconds.ago)
   end
 
   context 'merge request reviewers' do
@@ -102,6 +107,13 @@ RSpec.describe 'Merge requests > User lists merge requests' do
     expect(count_merge_requests).to eq(3)
   end
 
+  it 'sorts by merged at' do
+    visit_merge_requests(project, sort: sort_value_merged_date)
+
+    expect(first_merge_request).to include('markdown')
+    expect(count_merge_requests).to eq(3)
+  end
+
   it 'filters on one label and sorts by due date' do
     label = create(:label, project: project)
     create(:label_link, label: label, target: @fix)

spec/models/merge_request_spec.rb

@@ -441,6 +441,22 @@ RSpec.describe MergeRequest, factory_default: :keep do
     end
   end
 
+  describe '.join_metrics' do
+    let_it_be(:join_condition) { '"merge_request_metrics"."target_project_id" = 1' }
+
+    context 'when a no target_project_id is available' do
+      it 'moves target_project_id condition to the merge request metrics' do
+        expect(described_class.join_metrics(1).to_sql).to include(join_condition)
+      end
+    end
+
+    context 'when a target_project_id is present in the where conditions' do
+      it 'moves target_project_id condition to the merge request metrics' do
+        expect(described_class.where(target_project_id: 1).join_metrics.to_sql).to include(join_condition)
+      end
+    end
+  end
+
   describe '.by_related_commit_sha' do
     subject { described_class.by_related_commit_sha(sha) }