diff --git a/app/assets/javascripts/blob/viewer/index.js b/app/assets/javascripts/blob/viewer/index.js index b4cd0d5d875..4741152afce 100644 --- a/app/assets/javascripts/blob/viewer/index.js +++ b/app/assets/javascripts/blob/viewer/index.js @@ -62,6 +62,7 @@ export default class BlobViewer { this.switcher = document.querySelector('.js-blob-viewer-switcher'); this.switcherBtns = document.querySelectorAll('.js-blob-viewer-switch-btn'); this.copySourceBtn = document.querySelector('.js-copy-blob-source-btn'); + this.copySourceBtnTooltip = document.querySelector('.js-copy-blob-source-btn-tooltip'); this.simpleViewer = this.$fileHolder[0].querySelector('.blob-viewer[data-type="simple"]'); this.richViewer = this.$fileHolder[0].querySelector('.blob-viewer[data-type="rich"]'); @@ -109,23 +110,23 @@ export default class BlobViewer { toggleCopyButtonState() { if (!this.copySourceBtn) return; if (this.simpleViewer.getAttribute('data-loaded')) { - this.copySourceBtn.setAttribute('title', __('Copy file contents')); + this.copySourceBtnTooltip.setAttribute('title', __('Copy file contents')); this.copySourceBtn.classList.remove('disabled'); } else if (this.activeViewer === this.simpleViewer) { - this.copySourceBtn.setAttribute( + this.copySourceBtnTooltip.setAttribute( 'title', __('Wait for the file to load to copy its contents'), ); this.copySourceBtn.classList.add('disabled'); } else { - this.copySourceBtn.setAttribute( + this.copySourceBtnTooltip.setAttribute( 'title', __('Switch to the source to copy the file contents'), ); this.copySourceBtn.classList.add('disabled'); } - fixTitle($(this.copySourceBtn)); + fixTitle($(this.copySourceBtnTooltip)); } switchToViewer(name) { diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue index d022579ef54..adc4bdf745e 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue @@ -246,9 +246,6 @@ export default {
diff --git a/app/assets/javascripts/vue_merge_request_widget/mixins/mr_widget_pipeline.js b/app/assets/javascripts/vue_merge_request_widget/mixins/mr_widget_pipeline.js index 96e8bb45e34..7b77d7475bc 100644 --- a/app/assets/javascripts/vue_merge_request_widget/mixins/mr_widget_pipeline.js +++ b/app/assets/javascripts/vue_merge_request_widget/mixins/mr_widget_pipeline.js @@ -7,9 +7,4 @@ export default { return []; }, }, - methods: { - hasDownstream() { - return false; - }, - }, }; diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index 28a947a6ca1..41bbd0fddd5 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -235,7 +235,9 @@ module BlobHelper def copy_blob_source_button(blob) return unless blob.rendered_as_text?(ignore_errors: false) - clipboard_button(target: ".blob-content[data-blob-id='#{blob.id}'] > pre", class: "btn gl-button btn-default btn-icon js-copy-blob-source-btn", title: _("Copy file contents")) + content_tag(:span, class: 'btn-group has-tooltip js-copy-blob-source-btn-tooltip') do + clipboard_button(target: ".blob-content[data-blob-id='#{blob.id}'] > pre", class: "btn gl-button btn-default btn-icon js-copy-blob-source-btn", hide_tooltip: true) + end end def open_raw_blob_button(blob) diff --git a/app/models/bulk_imports/entity.rb b/app/models/bulk_imports/entity.rb index ae1e3693809..9127dab56a6 100644 --- a/app/models/bulk_imports/entity.rb +++ b/app/models/bulk_imports/entity.rb @@ -118,8 +118,8 @@ class BulkImports::Entity < ApplicationRecord if source.self_and_descendants.any? { |namespace| namespace.full_path == destination_namespace } errors.add( - :destination_namespace, - s_('BulkImport|destination group cannot be part of the source group tree') + :base, + s_('BulkImport|Import failed: Destination cannot be a subgroup of the source group. Change the destination and try again.') ) end end diff --git a/app/models/group.rb b/app/models/group.rb index 82142a7fc3f..a62a2b3a5fb 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -33,8 +33,6 @@ class Group < Namespace has_many :members_and_requesters, as: :source, class_name: 'GroupMember' has_many :milestones - has_many :iterations - has_many :iterations_cadences, class_name: 'Iterations::Cadence' has_many :services has_many :shared_group_links, foreign_key: :shared_with_group_id, class_name: 'GroupGroupLink' has_many :shared_with_group_links, foreign_key: :shared_group_id, class_name: 'GroupGroupLink' diff --git a/app/models/iteration.rb b/app/models/iteration.rb index ccf357a1f6b..7483d04aab8 100644 --- a/app/models/iteration.rb +++ b/app/models/iteration.rb @@ -1,172 +1,16 @@ # frozen_string_literal: true +# Placeholder class for model that is implemented in EE class Iteration < ApplicationRecord self.table_name = 'sprints' - attr_accessor :skip_future_date_validation - attr_accessor :skip_project_validation - - STATE_ENUM_MAP = { - upcoming: 1, - started: 2, - closed: 3 - }.with_indifferent_access.freeze - - include AtomicInternalId - include Timebox - - belongs_to :project - belongs_to :group - belongs_to :iterations_cadence, class_name: 'Iterations::Cadence', foreign_key: :iterations_cadence_id, inverse_of: :iterations - - has_internal_id :iid, scope: :project - has_internal_id :iid, scope: :group - - validates :start_date, presence: true - validates :due_date, presence: true - validates :iterations_cadence, presence: true, unless: -> { project_id.present? } - - validate :dates_do_not_overlap, if: :start_or_due_dates_changed? - validate :future_date, if: :start_or_due_dates_changed?, unless: :skip_future_date_validation - validate :no_project, unless: :skip_project_validation - validate :validate_group - - before_validation :set_iterations_cadence, unless: -> { project_id.present? } - before_create :set_past_iteration_state - - scope :upcoming, -> { with_state(:upcoming) } - scope :started, -> { with_state(:started) } - scope :closed, -> { with_state(:closed) } - - scope :within_timeframe, -> (start_date, end_date) do - where('start_date <= ?', end_date).where('due_date >= ?', start_date) + def self.reference_prefix + '*iteration:' end - scope :start_date_passed, -> { where('start_date <= ?', Date.current).where('due_date >= ?', Date.current) } - scope :due_date_passed, -> { where('due_date < ?', Date.current) } - - state_machine :state_enum, initial: :upcoming do - event :start do - transition upcoming: :started - end - - event :close do - transition [:upcoming, :started] => :closed - end - - state :upcoming, value: Iteration::STATE_ENUM_MAP[:upcoming] - state :started, value: Iteration::STATE_ENUM_MAP[:started] - state :closed, value: Iteration::STATE_ENUM_MAP[:closed] - end - - # Alias to state machine .with_state_enum method - # This needs to be defined after the state machine block to avoid errors - class << self - alias_method :with_state, :with_state_enum - alias_method :with_states, :with_state_enums - - def filter_by_state(iterations, state) - case state - when 'closed' then iterations.closed - when 'started' then iterations.started - when 'upcoming' then iterations.upcoming - when 'opened' then iterations.started.or(iterations.upcoming) - when 'all' then iterations - else raise ArgumentError, "Unknown state filter: #{state}" - end - end - - def reference_prefix - '*iteration:' - end - - def reference_pattern - nil - end - end - - def state - STATE_ENUM_MAP.key(state_enum) - end - - def state=(value) - self.state_enum = STATE_ENUM_MAP[value] - end - - def resource_parent - group || project - end - - private - - def parent_group - group || project.group - end - - def start_or_due_dates_changed? - start_date_changed? || due_date_changed? - end - - # ensure dates do not overlap with other Iterations in the same cadence tree - def dates_do_not_overlap - return unless iterations_cadence - return unless iterations_cadence.iterations.where.not(id: self.id).within_timeframe(start_date, due_date).exists? - - # for now we only have a single default cadence within a group just to wrap the iterations into a set. - # once we introduce multiple cadences per group we need to change this message. - # related issue: https://gitlab.com/gitlab-org/gitlab/-/issues/299312 - errors.add(:base, s_("Iteration|Dates cannot overlap with other existing Iterations within this group")) - end - - def future_date - if start_or_due_dates_changed? - errors.add(:start_date, s_("Iteration|cannot be more than 500 years in the future")) if start_date > 500.years.from_now - errors.add(:due_date, s_("Iteration|cannot be more than 500 years in the future")) if due_date > 500.years.from_now - end - end - - def no_project - return unless project_id.present? - - errors.add(:project_id, s_("is not allowed. We do not currently support project-level iterations")) - end - - def set_past_iteration_state - # if we create an iteration in the past, we set the state to closed right away, - # no need to wait for IterationsUpdateStatusWorker to do so. - self.state = :closed if due_date < Date.current - end - - # TODO: this method should be removed as part of https://gitlab.com/gitlab-org/gitlab/-/issues/296099 - def set_iterations_cadence - return if iterations_cadence - # For now we support only group iterations - # issue to clarify project iterations: https://gitlab.com/gitlab-org/gitlab/-/issues/299864 - return unless group - - # we need this as we use the cadence to validate the dates overlap for this iteration, - # so in the case this runs before background migration we need to first set all iterations - # in this group to a cadence before we can validate the dates overlap. - default_cadence = find_or_create_default_cadence - group.iterations.where(iterations_cadence_id: nil).update_all(iterations_cadence_id: default_cadence.id) - - self.iterations_cadence = default_cadence - end - - def find_or_create_default_cadence - cadence_title = "#{group.name} Iterations" - start_date = self.start_date || Date.today - - ::Iterations::Cadence.create_with(title: cadence_title, start_date: start_date).safe_find_or_create_by!(group: group) - end - - # TODO: remove this as part of https://gitlab.com/gitlab-org/gitlab/-/issues/296100 - def validate_group - return unless iterations_cadence - return if iterations_cadence.group_id == group_id - - errors.add(:group, s_('is not valid. The iteration group has to match the iteration cadence group.')) + def self.reference_pattern + nil end end -Iteration.prepend_if_ee('EE::Iteration') +Iteration.prepend_if_ee('::EE::Iteration') diff --git a/app/models/iterations/cadence.rb b/app/models/iterations/cadence.rb index 4f8e148d18f..7945fb1785a 100644 --- a/app/models/iterations/cadence.rb +++ b/app/models/iterations/cadence.rb @@ -1,14 +1,8 @@ # frozen_string_literal: true +# Placeholder class for model that is implemented in EE class Iterations::Cadence < ApplicationRecord self.table_name = 'iterations_cadences' - - belongs_to :group - has_many :iterations, foreign_key: :iterations_cadence_id, inverse_of: :iterations_cadence - - validates :title, presence: true - validates :start_date, presence: true - validates :group_id, presence: true - validates :active, presence: true - validates :automatic, presence: true end + +Iterations::Cadence.prepend_if_ee('::EE::Iterations::Cadence') diff --git a/app/views/notify/change_in_merge_request_draft_status_email.html.haml b/app/views/notify/change_in_merge_request_draft_status_email.html.haml new file mode 100644 index 00000000000..5604a30d9f1 --- /dev/null +++ b/app/views/notify/change_in_merge_request_draft_status_email.html.haml @@ -0,0 +1,2 @@ +%p + = _('%{username} changed the draft status of merge request %{mr_reference}' % {username: sanitize_name(@updated_by_user.name), mr_reference: @merge_request.to_reference }) diff --git a/app/views/notify/change_in_merge_request_draft_status_email.text.erb b/app/views/notify/change_in_merge_request_draft_status_email.text.erb new file mode 100644 index 00000000000..4e2df2dff1d --- /dev/null +++ b/app/views/notify/change_in_merge_request_draft_status_email.text.erb @@ -0,0 +1 @@ +<%= "#{sanitize_name(@updated_by_user.name)} changed the draft status of merge request #{@merge_request.to_reference}" %> diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml index 4b198717790..ff9535af233 100644 --- a/app/views/projects/diffs/_file.html.haml +++ b/app/views/projects/diffs/_file.html.haml @@ -16,8 +16,9 @@ - unless diff_file.submodule? .file-actions.d-none.d-sm-block - if diff_file.blob&.readable_text? - = link_to '#', class: 'js-toggle-diff-comments btn gl-button active has-tooltip', title: _("Toggle comments for this file"), disabled: @diff_notes_disabled do - = sprite_icon('comment') + %span.has-tooltip{ title: _("Toggle comments for this file") } + = link_to '#', class: 'js-toggle-diff-comments btn gl-button active', disabled: @diff_notes_disabled do + = sprite_icon('comment') \ - if editable_diff?(diff_file) - link_opts = @merge_request.persisted? ? { from_merge_request_iid: @merge_request.iid } : {} diff --git a/changelogs/unreleased/15332-add-draft-status-change-email.yml b/changelogs/unreleased/15332-add-draft-status-change-email.yml new file mode 100644 index 00000000000..4592395d1b2 --- /dev/null +++ b/changelogs/unreleased/15332-add-draft-status-change-email.yml @@ -0,0 +1,5 @@ +--- +title: Add notification templates for merge request draft/WIP status change events +merge_request: 54870 +author: +type: other diff --git a/changelogs/unreleased/ck3g-remove-MergeRequestAssigneesMigrationProgressCheck-background-migrat.yml b/changelogs/unreleased/ck3g-remove-MergeRequestAssigneesMigrationProgressCheck-background-migrat.yml new file mode 100644 index 00000000000..6cba82c7a5f --- /dev/null +++ b/changelogs/unreleased/ck3g-remove-MergeRequestAssigneesMigrationProgressCheck-background-migrat.yml @@ -0,0 +1,5 @@ +--- +title: Remove MergeRequestAssigneesMigrationProgressCheck background migration +merge_request: 54943 +author: +type: changed diff --git a/changelogs/unreleased/kassio-bulkimports-better-destination-validation-error-message.yml b/changelogs/unreleased/kassio-bulkimports-better-destination-validation-error-message.yml new file mode 100644 index 00000000000..5a87d152630 --- /dev/null +++ b/changelogs/unreleased/kassio-bulkimports-better-destination-validation-error-message.yml @@ -0,0 +1,5 @@ +--- +title: Better error message when import fails due to backend validation +merge_request: 54827 +author: +type: changed diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index ba44b900dbb..151af995ff6 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -533,7 +533,7 @@ Settings.cron_jobs['users_create_statistics_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['users_create_statistics_worker']['cron'] ||= '2 15 * * *' Settings.cron_jobs['users_create_statistics_worker']['job_class'] = 'Users::CreateStatisticsWorker' Settings.cron_jobs['authorized_project_update_periodic_recalculate_worker'] ||= Settingslogic.new({}) -Settings.cron_jobs['authorized_project_update_periodic_recalculate_worker']['cron'] ||= '45 1 * * 6' +Settings.cron_jobs['authorized_project_update_periodic_recalculate_worker']['cron'] ||= '45 1 1,15 * *' Settings.cron_jobs['authorized_project_update_periodic_recalculate_worker']['job_class'] = 'AuthorizedProjectUpdate::PeriodicRecalculateWorker' Settings.cron_jobs['update_container_registry_info_worker'] ||= Settingslogic.new({}) Settings.cron_jobs['update_container_registry_info_worker']['cron'] ||= '0 0 * * *' diff --git a/db/post_migrate/20190402224749_schedule_merge_request_assignees_migration_progress_check.rb b/db/post_migrate/20190402224749_schedule_merge_request_assignees_migration_progress_check.rb deleted file mode 100644 index 8ec6a4a24ec..00000000000 --- a/db/post_migrate/20190402224749_schedule_merge_request_assignees_migration_progress_check.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -class ScheduleMergeRequestAssigneesMigrationProgressCheck < ActiveRecord::Migration[5.0] - include Gitlab::Database::MigrationHelpers - - MIGRATION = 'MergeRequestAssigneesMigrationProgressCheck' - - DOWNTIME = false - - disable_ddl_transaction! - - def up - BackgroundMigrationWorker.perform_async(MIGRATION) - end - - def down - end -end diff --git a/db/schema_migrations/20190402224749 b/db/schema_migrations/20190402224749 deleted file mode 100644 index 3d02e4435c5..00000000000 --- a/db/schema_migrations/20190402224749 +++ /dev/null @@ -1 +0,0 @@ -f655a3f9f1f41710ae501c3e4ef0893791c28971d87e12f87d4b65131502b812 \ No newline at end of file diff --git a/lib/gitlab/background_migration/merge_request_assignees_migration_progress_check.rb b/lib/gitlab/background_migration/merge_request_assignees_migration_progress_check.rb deleted file mode 100644 index de0c357ab1c..00000000000 --- a/lib/gitlab/background_migration/merge_request_assignees_migration_progress_check.rb +++ /dev/null @@ -1,43 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module BackgroundMigration - # rubocop: disable Style/Documentation - class MergeRequestAssigneesMigrationProgressCheck - include Gitlab::Utils::StrongMemoize - - RESCHEDULE_DELAY = 3.hours - WORKER = 'PopulateMergeRequestAssigneesTable' - DeadJobsError = Class.new(StandardError) - - def perform - raise DeadJobsError, "Only dead background jobs in the queue for #{WORKER}" if !ongoing? && dead_jobs? - - if ongoing? - BackgroundMigrationWorker.perform_in(RESCHEDULE_DELAY, self.class.name) - else - Feature.enable(:multiple_merge_request_assignees) - end - end - - private - - def dead_jobs? - strong_memoize(:dead_jobs) do - migration_klass.dead_jobs?(WORKER) - end - end - - def ongoing? - strong_memoize(:ongoing) do - migration_klass.exists?(WORKER) || migration_klass.retrying_jobs?(WORKER) - end - end - - def migration_klass - Gitlab::BackgroundMigration - end - end - # rubocop: enable Style/Documentation - end -end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index c3937572638..b93be761110 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -5095,6 +5095,9 @@ msgstr "" msgid "BulkImport|From source group" msgstr "" +msgid "BulkImport|Import failed: Destination cannot be a subgroup of the source group. Change the destination and try again." +msgstr "" + msgid "BulkImport|Import groups from GitLab" msgstr "" @@ -5122,9 +5125,6 @@ msgstr "" msgid "BulkImport|You have no groups to import" msgstr "" -msgid "BulkImport|destination group cannot be part of the source group tree" -msgstr "" - msgid "BulkImport|expected an associated Group but has an associated Project" msgstr "" diff --git a/spec/factories/iteration_cadences.rb b/spec/factories/iteration_cadences.rb deleted file mode 100644 index b36f15e3dd4..00000000000 --- a/spec/factories/iteration_cadences.rb +++ /dev/null @@ -1,13 +0,0 @@ -# frozen_string_literal: true - -FactoryBot.define do - sequence(:cadence_sequential_date) do |n| - n.days.from_now - end - - factory :iterations_cadence, class: 'Iterations::Cadence' do - title - group - start_date { generate(:cadence_sequential_date) } - end -end diff --git a/spec/factories/iterations.rb b/spec/factories/iterations.rb deleted file mode 100644 index bd61cd469af..00000000000 --- a/spec/factories/iterations.rb +++ /dev/null @@ -1,66 +0,0 @@ -# frozen_string_literal: true - -FactoryBot.define do - sequence(:sequential_date) do |n| - n.days.from_now - end - - factory :iteration do - title - start_date { generate(:sequential_date) } - due_date { generate(:sequential_date) } - - transient do - project { nil } - group { nil } - project_id { nil } - group_id { nil } - resource_parent { nil } - end - - trait :upcoming do - state_enum { Iteration::STATE_ENUM_MAP[:upcoming] } - end - - trait :started do - state_enum { Iteration::STATE_ENUM_MAP[:started] } - end - - trait :closed do - state_enum { Iteration::STATE_ENUM_MAP[:closed] } - end - - trait(:skip_future_date_validation) do - after(:stub, :build) do |iteration| - iteration.skip_future_date_validation = true - end - end - - trait(:skip_project_validation) do - after(:stub, :build) do |iteration| - iteration.skip_project_validation = true - end - end - - after(:build, :stub) do |iteration, evaluator| - if evaluator.group - iteration.group = evaluator.group - elsif evaluator.group_id - iteration.group_id = evaluator.group_id - elsif evaluator.project - iteration.project = evaluator.project - elsif evaluator.project_id - iteration.project_id = evaluator.project_id - elsif evaluator.resource_parent - id = evaluator.resource_parent.id - evaluator.resource_parent.is_a?(Group) ? evaluator.group_id = id : evaluator.project_id = id - else - iteration.group = create(:group) - end - end - - factory :upcoming_iteration, traits: [:upcoming] - factory :started_iteration, traits: [:started] - factory :closed_iteration, traits: [:closed] - end -end diff --git a/spec/frontend/blob/viewer/index_spec.js b/spec/frontend/blob/viewer/index_spec.js index 7449de48ec0..d793b5eb190 100644 --- a/spec/frontend/blob/viewer/index_spec.js +++ b/spec/frontend/blob/viewer/index_spec.js @@ -85,9 +85,11 @@ describe('Blob viewer', () => { describe('copy blob button', () => { let copyButton; + let copyButtonTooltip; beforeEach(() => { copyButton = document.querySelector('.js-copy-blob-source-btn'); + copyButtonTooltip = document.querySelector('.js-copy-blob-source-btn-tooltip'); }); it('disabled on load', () => { @@ -95,7 +97,7 @@ describe('Blob viewer', () => { }); it('has tooltip when disabled', () => { - expect(copyButton.getAttribute('title')).toBe( + expect(copyButtonTooltip.getAttribute('title')).toBe( 'Switch to the source to copy the file contents', ); }); @@ -131,7 +133,7 @@ describe('Blob viewer', () => { document.querySelector('.js-blob-viewer-switch-btn[data-viewer="simple"]').click(); setImmediate(() => { - expect(copyButton.getAttribute('title')).toBe('Copy file contents'); + expect(copyButtonTooltip.getAttribute('title')).toBe('Copy file contents'); done(); }); diff --git a/spec/lib/gitlab/background_migration/merge_request_assignees_migration_progress_check_spec.rb b/spec/lib/gitlab/background_migration/merge_request_assignees_migration_progress_check_spec.rb deleted file mode 100644 index 85a9c88ebff..00000000000 --- a/spec/lib/gitlab/background_migration/merge_request_assignees_migration_progress_check_spec.rb +++ /dev/null @@ -1,99 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::BackgroundMigration::MergeRequestAssigneesMigrationProgressCheck do - context 'rescheduling' do - context 'when there are ongoing and no dead jobs' do - it 'reschedules check' do - allow(Gitlab::BackgroundMigration).to receive(:exists?) - .with('PopulateMergeRequestAssigneesTable') - .and_return(true) - - allow(Gitlab::BackgroundMigration).to receive(:dead_jobs?) - .with('PopulateMergeRequestAssigneesTable') - .and_return(false) - - expect(BackgroundMigrationWorker).to receive(:perform_in).with(described_class::RESCHEDULE_DELAY, described_class.name) - - described_class.new.perform - end - end - - context 'when there are ongoing and dead jobs' do - it 'reschedules check' do - allow(Gitlab::BackgroundMigration).to receive(:exists?) - .with('PopulateMergeRequestAssigneesTable') - .and_return(true) - - allow(Gitlab::BackgroundMigration).to receive(:dead_jobs?) - .with('PopulateMergeRequestAssigneesTable') - .and_return(true) - - expect(BackgroundMigrationWorker).to receive(:perform_in).with(described_class::RESCHEDULE_DELAY, described_class.name) - - described_class.new.perform - end - end - - context 'when there retrying jobs and no scheduled' do - it 'reschedules check' do - allow(Gitlab::BackgroundMigration).to receive(:exists?) - .with('PopulateMergeRequestAssigneesTable') - .and_return(false) - - allow(Gitlab::BackgroundMigration).to receive(:retrying_jobs?) - .with('PopulateMergeRequestAssigneesTable') - .and_return(true) - - expect(BackgroundMigrationWorker).to receive(:perform_in).with(described_class::RESCHEDULE_DELAY, described_class.name) - - described_class.new.perform - end - end - end - - context 'when there are no scheduled, or retrying or dead' do - before do - stub_feature_flags(multiple_merge_request_assignees: false) - end - - it 'enables feature' do - allow(Gitlab::BackgroundMigration).to receive(:exists?) - .with('PopulateMergeRequestAssigneesTable') - .and_return(false) - - allow(Gitlab::BackgroundMigration).to receive(:retrying_jobs?) - .with('PopulateMergeRequestAssigneesTable') - .and_return(false) - - allow(Gitlab::BackgroundMigration).to receive(:dead_jobs?) - .with('PopulateMergeRequestAssigneesTable') - .and_return(false) - - described_class.new.perform - - expect(Feature.enabled?(:multiple_merge_request_assignees, type: :licensed)).to eq(true) - end - end - - context 'when there are only dead jobs' do - it 'raises DeadJobsError error' do - allow(Gitlab::BackgroundMigration).to receive(:exists?) - .with('PopulateMergeRequestAssigneesTable') - .and_return(false) - - allow(Gitlab::BackgroundMigration).to receive(:retrying_jobs?) - .with('PopulateMergeRequestAssigneesTable') - .and_return(false) - - allow(Gitlab::BackgroundMigration).to receive(:dead_jobs?) - .with('PopulateMergeRequestAssigneesTable') - .and_return(true) - - expect { described_class.new.perform } - .to raise_error(described_class::DeadJobsError, - "Only dead background jobs in the queue for #{described_class::WORKER}") - end - end -end diff --git a/spec/migrations/schedule_merge_request_assignees_migration_progress_check_spec.rb b/spec/migrations/schedule_merge_request_assignees_migration_progress_check_spec.rb deleted file mode 100644 index 0a69f49f10d..00000000000 --- a/spec/migrations/schedule_merge_request_assignees_migration_progress_check_spec.rb +++ /dev/null @@ -1,16 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' -require Rails.root.join('db', 'post_migrate', '20190402224749_schedule_merge_request_assignees_migration_progress_check.rb') - -RSpec.describe ScheduleMergeRequestAssigneesMigrationProgressCheck do - describe '#up' do - it 'schedules MergeRequestAssigneesMigrationProgressCheck background job' do - expect(BackgroundMigrationWorker).to receive(:perform_async) - .with(described_class::MIGRATION) - .and_call_original - - subject.up - end - end -end diff --git a/spec/models/bulk_imports/entity_spec.rb b/spec/models/bulk_imports/entity_spec.rb index a3a22c386c9..17ab4d5954c 100644 --- a/spec/models/bulk_imports/entity_spec.rb +++ b/spec/models/bulk_imports/entity_spec.rb @@ -103,7 +103,9 @@ RSpec.describe BulkImports::Entity, type: :model do ) expect(entity).not_to be_valid - expect(entity.errors).to include(:destination_namespace) + expect(entity.errors).to include(:base) + expect(entity.errors[:base]) + .to include('Import failed: Destination cannot be a subgroup of the source group. Change the destination and try again.') end it 'is invalid if destination namespace is a descendant of the source' do @@ -118,7 +120,8 @@ RSpec.describe BulkImports::Entity, type: :model do ) expect(entity).not_to be_valid - expect(entity.errors).to include(:destination_namespace) + expect(entity.errors[:base]) + .to include('Import failed: Destination cannot be a subgroup of the source group. Change the destination and try again.') end end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 7d18e0503d8..528ce661370 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -25,7 +25,6 @@ RSpec.describe Group do it { is_expected.to have_many(:clusters).class_name('Clusters::Cluster') } it { is_expected.to have_many(:container_repositories) } it { is_expected.to have_many(:milestones) } - it { is_expected.to have_many(:iterations) } it { is_expected.to have_many(:group_deploy_keys) } it { is_expected.to have_many(:services) } it { is_expected.to have_one(:dependency_proxy_setting) } diff --git a/spec/models/iteration_spec.rb b/spec/models/iteration_spec.rb deleted file mode 100644 index 7c57f08b2bd..00000000000 --- a/spec/models/iteration_spec.rb +++ /dev/null @@ -1,438 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Iteration do - let(:set_cadence) { nil } - - let_it_be(:group) { create(:group) } - let_it_be(:project) { create(:project, group: group) } - - describe 'associations' do - it { is_expected.to belong_to(:project) } - it { is_expected.to belong_to(:group) } - it { is_expected.to belong_to(:iterations_cadence).inverse_of(:iterations) } - end - - describe "#iid" do - it "is properly scoped on project and group" do - iteration1 = create(:iteration, :skip_project_validation, project: project) - iteration2 = create(:iteration, :skip_project_validation, project: project) - iteration3 = create(:iteration, group: group) - iteration4 = create(:iteration, group: group) - iteration5 = create(:iteration, :skip_project_validation, project: project) - - want = { - iteration1: 1, - iteration2: 2, - iteration3: 1, - iteration4: 2, - iteration5: 3 - } - got = { - iteration1: iteration1.iid, - iteration2: iteration2.iid, - iteration3: iteration3.iid, - iteration4: iteration4.iid, - iteration5: iteration5.iid - } - expect(got).to eq(want) - end - end - - describe 'setting iteration cadence' do - let_it_be(:iterations_cadence) { create(:iterations_cadence, group: group, start_date: 10.days.ago) } - let(:iteration) { create(:iteration, group: group, iterations_cadence: set_cadence, start_date: 2.days.from_now) } - - context 'when iterations_cadence is set correctly' do - let(:set_cadence) { iterations_cadence} - - it 'does not change the iterations_cadence' do - expect(iteration.iterations_cadence).to eq(iterations_cadence) - end - end - - context 'when iterations_cadence exists for the group' do - let(:set_cadence) { nil } - - it 'sets the iterations_cadence to the existing record' do - expect(iteration.iterations_cadence).to eq(iterations_cadence) - end - end - - context 'when iterations_cadence does not exists for the group' do - let_it_be(:group) { create(:group, name: 'Test group')} - let(:iteration) { build(:iteration, group: group, iterations_cadence: set_cadence) } - - it 'creates a default iterations_cadence and uses it for the iteration' do - expect { iteration.save! }.to change { Iterations::Cadence.count }.by(1) - end - - it 'sets the newly created iterations_cadence to the record' do - iteration.save! - - expect(iteration.iterations_cadence).to eq(Iterations::Cadence.last) - end - - it 'creates the iterations_cadence with the correct attributes' do - iteration.save! - - cadence = Iterations::Cadence.last - - expect(cadence.reload.start_date).to eq(iteration.start_date) - expect(cadence.title).to eq('Test group Iterations') - end - end - - context 'when iteration is a project iteration' do - it 'does not set the iterations_cadence' do - iteration = create(:iteration, iterations_cadence: nil, project: project, skip_project_validation: true) - - expect(iteration.reload.iterations_cadence).to be_nil - end - end - end - - describe '.filter_by_state' do - let_it_be(:closed_iteration) { create(:iteration, :closed, :skip_future_date_validation, group: group, start_date: 8.days.ago, due_date: 2.days.ago) } - let_it_be(:started_iteration) { create(:iteration, :started, :skip_future_date_validation, group: group, start_date: 1.day.ago, due_date: 6.days.from_now) } - let_it_be(:upcoming_iteration) { create(:iteration, :upcoming, group: group, start_date: 1.week.from_now, due_date: 2.weeks.from_now) } - - shared_examples_for 'filter_by_state' do - it 'filters by the given state' do - expect(described_class.filter_by_state(Iteration.all, state)).to match(expected_iterations) - end - end - - context 'filtering by closed iterations' do - it_behaves_like 'filter_by_state' do - let(:state) { 'closed' } - let(:expected_iterations) { [closed_iteration] } - end - end - - context 'filtering by started iterations' do - it_behaves_like 'filter_by_state' do - let(:state) { 'started' } - let(:expected_iterations) { [started_iteration] } - end - end - - context 'filtering by opened iterations' do - it_behaves_like 'filter_by_state' do - let(:state) { 'opened' } - let(:expected_iterations) { [started_iteration, upcoming_iteration] } - end - end - - context 'filtering by upcoming iterations' do - it_behaves_like 'filter_by_state' do - let(:state) { 'upcoming' } - let(:expected_iterations) { [upcoming_iteration] } - end - end - - context 'filtering by "all"' do - it_behaves_like 'filter_by_state' do - let(:state) { 'all' } - let(:expected_iterations) { [closed_iteration, started_iteration, upcoming_iteration] } - end - end - - context 'filtering by nonexistent filter' do - it 'raises ArgumentError' do - expect { described_class.filter_by_state(Iteration.none, 'unknown') }.to raise_error(ArgumentError, 'Unknown state filter: unknown') - end - end - end - - context 'Validations' do - subject { build(:iteration, group: group, start_date: start_date, due_date: due_date) } - - describe 'when iteration belongs to project' do - subject { build(:iteration, project: project, start_date: Time.current, due_date: 1.day.from_now) } - - it 'is invalid' do - expect(subject).not_to be_valid - expect(subject.errors[:project_id]).to include('is not allowed. We do not currently support project-level iterations') - end - end - - describe '#dates_do_not_overlap' do - let_it_be(:existing_iteration) { create(:iteration, group: group, start_date: 4.days.from_now, due_date: 1.week.from_now) } - - context 'when no Iteration dates overlap' do - let(:start_date) { 2.weeks.from_now } - let(:due_date) { 3.weeks.from_now } - - it { is_expected.to be_valid } - end - - context 'when updated iteration dates overlap with its own dates' do - it 'is valid' do - existing_iteration.start_date = 5.days.from_now - - expect(existing_iteration).to be_valid - end - end - - context 'when dates overlap' do - let(:start_date) { 5.days.from_now } - let(:due_date) { 6.days.from_now } - - shared_examples_for 'overlapping dates' do |skip_constraint_test: false| - context 'when start_date overlaps' do - let(:start_date) { 5.days.from_now } - let(:due_date) { 3.weeks.from_now } - - it 'is not valid' do - expect(subject).not_to be_valid - expect(subject.errors[:base]).to include('Dates cannot overlap with other existing Iterations within this group') - end - - unless skip_constraint_test - it 'is not valid even if forced' do - subject.validate # to generate iid/etc - expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/) - end - end - end - - context 'when due_date overlaps' do - let(:start_date) { Time.current } - let(:due_date) { 6.days.from_now } - - it 'is not valid' do - expect(subject).not_to be_valid - expect(subject.errors[:base]).to include('Dates cannot overlap with other existing Iterations within this group') - end - - unless skip_constraint_test - it 'is not valid even if forced' do - subject.validate # to generate iid/etc - expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/) - end - end - end - - context 'when both overlap' do - it 'is not valid' do - expect(subject).not_to be_valid - expect(subject.errors[:base]).to include('Dates cannot overlap with other existing Iterations within this group') - end - - unless skip_constraint_test - it 'is not valid even if forced' do - subject.validate # to generate iid/etc - expect { subject.save!(validate: false) }.to raise_exception(ActiveRecord::StatementInvalid, /#{constraint_name}/) - end - end - end - end - - context 'group' do - it_behaves_like 'overlapping dates' do - let(:constraint_name) { 'iteration_start_and_due_date_iterations_cadence_id_constraint' } - end - - context 'different group' do - let(:group) { create(:group) } - - it { is_expected.to be_valid } - - it 'does not trigger exclusion constraints' do - expect { subject.save! }.not_to raise_exception - end - end - - context 'sub-group' do - let(:subgroup) { create(:group, parent: group) } - - subject { build(:iteration, group: subgroup, start_date: start_date, due_date: due_date) } - - it { is_expected.to be_valid } - end - end - - # Skipped. Pending https://gitlab.com/gitlab-org/gitlab/-/issues/299864 - xcontext 'project' do - let_it_be(:existing_iteration) { create(:iteration, :skip_project_validation, project: project, start_date: 4.days.from_now, due_date: 1.week.from_now) } - - subject { build(:iteration, :skip_project_validation, project: project, start_date: start_date, due_date: due_date) } - - it_behaves_like 'overlapping dates' do - let(:constraint_name) { 'iteration_start_and_due_daterange_project_id_constraint' } - end - - context 'different project' do - let(:project) { create(:project) } - - it { is_expected.to be_valid } - - it 'does not trigger exclusion constraints' do - expect { subject.save! }.not_to raise_exception - end - end - - context 'in a group' do - let(:group) { create(:group) } - - subject { build(:iteration, group: group, start_date: start_date, due_date: due_date) } - - it { is_expected.to be_valid } - - it 'does not trigger exclusion constraints' do - expect { subject.save! }.not_to raise_exception - end - end - - context 'project in a group' do - let_it_be(:project) { create(:project, group: create(:group)) } - let_it_be(:existing_iteration) { create(:iteration, :skip_project_validation, project: project, start_date: 4.days.from_now, due_date: 1.week.from_now) } - - subject { build(:iteration, :skip_project_validation, project: project, start_date: start_date, due_date: due_date) } - - it_behaves_like 'overlapping dates' do - let(:constraint_name) { 'iteration_start_and_due_daterange_project_id_constraint' } - end - end - end - end - end - - describe '#future_date' do - context 'when dates are in the future' do - let(:start_date) { Time.current } - let(:due_date) { 1.week.from_now } - - it { is_expected.to be_valid } - end - - context 'when start_date is in the past' do - let(:start_date) { 1.week.ago } - let(:due_date) { 1.week.from_now } - - it { is_expected.to be_valid } - end - - context 'when due_date is in the past' do - let(:start_date) { 2.weeks.ago } - let(:due_date) { 1.week.ago } - - it { is_expected.to be_valid } - end - - context 'when due_date is before start date' do - let(:start_date) { Time.current } - let(:due_date) { 1.week.ago } - - it 'is not valid' do - expect(subject).not_to be_valid - expect(subject.errors[:due_date]).to include('must be greater than start date') - end - end - - context 'when start_date is over 500 years in the future' do - let(:start_date) { 501.years.from_now } - let(:due_date) { Time.current } - - it 'is not valid' do - expect(subject).not_to be_valid - expect(subject.errors[:start_date]).to include('cannot be more than 500 years in the future') - end - end - - context 'when due_date is over 500 years in the future' do - let(:start_date) { Time.current } - let(:due_date) { 501.years.from_now } - - it 'is not valid' do - expect(subject).not_to be_valid - expect(subject.errors[:due_date]).to include('cannot be more than 500 years in the future') - end - end - end - end - - context 'time scopes' do - let_it_be(:project) { create(:project, :empty_repo) } - let_it_be(:iteration_1) { create(:iteration, :skip_future_date_validation, :skip_project_validation, project: project, start_date: 3.days.ago, due_date: 1.day.from_now) } - let_it_be(:iteration_2) { create(:iteration, :skip_future_date_validation, :skip_project_validation, project: project, start_date: 10.days.ago, due_date: 4.days.ago) } - let_it_be(:iteration_3) { create(:iteration, :skip_project_validation, project: project, start_date: 4.days.from_now, due_date: 1.week.from_now) } - - describe 'start_date_passed' do - it 'returns iterations where start_date is in the past but due_date is in the future' do - expect(described_class.start_date_passed).to contain_exactly(iteration_1) - end - end - - describe 'due_date_passed' do - it 'returns iterations where due date is in the past' do - expect(described_class.due_date_passed).to contain_exactly(iteration_2) - end - end - end - - describe '#validate_group' do - let_it_be(:iterations_cadence) { create(:iterations_cadence, group: group) } - - context 'when the iteration and iteration cadence groups are same' do - it 'is valid' do - iteration = build(:iteration, group: group, iterations_cadence: iterations_cadence) - - expect(iteration).to be_valid - end - end - - context 'when the iteration and iteration cadence groups are different' do - it 'is invalid' do - other_group = create(:group) - iteration = build(:iteration, group: other_group, iterations_cadence: iterations_cadence) - - expect(iteration).not_to be_valid - end - end - - context 'when the iteration belongs to a project and the iteration cadence is set' do - it 'is invalid' do - iteration = build(:iteration, project: project, iterations_cadence: iterations_cadence, skip_project_validation: true) - - expect(iteration).to be_invalid - end - end - - context 'when the iteration belongs to a project and the iteration cadence is not set' do - it 'is valid' do - iteration = build(:iteration, project: project, skip_project_validation: true) - - expect(iteration).to be_valid - end - end - end - - describe '.within_timeframe' do - let_it_be(:now) { Time.current } - let_it_be(:project) { create(:project, :empty_repo) } - let_it_be(:iteration_1) { create(:iteration, :skip_project_validation, project: project, start_date: now, due_date: 1.day.from_now) } - let_it_be(:iteration_2) { create(:iteration, :skip_project_validation, project: project, start_date: 2.days.from_now, due_date: 3.days.from_now) } - let_it_be(:iteration_3) { create(:iteration, :skip_project_validation, project: project, start_date: 4.days.from_now, due_date: 1.week.from_now) } - - it 'returns iterations with start_date and/or end_date between timeframe' do - iterations = described_class.within_timeframe(2.days.from_now, 3.days.from_now) - - expect(iterations).to match_array([iteration_2]) - end - - it 'returns iterations which starts before the timeframe' do - iterations = described_class.within_timeframe(1.day.from_now, 3.days.from_now) - - expect(iterations).to match_array([iteration_1, iteration_2]) - end - - it 'returns iterations which ends after the timeframe' do - iterations = described_class.within_timeframe(3.days.from_now, 5.days.from_now) - - expect(iterations).to match_array([iteration_2, iteration_3]) - end - end -end diff --git a/spec/models/iterations/cadence_spec.rb b/spec/models/iterations/cadence_spec.rb deleted file mode 100644 index cdeeef97580..00000000000 --- a/spec/models/iterations/cadence_spec.rb +++ /dev/null @@ -1,22 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Iterations::Cadence do - describe 'associations' do - subject { build(:iterations_cadence) } - - it { is_expected.to belong_to(:group) } - it { is_expected.to have_many(:iterations).inverse_of(:iterations_cadence) } - end - - describe 'validations' do - subject { build(:iterations_cadence) } - - it { is_expected.to validate_presence_of(:title) } - it { is_expected.to validate_presence_of(:start_date) } - it { is_expected.to validate_presence_of(:group_id) } - it { is_expected.to validate_presence_of(:active) } - it { is_expected.to validate_presence_of(:automatic) } - end -end diff --git a/spec/services/issuable/bulk_update_service_spec.rb b/spec/services/issuable/bulk_update_service_spec.rb index 79543fe9f5d..c749f282cd3 100644 --- a/spec/services/issuable/bulk_update_service_spec.rb +++ b/spec/services/issuable/bulk_update_service_spec.rb @@ -31,23 +31,6 @@ RSpec.describe Issuable::BulkUpdateService do end end - shared_examples 'updates iterations' do - it 'succeeds' do - result = bulk_update(issuables, sprint_id: iteration.id) - - expect(result.success?).to be_truthy - expect(result.payload[:count]).to eq(issuables.count) - end - - it 'updates the issuables iteration' do - bulk_update(issuables, sprint_id: iteration.id) - - issuables.each do |issuable| - expect(issuable.reload.iteration).to eq(iteration) - end - end - end - shared_examples 'updating labels' do def create_issue_with_labels(labels) create(:labeled_issue, project: project, labels: labels) @@ -250,21 +233,6 @@ RSpec.describe Issuable::BulkUpdateService do it_behaves_like 'updates milestones' end - describe 'updating iterations' do - let_it_be(:group) { create(:group) } - let_it_be(:project) { create(:project, group: group) } - let_it_be(:issuables) { [create(:issue, project: project)] } - let_it_be(:iteration) { create(:iteration, group: group) } - - let(:parent) { project } - - before do - group.add_reporter(user) - end - - it_behaves_like 'updates iterations' - end - describe 'updating labels' do let(:bug) { create(:label, project: project) } let(:regression) { create(:label, project: project) } @@ -347,19 +315,6 @@ RSpec.describe Issuable::BulkUpdateService do end end - describe 'updating iterations' do - let_it_be(:iteration) { create(:iteration, group: group) } - let_it_be(:project) { create(:project, :repository, group: group) } - - context 'when issues' do - let_it_be(:issue1) { create(:issue, project: project) } - let_it_be(:issue2) { create(:issue, project: project) } - let_it_be(:issuables) { [issue1, issue2] } - - it_behaves_like 'updates iterations' - end - end - describe 'updating labels' do let(:project) { create(:project, :repository, group: group) } let(:bug) { create(:group_label, group: group) } diff --git a/spec/views/notify/change_in_merge_request_draft_status_email.html.haml_spec.rb b/spec/views/notify/change_in_merge_request_draft_status_email.html.haml_spec.rb new file mode 100644 index 00000000000..6c25eba03b9 --- /dev/null +++ b/spec/views/notify/change_in_merge_request_draft_status_email.html.haml_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'notify/change_in_merge_request_draft_status_email.html.haml' do + let(:user) { create(:user) } + let(:merge_request) { create(:merge_request) } + + before do + assign(:updated_by_user, user) + assign(:merge_request, merge_request) + end + + it 'renders the email correctly' do + render + + expect(rendered).to have_content("#{user.name} changed the draft status of merge request #{merge_request.to_reference}") + end +end diff --git a/spec/views/notify/change_in_merge_request_draft_status_email.text.erb_spec.rb b/spec/views/notify/change_in_merge_request_draft_status_email.text.erb_spec.rb new file mode 100644 index 00000000000..a05c20fd8c4 --- /dev/null +++ b/spec/views/notify/change_in_merge_request_draft_status_email.text.erb_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true +require 'spec_helper' + +RSpec.describe 'notify/change_in_merge_request_draft_status_email.text.erb' do + let(:user) { create(:user) } + let(:merge_request) { create(:merge_request) } + + before do + assign(:updated_by_user, user) + assign(:merge_request, merge_request) + end + + it_behaves_like 'renders plain text email correctly' + + it 'renders the email correctly' do + render + + expect(rendered).to have_content("#{user.name} changed the draft status of merge request #{merge_request.to_reference}") + end +end