Merge branch '259608-add-autocomplete-suggestions-for-epics' into 'master'

Add search autocomplete suggestions for recently viewed epics

Closes #259608

See merge request gitlab-org/gitlab!43964

app/helpers/search_helper.rb

@@ -7,8 +7,7 @@ module SearchHelper
     return unless current_user
 
     resources_results = [
-      recent_merge_requests_autocomplete(term),
-      recent_issues_autocomplete(term),
+      recent_items_autocomplete(term),
       groups_autocomplete(term),
       projects_autocomplete(term)
     ].flatten
@@ -27,6 +26,10 @@ module SearchHelper
     end
   end
 
+  def recent_items_autocomplete(term)
+    recent_merge_requests_autocomplete(term) + recent_issues_autocomplete(term)
+  end
+
   def search_entries_info(collection, scope, term)
     return if collection.to_a.empty?
 
@@ -186,10 +189,10 @@ module SearchHelper
     end
   end
 
-  def recent_merge_requests_autocomplete(term, limit = 5)
+  def recent_merge_requests_autocomplete(term)
     return [] unless current_user
 
-    ::Gitlab::Search::RecentMergeRequests.new(user: current_user).search(term).limit(limit).map do |mr|
+    ::Gitlab::Search::RecentMergeRequests.new(user: current_user).search(term).map do |mr|
       {
         category: "Recent merge requests",
         id: mr.id,
@@ -200,10 +203,10 @@ module SearchHelper
     end
   end
 
-  def recent_issues_autocomplete(term, limit = 5)
+  def recent_issues_autocomplete(term)
     return [] unless current_user
 
-    ::Gitlab::Search::RecentIssues.new(user: current_user).search(term).limit(limit).map do |i|
+    ::Gitlab::Search::RecentIssues.new(user: current_user).search(term).map do |i|
       {
         category: "Recent issues",
         id: i.id,

doc/user/search/index.md

@@ -209,7 +209,8 @@ 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)
+- Recently viewed merge requests (try and type some word from the title of a recently viewed merge request)
+- Recently viewed epics (try and type some word from the title of a recently viewed epic)
 
 ## Basic search
 

ee/app/controllers/groups/epics_controller.rb

@@ -15,6 +15,7 @@ class Groups::EpicsController < Groups::ApplicationController
   before_action :authorize_update_issuable!, only: :update
   before_action :authorize_create_epic!, only: [:create, :new]
   before_action :verify_group_bulk_edit_enabled!, only: [:bulk_update]
+  after_action :log_epic_show, only: :show
 
   before_action do
     push_frontend_feature_flag(:vue_issuable_epic_sidebar, @group)
@@ -113,6 +114,12 @@ class Groups::EpicsController < Groups::ApplicationController
     @preload_for_collection ||= [:group, :author, :labels]
   end
 
+  def log_epic_show
+    return unless current_user && @epic
+
+    ::Gitlab::Search::RecentEpics.new(user: current_user).log_view(@epic)
+  end
+
   def authorize_create_epic!
     return render_404 unless can?(current_user, :create_epic, group)
   end

ee/app/helpers/ee/search_helper.rb

@@ -19,6 +19,11 @@ module EE
       options
     end
 
+    override :recent_items_autocomplete
+    def recent_items_autocomplete(term)
+      super + recent_epics_autocomplete(term)
+    end
+
     override :search_blob_title
     def search_blob_title(project, path)
       if @project
@@ -72,6 +77,20 @@ module EE
 
     private
 
+    def recent_epics_autocomplete(term)
+      return [] unless current_user
+
+      ::Gitlab::Search::RecentEpics.new(user: current_user).search(term).map do |e|
+        {
+          category: "Recent epics",
+          id: e.id,
+          label: search_result_sanitize(e.title),
+          url: epic_path(e),
+          avatar_url: e.group.avatar_url || ''
+        }
+      end
+    end
+
     def search_multiple_assignees?(type)
       context = @project.presence || @group.presence || :dashboard
 

ee/app/models/ee/epic.rb

@@ -17,6 +17,7 @@ module EE
       include FromUnion
       include EpicTreeSorting
       include Presentable
+      include IdInOrdered
 
       enum state_id: {
         opened: ::Epic.available_states[:opened],

ee/changelogs/unreleased/259608-add-autocomplete-suggestions-for-epics.yml

+---
+title: Add search autocomplete suggestions for recently viewed epics
+merge_request: 43964
+author:
+type: added

ee/lib/gitlab/search/recent_epics.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Search
+    class RecentEpics < RecentItems
+      extend ::Gitlab::Utils::Override
+
+      override :search
+      # rubocop: disable CodeReuse/ActiveRecord
+      def search(term)
+        epics = Epic.full_search(term, matched_columns: 'title')
+          .id_in_ordered(latest_ids).limit(::Gitlab::Search::RecentItems::SEARCH_LIMIT)
+
+        # Since EpicsFinder does not support searching globally (ie. applying
+        # global permissions) the most efficient option is just to load the
+        # last 5 matching recently viewed epics and then do an explicit
+        # permissions check
+        disallowed = epics.reject { |epic| Ability.allowed?(user, :read_epic, epic) }
+
+        return epics if disallowed.empty?
+
+        epics.where.not(id: disallowed.map(&:id))
+      end
+      # rubocop: enable CodeReuse/ActiveRecord
+
+      private
+
+      def type
+        Epic
+      end
+    end
+  end
+end

ee/spec/controllers/groups/epics_controller_spec.rb

@@ -285,6 +285,18 @@ RSpec.describe Groups::EpicsController do
           expect(response).to render_template 'groups/epics/show'
         end
 
+        it 'logs the view with Gitlab::Search::RecentEpics' do
+          group.add_developer(user)
+
+          recent_epics_double = instance_double(::Gitlab::Search::RecentEpics, log_view: nil)
+          expect(::Gitlab::Search::RecentEpics).to receive(:new).with(user: user).and_return(recent_epics_double)
+
+          show_epic
+
+          expect(response).to be_successful
+          expect(recent_epics_double).to have_received(:log_view).with(epic)
+        end
+
         context 'with unauthorized user' do
           it 'returns a not found 404 response' do
             show_epic

ee/spec/helpers/search_helper_spec.rb

@@ -51,6 +51,47 @@ RSpec.describe SearchHelper do
     end
   end
 
+  describe 'search_autocomplete_opts' do
+    context "with a user" do
+      let(:user) { create(:user) }
+
+      before do
+        allow(self).to receive(:current_user).and_return(user)
+      end
+
+      it 'includes the users recently viewed epics' do
+        recent_epics = instance_double(::Gitlab::Search::RecentEpics)
+        expect(::Gitlab::Search::RecentEpics).to receive(:new).with(user: user).and_return(recent_epics)
+        group1 = create(:group, :public, :with_avatar)
+        group2 = create(:group, :public)
+        epic1 = create(:epic, title: 'epic 1', group: group1)
+        epic2 = create(:epic, title: 'epic 2', group: group2)
+
+        expect(recent_epics).to receive(:search).with('the search term').and_return(Epic.id_in_ordered([epic1.id, epic2.id]))
+
+        results = search_autocomplete_opts("the search term")
+
+        expect(results.count).to eq(2)
+
+        expect(results[0]).to include({
+          category: 'Recent epics',
+          id: epic1.id,
+          label: 'epic 1',
+          url: Gitlab::Routing.url_helpers.group_epic_path(epic1.group, epic1),
+          avatar_url: group1.avatar_url
+        })
+
+        expect(results[1]).to include({
+          category: 'Recent epics',
+          id: epic2.id,
+          label: 'epic 2',
+          url: Gitlab::Routing.url_helpers.group_epic_path(epic2.group, epic2),
+          avatar_url: '' # This group didn't have an avatar so set this to ''
+        })
+      end
+    end
+  end
+
   describe '#project_autocomplete' do
     let(:user) { create(:user) }
 

ee/spec/lib/gitlab/search/recent_epics_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ::Gitlab::Search::RecentEpics do
+  let(:parent_type) { :group }
+
+  def create_item(content:, parent:)
+    create(:epic, title: content, group: parent)
+  end
+
+  before do
+    stub_licensed_features(epics: true)
+  end
+
+  it_behaves_like 'search recent items'
+end

lib/gitlab/search/recent_items.rb

@@ -6,9 +6,12 @@ module Gitlab
     # items. The #type and #finder methods are the only ones needed to be
     # implemented by classes inheriting from this.
     class RecentItems
-      ITEMS_LIMIT = 100
+      ITEMS_LIMIT = 100 # How much history to remember from the user
+      SEARCH_LIMIT = 5 # How many matching items to return from search
       EXPIRES_AFTER = 7.days
 
+      attr_reader :user
+
       def initialize(user:, items_limit: ITEMS_LIMIT, expires_after: EXPIRES_AFTER)
         @user = user
         @items_limit = items_limit
@@ -30,21 +33,25 @@ module Gitlab
       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
+        finder.new(user, search: term, in: 'title')
+          .execute
+          .limit(SEARCH_LIMIT).reorder(nil).id_in_ordered(latest_ids) # rubocop: disable CodeReuse/ActiveRecord
       end
 
       private
 
+      def latest_ids
+        with_redis do |redis|
+          redis.zrevrange(key, 0, @items_limit - 1)
+        end.map(&:to_i)
+      end
+
       def with_redis(&blk)
         Gitlab::Redis::SharedState.with(&blk) # rubocop: disable CodeReuse/ActiveRecord
       end
 
       def key
-        "recent_items:#{type.name.downcase}:#{@user.id}"
+        "recent_items:#{type.name.downcase}:#{user.id}"
       end
 
       def type

spec/helpers/search_helper_spec.rb

@@ -73,7 +73,7 @@ RSpec.describe SearchHelper do
         expect(result.keys).to match_array(%i[category id label url avatar_url])
       end
 
-      it 'includes the first 5 of the users recent issues' do
+      it 'includes the users recently viewed issues' do
         recent_issues = instance_double(::Gitlab::Search::RecentIssues)
         expect(::Gitlab::Search::RecentIssues).to receive(:new).with(user: user).and_return(recent_issues)
         project1 = create(:project, :with_avatar, namespace: user.namespace)
@@ -81,13 +81,11 @@ RSpec.describe SearchHelper do
         issue1 = create(:issue, title: 'issue 1', project: project1)
         issue2 = create(:issue, title: 'issue 2', project: project2)
 
-        other_issues = create_list(:issue, 5)
-
-        expect(recent_issues).to receive(:search).with('the search term').and_return(Issue.id_in_ordered([issue1.id, issue2.id, *other_issues.map(&:id)]))
+        expect(recent_issues).to receive(:search).with('the search term').and_return(Issue.id_in_ordered([issue1.id, issue2.id]))
 
         results = search_autocomplete_opts("the search term")
 
-        expect(results.count).to eq(5)
+        expect(results.count).to eq(2)
 
         expect(results[0]).to include({
           category: 'Recent issues',
@@ -106,7 +104,7 @@ RSpec.describe SearchHelper do
         })
       end
 
-      it 'includes the first 5 of the users recent merge requests' do
+      it 'includes the users recently viewed 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)
@@ -114,13 +112,11 @@ RSpec.describe SearchHelper do
         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)]))
+        expect(recent_merge_requests).to receive(:search).with('the search term').and_return(MergeRequest.id_in_ordered([merge_request1.id, merge_request2.id]))
 
         results = search_autocomplete_opts("the search term")
 
-        expect(results.count).to eq(5)
+        expect(results.count).to eq(2)
 
         expect(results[0]).to include({
           category: 'Recent merge requests',

spec/lib/gitlab/search/recent_issues_spec.rb

@@ -3,8 +3,10 @@
 require 'spec_helper'
 
 RSpec.describe ::Gitlab::Search::RecentIssues do
-  def create_item(content:, project:)
-    create(:issue, title: content, project: project)
+  let(:parent_type) { :project }
+
+  def create_item(content:, parent:)
+    create(:issue, title: content, project: parent)
   end
 
   it_behaves_like 'search recent items'

spec/lib/gitlab/search/recent_merge_requests_spec.rb

@@ -3,8 +3,10 @@
 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)
+  let(:parent_type) { :project }
+
+  def create_item(content:, parent:)
+    create(:merge_request, :unique_branches, title: content, target_project: parent, source_project: parent)
   end
 
   it_behaves_like 'search recent items'

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) }
+  let_it_be(:recent_items) { described_class.new(user: user) }
+  let(:item) { create_item(content: 'hello world 1', parent: parent) }
+  let(:parent) { create(parent_type, :public) }
 
   describe '#log_view', :clean_gitlab_redis_shared_state do
     it 'adds the item to the recent items' do
@@ -18,13 +17,15 @@ RSpec.shared_examples 'search recent items' do
     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))
+      recent_items = described_class.new(user: user, items_limit: 3)
+
+      4.times do |i|
+        recent_items.log_view(create_item(content: "item #{i}", parent: parent))
       end
 
       results = recent_items.search('item')
 
-      expect(results.map(&:title)).to contain_exactly('item 6', 'item 5', 'item 4', 'item 3', 'item 2')
+      expect(results.map(&:title)).to contain_exactly('item 3', 'item 2', 'item 1')
     end
 
     it 'expires the items after expires_after' do
@@ -39,7 +40,7 @@ RSpec.shared_examples 'search recent items' do
 
     it 'does not include results logged for another user' do
       another_user = create(:user)
-      another_item = create_item(content: 'hello world 2', project: project)
+      another_item = create_item(content: 'hello world 2', parent: parent)
       described_class.new(user: another_user).log_view(another_item)
       recent_items.log_view(item)
 
@@ -50,11 +51,11 @@ RSpec.shared_examples 'search recent items' do
   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) }
+    let(:item1) { create_item(content: "matching item 1", parent: parent) }
+    let(:item2) { create_item(content: "matching item 2", parent: parent) }
+    let(:item3) { create_item(content: "matching item 3", parent: parent) }
+    let(:non_matching_item) { create_item(content: "different item", parent: parent) }
+    let!(:non_viewed_item) { create_item(content: "matching but not viewed item", parent: parent) }
 
     before do
       recent_items.log_view(item1)
@@ -74,14 +75,24 @@ RSpec.shared_examples 'search recent items' do
     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)
+      private_parent = create(parent_type, :public)
+      private_item = create_item(content: 'matching item title', parent: private_parent)
 
       recent_items.log_view(private_item)
 
-      private_project.update!(visibility_level: Project::PRIVATE)
+      private_parent.update!(visibility_level: ::Gitlab::VisibilityLevel::PRIVATE)
 
       expect(recent_items.search('matching')).not_to include(private_item)
     end
+
+    it "limits results to #{Gitlab::Search::RecentItems::SEARCH_LIMIT} items" do
+      (Gitlab::Search::RecentItems::SEARCH_LIMIT + 1).times do |i|
+        recent_items.log_view(create_item(content: "item #{i}", parent: parent))
+      end
+
+      results = recent_items.search('item')
+
+      expect(results.count).to eq(Gitlab::Search::RecentItems::SEARCH_LIMIT)
+    end
   end
 end