Merge branch '249091-autocomplete-recent-mrs' into 'master'

Add autocomplete search suggestions for recently viewed merge requests

See merge request gitlab-org/gitlab!42560

app/controllers/projects/merge_requests_controller.rb

@@ -50,6 +50,8 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
   around_action :allow_gitaly_ref_name_caching, only: [:index, :show, :discussions]
+  after_action :log_merge_request_show, only: [:show]
+
   feature_category :source_code_management,
                    unless: -> (action) { action.ends_with?("_reports") }
   feature_category :code_testing,
@@ -450,6 +452,12 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     end
   end
+  def log_merge_request_show
+    return unless current_user && @merge_request
+
+    ::Gitlab::Search::RecentMergeRequests.new(user: current_user).log_view(@merge_request)
+  end
+
   def authorize_read_actual_head_pipeline!
     return render_404 unless can?(current_user, :read_build, merge_request.actual_head_pipeline)
   end

app/helpers/search_helper.rb

@@ -7,6 +7,7 @@ module SearchHelper
     return unless current_user
     resources_results = [
+      recent_merge_requests_autocomplete(term),
       recent_issues_autocomplete(term),
       groups_autocomplete(term),
       projects_autocomplete(term)
@@ -180,6 +181,20 @@ module SearchHelper
     end
   end
+  def recent_merge_requests_autocomplete(term, limit = 5)
+    return [] unless current_user
+
+    ::Gitlab::Search::RecentMergeRequests.new(user: current_user).search(term).limit(limit).map do |mr|
+      {
+        category: "Recent merge requests",
+        id: mr.id,
+        label: search_result_sanitize(mr.title),
+        url: merge_request_path(mr),
+        avatar_url: mr.project.avatar_url || ''
+      }
+    end
+  end
+
   def recent_issues_autocomplete(term, limit = 5)
     return [] unless current_user

app/models/merge_request.rb

@@ -21,6 +21,7 @@ class MergeRequest < ApplicationRecord
   include MilestoneEventable
   include StateEventable
   include ApprovableBase
+  include IdInOrdered
   extend ::Gitlab::Utils::Override

changelogs/unreleased/249091-autocomplete-recent-mrs.yml

+---
+title: Add autocomplete search suggestions for recent merge requests
+merge_request: 42560
+author:
+type: added

doc/user/search/index.md

@@ -171,6 +171,7 @@ You can also type in this search bar to see autocomplete suggestions for:
 - Project feature pages (try and type **milestones**)
 - Various settings pages (try and type **user settings**)
 - Recently viewed issues (try and type some word from the title of a recently viewed issue)
+- Recently viewed merge requests (try and type some word from the title of a recently merge request)
 ## To-Do List

lib/gitlab/search/recent_issues.rb

@@ -2,51 +2,16 @@
 module Gitlab
   module Search
-    class RecentIssues
-      ITEMS_LIMIT = 100
-      EXPIRES_AFTER = 7.days
-
-      def initialize(user:, items_limit: ITEMS_LIMIT, expires_after: EXPIRES_AFTER)
-        @user = user
-        @items_limit = items_limit
-        @expires_after = expires_after
-      end
-
-      def log_view(issue)
-        with_redis do |redis|
-          redis.zadd(key, Time.now.to_f, issue.id)
-          redis.expire(key, @expires_after)
-
-          # There is a race condition here where we could end up removing an
-          # item from 2 places concurrently but this is fine since worst case
-          # scenario we remove an extra item from the end of the list.
-          if redis.zcard(key) > @items_limit
-            redis.zremrangebyrank(key, 0, 0) # Remove least recent
-          end
-        end
-      end
-
-      def search(term)
-        ids = with_redis do |redis|
-          redis.zrevrange(key, 0, @items_limit - 1)
-        end.map(&:to_i)
-
-        IssuesFinder.new(@user, search: term, in: 'title').execute.reorder(nil).id_in_ordered(ids) # rubocop: disable CodeReuse/ActiveRecord
-      end
-
+    class RecentIssues < RecentItems
       private
-      def with_redis(&blk)
-        Gitlab::Redis::SharedState.with(&blk) # rubocop: disable CodeReuse/ActiveRecord
-      end
-
-      def key
-        "recent_items:#{type.name.downcase}:#{@user.id}"
-      end
-
       def type
         Issue
       end
+
+      def finder
+        IssuesFinder
+      end
     end
   end
 end

lib/gitlab/search/recent_items.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Search
+    # This is an abstract class used for storing/searching recently viewed
+    # items. The #type and #finder methods are the only ones needed to be
+    # implemented by classes inheriting from this.
+    class RecentItems
+      ITEMS_LIMIT = 100
+      EXPIRES_AFTER = 7.days
+
+      def initialize(user:, items_limit: ITEMS_LIMIT, expires_after: EXPIRES_AFTER)
+        @user = user
+        @items_limit = items_limit
+        @expires_after = expires_after
+      end
+
+      def log_view(item)
+        with_redis do |redis|
+          redis.zadd(key, Time.now.to_f, item.id)
+          redis.expire(key, @expires_after)
+
+          # There is a race condition here where we could end up removing an
+          # item from 2 places concurrently but this is fine since worst case
+          # scenario we remove an extra item from the end of the list.
+          if redis.zcard(key) > @items_limit
+            redis.zremrangebyrank(key, 0, 0) # Remove least recent
+          end
+        end
+      end
+
+      def search(term)
+        ids = with_redis do |redis|
+          redis.zrevrange(key, 0, @items_limit - 1)
+        end.map(&:to_i)
+
+        finder.new(@user, search: term, in: 'title').execute.reorder(nil).id_in_ordered(ids) # rubocop: disable CodeReuse/ActiveRecord
+      end
+
+      private
+
+      def with_redis(&blk)
+        Gitlab::Redis::SharedState.with(&blk) # rubocop: disable CodeReuse/ActiveRecord
+      end
+
+      def key
+        "recent_items:#{type.name.downcase}:#{@user.id}"
+      end
+
+      def type
+        raise NotImplementedError
+      end
+
+      def finder
+        raise NotImplementedError
+      end
+    end
+  end
+end

lib/gitlab/search/recent_merge_requests.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Search
+    class RecentMergeRequests < RecentItems
+      private
+
+      def type
+        MergeRequest
+      end
+
+      def finder
+        MergeRequestsFinder
+      end
+    end
+  end
+end

spec/controllers/projects/issues_controller_spec.rb

@@ -1029,7 +1029,8 @@ RSpec.describe Projects::IssuesController do
         go(id: issue.to_param)
-        expect(recent_issues_double).to have_received(:log_view)
+        expect(response).to be_successful
+        expect(recent_issues_double).to have_received(:log_view).with(issue)
       end
       context 'when not logged in' do

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -123,6 +123,16 @@ RSpec.describe Projects::MergeRequestsController do
         expect(response).to be_successful
       end
+      it 'logs the view with Gitlab::Search::RecentMergeRequests' do
+        recent_merge_requests_double = instance_double(::Gitlab::Search::RecentMergeRequests, log_view: nil)
+        expect(::Gitlab::Search::RecentMergeRequests).to receive(:new).with(user: user).and_return(recent_merge_requests_double)
+
+        go(format: :html)
+
+        expect(response).to be_successful
+        expect(recent_merge_requests_double).to have_received(:log_view).with(merge_request)
+      end
+
       context "that is invalid" do
         let(:merge_request) { create(:invalid_merge_request, target_project: project, source_project: project) }

spec/helpers/search_helper_spec.rb

@@ -106,6 +106,39 @@ RSpec.describe SearchHelper do
         })
       end
+      it 'includes the first 5 of the users recent merge requests' do
+        recent_merge_requests = instance_double(::Gitlab::Search::RecentMergeRequests)
+        expect(::Gitlab::Search::RecentMergeRequests).to receive(:new).with(user: user).and_return(recent_merge_requests)
+        project1 = create(:project, :with_avatar, namespace: user.namespace)
+        project2 = create(:project, namespace: user.namespace)
+        merge_request1 = create(:merge_request, :unique_branches, title: 'Merge request 1', target_project: project1, source_project: project1)
+        merge_request2 = create(:merge_request, :unique_branches, title: 'Merge request 2', target_project: project2, source_project: project2)
+
+        other_merge_requests = create_list(:merge_request, 5)
+
+        expect(recent_merge_requests).to receive(:search).with('the search term').and_return(MergeRequest.id_in_ordered([merge_request1.id, merge_request2.id, *other_merge_requests.map(&:id)]))
+
+        results = search_autocomplete_opts("the search term")
+
+        expect(results.count).to eq(5)
+
+        expect(results[0]).to include({
+          category: 'Recent merge requests',
+          id: merge_request1.id,
+          label: 'Merge request 1',
+          url: Gitlab::Routing.url_helpers.project_merge_request_path(merge_request1.project, merge_request1),
+          avatar_url: project1.avatar_url
+        })
+
+        expect(results[1]).to include({
+          category: 'Recent merge requests',
+          id: merge_request2.id,
+          label: 'Merge request 2',
+          url: Gitlab::Routing.url_helpers.project_merge_request_path(merge_request2.project, merge_request2),
+          avatar_url: '' # This project didn't have an avatar so set this to ''
+        })
+      end
+
       it "does not include the public group" do
         group = create(:group)
         expect(search_autocomplete_opts(group.name).size).to eq(0)

spec/lib/gitlab/search/recent_issues_spec.rb

@@ -2,86 +2,10 @@
 require 'spec_helper'
-RSpec.describe ::Gitlab::Search::RecentIssues, :clean_gitlab_redis_shared_state do
-  let(:user) { create(:user) }
-  let(:issue) { create(:issue, title: 'hello world 1', project: project) }
-  let(:recent_issues) { described_class.new(user: user, items_limit: 5) }
-  let(:project) { create(:project, :public) }
-
-  describe '#log_viewing' do
-    it 'adds the item to the recent items' do
-      recent_issues.log_view(issue)
-
-      results = recent_issues.search('hello')
-
-      expect(results).to eq([issue])
-    end
-
-    it 'removes an item when it exceeds the size items_limit' do
-      (1..6).each do |i|
-        recent_issues.log_view(create(:issue, title: "issue #{i}", project: project))
-      end
-
-      results = recent_issues.search('issue')
-
-      expect(results.map(&:title)).to contain_exactly('issue 6', 'issue 5', 'issue 4', 'issue 3', 'issue 2')
-    end
-
-    it 'expires the items after expires_after' do
-      recent_issues = described_class.new(user: user, expires_after: 0)
-
-      recent_issues.log_view(issue)
-
-      results = recent_issues.search('hello')
-
-      expect(results).to be_empty
-    end
-
-    it 'does not include results logged for another user' do
-      another_user = create(:user)
-      another_issue = create(:issue, title: 'hello world 2', project: project)
-      described_class.new(user: another_user).log_view(another_issue)
-      recent_issues.log_view(issue)
-
-      results = recent_issues.search('hello')
-
-      expect(results).to eq([issue])
-    end
+RSpec.describe ::Gitlab::Search::RecentIssues do
+  def create_item(content:, project:)
+    create(:issue, title: content, project: project)
   end
-  describe '#search' do
-    let(:issue1) { create(:issue, title: "matching issue 1", project: project) }
-    let(:issue2) { create(:issue, title: "matching issue 2", project: project) }
-    let(:issue3) { create(:issue, title: "matching issue 3", project: project) }
-    let(:non_matching_issue) { create(:issue, title: "different issue", project: project) }
-    let!(:non_viewed_issued) { create(:issue, title: "matching but not viewed issue", project: project) }
-
-    before do
-      recent_issues.log_view(issue1)
-      recent_issues.log_view(issue2)
-      recent_issues.log_view(issue3)
-      recent_issues.log_view(non_matching_issue)
-    end
-
-    it 'matches partial text in the issue title' do
-      expect(recent_issues.search('matching')).to contain_exactly(issue1, issue2, issue3)
-    end
-
-    it 'returns results sorted by recently viewed' do
-      recent_issues.log_view(issue2)
-
-      expect(recent_issues.search('matching')).to eq([issue2, issue3, issue1])
-    end
-
-    it 'does not leak issues you no longer have access to' do
-      private_project = create(:project, :public, namespace: create(:group))
-      private_issue = create(:issue, project: private_project, title: 'matching issue title')
-
-      recent_issues.log_view(private_issue)
-
-      private_project.update!(visibility_level: Project::PRIVATE)
-
-      expect(recent_issues.search('matching')).not_to include(private_issue)
-    end
-  end
+  it_behaves_like 'search recent items'
 end

spec/lib/gitlab/search/recent_merge_requests_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ::Gitlab::Search::RecentMergeRequests do
+  def create_item(content:, project:)
+    create(:merge_request, :unique_branches, title: content, target_project: project, source_project: project)
+  end
+
+  it_behaves_like 'search recent items'
+end

spec/support/shared_examples/lib/gitlab/search/recent_items.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.shared_examples 'search recent items' do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:recent_items) { described_class.new(user: user, items_limit: 5) }
+  let(:item) { create_item(content: 'hello world 1', project: project) }
+  let(:project) { create(:project, :public) }
+
+  describe '#log_view', :clean_gitlab_redis_shared_state do
+    it 'adds the item to the recent items' do
+      recent_items.log_view(item)
+
+      results = recent_items.search('hello')
+
+      expect(results).to eq([item])
+    end
+
+    it 'removes an item when it exceeds the size items_limit' do
+      (1..6).each do |i|
+        recent_items.log_view(create_item(content: "item #{i}", project: project))
+      end
+
+      results = recent_items.search('item')
+
+      expect(results.map(&:title)).to contain_exactly('item 6', 'item 5', 'item 4', 'item 3', 'item 2')
+    end
+
+    it 'expires the items after expires_after' do
+      recent_items = described_class.new(user: user, expires_after: 0)
+
+      recent_items.log_view(item)
+
+      results = recent_items.search('hello')
+
+      expect(results).to be_empty
+    end
+
+    it 'does not include results logged for another user' do
+      another_user = create(:user)
+      another_item = create_item(content: 'hello world 2', project: project)
+      described_class.new(user: another_user).log_view(another_item)
+      recent_items.log_view(item)
+
+      results = recent_items.search('hello')
+
+      expect(results).to eq([item])
+    end
+  end
+
+  describe '#search', :clean_gitlab_redis_shared_state do
+    let(:item1) { create_item(content: "matching item 1", project: project) }
+    let(:item2) { create_item(content: "matching item 2", project: project) }
+    let(:item3) { create_item(content: "matching item 3", project: project) }
+    let(:non_matching_item) { create_item(content: "different item", project: project) }
+    let!(:non_viewed_item) { create_item(content: "matching but not viewed item", project: project) }
+
+    before do
+      recent_items.log_view(item1)
+      recent_items.log_view(item2)
+      recent_items.log_view(item3)
+      recent_items.log_view(non_matching_item)
+    end
+
+    it 'matches partial text in the item title' do
+      expect(recent_items.search('matching')).to contain_exactly(item1, item2, item3)
+    end
+
+    it 'returns results sorted by recently viewed' do
+      recent_items.log_view(item2)
+
+      expect(recent_items.search('matching')).to eq([item2, item3, item1])
+    end
+
+    it 'does not leak items you no longer have access to' do
+      private_project = create(:project, :public, namespace: create(:group))
+      private_item = create_item(content: 'matching item title', project: private_project)
+
+      recent_items.log_view(private_item)
+
+      private_project.update!(visibility_level: Project::PRIVATE)
+
+      expect(recent_items.search('matching')).not_to include(private_item)
+    end
+  end
+end