From 14c866bb2ff6550420257cee619828a7510433dc Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 3 Nov 2021 06:10:20 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- Gemfile | 2 +- Gemfile.lock | 4 ++-- .../concerns/merge_request_reviewer_state.rb | 8 ++++++++ config/boot.rb | 4 +++- doc/administration/environment_variables.md | 2 +- .../3_create/gitaly/distributed_reads_spec.rb | 10 +++++++--- .../user_merge_request_interaction_type_spec.rb | 5 +++-- spec/models/merge_request_assignee_spec.rb | 2 ++ spec/models/merge_request_reviewer_spec.rb | 2 ++ .../users/merge_request_interaction_spec.rb | 5 +++-- .../api/graphql/project/merge_request_spec.rb | 16 ++++++++-------- .../models/reviewer_state_shared_examples.rb | 15 +++++++++++++++ 12 files changed, 55 insertions(+), 20 deletions(-) create mode 100644 spec/support/shared_examples/models/reviewer_state_shared_examples.rb diff --git a/Gemfile b/Gemfile index 6146d6e011f..d954c75b876 100644 --- a/Gemfile +++ b/Gemfile @@ -4,7 +4,7 @@ source 'https://rubygems.org' gem 'rails', '~> 6.1.4.1' -gem 'bootsnap', '~> 1.4.6' +gem 'bootsnap', '~> 1.9.1', require: false # Responders respond_to and respond_with gem 'responders', '~> 3.0' diff --git a/Gemfile.lock b/Gemfile.lock index b82fdfc81c7..55f411f6a1f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -148,7 +148,7 @@ GEM rack (>= 0.9.0) bindata (2.4.10) binding_ninja (0.2.3) - bootsnap (1.4.6) + bootsnap (1.9.1) msgpack (~> 1.0) bootstrap_form (4.2.0) actionpack (>= 5.0) @@ -1403,7 +1403,7 @@ DEPENDENCIES benchmark-ips (~> 2.3.0) benchmark-memory (~> 0.1) better_errors (~> 2.9.0) - bootsnap (~> 1.4.6) + bootsnap (~> 1.9.1) bootstrap_form (~> 4.2.0) browser (~> 4.2) bullet (~> 6.1.3) diff --git a/app/models/concerns/merge_request_reviewer_state.rb b/app/models/concerns/merge_request_reviewer_state.rb index b73a601bdb0..ba3c55adf64 100644 --- a/app/models/concerns/merge_request_reviewer_state.rb +++ b/app/models/concerns/merge_request_reviewer_state.rb @@ -13,5 +13,13 @@ module MergeRequestReviewerState validates :state, presence: true, inclusion: { in: self.states.keys } + + after_initialize :set_state, unless: :persisted? + + def set_state + if Feature.enabled?(:mr_attention_requests, self.merge_request&.project, default_enabled: :yaml) + self.state = :attention_required + end + end end end diff --git a/config/boot.rb b/config/boot.rb index afa3c04c3c7..ec9470bc506 100644 --- a/config/boot.rb +++ b/config/boot.rb @@ -1,4 +1,6 @@ # frozen_string_literal: true require_relative 'bundler_setup' -require 'bootsnap/setup' if ENV['RAILS_ENV'] != 'production' || %w(1 yes true).include?(ENV['ENABLE_BOOTSNAP']) + +enable_bootsnap_default_value = ENV['RAILS_ENV'] != 'production' ? '1' : '0' +require 'bootsnap/setup' if %w(1 yes true).include?(ENV.fetch('ENABLE_BOOTSNAP', enable_bootsnap_default_value)) diff --git a/doc/administration/environment_variables.md b/doc/administration/environment_variables.md index 3af80363916..21e32d145bd 100644 --- a/doc/administration/environment_variables.md +++ b/doc/administration/environment_variables.md @@ -20,7 +20,7 @@ You can use the following environment variables to override certain values: | Variable | Type | Description | |--------------------------------------------|---------|---------------------------------------------------------------------------------------------------------| | `DATABASE_URL` | string | The database URL; is of the form: `postgresql://localhost/blog_development`. | -| `ENABLE_BOOTSNAP` | string | Enables Bootsnap for speeding up initial Rails boot (`1` to enable). | +| `ENABLE_BOOTSNAP` | string | Toggles [Bootsnap](https://github.com/Shopify/bootsnap) for speeding up initial Rails boot. Enabled by default for non-production environments. Set to `0` to disable. | | `EXTERNAL_URL` | string | Specify the external URL at the [time of installation](https://docs.gitlab.com/omnibus/settings/configuration.html#specifying-the-external-url-at-the-time-of-installation). | | `EXTERNAL_VALIDATION_SERVICE_TIMEOUT` | integer | Timeout, in seconds, for an [external CI/CD pipeline validation service](external_pipeline_validation.md). Default is `5`. | | `EXTERNAL_VALIDATION_SERVICE_URL` | string | URL to an [external CI/CD pipeline validation service](external_pipeline_validation.md). | diff --git a/qa/qa/specs/features/api/3_create/gitaly/distributed_reads_spec.rb b/qa/qa/specs/features/api/3_create/gitaly/distributed_reads_spec.rb index 1aea1bd1189..dfc2de02bf0 100644 --- a/qa/qa/specs/features/api/3_create/gitaly/distributed_reads_spec.rb +++ b/qa/qa/specs/features/api/3_create/gitaly/distributed_reads_spec.rb @@ -16,9 +16,15 @@ module QA end before do + praefect_manager.start_all_nodes praefect_manager.wait_for_replication(project.id) end + after do + # Leave the cluster in a suitable state for subsequent tests + praefect_manager.start_all_nodes + end + it 'reads from each node', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/quality/test_cases/1264' do pre_read_data = praefect_manager.query_read_distribution @@ -42,9 +48,7 @@ module QA after do # Leave the cluster in a suitable state for subsequent tests - praefect_manager.start_secondary_node - praefect_manager.wait_for_health_check_all_nodes - praefect_manager.wait_for_reliable_connection + praefect_manager.start_all_nodes end it 'does not read from the unhealthy node', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/quality/test_cases/1263' do diff --git a/spec/graphql/types/user_merge_request_interaction_type_spec.rb b/spec/graphql/types/user_merge_request_interaction_type_spec.rb index f424c9200ab..8da94d775c3 100644 --- a/spec/graphql/types/user_merge_request_interaction_type_spec.rb +++ b/spec/graphql/types/user_merge_request_interaction_type_spec.rb @@ -78,7 +78,7 @@ RSpec.describe GitlabSchema.types['UserMergeRequestInteraction'] do merge_request.reviewers << user end - it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['UNREVIEWED'].value) } + it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['ATTENTION_REQUIRED'].value) } it 'implies not reviewed' do expect(resolve(:reviewed)).to be false @@ -87,7 +87,8 @@ RSpec.describe GitlabSchema.types['UserMergeRequestInteraction'] do context 'when the user has provided a review' do before do - merge_request.merge_request_reviewers.create!(reviewer: user, state: MergeRequestReviewer.states['reviewed']) + reviewer = merge_request.merge_request_reviewers.create!(reviewer: user) + reviewer.update!(state: MergeRequestReviewer.states['reviewed']) end it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['REVIEWED'].value) } diff --git a/spec/models/merge_request_assignee_spec.rb b/spec/models/merge_request_assignee_spec.rb index 650bef888cf..5bb8e7184a3 100644 --- a/spec/models/merge_request_assignee_spec.rb +++ b/spec/models/merge_request_assignee_spec.rb @@ -39,4 +39,6 @@ RSpec.describe MergeRequestAssignee do end it_behaves_like 'having unique enum values' + + it_behaves_like 'having reviewer state' end diff --git a/spec/models/merge_request_reviewer_spec.rb b/spec/models/merge_request_reviewer_spec.rb index 1b8fd5da205..d69d60c94f0 100644 --- a/spec/models/merge_request_reviewer_spec.rb +++ b/spec/models/merge_request_reviewer_spec.rb @@ -9,6 +9,8 @@ RSpec.describe MergeRequestReviewer do it_behaves_like 'having unique enum values' + it_behaves_like 'having reviewer state' + describe 'associations' do it { is_expected.to belong_to(:merge_request).class_name('MergeRequest') } it { is_expected.to belong_to(:reviewer).class_name('User').inverse_of(:merge_request_reviewers) } diff --git a/spec/models/users/merge_request_interaction_spec.rb b/spec/models/users/merge_request_interaction_spec.rb index d333577fa1a..de75f26989c 100644 --- a/spec/models/users/merge_request_interaction_spec.rb +++ b/spec/models/users/merge_request_interaction_spec.rb @@ -61,7 +61,7 @@ RSpec.describe ::Users::MergeRequestInteraction do merge_request.reviewers << user end - it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['UNREVIEWED'].value) } + it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['ATTENTION_REQUIRED'].value) } it 'implies not reviewed' do expect(interaction).not_to be_reviewed @@ -70,7 +70,8 @@ RSpec.describe ::Users::MergeRequestInteraction do context 'when the user has provided a review' do before do - merge_request.merge_request_reviewers.create!(reviewer: user, state: MergeRequestReviewer.states['reviewed']) + reviewer = merge_request.merge_request_reviewers.create!(reviewer: user) + reviewer.update!(state: MergeRequestReviewer.states['reviewed']) end it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['REVIEWED'].value) } diff --git a/spec/requests/api/graphql/project/merge_request_spec.rb b/spec/requests/api/graphql/project/merge_request_spec.rb index 438ea9bb4c1..8317ea57794 100644 --- a/spec/requests/api/graphql/project/merge_request_spec.rb +++ b/spec/requests/api/graphql/project/merge_request_spec.rb @@ -347,7 +347,7 @@ RSpec.describe 'getting merge request information nested in a project' do expect(interaction_data).to contain_exactly a_hash_including( 'canMerge' => false, 'canUpdate' => can_update, - 'reviewState' => unreviewed, + 'reviewState' => attention_required, 'reviewed' => false, 'approved' => false ) @@ -380,8 +380,8 @@ RSpec.describe 'getting merge request information nested in a project' do describe 'scalability' do let_it_be(:other_users) { create_list(:user, 3) } - let(:unreviewed) do - { 'reviewState' => 'UNREVIEWED' } + let(:attention_required) do + { 'reviewState' => 'ATTENTION_REQUIRED' } end let(:reviewed) do @@ -413,9 +413,9 @@ RSpec.describe 'getting merge request information nested in a project' do expect { post_graphql(query) }.not_to exceed_query_limit(baseline) expect(interaction_data).to contain_exactly( - include(unreviewed), - include(unreviewed), - include(unreviewed), + include(attention_required), + include(attention_required), + include(attention_required), include(reviewed) ) end @@ -444,7 +444,7 @@ RSpec.describe 'getting merge request information nested in a project' do it_behaves_like 'when requesting information about MR interactions' do let(:field) { :reviewers } - let(:unreviewed) { 'UNREVIEWED' } + let(:attention_required) { 'ATTENTION_REQUIRED' } let(:can_update) { false } def assign_user(user) @@ -454,7 +454,7 @@ RSpec.describe 'getting merge request information nested in a project' do it_behaves_like 'when requesting information about MR interactions' do let(:field) { :assignees } - let(:unreviewed) { nil } + let(:attention_required) { nil } let(:can_update) { true } # assignees can update MRs def assign_user(user) diff --git a/spec/support/shared_examples/models/reviewer_state_shared_examples.rb b/spec/support/shared_examples/models/reviewer_state_shared_examples.rb new file mode 100644 index 00000000000..c2fac7fe25b --- /dev/null +++ b/spec/support/shared_examples/models/reviewer_state_shared_examples.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'having reviewer state' do + describe 'mr_attention_requests feature flag is disabled' do + before do + stub_feature_flags(mr_attention_requests: false) + end + + it { is_expected.to have_attributes(state: 'unreviewed') } + end + + describe 'mr_attention_requests feature flag is enabled' do + it { is_expected.to have_attributes(state: 'attention_required') } + end +end