diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2024-01-17 06:10:05 +0300 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2024-01-17 06:10:05 +0300 |
| commit | 4c872af312f27f2e2da967a6efebd76e88119caa (patch) | |
| tree | 096faafac1769e8a6bb9c6e71240e43ea90bf4e1 | |
| parent | 3f98f1e47b16b2b1d7a2e8a86252e002c2496098 (diff) | |
Add latest changes from gitlab-org/gitlab@master
36 files changed, 315 insertions, 90 deletions
diff --git a/.gitlab/ci/test-on-gdk/main.gitlab-ci.yml b/.gitlab/ci/test-on-gdk/main.gitlab-ci.yml index 711d5bf4a02..5efe66d9b47 100644 --- a/.gitlab/ci/test-on-gdk/main.gitlab-ci.yml +++ b/.gitlab/ci/test-on-gdk/main.gitlab-ci.yml @@ -142,7 +142,7 @@ gdk-qa-smoke: QA_SCENARIO: Test::Instance::Smoke QA_RUN_TYPE: gdk-qa-smoke -gdk-qa-reliable: +gdk-qa-blocking: extends: - .gdk-qa-base - .rules:gdk:qa-parallel @@ -151,7 +151,7 @@ gdk-qa-reliable: QA_RUN_TYPE: gdk-qa-blocking parallel: 10 -gdk-qa-reliable-selective: +gdk-qa-blocking-selective: extends: - .gdk-qa-base - .rules:gdk:qa-selective @@ -179,7 +179,7 @@ gdk-qa-smoke-with-load-balancer: - !reference [".rules:test:never-schedule-pipeline", rules] - !reference [".rules:test:gdk-load-balancer-changes", rules] -gdk-qa-reliable-with-load-balancer: +gdk-qa-blocking-with-load-balancer: extends: - .gdk-qa-base - .gdk-with-load-balancer-setup @@ -205,9 +205,12 @@ gdk-qa-non-blocking: QA_SCENARIO: Test::Instance::NonBlocking QA_RUN_TYPE: gdk-qa-non-blocking parallel: 5 + allow_failure: true rules: + # run tests on master pipelines to collect metrics and move tests to `blocking` job until `non-blocking` job + # is removed entirely + - if: '$CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH' - when: manual - allow_failure: true # ========================================== # Post test stage diff --git a/GITLAB_KAS_VERSION b/GITLAB_KAS_VERSION index 13f7272cfef..83d4962b6a5 100644 --- a/GITLAB_KAS_VERSION +++ b/GITLAB_KAS_VERSION @@ -1 +1 @@ -v16.9.0-rc1 +v16.9.0-rc2 diff --git a/app/models/member.rb b/app/models/member.rb index cd977969098..1771c92d5cd 100644 --- a/app/models/member.rb +++ b/app/models/member.rb @@ -31,6 +31,8 @@ class Member < ApplicationRecord delegate :name, :username, :email, :last_activity_on, to: :user, prefix: true + has_many :member_approvals, inverse_of: :member, class_name: 'Members::MemberApproval' + validates :expires_at, allow_blank: true, future_date: true validates :user, presence: true, unless: :invite? validates :source, presence: true diff --git a/app/models/members/members/member_approval.rb b/app/models/members/members/member_approval.rb new file mode 100644 index 00000000000..60318f9d4e5 --- /dev/null +++ b/app/models/members/members/member_approval.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Members + class MemberApproval < ApplicationRecord + enum status: { pending: 0, approved: 1, denied: 2 } + + belongs_to :member + belongs_to :member_namespace, class_name: 'Namespace' + belongs_to :requested_by, inverse_of: :requested_member_approvals, class_name: 'User', + optional: true + belongs_to :reviewed_by, inverse_of: :reviewed_member_approvals, class_name: 'User', + optional: true + + validates :new_access_level, presence: true + validates :old_access_level, presence: true + end +end diff --git a/app/models/project.rb b/app/models/project.rb index 8f82a947ba6..8ad463f1511 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -347,6 +347,8 @@ class Project < ApplicationRecord primary_key: :project_namespace_id, foreign_key: :member_namespace_id, inverse_of: :project, class_name: 'ProjectMember' has_many :members_and_requesters, as: :source, class_name: 'ProjectMember' + has_many :member_approvals, through: :members_and_requesters + has_many :namespace_members_and_requesters, -> { unscope(where: %i[source_id source_type]) }, primary_key: :project_namespace_id, foreign_key: :member_namespace_id, inverse_of: :project, class_name: 'ProjectMember' diff --git a/app/models/user.rb b/app/models/user.rb index c9873975cc9..dca6570661e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -299,6 +299,9 @@ class User < MainClusterwide::ApplicationRecord has_many :achievements, through: :user_achievements, class_name: 'Achievements::Achievement', inverse_of: :users has_many :vscode_settings, class_name: 'VsCode::Settings::VsCodeSetting', inverse_of: :user + has_many :requested_member_approvals, class_name: 'Members::MemberApproval', foreign_key: 'requested_by_id' + has_many :reviewed_member_approvals, class_name: 'Members::MemberApproval', foreign_key: 'reviewed_by_id' + # # Validations # diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index f5c5dceb611..1477307c67b 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -277,6 +277,11 @@ module MergeRequests def capture_suggested_reviewers_accepted(merge_request) # Implemented in EE end + + def remove_approval(merge_request) + MergeRequests::RemoveApprovalService.new(project: project, current_user: current_user) + .execute(merge_request) + end end end diff --git a/app/services/merge_requests/request_review_service.rb b/app/services/merge_requests/request_review_service.rb index 87b00aa088c..6af728f7039 100644 --- a/app/services/merge_requests/request_review_service.rb +++ b/app/services/merge_requests/request_review_service.rb @@ -14,6 +14,8 @@ module MergeRequests trigger_merge_request_reviewers_updated(merge_request) create_system_note(merge_request, user) + remove_approval(merge_request) if Feature.enabled?(:mr_request_changes, current_user) + success else error("Reviewer not found") diff --git a/app/services/merge_requests/update_reviewer_state_service.rb b/app/services/merge_requests/update_reviewer_state_service.rb index e2252f55fd3..e59c2b178db 100644 --- a/app/services/merge_requests/update_reviewer_state_service.rb +++ b/app/services/merge_requests/update_reviewer_state_service.rb @@ -23,12 +23,5 @@ module MergeRequests error("Reviewer not found") end end - - private - - def remove_approval(merge_request) - MergeRequests::RemoveApprovalService.new(project: project, current_user: current_user) - .execute(merge_request) - end end end diff --git a/config/gitlab_loose_foreign_keys.yml b/config/gitlab_loose_foreign_keys.yml index 644eab176bf..13f8b824278 100644 --- a/config/gitlab_loose_foreign_keys.yml +++ b/config/gitlab_loose_foreign_keys.yml @@ -225,6 +225,13 @@ groups_visits: - table: users column: user_id on_delete: async_delete +member_approvals: + - table: users + column: requested_by_id + on_delete: async_nullify + - table: users + column: reviewed_by_id + on_delete: async_nullify members: - table: users column: user_id diff --git a/db/docs/coverage_fuzzing_corpuses.yml b/db/docs/coverage_fuzzing_corpuses.yml index 38410c1a72d..a7bfe7810c7 100644 --- a/db/docs/coverage_fuzzing_corpuses.yml +++ b/db/docs/coverage_fuzzing_corpuses.yml @@ -7,4 +7,12 @@ feature_categories: description: Stores additional values describing corpuses used by coverage fuzzing introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/71704 milestone: '14.4' -gitlab_schema: gitlab_main +gitlab_schema: gitlab_main_cell +allow_cross_joins: +- gitlab_main_clusterwide +allow_cross_transactions: +- gitlab_main_clusterwide +allow_cross_foreign_keys: +- gitlab_main_clusterwide +sharding_key: + project_id: projects diff --git a/db/docs/member_approvals.yml b/db/docs/member_approvals.yml new file mode 100644 index 00000000000..544aea94ddd --- /dev/null +++ b/db/docs/member_approvals.yml @@ -0,0 +1,12 @@ +--- +table_name: member_approvals +classes: +- Members::MemberApproval +feature_categories: +- groups_and_projects +description: Stores approval requests for Member promotions along with old and new access_levels +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/140073 +milestone: '16.9' +gitlab_schema: gitlab_main_cell +sharding_key: + member_namespace_id: namespaces diff --git a/db/migrate/20240116100948_create_member_approvals.rb b/db/migrate/20240116100948_create_member_approvals.rb new file mode 100644 index 00000000000..384060207b8 --- /dev/null +++ b/db/migrate/20240116100948_create_member_approvals.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class CreateMemberApprovals < Gitlab::Database::Migration[2.2] + milestone '16.9' + + def change + create_table :member_approvals do |t| + t.datetime_with_timezone :reviewed_at + t.timestamps_with_timezone + + t.bigint :member_id, null: false + t.bigint :member_namespace_id, null: false + t.bigint :requested_by_id + t.bigint :reviewed_by_id + t.integer :new_access_level, null: false + t.integer :old_access_level, null: false + t.integer :status, null: false, default: 0, limit: 2 + end + + add_index :member_approvals, :requested_by_id, name: 'index_member_approval_on_requested_by_id' + add_index :member_approvals, :reviewed_by_id, name: 'index_member_approval_on_reviewed_by_id' + add_index :member_approvals, :member_id, name: 'index_member_approval_on_member_id' + add_index :member_approvals, :member_namespace_id, name: 'index_member_approval_on_member_namespace_id' + end +end diff --git a/db/migrate/20240116120144_add_foreign_key_for_member_to_member_approvals.rb b/db/migrate/20240116120144_add_foreign_key_for_member_to_member_approvals.rb new file mode 100644 index 00000000000..87757fe7015 --- /dev/null +++ b/db/migrate/20240116120144_add_foreign_key_for_member_to_member_approvals.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class AddForeignKeyForMemberToMemberApprovals < Gitlab::Database::Migration[2.2] + disable_ddl_transaction! + milestone '16.9' + + def up + add_concurrent_foreign_key :member_approvals, :members, column: :member_id, on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key :member_approvals, column: :member_id + end + end +end diff --git a/db/migrate/20240116133546_add_foreign_key_for_namespace_id_to_member_approvals.rb b/db/migrate/20240116133546_add_foreign_key_for_namespace_id_to_member_approvals.rb new file mode 100644 index 00000000000..1d8e4246608 --- /dev/null +++ b/db/migrate/20240116133546_add_foreign_key_for_namespace_id_to_member_approvals.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class AddForeignKeyForNamespaceIdToMemberApprovals < Gitlab::Database::Migration[2.2] + disable_ddl_transaction! + milestone '16.9' + + def up + add_concurrent_foreign_key :member_approvals, :namespaces, column: :member_namespace_id + end + + def down + with_lock_retries do + remove_foreign_key :member_approvals, column: :member_namespace_id + end + end +end diff --git a/db/schema_migrations/20240116100948 b/db/schema_migrations/20240116100948 new file mode 100644 index 00000000000..180c6b62fbe --- /dev/null +++ b/db/schema_migrations/20240116100948 @@ -0,0 +1 @@ +2d91b99f6c82ebf2d4b3bea6cc0a92ee6cf7f54dffdeb7d9fe3cd2b290b2a84f
\ No newline at end of file diff --git a/db/schema_migrations/20240116120144 b/db/schema_migrations/20240116120144 new file mode 100644 index 00000000000..9d01774ebdb --- /dev/null +++ b/db/schema_migrations/20240116120144 @@ -0,0 +1 @@ +353d71b88510f83491b0a59aed5300b94afceef9348f72edfc8e1fd35f610c2a
\ No newline at end of file diff --git a/db/schema_migrations/20240116133546 b/db/schema_migrations/20240116133546 new file mode 100644 index 00000000000..5e434eba9f4 --- /dev/null +++ b/db/schema_migrations/20240116133546 @@ -0,0 +1 @@ +279526eb13969a77d77dade50b72c15c9cff4d4b982b02542a703200421fc423
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index fd741fc3111..e2ff22c7a0f 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -18979,6 +18979,29 @@ CREATE SEQUENCE loose_foreign_keys_deleted_records_id_seq ALTER SEQUENCE loose_foreign_keys_deleted_records_id_seq OWNED BY loose_foreign_keys_deleted_records.id; +CREATE TABLE member_approvals ( + id bigint NOT NULL, + reviewed_at timestamp with time zone, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + member_id bigint NOT NULL, + member_namespace_id bigint NOT NULL, + requested_by_id bigint, + reviewed_by_id bigint, + new_access_level integer NOT NULL, + old_access_level integer NOT NULL, + status smallint DEFAULT 0 NOT NULL +); + +CREATE SEQUENCE member_approvals_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE member_approvals_id_seq OWNED BY member_approvals.id; + CREATE TABLE member_roles ( id bigint NOT NULL, namespace_id bigint, @@ -27389,6 +27412,8 @@ ALTER TABLE ONLY lists ALTER COLUMN id SET DEFAULT nextval('lists_id_seq'::regcl ALTER TABLE ONLY loose_foreign_keys_deleted_records ALTER COLUMN id SET DEFAULT nextval('loose_foreign_keys_deleted_records_id_seq'::regclass); +ALTER TABLE ONLY member_approvals ALTER COLUMN id SET DEFAULT nextval('member_approvals_id_seq'::regclass); + ALTER TABLE ONLY member_roles ALTER COLUMN id SET DEFAULT nextval('member_roles_id_seq'::regclass); ALTER TABLE ONLY members ALTER COLUMN id SET DEFAULT nextval('members_id_seq'::regclass); @@ -29761,6 +29786,9 @@ ALTER TABLE ONLY lists ALTER TABLE ONLY loose_foreign_keys_deleted_records ADD CONSTRAINT loose_foreign_keys_deleted_records_pkey PRIMARY KEY (partition, id); +ALTER TABLE ONLY member_approvals + ADD CONSTRAINT member_approvals_pkey PRIMARY KEY (id); + ALTER TABLE ONLY member_roles ADD CONSTRAINT member_roles_pkey PRIMARY KEY (id); @@ -34130,6 +34158,14 @@ CREATE INDEX index_manifest_states_on_verification_state ON dependency_proxy_man CREATE INDEX index_manifest_states_pending_verification ON dependency_proxy_manifest_states USING btree (verified_at NULLS FIRST) WHERE (verification_state = 0); +CREATE INDEX index_member_approval_on_member_id ON member_approvals USING btree (member_id); + +CREATE INDEX index_member_approval_on_member_namespace_id ON member_approvals USING btree (member_namespace_id); + +CREATE INDEX index_member_approval_on_requested_by_id ON member_approvals USING btree (requested_by_id); + +CREATE INDEX index_member_approval_on_reviewed_by_id ON member_approvals USING btree (reviewed_by_id); + CREATE INDEX index_member_roles_on_namespace_id ON member_roles USING btree (namespace_id); CREATE INDEX index_members_on_access_level ON members USING btree (access_level); @@ -38129,6 +38165,9 @@ ALTER TABLE ONLY project_pages_metadata ALTER TABLE ONLY group_deletion_schedules ADD CONSTRAINT fk_11e3ebfcdd FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE; +ALTER TABLE ONLY member_approvals + ADD CONSTRAINT fk_1383c72212 FOREIGN KEY (member_namespace_id) REFERENCES namespaces(id) ON DELETE CASCADE; + ALTER TABLE ONLY approval_group_rules ADD CONSTRAINT fk_1485c451e3 FOREIGN KEY (scan_result_policy_id) REFERENCES scan_result_policies(id) ON DELETE CASCADE; @@ -38825,6 +38864,9 @@ ALTER TABLE ONLY project_access_tokens ALTER TABLE ONLY vulnerability_reads ADD CONSTRAINT fk_b28c28abf1 FOREIGN KEY (scanner_id) REFERENCES vulnerability_scanners(id) ON DELETE CASCADE; +ALTER TABLE ONLY member_approvals + ADD CONSTRAINT fk_b2e4a4b68a FOREIGN KEY (member_id) REFERENCES members(id) ON DELETE CASCADE; + ALTER TABLE ONLY issues ADD CONSTRAINT fk_b37be69be6 FOREIGN KEY (work_item_type_id) REFERENCES work_item_types(id); diff --git a/doc/development/testing_guide/end_to_end/rspec_metadata_tests.md b/doc/development/testing_guide/end_to_end/rspec_metadata_tests.md index 5d7897331a2..ec3025bc7d7 100644 --- a/doc/development/testing_guide/end_to_end/rspec_metadata_tests.md +++ b/doc/development/testing_guide/end_to_end/rspec_metadata_tests.md @@ -17,7 +17,6 @@ This is a partial list of the [RSpec metadata](https://rspec.info/features/3-12/ | `:except` | The test is to be run in their typical execution contexts _except_ as specified. See [test execution context selection](execution_context_selection.md) for more information. | | `:external_api_calls` | The test requires interaction with a network external to the Docker network | | `:feature_flag` | The test uses a feature flag and therefore requires an administrator account to run. When `scope` is set to `:global`, the test will be skipped on all live .com environments. Otherwise, it will be skipped only on Canary, Production, and Pre-production. See [testing with feature flags](../../../development/testing_guide/end_to_end/feature_flags.md) for more details. | -| `:framework` | The test makes sanity assertions around the QA framework itself | | `:geo` | The test requires two GitLab Geo instances - a primary and a secondary - to be spun up. | | `:gitaly_cluster` | The test runs against a GitLab instance where repositories are stored on redundant Gitaly nodes behind a Praefect node. All nodes are [separate containers](../../../administration/gitaly/praefect.md#requirements). Tests that use this tag have a longer setup time since there are three additional containers that need to be started. | | `:github` | The test requires a GitHub personal access token. | @@ -42,12 +41,12 @@ This is a partial list of the [RSpec metadata](https://rspec.info/features/3-12/ | `:quarantine` | The test has been [quarantined](https://about.gitlab.com/handbook/engineering/quality/quality-engineering/debugging-qa-test-failures/#quarantining-tests), runs in a separate job that only includes quarantined tests, and is allowed to fail. The test is skipped in its regular job so that if it fails it doesn't hold up the pipeline. Note that you can also [quarantine a test only when it runs in a specific context](execution_context_selection.md#quarantine-a-test-for-a-specific-environment). | | `:relative_url` | The test requires a GitLab instance to be installed under a [relative URL](../../../install/relative_url.md). | | `:reliable` | The test has been [promoted to a reliable test](https://about.gitlab.com/handbook/engineering/quality/quality-engineering/reliable-tests/#promoting-an-existing-test-to-reliable) meaning it passes consistently in all pipelines, including merge requests. | +| `:blocking` | Temporary tag for tests running in `e2e:test-on-gdk` pipeline and not allowed to fail. Only tests that pass consistently should have this tag. Allows to scale blocking suite running against `gdk` independently of `:reliable`. | | `:repository_storage` | The test requires a GitLab instance to be configured to use multiple [repository storage paths](../../../administration/repository_storage_paths.md). Paired with the `:orchestrated` tag. | | `:requires_admin` | The test requires an administrator account. Tests with the tag are excluded when run against Canary and Production environments. | | `:requires_git_protocol_v2` | The test requires that Git protocol version 2 is enabled on the server. It's assumed to be enabled by default but if not the test can be skipped by setting `QA_CAN_TEST_GIT_PROTOCOL_V2` to `false`. | | `:requires_praefect` | The test requires that the GitLab instance uses [Gitaly Cluster](../../../administration/gitaly/praefect.md) (a.k.a. Praefect) as the repository storage . It's assumed to be used by default but if not the test can be skipped by setting `QA_CAN_TEST_PRAEFECT` to `false`. | | `:runner` | The test depends on and sets up a GitLab Runner instance, typically to run a pipeline. | -| `:sanity_feature_flags` | The test verifies the functioning of the feature flag handling part of the test framework | | `:skip_live_env` | The test is excluded when run against live deployed environments such as Staging, Canary, and Production. | | `:skip_fips_env` | The test is excluded when run against an environment in FIPS mode. | | `:skip_signup_disabled` | The test uses UI to sign up a new user and is skipped in any environment that does not allow new user registration via the UI. | diff --git a/doc/user/project/import/bitbucket_server.md b/doc/user/project/import/bitbucket_server.md index 41076bfbc67..572e0edcb90 100644 --- a/doc/user/project/import/bitbucket_server.md +++ b/doc/user/project/import/bitbucket_server.md @@ -45,7 +45,7 @@ To import your Bitbucket repositories: - Repository description - Git repository data - Pull requests -- Pull request comments, reviewers, approvals, and merge events +- Pull request comments, user mentions, reviewers, approvals, and merge events - LFS objects When importing, repository public access is retained. If a repository is private in Bitbucket, it's @@ -80,13 +80,25 @@ The following items are changed when they are imported: ## User assignment -> Importing approvals by email address or username [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/23586) in GitLab 16.7. +> - Importing approvals by email address or username [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/23586) in GitLab 16.7. +> - Matching user mentions with GitLab users [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/433008) in GitLab 16.8. + +FLAG: +On self-managed GitLab, matching user mentions with GitLab users is not available. To make it available per user, +an administrator can [enable the feature flag](../../../administration/feature_flags.md) named `bitbucket_server_import_stage_import_users`. +On GitLab.com, this feature is not available. When issues and pull requests are importing, the importer tries to find the author's email address with a confirmed email address in the GitLab user database. If no such user is available, the project creator is set as the author. The importer appends a note in the comment to mark the original creator. +`@mentions` on pull request descriptions and notes are matched to user profiles on a Bitbucket Server by using the user's email address. +If a user with the same email address is not found on GitLab, the `@mention` is made static. +For a user to be matched, they must have a GitLab role that provides at least read access to the project. + +If the project is public, GitLab only matches users who are invited to the project. + The importer creates any new namespaces (groups) if they don't exist. If the namespace is taken, the repository imports under the namespace of the user who started the import process. diff --git a/lib/gitlab/database/dictionary.rb b/lib/gitlab/database/dictionary.rb index d963d6f288e..ff9be3b8f4b 100644 --- a/lib/gitlab/database/dictionary.rb +++ b/lib/gitlab/database/dictionary.rb @@ -3,15 +3,35 @@ module Gitlab module Database class Dictionary + include Enumerable + + delegate :each, to: :@dictionary_entries + ALL_SCOPES = ['', 'views', 'deleted_tables'].freeze + def initialize(dictionary_entries) + @dictionary_entries = dictionary_entries + end + + def to_name_and_schema_mapping + @dictionary_entries.map(&:name_and_schema).to_h + end + + def find_by_table_name(name) + @dictionary_entries.find { |entry| entry.key_name == name } + end + + def find_all_by_schema(schema_name) + @dictionary_entries.select { |entry| entry.schema?(schema_name) } + end + def self.entries(scope = '') @entries ||= {} - @entries[scope] ||= Dir.glob(dictionary_path_globs(scope)).map do |file_path| - dictionary = Entry.new(file_path) - dictionary.validate! - dictionary - end + @entries[scope] ||= new( + Dir.glob(dictionary_path_globs(scope)).map do |file_path| + Entry.new(file_path).tap(&:validate!) + end + ) end def self.any_entry(name) @@ -24,9 +44,7 @@ module Gitlab end def self.entry(name, scope = '') - entries(scope).find do |entry| - entry.key_name == name - end + entries(scope).find_by_table_name(name) end private_class_method def self.dictionary_path_globs(scope) diff --git a/lib/gitlab/database/gitlab_schema.rb b/lib/gitlab/database/gitlab_schema.rb index e6f7dbec69c..f23e0a5112d 100644 --- a/lib/gitlab/database/gitlab_schema.rb +++ b/lib/gitlab/database/gitlab_schema.rb @@ -134,19 +134,19 @@ module Gitlab end def self.deleted_tables_to_schema - @deleted_tables_to_schema ||= ::Gitlab::Database::Dictionary.entries('deleted_tables').map(&:name_and_schema).to_h + @deleted_tables_to_schema ||= ::Gitlab::Database::Dictionary.entries('deleted_tables').to_name_and_schema_mapping end def self.deleted_views_to_schema - @deleted_views_to_schema ||= ::Gitlab::Database::Dictionary.entries('deleted_views').map(&:name_and_schema).to_h + @deleted_views_to_schema ||= ::Gitlab::Database::Dictionary.entries('deleted_views').to_name_and_schema_mapping end def self.tables_to_schema - @tables_to_schema ||= ::Gitlab::Database::Dictionary.entries.map(&:name_and_schema).to_h + @tables_to_schema ||= ::Gitlab::Database::Dictionary.entries.to_name_and_schema_mapping end def self.views_to_schema - @views_to_schema ||= ::Gitlab::Database::Dictionary.entries('views').map(&:name_and_schema).to_h + @views_to_schema ||= ::Gitlab::Database::Dictionary.entries('views').to_name_and_schema_mapping end def self.schema_names diff --git a/qa/qa/scenario/test/instance/blocking.rb b/qa/qa/scenario/test/instance/blocking.rb index 26bd71e04e8..e20b4e5f2e5 100644 --- a/qa/qa/scenario/test/instance/blocking.rb +++ b/qa/qa/scenario/test/instance/blocking.rb @@ -5,8 +5,7 @@ module QA module Test module Instance class Blocking < All - tags :reliable, - *Specs::Runner::DEFAULT_SKIPPED_TAGS.map { |tag| :"~#{tag}" } + tags :reliable, :blocking, *Specs::Runner::DEFAULT_SKIPPED_TAGS.map { |tag| :"~#{tag}" } end end end diff --git a/qa/qa/scenario/test/instance/non_blocking.rb b/qa/qa/scenario/test/instance/non_blocking.rb index 602fa60b646..aea5c60daa3 100644 --- a/qa/qa/scenario/test/instance/non_blocking.rb +++ b/qa/qa/scenario/test/instance/non_blocking.rb @@ -5,9 +5,7 @@ module QA module Test module Instance class NonBlocking < All - tags :"~reliable", - :"~smoke", - *Specs::Runner::DEFAULT_SKIPPED_TAGS.map { |tag| :"~#{tag}" } + tags :"~reliable", :"~blocking", :"~smoke", *Specs::Runner::DEFAULT_SKIPPED_TAGS.map { |tag| :"~#{tag}" } end end end diff --git a/qa/qa/scenario/test/instance/review_blocking.rb b/qa/qa/scenario/test/instance/review_blocking.rb deleted file mode 100644 index 41e760baf45..00000000000 --- a/qa/qa/scenario/test/instance/review_blocking.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -module QA - module Scenario - module Test - module Instance - class ReviewBlocking < All - tags :reliable, - :sanity_feature_flags, - :"~orchestrated", - :"~skip_signup_disabled" - end - end - end - end -end diff --git a/qa/qa/scenario/test/sanity/framework.rb b/qa/qa/scenario/test/sanity/framework.rb deleted file mode 100644 index 7835d2564f0..00000000000 --- a/qa/qa/scenario/test/sanity/framework.rb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -module QA - module Scenario - module Test - module Sanity - ## - # This scenario runs 1 passing example, and 1 failing example, and exits - # with a 1 exit code. - # - class Framework < Template - include Bootable - - tags :framework - end - end - end - end -end diff --git a/qa/qa/specs/runner.rb b/qa/qa/specs/runner.rb index 4eb799f34c1..dc550524ce0 100644 --- a/qa/qa/specs/runner.rb +++ b/qa/qa/specs/runner.rb @@ -12,7 +12,7 @@ module QA DEFAULT_TEST_PATH_ARGS = ['--', File.expand_path('./features', __dir__)].freeze DEFAULT_STD_ARGS = [$stderr, $stdout].freeze - DEFAULT_SKIPPED_TAGS = %w[orchestrated transient sanity_feature_flags].freeze + DEFAULT_SKIPPED_TAGS = %w[orchestrated transient].freeze def initialize @tty = false diff --git a/qa/spec/scenario/test/sanity/framework_spec.rb b/qa/spec/scenario/test/sanity/framework_spec.rb deleted file mode 100644 index 5ae8b123ec2..00000000000 --- a/qa/spec/scenario/test/sanity/framework_spec.rb +++ /dev/null @@ -1,7 +0,0 @@ -# frozen_string_literal: true - -RSpec.describe QA::Scenario::Test::Sanity::Framework do - it_behaves_like 'a QA scenario class' do - let(:tags) { [:framework] } - end -end diff --git a/qa/spec/specs/runner_spec.rb b/qa/spec/specs/runner_spec.rb index cbe5699a306..8d1a0d1afd0 100644 --- a/qa/spec/specs/runner_spec.rb +++ b/qa/spec/specs/runner_spec.rb @@ -10,7 +10,7 @@ RSpec.describe QA::Specs::Runner do end before do - stub_const('DEFAULT_SKIPPED_TAGS', %w[--tag ~orchestrated --tag ~transient --tag ~sanity_feature_flags].freeze) + stub_const('DEFAULT_SKIPPED_TAGS', %w[--tag ~orchestrated --tag ~transient].freeze) end describe '#perform' do diff --git a/spec/lib/gitlab/database/dictionary_spec.rb b/spec/lib/gitlab/database/dictionary_spec.rb index 59145842b24..2a7535ffb23 100644 --- a/spec/lib/gitlab/database/dictionary_spec.rb +++ b/spec/lib/gitlab/database/dictionary_spec.rb @@ -3,10 +3,12 @@ require 'spec_helper' RSpec.describe Gitlab::Database::Dictionary, feature_category: :database do + subject(:dictionary) { described_class.entries('') } + describe '.entries' do it 'all tables and views are unique' do - table_and_view_names = described_class.entries('') - table_and_view_names += described_class.entries('views') + table_and_view_names = dictionary.to_a + table_and_view_names += described_class.entries('views').to_a # ignore gitlab_internal due to `ar_internal_metadata`, `schema_migrations` table_and_view_names = table_and_view_names @@ -22,6 +24,51 @@ RSpec.describe Gitlab::Database::Dictionary, feature_category: :database do "Any duplicated table must be removed from db/docs/ or ee/db/docs/. " \ "More info: https://docs.gitlab.com/ee/development/database/database_dictionary.html" end + + it 'builds a Dictionary with validated Entry records' do + expect { dictionary }.not_to raise_error + + expect(dictionary).to be_instance_of(described_class) + expect(dictionary).to all(be_instance_of(Gitlab::Database::Dictionary::Entry)) + end + end + + describe '#to_name_and_schema_mapping' do + it 'returns a hash of name and schema mappings' do + expect(dictionary.to_name_and_schema_mapping).to include( + { + 'application_settings' => :gitlab_main_clusterwide, + 'members' => :gitlab_main_cell + } + ) + end + end + + describe '#find_by_table_name' do + it 'finds an entry by table name' do + entry = dictionary.find_by_table_name('application_settings') + expect(entry).to be_instance_of(Gitlab::Database::Dictionary::Entry) + expect(entry.key_name).to eq('application_settings') + expect(entry.gitlab_schema).to eq('gitlab_main_clusterwide') + end + + it 'returns nil if the entry is not found' do + entry = dictionary.find_by_table_name('non_existent_table') + expect(entry).to be_nil + end + end + + describe '#find_all_by_schema' do + it 'returns an array of entries with a given schema' do + entries = dictionary.find_all_by_schema('gitlab_main_cell') + expect(entries).to all(be_instance_of(Gitlab::Database::Dictionary::Entry)) + expect(entries).to all(have_attributes(gitlab_schema: 'gitlab_main_cell')) + end + + it 'returns an empty array if no entries match the schema' do + entries = dictionary.find_all_by_schema('non_existent_schema') + expect(entries).to be_empty + end end describe '.any_entry' do diff --git a/spec/lib/gitlab/database/no_new_tables_with_gitlab_main_schema_spec.rb b/spec/lib/gitlab/database/no_new_tables_with_gitlab_main_schema_spec.rb index 4fc62c6cc74..09cf7808042 100644 --- a/spec/lib/gitlab/database/no_new_tables_with_gitlab_main_schema_spec.rb +++ b/spec/lib/gitlab/database/no_new_tables_with_gitlab_main_schema_spec.rb @@ -51,16 +51,12 @@ RSpec.describe 'new tables with gitlab_main schema', feature_category: :cell do end def tables_having_gitlab_main_schema(starting_from_milestone:) - selected_data = gitlab_main_schema_tables.select do |entry| - entry.milestone.to_f >= starting_from_milestone + gitlab_main_schema_tables.filter_map do |entry| + entry.table_name if entry.milestone.to_f >= starting_from_milestone end - - selected_data.map(&:table_name) end def gitlab_main_schema_tables - ::Gitlab::Database::Dictionary.entries.select do |entry| - entry.schema?('gitlab_main') - end + ::Gitlab::Database::Dictionary.entries.find_all_by_schema('gitlab_main') end end diff --git a/spec/lib/gitlab/database/sharding_key_spec.rb b/spec/lib/gitlab/database/sharding_key_spec.rb index 67c1422af3c..dfd78bfacba 100644 --- a/spec/lib/gitlab/database/sharding_key_spec.rb +++ b/spec/lib/gitlab/database/sharding_key_spec.rb @@ -108,11 +108,11 @@ RSpec.describe 'new tables missing sharding_key', feature_category: :cell do end def tables_missing_sharding_key(starting_from_milestone:) - ::Gitlab::Database::Dictionary.entries.select do |entry| - entry.sharding_key.blank? && + ::Gitlab::Database::Dictionary.entries.filter_map do |entry| + entry.table_name if entry.sharding_key.blank? && entry.milestone.to_f >= starting_from_milestone && ::Gitlab::Database::GitlabSchema.cell_local?(entry.gitlab_schema) - end.map(&:table_name) + end end def all_tables_to_sharding_key diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 8da05ed7b7e..6d5c17176dc 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -176,6 +176,7 @@ project_members: - project - member_namespace - member_role +- member_approvals member_roles: - members - namespace @@ -838,6 +839,7 @@ project: - organization - dora_performance_scores - xray_reports +- member_approvals award_emoji: - awardable - user diff --git a/spec/models/members/members/member_approval_spec.rb b/spec/models/members/members/member_approval_spec.rb new file mode 100644 index 00000000000..ed012a5a7c0 --- /dev/null +++ b/spec/models/members/members/member_approval_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Members::MemberApproval, feature_category: :groups_and_projects do + describe 'associations' do + it { is_expected.to belong_to(:member) } + it { is_expected.to belong_to(:member_namespace) } + it { is_expected.to belong_to(:reviewed_by) } + it { is_expected.to belong_to(:requested_by) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:new_access_level) } + it { is_expected.to validate_presence_of(:old_access_level) } + end +end diff --git a/spec/services/merge_requests/request_review_service_spec.rb b/spec/services/merge_requests/request_review_service_spec.rb index a5f0d5b5c5a..7e2931288de 100644 --- a/spec/services/merge_requests/request_review_service_spec.rb +++ b/spec/services/merge_requests/request_review_service_spec.rb @@ -82,6 +82,29 @@ RSpec.describe MergeRequests::RequestReviewService, feature_category: :code_revi it_behaves_like 'triggers GraphQL subscription mergeRequestReviewersUpdated' do let(:action) { result } end + + it 'calls MergeRequests::RemoveApprovalService' do + expect_next_instance_of( + MergeRequests::RemoveApprovalService, + project: project, current_user: current_user + ) do |service| + expect(service).to receive(:execute).with(merge_request).and_return({ success: true }) + end + + service.execute(merge_request, user) + end + + describe 'mr_request_changes feature flag is disabled' do + before do + stub_feature_flags(mr_request_changes: false) + end + + it 'does not call MergeRequests::RemoveApprovalService' do + expect(MergeRequests::RemoveApprovalService).not_to receive(:new) + + service.execute(merge_request, user) + end + end end end end |