diff --git a/app/assets/javascripts/captcha/init_recaptcha_script.js b/app/assets/javascripts/captcha/init_recaptcha_script.js index adc69862a85..b9df7604ed1 100644 --- a/app/assets/javascripts/captcha/init_recaptcha_script.js +++ b/app/assets/javascripts/captcha/init_recaptcha_script.js @@ -2,9 +2,6 @@ import { memoize } from 'lodash'; export const RECAPTCHA_API_URL_PREFIX = 'https://www.google.com/recaptcha/api.js'; -/** - * The name which will be used for the reCAPTCHA script's onload callback - */ export const RECAPTCHA_ONLOAD_CALLBACK_NAME = 'recaptchaOnloadCallback'; /** @@ -21,9 +18,7 @@ export const RECAPTCHA_ONLOAD_CALLBACK_NAME = 'recaptchaOnloadCallback'; * */ export const initRecaptchaScript = memoize(() => { - /** - * Appends the the reCAPTCHA script tag to the head of document - */ + // Appends the the reCAPTCHA script tag to the head of document const appendRecaptchaScript = () => { const script = document.createElement('script'); script.src = `${RECAPTCHA_API_URL_PREFIX}?onload=${RECAPTCHA_ONLOAD_CALLBACK_NAME}&render=explicit`; @@ -31,11 +26,14 @@ export const initRecaptchaScript = memoize(() => { document.head.appendChild(script); }; - /** - * Returns a Promise which is fulfilled after the reCAPTCHA script is loaded - */ return new Promise((resolve) => { - window[RECAPTCHA_ONLOAD_CALLBACK_NAME] = resolve; + // This global callback resolves the Promise and is passed by name to the reCAPTCHA script. + window[RECAPTCHA_ONLOAD_CALLBACK_NAME] = (val) => { + // Let's clean up after ourselves. This is also important for testing, because `window` is NOT cleared between tests. + // https://github.com/facebook/jest/issues/1224#issuecomment-444586798. + delete window[RECAPTCHA_ONLOAD_CALLBACK_NAME]; + resolve(val); + }; appendRecaptchaScript(); }); }); diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 27f66fb8f19..ffa33bed7e6 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -803,7 +803,7 @@ module Ci variables.concat(merge_request.predefined_variables) end - if Gitlab::Ci::Features.pipeline_open_merge_requests?(project) && open_merge_requests_refs.any? + if open_merge_requests_refs.any? variables.append(key: 'CI_OPEN_MERGE_REQUESTS', value: open_merge_requests_refs.join(',')) end diff --git a/app/services/packages/debian/destroy_distribution_service.rb b/app/services/packages/debian/destroy_distribution_service.rb new file mode 100644 index 00000000000..bef1127fece --- /dev/null +++ b/app/services/packages/debian/destroy_distribution_service.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module Packages + module Debian + class DestroyDistributionService + def initialize(distribution) + @distribution = distribution + end + + def execute + destroy_distribution + end + + private + + def destroy_distribution + if @distribution.destroy + success + else + error("Unable to destroy Debian #{@distribution.model_name.human.downcase}") + end + end + + def success + ServiceResponse.success + end + + def error(message) + ServiceResponse.error(message: message, payload: { distribution: @distribution }) + end + end + end +end diff --git a/app/services/packages/debian/update_distribution_service.rb b/app/services/packages/debian/update_distribution_service.rb new file mode 100644 index 00000000000..5bb59b854e9 --- /dev/null +++ b/app/services/packages/debian/update_distribution_service.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +module Packages + module Debian + class UpdateDistributionService + def initialize(distribution, params) + @distribution, @params = distribution, params + + @components = params.delete(:components) + + @architectures = params.delete(:architectures) + @architectures += ['all'] unless @architectures.nil? + + @errors = [] + end + + def execute + update_distribution + end + + private + + attr_reader :distribution, :params, :components, :architectures, :errors + + def append_errors(record, prefix = '') + return if record.valid? + + prefix = "#{prefix} " unless prefix.empty? + @errors += record.errors.full_messages.map { |message| "#{prefix}#{message}" } + end + + def update_distribution + distribution.transaction do + if distribution.update(params) + update_components if components + update_architectures if architectures + + success + else + append_errors(distribution) + error + end + end || error + end + + def update_components + update_objects(distribution.components, components, error_label: 'Component') + end + + def update_architectures + update_objects(distribution.architectures, architectures, error_label: 'Architecture') + end + + def update_objects(objects, object_names_from_params, error_label: ) + current_object_names = objects.map(&:name) + missing_object_names = object_names_from_params - current_object_names + extra_object_names = current_object_names - object_names_from_params + + missing_object_names.each do |name| + new_object = objects.create(name: name) + append_errors(new_object, error_label) + raise ActiveRecord::Rollback unless new_object.persisted? + end + + extra_object_names.each do |name| + object = objects.with_name(name).first + raise ActiveRecord::Rollback unless object.destroy + end + end + + def success + ServiceResponse.success(payload: { distribution: distribution }) + end + + def error + ServiceResponse.error(message: errors.to_sentence, payload: { distribution: distribution }) + end + end + end +end diff --git a/config/feature_flags/development/ci_pipeline_open_merge_requests.yml b/config/feature_flags/development/ci_pipeline_open_merge_requests.yml deleted file mode 100644 index 7e2ae1edd34..00000000000 --- a/config/feature_flags/development/ci_pipeline_open_merge_requests.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: ci_pipeline_open_merge_requests -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38673 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/292727 -milestone: '13.7' -group: group::memory -type: development -default_enabled: true diff --git a/lib/gitlab/ci/features.rb b/lib/gitlab/ci/features.rb index c69b11506e9..54c393d8167 100644 --- a/lib/gitlab/ci/features.rb +++ b/lib/gitlab/ci/features.rb @@ -55,10 +55,6 @@ module Gitlab ::Feature.enabled?(:ci_trace_log_invalid_chunks, project, type: :ops, default_enabled: false) end - def self.pipeline_open_merge_requests?(project) - ::Feature.enabled?(:ci_pipeline_open_merge_requests, project, default_enabled: true) - end - def self.ci_pipeline_editor_page_enabled?(project) ::Feature.enabled?(:ci_pipeline_editor_page, project, default_enabled: :yaml) end diff --git a/qa/qa/flow/project.rb b/qa/qa/flow/project.rb index db42a3a3594..8a9e2c86332 100644 --- a/qa/qa/flow/project.rb +++ b/qa/qa/flow/project.rb @@ -5,16 +5,6 @@ module QA module Project module_function - def add_member(project:, username:) - project.visit! - - Page::Project::Menu.perform(&:click_members) - - Page::Project::Members.perform do |member_settings| - member_settings.add_member(username) - end - end - def go_to_create_project_from_template if Page::Project::NewExperiment.perform(&:shown?) Page::Project::NewExperiment.perform(&:click_create_from_template_link) diff --git a/qa/qa/specs/features/browser_ui/2_plan/email/trigger_email_notification_spec.rb b/qa/qa/specs/features/browser_ui/2_plan/email/trigger_email_notification_spec.rb index 418865a63d2..fbaf4241cec 100644 --- a/qa/qa/specs/features/browser_ui/2_plan/email/trigger_email_notification_spec.rb +++ b/qa/qa/specs/features/browser_ui/2_plan/email/trigger_email_notification_spec.rb @@ -20,7 +20,12 @@ module QA end it 'is received by a user for project invitation', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/676' do - Flow::Project.add_member(project: project, username: user.username) + project.visit! + + Page::Project::Menu.perform(&:click_members) + Page::Project::Members.perform do |member_settings| + member_settings.add_member(user.username) + end expect(page).to have_content(/@#{user.username}(\n| )?Given access/) diff --git a/qa/qa/specs/features/browser_ui/2_plan/issue/check_mentions_for_xss_spec.rb b/qa/qa/specs/features/browser_ui/2_plan/issue/check_mentions_for_xss_spec.rb index e394f6b1e9c..a7f862e8911 100644 --- a/qa/qa/specs/features/browser_ui/2_plan/issue/check_mentions_for_xss_spec.rb +++ b/qa/qa/specs/features/browser_ui/2_plan/issue/check_mentions_for_xss_spec.rb @@ -20,7 +20,7 @@ module QA before do Flow::Login.sign_in - Flow::Project.add_member(project: project, username: user.username) + project.add_member(user) Resource::Issue.fabricate_via_api! do |issue| issue.project = project diff --git a/spec/frontend/captcha/init_recaptcha_script_spec.js b/spec/frontend/captcha/init_recaptcha_script_spec.js index 77f78d36991..df114821651 100644 --- a/spec/frontend/captcha/init_recaptcha_script_spec.js +++ b/spec/frontend/captcha/init_recaptcha_script_spec.js @@ -7,19 +7,11 @@ import { describe('initRecaptchaScript', () => { afterEach(() => { - // NOTE: The DOM is guaranteed to be clean at the start of a new test file, but it isn't cleaned - // between examples within a file, so we need to clean it after each one. See more context here: - // - https://github.com/facebook/jest/issues/1224 - // - https://stackoverflow.com/questions/42805128/does-jest-reset-the-jsdom-document-after-every-suite-or-test - // - // Also note as mentioned in https://github.com/facebook/jest/issues/1224#issuecomment-444586798 - // that properties of `window` are NOT cleared between test files. So, we are also - // explicitly unsetting it. document.head.innerHTML = ''; - window[RECAPTCHA_ONLOAD_CALLBACK_NAME] = undefined; clearMemoizeCache(); }); + const getScriptOnload = () => window[RECAPTCHA_ONLOAD_CALLBACK_NAME]; const triggerScriptOnload = (...args) => window[RECAPTCHA_ONLOAD_CALLBACK_NAME](...args); describe('when called', () => { @@ -51,6 +43,7 @@ describe('initRecaptchaScript', () => { triggerScriptOnload(instance); await expect(result).resolves.toBe(instance); + expect(getScriptOnload()).toBeUndefined(); }); }); }); diff --git a/spec/services/packages/debian/destroy_distribution_service_spec.rb b/spec/services/packages/debian/destroy_distribution_service_spec.rb new file mode 100644 index 00000000000..d658b0ed66e --- /dev/null +++ b/spec/services/packages/debian/destroy_distribution_service_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::Debian::DestroyDistributionService do + RSpec.shared_examples 'Destroy Debian Distribution' do |expected_message| + it 'returns ServiceResponse', :aggregate_failures do + if expected_message.nil? + expect { response } + .to change { container.debian_distributions.klass.all.count } + .from(1).to(0) + .and change { container.debian_distributions.count } + .from(1).to(0) + .and change { component1.class.all.count } + .from(2).to(0) + .and change { architecture1.class.all.count } + .from(3).to(0) + else + expect { response } + .to not_change { container.debian_distributions.klass.all.count } + .and not_change { container.debian_distributions.count } + .and not_change { component1.class.all.count } + .and not_change { architecture1.class.all.count } + end + + expect(response).to be_a(ServiceResponse) + expect(response.success?).to eq(expected_message.nil?) + expect(response.error?).to eq(!expected_message.nil?) + expect(response.message).to eq(expected_message) + + if expected_message.nil? + expect(response.payload).to eq({}) + else + expect(response.payload).to eq(distribution: distribution) + end + end + end + + RSpec.shared_examples 'Debian Destroy Distribution Service' do |container_type, can_freeze| + context "with a Debian #{container_type} distribution" do + let_it_be(:container, freeze: can_freeze) { create(container_type) } # rubocop:disable Rails/SaveBang + let_it_be(:distribution, freeze: can_freeze) { create("debian_#{container_type}_distribution", container: container) } + let_it_be(:component1, freeze: can_freeze) { create("debian_#{container_type}_component", distribution: distribution, name: 'component1') } + let_it_be(:component2, freeze: can_freeze) { create("debian_#{container_type}_component", distribution: distribution, name: 'component2') } + let_it_be(:architecture0, freeze: true) { create("debian_#{container_type}_architecture", distribution: distribution, name: 'all') } + let_it_be(:architecture1, freeze: can_freeze) { create("debian_#{container_type}_architecture", distribution: distribution, name: 'architecture1') } + let_it_be(:architecture2, freeze: can_freeze) { create("debian_#{container_type}_architecture", distribution: distribution, name: 'architecture2') } + + subject { described_class.new(distribution) } + + let(:response) { subject.execute } + + context 'with a distribution' do + it_behaves_like 'Destroy Debian Distribution' + end + + context 'when destroy fails' do + before do + expect(distribution).to receive(:destroy).and_return(false) + end + + it_behaves_like 'Destroy Debian Distribution', "Unable to destroy Debian #{container_type} distribution" + end + end + end + + it_behaves_like 'Debian Destroy Distribution Service', :project, true + it_behaves_like 'Debian Destroy Distribution Service', :group, false +end diff --git a/spec/services/packages/debian/update_distribution_service_spec.rb b/spec/services/packages/debian/update_distribution_service_spec.rb new file mode 100644 index 00000000000..c60e8e3f735 --- /dev/null +++ b/spec/services/packages/debian/update_distribution_service_spec.rb @@ -0,0 +1,144 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Packages::Debian::UpdateDistributionService do + RSpec.shared_examples 'Update Debian Distribution' do |expected_message, expected_components, expected_architectures| + it 'returns ServiceResponse', :aggregate_failures do + expect(distribution).to receive(:update).with(simple_params).and_call_original if expected_message.nil? + + expect { response } + .to not_change { container.debian_distributions.klass.all.count } + .and not_change { container.debian_distributions.count } + .and not_change { component1.class.all.count } + .and not_change { architecture1.class.all.count } + + expect(response).to be_a(ServiceResponse) + expect(response.success?).to eq(expected_message.nil?) + expect(response.error?).to eq(!expected_message.nil?) + expect(response.message).to eq(expected_message) + + expect(response.payload).to eq(distribution: distribution) + + distribution.reload + distribution.components.reload + distribution.architectures.reload + + if expected_message.nil? + simple_params.each_pair do |name, value| + expect(distribution.send(name)).to eq(value) + end + else + original_params.each_pair do |name, value| + expect(distribution.send(name)).to eq(value) + end + end + + expect(distribution.components.map(&:name)).to contain_exactly(*expected_components) + expect(distribution.architectures.map(&:name)).to contain_exactly(*expected_architectures) + end + end + + RSpec.shared_examples 'Debian Update Distribution Service' do |container_type, can_freeze| + context "with a Debian #{container_type} distribution" do + let_it_be(:container, freeze: can_freeze) { create(container_type) } # rubocop:disable Rails/SaveBang + let_it_be(:distribution, reload: true) { create("debian_#{container_type}_distribution", container: container) } + let_it_be(:component1) { create("debian_#{container_type}_component", distribution: distribution, name: 'component1') } + let_it_be(:component2) { create("debian_#{container_type}_component", distribution: distribution, name: 'component2') } + let_it_be(:architecture0) { create("debian_#{container_type}_architecture", distribution: distribution, name: 'all') } + let_it_be(:architecture1) { create("debian_#{container_type}_architecture", distribution: distribution, name: 'architecture1') } + let_it_be(:architecture2) { create("debian_#{container_type}_architecture", distribution: distribution, name: 'architecture2') } + + let(:original_params) do + { + suite: nil, + origin: nil, + label: nil, + version: nil, + description: nil, + valid_time_duration_seconds: nil, + automatic: true, + automatic_upgrades: false + } + end + + let(:params) { {} } + let(:simple_params) { params.except(:components, :architectures) } + + subject { described_class.new(distribution, params) } + + let(:response) { subject.execute } + + context 'with valid simple params' do + let(:params) do + { + suite: 'my-suite', + origin: 'my-origin', + label: 'my-label', + version: '42.0', + description: 'my-description', + valid_time_duration_seconds: 7.days, + automatic: false, + automatic_upgrades: true + } + end + + it_behaves_like 'Update Debian Distribution', nil, %w[component1 component2], %w[all architecture1 architecture2] + end + + context 'with invalid simple params' do + let(:params) do + { + suite: 'suite erronée', + origin: 'origin erronée', + label: 'label erronée', + version: 'version erronée', + description: 'description erronée', + valid_time_duration_seconds: 1.hour + } + end + + it_behaves_like 'Update Debian Distribution', 'Suite is invalid, Origin is invalid, Label is invalid, Version is invalid, and Valid time duration seconds must be greater than or equal to 86400', %w[component1 component2], %w[all architecture1 architecture2] + end + + context 'with valid components and architectures' do + let(:params) do + { + suite: 'my-suite', + components: %w[component2 component3], + architectures: %w[architecture2 architecture3] + } + end + + it_behaves_like 'Update Debian Distribution', nil, %w[component2 component3], %w[all architecture2 architecture3] + end + + context 'with invalid components' do + let(:params) do + { + suite: 'my-suite', + components: %w[component2 erroné], + architectures: %w[architecture2 architecture3] + } + end + + it_behaves_like 'Update Debian Distribution', 'Component Name is invalid', %w[component1 component2], %w[all architecture1 architecture2] + end + + context 'with invalid architectures' do + let(:params) do + { + suite: 'my-suite', + components: %w[component2 component3], + architectures: %w[architecture2 erroné] + } + end + + it_behaves_like 'Update Debian Distribution', 'Architecture Name is invalid', %w[component1 component2], %w[all architecture1 architecture2] + end + end + end + + it_behaves_like 'Debian Update Distribution Service', :project, true + it_behaves_like 'Debian Update Distribution Service', :group, false +end