Add latest changes from gitlab-org/gitlab@master

9 files changed, 205 insertions, 55 deletions

diff --git a/spec/features/groups/import_export/connect_instance_spec.rb b/spec/features/groups/import_export/connect_instance_spec.rb
index 552b599a3f3..ae03e64cf59 100644
--- a/spec/features/groups/import_export/connect_instance_spec.rb
+++ b/spec/features/groups/import_export/connect_instance_spec.rb
@@ -34,7 +34,7 @@ RSpec.describe 'Import/Export - Connect to another instance', :js do
 
       click_on 'Connect instance'
 
-      expect(page).to have_content 'Showing 1-1 of 42 groups from %{url}' % { url: source_url }
+      expect(page).to have_content 'Showing 1-1 of 42 groups that you own from %{url}' % { url: source_url }
       expect(page).to have_content 'stub-group'
 
       visit '/'
diff --git a/spec/frontend/import_entities/import_groups/components/import_table_spec.js b/spec/frontend/import_entities/import_groups/components/import_table_spec.js
index 1939e43e5dc..0279ad454d2 100644
--- a/spec/frontend/import_entities/import_groups/components/import_table_spec.js
+++ b/spec/frontend/import_entities/import_groups/components/import_table_spec.js
@@ -122,7 +122,7 @@ describe('import table', () => {
       });
       await waitForPromises();
 
-      expect(wrapper.find(GlEmptyState).props().title).toBe('You have no groups to import');
+      expect(wrapper.find(GlEmptyState).props().title).toBe(i18n.NO_GROUPS_FOUND);
     });
   });
 
@@ -297,7 +297,7 @@ describe('import table', () => {
       wrapper.find(PaginationLinks).props().change(REQUESTED_PAGE);
       await waitForPromises();
 
-      expect(wrapper.text()).toContain('Showing 21-21 of 38 groups from');
+      expect(wrapper.text()).toContain('Showing 21-21 of 38 groups that you own from');
     });
   });
 
@@ -349,7 +349,9 @@ describe('import table', () => {
       await setFilter(FILTER_VALUE);
       await waitForPromises();
 
-      expect(wrapper.text()).toContain('Showing 1-1 of 40 groups matching filter "foo" from');
+      expect(wrapper.text()).toContain(
+        'Showing 1-1 of 40 groups that you own matching filter "foo" from',
+      );
     });
 
     it('properly resets filter in graphql query when search box is cleared', async () => {
diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb
index 5d9963bf3ea..3fe3e37b57f 100644
--- a/spec/lib/gitlab/database/migration_helpers_spec.rb
+++ b/spec/lib/gitlab/database/migration_helpers_spec.rb
@@ -2079,6 +2079,8 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
         t.integer :other_id
         t.timestamps
       end
+
+      allow(model).to receive(:transaction_open?).and_return(false)
     end
 
     context 'when the target table does not exist' do
@@ -2191,6 +2193,8 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
         t.timestamps
       end
 
+      allow(model).to receive(:transaction_open?).and_return(false)
+
       model.initialize_conversion_of_integer_to_bigint(table, columns, primary_key: primary_key)
       model.backfill_conversion_of_integer_to_bigint(table, columns, primary_key: primary_key)
     end
@@ -2246,6 +2250,10 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
       configuration.merge(gitlab_schema: Gitlab::Database.gitlab_schemas_for_connection(model.connection).first)
     end
 
+    before do
+      allow(model).to receive(:transaction_open?).and_return(false)
+    end
+
     subject(:ensure_batched_background_migration_is_finished) { model.ensure_batched_background_migration_is_finished(**configuration) }
 
     it 'raises an error when migration exists and is not marked as finished' do
diff --git a/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb b/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb
index 87b6c65bf7e..daddc510963 100644
--- a/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb
+++ b/spec/lib/gitlab/database/migrations/batched_background_migration_helpers_spec.rb
@@ -13,6 +13,8 @@ RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers d
     before do
       allow(Gitlab::Database::PgClass).to receive(:for_table).and_call_original
       expect(Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas).to receive(:require_dml_mode!)
+
+      allow(migration).to receive(:transaction_open?).and_return(false)
     end
 
     context 'when such migration already exists' do
@@ -165,6 +167,17 @@ RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers d
 
           expect(Gitlab::Database::BackgroundMigration::BatchedMigration.last).to be_finished
         end
+
+        context 'when within transaction' do
+          before do
+            allow(migration).to receive(:transaction_open?).and_return(true)
+          end
+
+          it 'does raise an exception' do
+            expect { migration.queue_batched_background_migration('MyJobClass', :events, :id, job_interval: 5.minutes)}
+              .to raise_error /`queue_batched_background_migration` cannot be run inside a transaction./
+          end
+        end
       end
     end
 
@@ -188,6 +201,8 @@ RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers d
 
     before do
       expect(Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas).to receive(:require_dml_mode!)
+
+      allow(migration).to receive(:transaction_open?).and_return(false)
     end
 
     it 'finalizes the migration' do
@@ -226,6 +241,17 @@ RSpec.describe Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers d
         end.to raise_error /is currently not supported when running in decomposed/
       end
     end
+
+    context 'when within transaction' do
+      before do
+        allow(migration).to receive(:transaction_open?).and_return(true)
+      end
+
+      it 'does raise an exception' do
+        expect { migration.finalize_batched_background_migration(job_class_name: 'MyJobClass', table_name: :projects, column_name: :id, job_arguments: []) }
+          .to raise_error /`finalize_batched_background_migration` cannot be run inside a transaction./
+      end
+    end
   end
 
   describe '#delete_batched_background_migration' do
diff --git a/spec/lib/gitlab/database/migrations/test_batched_background_runner_spec.rb b/spec/lib/gitlab/database/migrations/test_batched_background_runner_spec.rb
index fbfff1268cc..2f3d44f6f8f 100644
--- a/spec/lib/gitlab/database/migrations/test_batched_background_runner_spec.rb
+++ b/spec/lib/gitlab/database/migrations/test_batched_background_runner_spec.rb
@@ -4,7 +4,6 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::Database::Migrations::TestBatchedBackgroundRunner, :freeze_time do
   include Gitlab::Database::MigrationHelpers
-  include Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers
   include Database::MigrationTestingHelpers
 
   let(:result_dir) { Dir.mktmpdir }
@@ -13,6 +12,10 @@ RSpec.describe Gitlab::Database::Migrations::TestBatchedBackgroundRunner, :freez
     FileUtils.rm_rf(result_dir)
   end
 
+  let(:migration) do
+    ActiveRecord::Migration.new.extend(Gitlab::Database::Migrations::BatchedBackgroundMigrationHelpers)
+  end
+
   let(:connection) { ApplicationRecord.connection }
 
   let(:table_name) { "_test_column_copying"}
@@ -26,11 +29,13 @@ RSpec.describe Gitlab::Database::Migrations::TestBatchedBackgroundRunner, :freez
 
       insert into #{table_name} (id) select i from generate_series(1, 1000) g(i);
     SQL
+
+    allow(migration).to receive(:transaction_open?).and_return(false)
   end
 
   context 'running a real background migration' do
     it 'runs sampled jobs from the batched background migration' do
-      queue_batched_background_migration('CopyColumnUsingBackgroundMigrationJob',
+      migration.queue_batched_background_migration('CopyColumnUsingBackgroundMigrationJob',
                                          table_name, :id,
                                          :id, :data,
                                          batch_size: 100,
@@ -46,7 +51,9 @@ RSpec.describe Gitlab::Database::Migrations::TestBatchedBackgroundRunner, :freez
     let(:migration_name) { 'TestBackgroundMigration' }
 
     before do
-      queue_batched_background_migration(migration_name, table_name, :id, job_interval: 5.minutes, batch_size: 100)
+      migration.queue_batched_background_migration(
+        migration_name, table_name, :id, job_interval: 5.minutes, batch_size: 100
+      )
     end
 
     it 'samples jobs' do
@@ -67,13 +74,13 @@ RSpec.describe Gitlab::Database::Migrations::TestBatchedBackgroundRunner, :freez
         travel 3.days
 
         new_migration = define_background_migration('NewMigration') { travel 1.second }
-        queue_batched_background_migration('NewMigration', table_name, :id,
+        migration.queue_batched_background_migration('NewMigration', table_name, :id,
                                            job_interval: 5.minutes,
                                            batch_size: 10,
                                            sub_batch_size: 5)
 
         other_new_migration = define_background_migration('NewMigration2') { travel 2.seconds }
-        queue_batched_background_migration('NewMigration2', table_name, :id,
+        migration.queue_batched_background_migration('NewMigration2', table_name, :id,
                                            job_interval: 5.minutes,
                                            batch_size: 10,
                                            sub_batch_size: 5)
diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index 62c1da8c990..9b1cfeac4c5 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -336,6 +336,8 @@ integrations:
 - jira_tracker_data
 - zentao_tracker_data
 - issue_tracker_data
+# dingtalk_tracker_data JiHu-specific, see https://jihulab.com/gitlab-cn/gitlab/-/merge_requests/417
+- dingtalk_tracker_data
 hooks:
 - project
 - web_hook_logs
@@ -414,6 +416,8 @@ project:
 - pushover_integration
 - jira_integration
 - zentao_integration
+# dingtalk_integration JiHu-specific, see https://jihulab.com/gitlab-cn/gitlab/-/merge_requests/417
+- dingtalk_integration
 - redmine_integration
 - youtrack_integration
 - custom_issue_tracker_integration
diff --git a/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb b/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb
index eefe5bfc6c4..7d4268f74e9 100644
--- a/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb
+++ b/spec/models/preloaders/user_max_access_level_in_projects_preloader_spec.rb
@@ -7,8 +7,10 @@ RSpec.describe Preloaders::UserMaxAccessLevelInProjectsPreloader do
   let_it_be(:project_1) { create(:project) }
   let_it_be(:project_2) { create(:project) }
   let_it_be(:project_3) { create(:project) }
+  let_it_be(:project_4) { create(:project) }
+  let_it_be(:project_5) { create(:project) }
 
-  let(:projects) { [project_1, project_2, project_3] }
+  let(:projects) { [project_1, project_2, project_3, project_4, project_5] }
   let(:query) { projects.each { |project| user.can?(:read_project, project) } }
 
   before do
@@ -17,8 +19,11 @@ RSpec.describe Preloaders::UserMaxAccessLevelInProjectsPreloader do
   end
 
   context 'without preloader' do
-    it 'runs N queries' do
-      expect { query }.to make_queries(projects.size)
+    it 'runs some queries' do
+      # we have an existing N+1, one for each project for which user is not a member
+      # in this spec, project_3, project_4, project_5
+      # https://gitlab.com/gitlab-org/gitlab/-/issues/362890
+      expect { query }.to make_queries(projects.size + 3)
     end
   end
 
@@ -34,7 +39,7 @@ RSpec.describe Preloaders::UserMaxAccessLevelInProjectsPreloader do
     end
 
     context 'when projects is an array of IDs' do
-      let(:projects_arg) { [project_1.id, project_2.id, project_3.id] }
+      let(:projects_arg) { projects.map(&:id) }
 
       it 'avoids N+1 queries' do
         expect { query }.not_to make_queries
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index ca4ca2eb7a0..ce97fc0c77e 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -433,6 +433,80 @@ RSpec.describe ProjectPolicy do
     end
   end
 
+  context 'owner access' do
+    let!(:owner_user) { create(:user) }
+    let!(:owner_of_different_thing) { create(:user) }
+    let(:stranger) { create(:user) }
+
+    shared_examples 'owner access for personal and group projects' do
+      before do
+        stub_feature_flags(faster_owner_access: faster_owner_access_enabled)
+      end
+
+      context 'personal project' do
+        let!(:project) { create(:project) }
+        let!(:project2) { create(:project) }
+
+        before do
+          project.add_guest(guest)
+          project.add_reporter(reporter)
+          project.add_developer(developer)
+          project.add_maintainer(maintainer)
+          project2.add_owner(owner_of_different_thing)
+        end
+
+        it 'allows owner access', :aggregate_failures do
+          expect(described_class.new(owner_of_different_thing, project)).to be_disallowed(:owner_access)
+          expect(described_class.new(stranger, project)).to be_disallowed(:owner_access)
+          expect(described_class.new(guest, project)).to be_disallowed(:owner_access)
+          expect(described_class.new(reporter, project)).to be_disallowed(:owner_access)
+          expect(described_class.new(developer, project)).to be_disallowed(:owner_access)
+          expect(described_class.new(maintainer, project)).to be_disallowed(:owner_access)
+          expect(described_class.new(project.owner, project)).to be_allowed(:owner_access)
+        end
+      end
+
+      context 'group project' do
+        let(:group) { create(:group) }
+        let!(:group2) { create(:group) }
+        let!(:project) { create(:project, group: group) }
+
+        context 'group members' do
+          before do
+            group.add_guest(guest)
+            group.add_reporter(reporter)
+            group.add_developer(developer)
+            group.add_maintainer(maintainer)
+            group.add_owner(owner_user)
+            group2.add_owner(owner_of_different_thing)
+          end
+
+          it 'allows owner access', :aggregate_failures do
+            expect(described_class.new(owner_of_different_thing, project)).to be_disallowed(:owner_access)
+            expect(described_class.new(stranger, project)).to be_disallowed(:owner_access)
+            expect(described_class.new(guest, project)).to be_disallowed(:owner_access)
+            expect(described_class.new(reporter, project)).to be_disallowed(:owner_access)
+            expect(described_class.new(developer, project)).to be_disallowed(:owner_access)
+            expect(described_class.new(maintainer, project)).to be_disallowed(:owner_access)
+            expect(described_class.new(owner_user, project)).to be_allowed(:owner_access)
+          end
+        end
+      end
+    end
+
+    context 'when faster_owner_access feature is enabled' do
+      let(:faster_owner_access_enabled) { true }
+
+      it_behaves_like 'owner access for personal and group projects'
+    end
+
+    context 'when faster_owner_access feature is not enabled' do
+      let(:faster_owner_access_enabled) { false }
+
+      it_behaves_like 'owner access for personal and group projects'
+    end
+  end
+
   context 'reading a project' do
     it 'allows access when a user has read access to the repo' do
       expect(described_class.new(owner, project)).to be_allowed(:read_project)
diff --git a/spec/services/notification_recipients/build_service_spec.rb b/spec/services/notification_recipients/build_service_spec.rb
index ff54d6ccd2f..b348d95ecfc 100644
--- a/spec/services/notification_recipients/build_service_spec.rb
+++ b/spec/services/notification_recipients/build_service_spec.rb
@@ -14,6 +14,9 @@ RSpec.describe NotificationRecipients::BuildService do
 
     shared_examples 'no N+1 queries' do
       it 'avoids N+1 queries', :request_store do
+        # existing N+1 due to multiple users having to be looked up in the project_authorizations table
+        threshold = Feature.enabled?(:faster_owner_access) && project.private? ? 1 : 0
+
         create_user
 
         service.build_new_note_recipients(note)
@@ -24,37 +27,46 @@ RSpec.describe NotificationRecipients::BuildService do
 
         create_user
 
-        expect { service.build_new_note_recipients(note) }.not_to exceed_query_limit(control_count)
+        expect { service.build_new_note_recipients(note) }.not_to exceed_query_limit(control_count).with_threshold(threshold)
       end
     end
 
-    context 'when there are multiple watchers' do
-      def create_user
-        watcher = create(:user)
-        create(:notification_setting, source: project, user: watcher, level: :watch)
+    [true, false].each do |value|
+      context "when faster_owner_access feature is #{value ? 'enabled' : 'not enabled'}" do
+        before do
+          # test both feature flag values
+          stub_feature_flags(faster_owner_access: value)
+        end
+
+        context 'when there are multiple watchers' do
+          def create_user
+            watcher = create(:user)
+            create(:notification_setting, source: project, user: watcher, level: :watch)
+
+            other_projects.each do |other_project|
+              create(:notification_setting, source: other_project, user: watcher, level: :watch)
+            end
+          end
 
-        other_projects.each do |other_project|
-          create(:notification_setting, source: other_project, user: watcher, level: :watch)
+          include_examples 'no N+1 queries'
         end
-      end
 
-      include_examples 'no N+1 queries'
-    end
+        context 'when there are multiple subscribers' do
+          def create_user
+            subscriber = create(:user)
+            issue.subscriptions.create!(user: subscriber, project: project, subscribed: true)
+          end
 
-    context 'when there are multiple subscribers' do
-      def create_user
-        subscriber = create(:user)
-        issue.subscriptions.create!(user: subscriber, project: project, subscribed: true)
-      end
+          include_examples 'no N+1 queries'
 
-      include_examples 'no N+1 queries'
+          context 'when the project is private' do
+            before do
+              project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
+            end
 
-      context 'when the project is private' do
-        before do
-          project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
+            include_examples 'no N+1 queries'
+          end
         end
-
-        include_examples 'no N+1 queries'
       end
     end
   end
@@ -66,6 +78,9 @@ RSpec.describe NotificationRecipients::BuildService do
 
     shared_examples 'no N+1 queries' do
       it 'avoids N+1 queries', :request_store do
+        # existing N+1 due to multiple users having to be looked up in the project_authorizations table
+        threshold = Feature.enabled?(:faster_owner_access) && project.private? ? 1 : 0
+
         create_user
 
         service.build_new_review_recipients(review)
@@ -76,37 +91,46 @@ RSpec.describe NotificationRecipients::BuildService do
 
         create_user
 
-        expect { service.build_new_review_recipients(review) }.not_to exceed_query_limit(control_count)
+        expect { service.build_new_review_recipients(review) }.not_to exceed_query_limit(control_count).with_threshold(threshold)
       end
     end
 
-    context 'when there are multiple watchers' do
-      def create_user
-        watcher = create(:user)
-        create(:notification_setting, source: project, user: watcher, level: :watch)
+    [true, false].each do |value|
+      context "when faster_owner_access feature is #{value ? 'enabled' : 'not enabled'}" do
+        before do
+          # test both feature flag values
+          stub_feature_flags(faster_owner_access: value)
+        end
+
+        context 'when there are multiple watchers' do
+          def create_user
+            watcher = create(:user)
+            create(:notification_setting, source: project, user: watcher, level: :watch)
+
+            other_projects.each do |other_project|
+              create(:notification_setting, source: other_project, user: watcher, level: :watch)
+            end
+          end
 
-        other_projects.each do |other_project|
-          create(:notification_setting, source: other_project, user: watcher, level: :watch)
+          include_examples 'no N+1 queries'
         end
-      end
 
-      include_examples 'no N+1 queries'
-    end
+        context 'when there are multiple subscribers' do
+          def create_user
+            subscriber = create(:user)
+            merge_request.subscriptions.create!(user: subscriber, project: project, subscribed: true)
+          end
 
-    context 'when there are multiple subscribers' do
-      def create_user
-        subscriber = create(:user)
-        merge_request.subscriptions.create!(user: subscriber, project: project, subscribed: true)
-      end
+          include_examples 'no N+1 queries'
 
-      include_examples 'no N+1 queries'
+          context 'when the project is private' do
+            before do
+              project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
+            end
 
-      context 'when the project is private' do
-        before do
-          project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
+            include_examples 'no N+1 queries'
+          end
         end
-
-        include_examples 'no N+1 queries'
       end
     end
   end