Automatic merge of gitlab-org/gitlab master

app/assets/javascripts/api.js

@@ -870,6 +870,14 @@ const Api = {
     return axios.put(url, freezePeriod);
   },
 
+  deleteFreezePeriod(id, freezePeriodId) {
+    const url = Api.buildUrl(this.freezePeriodPath)
+      .replace(':id', encodeURIComponent(id))
+      .replace(':freeze_period_id', encodeURIComponent(freezePeriodId));
+
+    return axios.delete(url);
+  },
+
   trackRedisCounterEvent(event) {
     if (!gon.features?.usageDataApi) {
       return null;

app/assets/javascripts/deploy_freeze/components/deploy_freeze_table.vue

 <script>
-import { GlTable, GlButton, GlModalDirective, GlSprintf } from '@gitlab/ui';
+import { GlTable, GlButton, GlModal, GlModalDirective, GlSprintf } from '@gitlab/ui';
 import { mapState, mapActions } from 'vuex';
 import { s__ } from '~/locale';
 
@@ -21,21 +21,42 @@ export default {
       key: 'edit',
       label: s__('DeployFreeze|Edit'),
     },
+    {
+      key: 'delete',
+      label: s__('DeployFreeze|Delete'),
+    },
   ],
   translations: {
     addDeployFreeze: s__('DeployFreeze|Add deploy freeze'),
+    deleteDeployFreezeTitle: s__('DeployFreeze|Delete deploy freeze?'),
+    deleteDeployFreezeMessage: s__(
+      'DeployFreeze|Deploy freeze from %{start} to %{end} in %{timezone} will be removed. Are you sure?',
+    ),
     emptyStateText: s__(
       'DeployFreeze|No deploy freezes exist for this project. To add one, select %{strongStart}Add deploy freeze%{strongEnd}',
     ),
   },
+  modal: {
+    id: 'deleteFreezePeriodModal',
+    actionPrimary: {
+      text: s__('DeployFreeze|Delete freeze period'),
+      attributes: { variant: 'danger', 'data-testid': 'modal-confirm' },
+    },
+  },
   components: {
     GlTable,
     GlButton,
+    GlModal,
     GlSprintf,
   },
   directives: {
     GlModal: GlModalDirective,
   },
+  data() {
+    return {
+      freezePeriodToDelete: null,
+    };
+  },
   computed: {
     ...mapState(['freezePeriods']),
     tableIsNotEmpty() {
@@ -46,7 +67,14 @@ export default {
     this.fetchFreezePeriods();
   },
   methods: {
-    ...mapActions(['fetchFreezePeriods', 'setFreezePeriod']),
+    ...mapActions(['fetchFreezePeriods', 'setFreezePeriod', 'deleteFreezePeriod']),
+    handleDeleteFreezePeriod(freezePeriod) {
+      this.freezePeriodToDelete = freezePeriod;
+    },
+    confirmDeleteFreezePeriod() {
+      this.deleteFreezePeriod(this.freezePeriodToDelete);
+      this.freezePeriodToDelete = null;
+    },
   },
 };
 </script>
@@ -72,6 +100,18 @@ export default {
           @click="setFreezePeriod(item)"
         />
       </template>
+      <template #cell(delete)="{ item }">
+        <gl-button
+          v-gl-modal="$options.modal.id"
+          category="secondary"
+          variant="danger"
+          icon="remove"
+          :aria-label="$options.modal.actionPrimary.text"
+          :loading="item.isDeleting"
+          data-testid="delete-deploy-freeze"
+          @click="handleDeleteFreezePeriod(item)"
+        />
+      </template>
       <template #empty>
         <p data-testid="empty-freeze-periods" class="gl-text-center text-plain">
           <gl-sprintf :message="$options.translations.emptyStateText">
@@ -90,5 +130,24 @@ export default {
     >
       {{ $options.translations.addDeployFreeze }}
     </gl-button>
+    <gl-modal
+      :title="$options.translations.deleteDeployFreezeTitle"
+      :modal-id="$options.modal.id"
+      :action-primary="$options.modal.actionPrimary"
+      static
+      @primary="confirmDeleteFreezePeriod"
+    >
+      <template v-if="freezePeriodToDelete">
+        <gl-sprintf :message="$options.translations.deleteDeployFreezeMessage">
+          <template #start>
+            <code>{{ freezePeriodToDelete.freezeStart }}</code>
+          </template>
+          <template #end>
+            <code>{{ freezePeriodToDelete.freezeEnd }}</code>
+          </template>
+          <template #timezone>{{ freezePeriodToDelete.cronTimezone.formattedTimezone }}</template>
+        </gl-sprintf>
+      </template>
+    </gl-modal>
   </div>
 </template>

app/assets/javascripts/deploy_freeze/store/actions.js

@@ -52,6 +52,22 @@ export const updateFreezePeriod = (store) =>
     }),
   );
 
+export const deleteFreezePeriod = ({ state, commit }, { id }) => {
+  commit(types.REQUEST_DELETE_FREEZE_PERIOD, id);
+
+  return Api.deleteFreezePeriod(state.projectId, id)
+    .then(() => commit(types.RECEIVE_DELETE_FREEZE_PERIOD_SUCCESS, id))
+    .catch((e) => {
+      createFlash({
+        message: __('Error: Unable to delete deploy freeze'),
+      });
+      commit(types.RECEIVE_DELETE_FREEZE_PERIOD_ERROR, id);
+
+      // eslint-disable-next-line no-console
+      console.error('[gitlab] Unable to delete deploy freeze:', e);
+    });
+};
+
 export const fetchFreezePeriods = ({ commit, state }) => {
   commit(types.REQUEST_FREEZE_PERIODS);
 

app/assets/javascripts/deploy_freeze/store/mutation_types.js

@@ -10,4 +10,8 @@ export const SET_SELECTED_ID = 'SET_SELECTED_ID';
 export const SET_FREEZE_START_CRON = 'SET_FREEZE_START_CRON';
 export const SET_FREEZE_END_CRON = 'SET_FREEZE_END_CRON';
 
+export const REQUEST_DELETE_FREEZE_PERIOD = 'REQUEST_DELETE_FREEZE_PERIOD';
+export const RECEIVE_DELETE_FREEZE_PERIOD_SUCCESS = 'RECEIVE_DELETE_FREEZE_PERIOD_SUCCESS';
+export const RECEIVE_DELETE_FREEZE_PERIOD_ERROR = 'RECEIVE_DELETE_FREEZE_PERIOD_ERROR';
+
 export const RESET_MODAL = 'RESET_MODAL';

app/assets/javascripts/deploy_freeze/store/mutations.js

+import Vue from 'vue';
 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
+import { secondsToHours } from '~/lib/utils/datetime_utility';
 import * as types from './mutation_types';
 
-const formatTimezoneName = (freezePeriod, timezoneList) =>
-  convertObjectPropsToCamelCase({
+const formatTimezoneName = (freezePeriod, timezoneList) => {
+  const tz = timezoneList.find((timezone) => timezone.identifier === freezePeriod.cron_timezone);
+  return convertObjectPropsToCamelCase({
     ...freezePeriod,
     cron_timezone: {
-      formattedTimezone: timezoneList.find((tz) => tz.identifier === freezePeriod.cron_timezone)
-        ?.name,
+      formattedTimezone: tz && `[UTC ${secondsToHours(tz.offset)}] ${tz.name}`,
       identifier: freezePeriod.cron_timezone,
     },
   });
+};
+
+const setFreezePeriodIsDeleting = (state, id, isDeleting) => {
+  const freezePeriod = state.freezePeriods.find((f) => f.id === id);
+
+  if (!freezePeriod) {
+    return;
+  }
+
+  Vue.set(freezePeriod, 'isDeleting', isDeleting);
+};
 
 export default {
   [types.REQUEST_FREEZE_PERIODS](state) {
@@ -53,6 +66,18 @@ export default {
     state.selectedId = id;
   },
 
+  [types.REQUEST_DELETE_FREEZE_PERIOD](state, id) {
+    setFreezePeriodIsDeleting(state, id, true);
+  },
+
+  [types.RECEIVE_DELETE_FREEZE_PERIOD_SUCCESS](state, id) {
+    state.freezePeriods = state.freezePeriods.filter((f) => f.id !== id);
+  },
+
+  [types.RECEIVE_DELETE_FREEZE_PERIOD_ERROR](state, id) {
+    setFreezePeriodIsDeleting(state, id, false);
+  },
+
   [types.RESET_MODAL](state) {
     state.freezeStartCron = '';
     state.freezeEndCron = '';

app/controllers/registrations_controller.rb

@@ -21,9 +21,10 @@ class RegistrationsController < Devise::RegistrationsController
 
   def create
     set_user_state
-    accept_pending_invitations
 
     super do |new_user|
+      accept_pending_invitations if new_user.persisted?
+
       persist_accepted_terms_if_required(new_user)
       set_role_required(new_user)
 

app/graphql/types/ci/stage_type.rb

@@ -52,15 +52,22 @@ module Types
 
       # rubocop: disable CodeReuse/ActiveRecord
       def jobs_for_pipeline(pipeline, stage_ids, include_needs)
-        builds_results = pipeline.latest_builds.where(stage_id: stage_ids).preload(:job_artifacts, :project)
-        bridges_results = pipeline.bridges.where(stage_id: stage_ids).preload(:project)
-        builds_results = builds_results.preload(:needs) if include_needs
-        bridges_results = bridges_results.preload(:needs) if include_needs
-        commit_status_results = pipeline.latest_statuses.where(stage_id: stage_ids)
+        jobs = pipeline.statuses.latest.where(stage_id: stage_ids)
 
-        results = builds_results | bridges_results | commit_status_results
+        common_relations = [:project]
+        common_relations << :needs if include_needs
 
-        results.group_by(&:stage_id)
+        preloaders = {
+          ::Ci::Build => [:metadata, :job_artifacts],
+          ::Ci::Bridge => [:metadata, :downstream_pipeline],
+          ::GenericCommitStatus => []
+        }
+
+        preloaders.each do |klass, relations|
+          ActiveRecord::Associations::Preloader.new.preload(jobs.select { |job| job.is_a?(klass) }, relations + common_relations)
+        end
+
+        jobs.group_by(&:stage_id)
       end
       # rubocop: enable CodeReuse/ActiveRecord
     end

app/models/ci/pipeline.rb

@@ -66,6 +66,7 @@ module Ci
     has_many :processables, class_name: 'Ci::Processable', foreign_key: :commit_id, inverse_of: :pipeline
     has_many :bridges, class_name: 'Ci::Bridge', foreign_key: :commit_id, inverse_of: :pipeline
     has_many :builds, foreign_key: :commit_id, inverse_of: :pipeline
+    has_many :generic_commit_statuses, foreign_key: :commit_id, inverse_of: :pipeline, class_name: 'GenericCommitStatus'
     has_many :job_artifacts, through: :builds
     has_many :trigger_requests, dependent: :destroy, foreign_key: :commit_id # rubocop:disable Cop/ActiveRecordDependent
     has_many :variables, class_name: 'Ci::PipelineVariable'

app/models/member.rb

@@ -278,12 +278,14 @@ class Member < ApplicationRecord
 
   def accept_invite!(new_user)
     return false unless invite?
+    return false unless new_user
+
+    self.user = new_user
+    return false unless self.user.save
 
     self.invite_token = nil
     self.invite_accepted_at = Time.current.utc
 
-    self.user = new_user
-
     saved = self.save
 
     after_accept_invite if saved

app/models/note.rb

@@ -582,7 +582,7 @@ class Note < ApplicationRecord
   end
 
   def post_processed_cache_key
-    cache_key_items = [cache_key, author.cache_key]
+    cache_key_items = [cache_key, author&.cache_key]
     cache_key_items << Digest::SHA1.hexdigest(redacted_note_html) if redacted_note_html.present?
 
     cache_key_items.join(':')

doc/user/project/releases/index.md

@@ -186,7 +186,8 @@ To subscribe to notifications for releases:
 
 ## Prevent unintentional releases by setting a deploy freeze
 
-> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/29382) in GitLab 13.0.
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/29382) in GitLab 13.0.
+> - The ability to delete freeze periods through the UI was [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/212451) in GitLab 14.3.
 
 Prevent unintended production releases during a period of time you specify by
 setting a [*deploy freeze* period](../../../ci/environments/deployment_safety.md).
@@ -219,11 +220,8 @@ To set a deploy freeze window in the UI, complete these steps:
 1. Click **Add deploy freeze** to open the deploy freeze modal.
 1. Enter the start time, end time, and timezone of the desired deploy freeze period.
 1. Click **Add deploy freeze** in the modal.
-1. After the deploy freeze is saved, you can edit it by selecting the edit button (**{pencil}**).
-   ![Deploy freeze modal for setting a deploy freeze period](img/deploy_freeze_v13_10.png)
-
-WARNING:
-To delete a deploy freeze, use the [Freeze Periods API](../../../api/freeze_periods.md).
+1. After the deploy freeze is saved, you can edit it by selecting the edit button (**{pencil}**) and remove it by selecting the delete button (**{remove}**).
+   ![Deploy freeze modal for setting a deploy freeze period](img/deploy_freeze_v14_3.png)
 
 If a project contains multiple freeze periods, all periods apply. If they overlap, the freeze covers the
 complete overlapping period.

locale/gitlab.pot

@@ -11068,6 +11068,18 @@ msgstr ""
 msgid "DeployFreeze|Add deploy freeze"
 msgstr ""
 
+msgid "DeployFreeze|Delete"
+msgstr ""
+
+msgid "DeployFreeze|Delete deploy freeze?"
+msgstr ""
+
+msgid "DeployFreeze|Delete freeze period"
+msgstr ""
+
+msgid "DeployFreeze|Deploy freeze from %{start} to %{end} in %{timezone} will be removed. Are you sure?"
+msgstr ""
+
 msgid "DeployFreeze|Edit"
 msgstr ""
 
@@ -13262,6 +13274,9 @@ msgstr ""
 msgid "Error: Unable to create deploy freeze"
 msgstr ""
 
+msgid "Error: Unable to delete deploy freeze"
+msgstr ""
+
 msgid "Error: Unable to find AWS role for current user"
 msgstr ""
 

spec/controllers/registrations_controller_spec.rb

@@ -283,6 +283,26 @@ RSpec.describe RegistrationsController do
           end
         end
 
+        context 'when the registration fails' do
+          let_it_be(:member) { create(:project_member, :invited) }
+          let_it_be(:missing_user_params) do
+            { username: '', email: member.invite_email, password: 'Any_password' }
+          end
+
+          let_it_be(:user_params) { { user: missing_user_params } }
+
+          let(:session_params) { { invite_email: member.invite_email } }
+
+          subject { post(:create, params: user_params, session: session_params) }
+
+          it 'does not delete the invitation or register the new user' do
+            subject
+
+            expect(member.invite_token).not_to be_nil
+            expect(controller.current_user).to be_nil
+          end
+        end
+
         context 'when soft email confirmation is enabled' do
           before do
             stub_feature_flags(soft_email_confirmation: true)

spec/features/invites_spec.rb

@@ -189,6 +189,16 @@ RSpec.describe 'Group or Project invitations', :aggregate_failures do
       end
 
       context 'email confirmation enabled' do
+        context 'when user is not valid in sign up form' do
+          let(:new_user) { build_stubbed(:user, first_name: '', last_name: '') }
+
+          it 'fails sign up and redirects back to sign up', :aggregate_failures do
+            expect { fill_in_sign_up_form(new_user) }.not_to change { User.count }
+            expect(page).to have_content('prohibited this user from being saved')
+            expect(current_path).to eq(user_registration_path)
+          end
+        end
+
         context 'with invite email acceptance', :snowplow do
           it 'tracks the accepted invite' do
             fill_in_sign_up_form(new_user)

spec/frontend/deploy_freeze/components/deploy_freeze_table_spec.js

+import { GlModal } from '@gitlab/ui';
 import { createLocalVue, mount } from '@vue/test-utils';
 import Vuex from 'vuex';
 import DeployFreezeTable from '~/deploy_freeze/components/deploy_freeze_table.vue';
@@ -29,6 +30,8 @@ describe('Deploy freeze table', () => {
   const findAddDeployFreezeButton = () => wrapper.find('[data-testid="add-deploy-freeze"]');
   const findEditDeployFreezeButton = () => wrapper.find('[data-testid="edit-deploy-freeze"]');
   const findDeployFreezeTable = () => wrapper.find('[data-testid="deploy-freeze-table"]');
+  const findDeleteDeployFreezeButton = () => wrapper.find('[data-testid="delete-deploy-freeze"]');
+  const findDeleteDeployFreezeModal = () => wrapper.findComponent(GlModal);
 
   beforeEach(() => {
     createComponent();
@@ -73,6 +76,29 @@ describe('Deploy freeze table', () => {
           store.state.freezePeriods[0],
         );
       });
+
+      it('displays delete deploy freeze button', () => {
+        expect(findDeleteDeployFreezeButton().exists()).toBe(true);
+      });
+
+      it('confirms a user wants to delete a deploy freeze', async () => {
+        const [{ freezeStart, freezeEnd, cronTimezone }] = store.state.freezePeriods;
+        await findDeleteDeployFreezeButton().trigger('click');
+        const modal = findDeleteDeployFreezeModal();
+        expect(modal.text()).toContain(
+          `Deploy freeze from ${freezeStart} to ${freezeEnd} in ${cronTimezone.formattedTimezone} will be removed.`,
+        );
+      });
+
+      it('deletes the freeze period on confirmation', async () => {
+        await findDeleteDeployFreezeButton().trigger('click');
+        const modal = findDeleteDeployFreezeModal();
+        modal.vm.$emit('primary');
+        expect(store.dispatch).toHaveBeenCalledWith(
+          'deleteFreezePeriod',
+          store.state.freezePeriods[0],
+        );
+      });
     });
   });
 

spec/frontend/deploy_freeze/store/actions_spec.js

@@ -12,6 +12,7 @@ jest.mock('~/api.js');
 jest.mock('~/flash.js');
 
 describe('deploy freeze store actions', () => {
+  const freezePeriodFixture = freezePeriodsFixture[0];
   let mock;
   let state;
 
@@ -24,6 +25,7 @@ describe('deploy freeze store actions', () => {
     Api.freezePeriods.mockResolvedValue({ data: freezePeriodsFixture });
     Api.createFreezePeriod.mockResolvedValue();
     Api.updateFreezePeriod.mockResolvedValue();
+    Api.deleteFreezePeriod.mockResolvedValue();
   });
 
   afterEach(() => {
@@ -195,4 +197,46 @@ describe('deploy freeze store actions', () => {
       );
     });
   });
+
+  describe('deleteFreezePeriod', () => {
+    it('dispatch correct actions on deleting a freeze period', () => {
+      testAction(
+        actions.deleteFreezePeriod,
+        freezePeriodFixture,
+        state,
+        [
+          { type: 'REQUEST_DELETE_FREEZE_PERIOD', payload: freezePeriodFixture.id },
+          { type: 'RECEIVE_DELETE_FREEZE_PERIOD_SUCCESS', payload: freezePeriodFixture.id },
+        ],
+        [],
+        () =>
+          expect(Api.deleteFreezePeriod).toHaveBeenCalledWith(
+            state.projectId,
+            freezePeriodFixture.id,
+          ),
+      );
+    });
+
+    it('should show flash error and set error in state on delete failure', () => {
+      const errorSpy = jest.spyOn(console, 'error').mockImplementation();
+      const error = new Error();
+      Api.deleteFreezePeriod.mockRejectedValue(error);
+
+      testAction(
+        actions.deleteFreezePeriod,
+        freezePeriodFixture,
+        state,
+        [
+          { type: 'REQUEST_DELETE_FREEZE_PERIOD', payload: freezePeriodFixture.id },
+          { type: 'RECEIVE_DELETE_FREEZE_PERIOD_ERROR', payload: freezePeriodFixture.id },
+        ],
+        [],
+        () => {
+          expect(createFlash).toHaveBeenCalled();
+
+          expect(errorSpy).toHaveBeenCalledWith('[gitlab] Unable to delete deploy freeze:', error);
+        },
+      );
+    });
+  });
 });

spec/frontend/deploy_freeze/store/mutations_spec.js

@@ -28,9 +28,9 @@ describe('Deploy freeze mutations', () => {
   describe('RECEIVE_FREEZE_PERIODS_SUCCESS', () => {
     it('should set freeze periods and format timezones from identifiers to names', () => {
       const timezoneNames = {
-        'Europe/Berlin': 'Berlin',
-        'Etc/UTC': 'UTC',
-        'America/New_York': 'Eastern Time (US & Canada)',
+        'Europe/Berlin': '[UTC 2] Berlin',
+        'Etc/UTC': '[UTC 0] UTC',
+        'America/New_York': '[UTC -4] Eastern Time (US & Canada)',
       };
 
       mutations[types.RECEIVE_FREEZE_PERIODS_SUCCESS](stateCopy, freezePeriodsFixture);

spec/lib/gitlab/import_export/all_models.yml

@@ -224,6 +224,7 @@ ci_pipelines:
 - builds
 - bridges
 - processables
+- generic_commit_statuses
 - trigger_requests
 - variables
 - auto_canceled_by

spec/models/member_spec.rb

@@ -645,6 +645,16 @@ RSpec.describe Member do
 
       expect(user.authorized_projects.reload).to include(project)
     end
+
+    it 'does not accept the invite if saving a new user fails' do
+      invalid_user = User.new(first_name: '', last_name: '')
+
+      member.accept_invite! invalid_user
+
+      expect(member.invite_accepted_at).to be_nil
+      expect(member.invite_token).not_to be_nil
+      expect_any_instance_of(Member).not_to receive(:after_accept_invite)
+    end
   end
 
   describe "#decline_invite!" do

spec/models/note_spec.rb

@@ -1576,6 +1576,14 @@ RSpec.describe Note do
       expect(note.post_processed_cache_key).to eq("#{note.cache_key}:#{note.author.cache_key}")
     end
 
+    context 'when note has no author' do
+      let(:note) { build(:note, author: nil) }
+
+      it 'returns cache key only' do
+        expect(note.post_processed_cache_key).to eq("#{note.cache_key}:")
+      end
+    end
+
     context 'when note has redacted_note_html' do
       let(:redacted_note_html) { 'redacted note html' }
 

spec/requests/api/graphql/project/pipeline_spec.rb

@@ -311,6 +311,10 @@ RSpec.describe 'getting pipeline information nested in a project' do
     end
 
     it 'does not generate N+1 queries', :request_store, :use_sql_query_cache do
+      # create extra statuses
+      create(:generic_commit_status, :pending, name: 'generic-build-a', pipeline: pipeline, stage_idx: 0, stage: 'build')
+      create(:ci_bridge, :failed, name: 'deploy-a', pipeline: pipeline, stage_idx: 2, stage: 'deploy')
+
       # warm up
       post_graphql(query, current_user: current_user)
 
@@ -318,9 +322,11 @@ RSpec.describe 'getting pipeline information nested in a project' do
         post_graphql(query, current_user: current_user)
       end
 
-      create(:ci_build, name: 'test-a', pipeline: pipeline, stage_idx: 1, stage: 'test')
-      create(:ci_build, name: 'test-b', pipeline: pipeline, stage_idx: 1, stage: 'test')
-      create(:ci_build, name: 'deploy-a', pipeline: pipeline, stage_idx: 2, stage: 'deploy')
+      create(:generic_commit_status, :pending, name: 'generic-build-b', pipeline: pipeline, stage_idx: 0, stage: 'build')
+      create(:ci_build, :failed, name: 'test-a', pipeline: pipeline, stage_idx: 1, stage: 'test')
+      create(:ci_build, :running, name: 'test-b', pipeline: pipeline, stage_idx: 1, stage: 'test')
+      create(:ci_build, :pending, name: 'deploy-b', pipeline: pipeline, stage_idx: 2, stage: 'deploy')
+      create(:ci_bridge, :failed, name: 'deploy-c', pipeline: pipeline, stage_idx: 2, stage: 'deploy')
 
       expect do
         post_graphql(query, current_user: current_user)