diff --git a/app/finders/notes_finder.rb b/app/finders/notes_finder.rb index 412f57b86d21cbbcdaabe00922d1e86846121cbb..2f88a787f258abf1122dc7bf21740c92d9d2bc7d 100644 --- a/app/finders/notes_finder.rb +++ b/app/finders/notes_finder.rb @@ -23,6 +23,7 @@ def initialize(current_user, params = {}) @project = params[:project] @current_user = current_user @params = params.dup + @organization_id = @params.delete(:organization_id) @target_type = @params[:target_type] end @@ -118,9 +119,9 @@ def noteables_for_type(noteable_type) when "merge_request" MergeRequestsFinder.new(@current_user, project_id: @project.id).execute # rubocop: disable CodeReuse/Finder when "snippet", "project_snippet" - SnippetsFinder.new(@current_user, project: @project).execute # rubocop: disable CodeReuse/Finder + SnippetsFinder.new(@current_user, organization_id: @organization_id, project: @project).execute # rubocop: disable CodeReuse/Finder when "personal_snippet" - SnippetsFinder.new(@current_user, only_personal: true).execute # rubocop: disable CodeReuse/Finder + SnippetsFinder.new(@current_user, organization_id: @organization_id, only_personal: true).execute # rubocop: disable CodeReuse/Finder when "wiki_page/meta" WikiPage::Meta.for_project(@project) else diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb index b536ce2b3b559a448834f205c60603746a08437c..1a3bab3b472d48ef82b85dbc4e7e442a87d67024 100644 --- a/app/services/projects/autocomplete_service.rb +++ b/app/services/projects/autocomplete_service.rb @@ -38,7 +38,13 @@ def commands(noteable) end def snippets - SnippetsFinder.new(current_user, project: project).execute.select([:id, :title]) + # rubocop: disable Gitlab/AvoidCurrentOrganization -- Will be refactored to accept organization_id parameter + SnippetsFinder.new( + current_user, + organization_id: Current.organization.id, + project: project + ).execute.select([:id, :title]) + # rubocop: enable Gitlab/AvoidCurrentOrganization end def wikis diff --git a/app/services/snippets/count_service.rb b/app/services/snippets/count_service.rb index c28768aea3e7273728eda91f9a2394c5ed6e3ea0..112a4dac6ef0bc7293e47eedd1d45db975fca595 100644 --- a/app/services/snippets/count_service.rb +++ b/app/services/snippets/count_service.rb @@ -34,14 +34,19 @@ # Either a project or an author *must* be supplied. module Snippets class CountService - def initialize(current_user, author: nil, project: nil) + def initialize(current_user, organization_id: nil, author: nil, project: nil) if !author && !project raise( ArgumentError, 'Must provide either an author or a project' ) end - @snippets_finder = SnippetsFinder.new(current_user, author: author, project: project) + @snippets_finder = SnippetsFinder.new( + current_user, + organization_id: organization_id, + author: author, + project: project + ) end def execute diff --git a/ee/lib/ee/gitlab/snippet_search_results.rb b/ee/lib/ee/gitlab/snippet_search_results.rb index 0311f8882523e3f83970ff930fdc3d78cb77ac6e..8bd99dfb4c8e3cfeeba7a27f07c75ec308d39ab9 100644 --- a/ee/lib/ee/gitlab/snippet_search_results.rb +++ b/ee/lib/ee/gitlab/snippet_search_results.rb @@ -9,9 +9,10 @@ module SnippetSearchResults # https://gitlab.com/gitlab-org/gitlab/issues/26123 override :finder_params def finder_params - return super unless ::Gitlab.com? + params = super + return params unless ::Gitlab.com? - { authorized_and_user_personal: true } + params.merge(authorized_and_user_personal: true) end end end diff --git a/ee/spec/lib/ee/gitlab/snippet_search_results_spec.rb b/ee/spec/lib/ee/gitlab/snippet_search_results_spec.rb index 8ddb5f25dffead7f79f45474deeba19432034321..7034b4d7107632f75a47d3115a5f69629b75bc7a 100644 --- a/ee/spec/lib/ee/gitlab/snippet_search_results_spec.rb +++ b/ee/spec/lib/ee/gitlab/snippet_search_results_spec.rb @@ -1,7 +1,13 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe Gitlab::SnippetSearchResults do +RSpec.describe Gitlab::SnippetSearchResults, :with_current_organization, feature_category: :global_search do + before do + # Since this doesn't go through a request flow, we need to manually set Current.organization + Current.organization = current_organization + allow(Gitlab).to receive(:com?).and_return(com_value) + end + let_it_be(:snippet) { create(:project_snippet, title: 'foo', description: 'foo') } let(:user) { snippet.author } @@ -9,13 +15,12 @@ subject { described_class.new(user, 'foo').objects('snippet_titles') } - before do - allow(Gitlab).to receive(:com?).and_return(com_value) - end - context 'when all requirements are met' do it 'calls the finder with the restrictive scope' do - expect(SnippetsFinder).to receive(:new).with(user, { authorized_and_user_personal: true }).and_call_original + expect(SnippetsFinder).to receive(:new).with( + user, + { authorized_and_user_personal: true, organization_id: current_organization.id } + ).and_call_original subject end @@ -25,7 +30,10 @@ let(:com_value) { false } it 'calls the finder with the restrictive scope' do - expect(SnippetsFinder).to receive(:new).with(user, {}).and_call_original + expect(SnippetsFinder).to receive(:new).with( + user, + { organization_id: current_organization.id } + ).and_call_original subject end diff --git a/lib/gitlab/snippet_search_results.rb b/lib/gitlab/snippet_search_results.rb index b77f48d1a2c82027cdeb4b184ecdfbf9eba1a40f..241fde412ea09423e14d1f61460638c1a6f4303e 100644 --- a/lib/gitlab/snippet_search_results.rb +++ b/lib/gitlab/snippet_search_results.rb @@ -36,7 +36,9 @@ def paginated_objects(relation, page, per_page) end def finder_params - {} + # rubocop: disable Gitlab/AvoidCurrentOrganization -- Will be refactored to accept organization_id parameter + { organization_id: ::Current.organization.id } + # rubocop: enable Gitlab/AvoidCurrentOrganization end end end diff --git a/spec/finders/notes_finder_spec.rb b/spec/finders/notes_finder_spec.rb index ea4aa2ba31ecbc8446c6693f5ea6f71b70b4a29b..22995a35f16e9a7d6e4e6bb86b157ceef2854e85 100644 --- a/spec/finders/notes_finder_spec.rb +++ b/spec/finders/notes_finder_spec.rb @@ -2,12 +2,13 @@ require 'spec_helper' -RSpec.describe NotesFinder do +RSpec.describe NotesFinder, :with_current_organization, feature_category: :team_planning do let(:user) { create(:user) } let(:project) { create(:project) } before do project.add_maintainer(user) + Current.organization = current_organization end describe '#execute' do @@ -273,6 +274,33 @@ end end end + + context 'organization_id parameter for snippet noteables' do + %w[snippet project_snippet personal_snippet].each do |noteable_type| + context "when target_type is #{noteable_type}" do + let(:snippet) do + case noteable_type + when 'snippet', 'project_snippet' + create(:project_snippet, project: project) + when 'personal_snippet' + create(:personal_snippet, author: user) + end + end + + let(:note) { create(:note, noteable: snippet, project: noteable_type == 'personal_snippet' ? nil : project) } + let(:params) { { organization_id: current_organization.id, project: project, target_type: noteable_type, target_id: note.noteable.id } } + + it 'passes organization_id to SnippetsFinder' do + expect(SnippetsFinder).to receive(:new).with( + user, + hash_including(organization_id: current_organization.id) + ).and_call_original + + described_class.new(user, params).execute + end + end + end + end end context 'for explicit target' do diff --git a/spec/lib/gitlab/snippet_search_results_spec.rb b/spec/lib/gitlab/snippet_search_results_spec.rb index 9bb6975ede05d23c76b10dadefa2cb4033891b28..4ea7e6686c2c3dbbb932f891e9ee9c495869291b 100644 --- a/spec/lib/gitlab/snippet_search_results_spec.rb +++ b/spec/lib/gitlab/snippet_search_results_spec.rb @@ -2,9 +2,14 @@ require 'spec_helper' -RSpec.describe Gitlab::SnippetSearchResults do +RSpec.describe Gitlab::SnippetSearchResults, :with_current_organization, feature_category: :global_search do include SearchHelpers + before do + # Since this doesn't go through a request flow, we need to manually set Current.organization + Current.organization = current_organization + end + let_it_be(:snippet) { create(:personal_snippet, content: 'foo', file_name: 'foo') } let(:results) { described_class.new(snippet.author, 'foo') } @@ -37,4 +42,10 @@ expect(results.objects('snippet_titles', page: 1, per_page: 2).count).to eq(2) end end + + describe '#finder_params' do + it 'includes organization_id' do + expect(results.send(:finder_params)).to include(organization_id: current_organization.id) + end + end end diff --git a/spec/services/projects/autocomplete_service_spec.rb b/spec/services/projects/autocomplete_service_spec.rb index 1c3cd950b9b0081cbcad3b9dda52f1f2c1b2b252..0e57ca2cdf06b3744628f7362a4a29d592d69055 100644 --- a/spec/services/projects/autocomplete_service_spec.rb +++ b/spec/services/projects/autocomplete_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::AutocompleteService, feature_category: :groups_and_projects do +RSpec.describe Projects::AutocompleteService, :with_current_organization, feature_category: :groups_and_projects do let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, :public, group: group) } let_it_be(:owner) { create(:user, owner_of: project) } @@ -364,4 +364,22 @@ def expect_labels_to_equal(labels, expected_labels) end end end + + describe '#snippets' do + let_it_be(:user) { create(:user) } + + before do + # Since this doesn't go through a request flow, we need to manually set Current.organization + Current.organization = current_organization + end + + it 'passes organization_id to SnippetsFinder' do + expect(SnippetsFinder).to receive(:new).with( + user, + hash_including(organization_id: current_organization.id) + ).and_call_original + + described_class.new(project, user).snippets + end + end end diff --git a/spec/services/snippets/count_service_spec.rb b/spec/services/snippets/count_service_spec.rb index 4ad9b07d5189f11b7dd2e61c9bfd9c198cc2978a..15bd6854e13b8c97a2586dd736702db078641b8c 100644 --- a/spec/services/snippets/count_service_spec.rb +++ b/spec/services/snippets/count_service_spec.rb @@ -2,7 +2,12 @@ require 'spec_helper' -RSpec.describe Snippets::CountService, feature_category: :source_code_management do +RSpec.describe Snippets::CountService, :with_current_organization, feature_category: :source_code_management do + before do + # Since this doesn't go through a request flow, we need to manually set Current.organization + Current.organization = current_organization + end + let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project, :public) } @@ -14,17 +19,17 @@ it 'uses the SnippetsFinder to scope snippets by user' do expect(SnippetsFinder) .to receive(:new) - .with(user, author: user, project: nil) + .with(user, author: user, project: nil, organization_id: current_organization.id) - described_class.new(user, author: user) + described_class.new(user, organization_id: current_organization.id, author: user) end it 'allows scoping to project' do expect(SnippetsFinder) .to receive(:new) - .with(user, author: nil, project: project) + .with(user, author: nil, project: project, organization_id: current_organization.id) - described_class.new(user, project: project) + described_class.new(user, organization_id: current_organization.id, project: project) end end