Merge branch '208735-application-setting-for-container-expiration-policies' into 'master'

Docker expiration policies wire application settings

See merge request gitlab-org/gitlab!28640

app/assets/javascripts/registry/settings/components/registry_settings_app.vue

 <script>
-import { mapActions, mapState } from 'vuex';
+import { mapActions, mapGetters, mapState } from 'vuex';
 import { GlAlert, GlSprintf, GlLink } from '@gitlab/ui';
 import { s__ } from '~/locale';
 
@@ -15,8 +15,15 @@ export default {
     GlLink,
   },
   i18n: {
-    unavailableFeatureText: s__(
-      'ContainerRegistry|Currently, the Container Registry tag expiration feature is not available for projects created before GitLab version 12.8. For updates and more information, visit Issue %{linkStart}#196124%{linkEnd}',
+    unavailableFeatureTitle: s__(
+      `ContainerRegistry|Container Registry tag expiration and retention policy is disabled`,
+    ),
+    unavailableFeatureIntroText: s__(
+      `ContainerRegistry|The Container Registry tag expiration and retention policies for this project have not been enabled.`,
+    ),
+    unavailableUserFeatureText: s__(`ContainerRegistry|Please contact your administrator.`),
+    unavailableAdminFeatureText: s__(
+      `ContainerRegistry| Please visit the %{linkStart}administration settings%{linkEnd} to enable this feature.`,
     ),
     fetchSettingsErrorText: FETCH_SETTINGS_ERROR_MESSAGE,
   },
@@ -26,10 +33,19 @@ export default {
     };
   },
   computed: {
-    ...mapState(['isDisabled']),
+    ...mapState(['isAdmin', 'adminSettingsPath']),
+    ...mapGetters({ isDisabled: 'getIsDisabled' }),
     showSettingForm() {
       return !this.isDisabled && !this.fetchSettingsError;
     },
+    showDisabledFormMessage() {
+      return this.isDisabled && !this.fetchSettingsError;
+    },
+    unavailableFeatureMessage() {
+      return this.isAdmin
+        ? this.$options.i18n.unavailableAdminFeatureText
+        : this.$options.i18n.unavailableUserFeatureText;
+    },
   },
   mounted() {
     this.fetchSettings().catch(() => {
@@ -59,16 +75,21 @@ export default {
     </ul>
     <settings-form v-if="showSettingForm" />
     <template v-else>
-      <gl-alert v-if="isDisabled" :dismissible="false">
-        <p>
-          <gl-sprintf :message="$options.i18n.unavailableFeatureText">
-            <template #link="{content}">
-              <gl-link href="https://gitlab.com/gitlab-org/gitlab/issues/196124" target="_blank">
-                {{ content }}
-              </gl-link>
-            </template>
-          </gl-sprintf>
-        </p>
+      <gl-alert
+        v-if="showDisabledFormMessage"
+        :dismissible="false"
+        :title="$options.i18n.unavailableFeatureTitle"
+        variant="tip"
+      >
+        {{ $options.i18n.unavailableFeatureIntroText }}
+
+        <gl-sprintf :message="unavailableFeatureMessage">
+          <template #link="{ content }">
+            <gl-link :href="adminSettingsPath" target="_blank">
+              {{ content }}
+            </gl-link>
+          </template>
+        </gl-sprintf>
       </gl-alert>
       <gl-alert v-else-if="fetchSettingsError" variant="warning" :dismissible="false">
         <gl-sprintf :message="$options.i18n.fetchSettingsErrorText" />

app/assets/javascripts/registry/settings/store/actions.js

@@ -5,11 +5,7 @@ export const setInitialState = ({ commit }, data) => commit(types.SET_INITIAL_ST
 export const updateSettings = ({ commit }, data) => commit(types.UPDATE_SETTINGS, data);
 export const toggleLoading = ({ commit }) => commit(types.TOGGLE_LOADING);
 export const receiveSettingsSuccess = ({ commit }, data) => {
-  if (data) {
-    commit(types.SET_SETTINGS, data);
-  } else {
-    commit(types.SET_IS_DISABLED, true);
-  }
+  commit(types.SET_SETTINGS, data);
 };
 export const resetSettings = ({ commit }) => commit(types.RESET_SETTINGS);
 

app/assets/javascripts/registry/settings/store/getters.js

@@ -19,3 +19,7 @@ export const getSettings = (state, getters) => ({
 });
 
 export const getIsEdited = state => !isEqual(state.original, state.settings);
+
+export const getIsDisabled = state => {
+  return !(state.original || state.enableHistoricEntries);
+};

app/assets/javascripts/registry/settings/store/mutation_types.js

@@ -3,4 +3,3 @@ export const UPDATE_SETTINGS = 'UPDATE_SETTINGS';
 export const TOGGLE_LOADING = 'TOGGLE_LOADING';
 export const SET_SETTINGS = 'SET_SETTINGS';
 export const RESET_SETTINGS = 'RESET_SETTINGS';
-export const SET_IS_DISABLED = 'SET_IS_DISABLED';

app/assets/javascripts/registry/settings/store/mutations.js

+import { parseBoolean } from '~/lib/utils/common_utils';
 import * as types from './mutation_types';
 
 export default {
@@ -8,19 +9,19 @@ export default {
       keepN: JSON.parse(initialState.keepNOptions),
       olderThan: JSON.parse(initialState.olderThanOptions),
     };
+    state.enableHistoricEntries = parseBoolean(initialState.enableHistoricEntries);
+    state.isAdmin = parseBoolean(initialState.isAdmin);
+    state.adminSettingsPath = initialState.adminSettingsPath;
   },
   [types.UPDATE_SETTINGS](state, data) {
     state.settings = { ...state.settings, ...data.settings };
   },
   [types.SET_SETTINGS](state, settings) {
-    state.settings = settings;
+    state.settings = settings ?? state.settings;
     state.original = Object.freeze(settings);
   },
-  [types.SET_IS_DISABLED](state, isDisabled) {
-    state.isDisabled = isDisabled;
-  },
   [types.RESET_SETTINGS](state) {
-    state.settings = { ...state.original };
+    state.settings = Object.assign({}, state.original);
   },
   [types.TOGGLE_LOADING](state) {
     state.isLoading = !state.isLoading;

app/assets/javascripts/registry/settings/store/state.js

@@ -8,9 +8,17 @@ export default () => ({
    */
   isLoading: false,
   /*
-   * Boolean to determine if the user is allowed to interact with the form
+   * Boolean to determine if the user is an admin
    */
-  isDisabled: false,
+  isAdmin: false,
+  /*
+   * String containing the full path to the admin config page for CI/CD
+   */
+  adminSettingsPath: '',
+  /*
+   * Boolean to determine if project created before 12.8 can use this feature
+   */
+  enableHistoricEntries: false,
   /*
    * This contains the data shown and manipulated in the UI
    * Has the following structure:
@@ -24,9 +32,9 @@ export default () => ({
    */
   settings: {},
   /*
-   * Same structure as settings, above but Frozen object and used only in case the user clicks 'cancel'
+   * Same structure as settings, above but Frozen object and used only in case the user clicks 'cancel', initialized to null
    */
-  original: {},
+  original: null,
   /*
    * Contains the options used to populate the form selects
    */

app/views/projects/registry/settings/_index.haml

 #js-registry-settings{ data: { project_id: @project.id,
     cadence_options: cadence_options.to_json,
     keep_n_options: keep_n_options.to_json,
-    older_than_options: older_than_options.to_json} }
+    older_than_options: older_than_options.to_json,
+    is_admin: current_user&.admin.to_s,
+    admin_settings_path: ci_cd_admin_application_settings_path(anchor: 'js-registry-settings'),
+    enable_historic_entries: Gitlab::CurrentSettings.try(:container_expiration_policies_enable_historic_entries).to_s} }

locale/gitlab.pot

@@ -5462,6 +5462,9 @@ msgstr ""
 msgid "Container repositories sync capacity"
 msgstr ""
 
+msgid "ContainerRegistry| Please visit the %{linkStart}administration settings%{linkEnd} to enable this feature."
+msgstr ""
+
 msgid "ContainerRegistry|%{imageName} tags"
 msgstr ""
 
@@ -5477,6 +5480,9 @@ msgstr ""
 msgid "ContainerRegistry|Container Registry"
 msgstr ""
 
+msgid "ContainerRegistry|Container Registry tag expiration and retention policy is disabled"
+msgstr ""
+
 msgid "ContainerRegistry|Copy build command"
 msgstr ""
 
@@ -5486,9 +5492,6 @@ msgstr ""
 msgid "ContainerRegistry|Copy push command"
 msgstr ""
 
-msgid "ContainerRegistry|Currently, the Container Registry tag expiration feature is not available for projects created before GitLab version 12.8. For updates and more information, visit Issue %{linkStart}#196124%{linkEnd}"
-msgstr ""
-
 msgid "ContainerRegistry|Docker connection error"
 msgstr ""
 
@@ -5537,6 +5540,9 @@ msgstr ""
 msgid "ContainerRegistry|Number of tags to retain:"
 msgstr ""
 
+msgid "ContainerRegistry|Please contact your administrator."
+msgstr ""
+
 msgid "ContainerRegistry|Push an image"
 msgstr ""
 
@@ -5593,6 +5599,9 @@ msgstr ""
 msgid "ContainerRegistry|Tags deleted successfully"
 msgstr ""
 
+msgid "ContainerRegistry|The Container Registry tag expiration and retention policies for this project have not been enabled."
+msgstr ""
+
 msgid "ContainerRegistry|The last tag related to this image was recently removed. This empty image and any associated data will be automatically removed as part of the regular Garbage Collection process. If you have any questions, contact your administrator."
 msgstr ""
 

spec/frontend/registry/settings/components/registry_settings_app_spec.js

 import { shallowMount } from '@vue/test-utils';
-import { GlAlert } from '@gitlab/ui';
+import { GlAlert, GlSprintf, GlLink } from '@gitlab/ui';
 import component from '~/registry/settings/components/registry_settings_app.vue';
 import SettingsForm from '~/registry/settings/components/settings_form.vue';
 import { createStore } from '~/registry/settings/store/';
-import { SET_IS_DISABLED } from '~/registry/settings/store/mutation_types';
+import { SET_SETTINGS, SET_INITIAL_STATE } from '~/registry/settings/store/mutation_types';
 import { FETCH_SETTINGS_ERROR_MESSAGE } from '~/registry/shared/constants';
+import { stringifiedFormOptions } from '../../shared/mock_data';
 
 describe('Registry Settings App', () => {
   let wrapper;
@@ -13,14 +14,14 @@ describe('Registry Settings App', () => {
   const findSettingsComponent = () => wrapper.find(SettingsForm);
   const findAlert = () => wrapper.find(GlAlert);
 
-  const mountComponent = ({ dispatchMock = 'mockResolvedValue', isDisabled = false } = {}) => {
-    store = createStore();
-    store.commit(SET_IS_DISABLED, isDisabled);
+  const mountComponent = ({ dispatchMock = 'mockResolvedValue' } = {}) => {
     const dispatchSpy = jest.spyOn(store, 'dispatch');
-    if (dispatchMock) {
-      dispatchSpy[dispatchMock]();
-    }
+    dispatchSpy[dispatchMock]();
+
     wrapper = shallowMount(component, {
+      stubs: {
+        GlSprintf,
+      },
       mocks: {
         $toast: {
           show: jest.fn(),
@@ -30,11 +31,16 @@ describe('Registry Settings App', () => {
     });
   };
 
+  beforeEach(() => {
+    store = createStore();
+  });
+
   afterEach(() => {
     wrapper.destroy();
   });
 
   it('renders', () => {
+    store.commit(SET_SETTINGS, { foo: 'bar' });
     mountComponent();
     expect(wrapper.element).toMatchSnapshot();
   });
@@ -45,13 +51,15 @@ describe('Registry Settings App', () => {
   });
 
   it('renders the setting form', () => {
+    store.commit(SET_SETTINGS, { foo: 'bar' });
     mountComponent();
     expect(findSettingsComponent().exists()).toBe(true);
   });
 
-  describe('isDisabled', () => {
+  describe('the form is disabled', () => {
     beforeEach(() => {
-      mountComponent({ isDisabled: true });
+      store.commit(SET_SETTINGS, undefined);
+      mountComponent();
     });
 
     it('the form is hidden', () => {
@@ -59,9 +67,27 @@ describe('Registry Settings App', () => {
     });
 
     it('shows an alert', () => {
-      expect(findAlert().html()).toContain(
-        'Currently, the Container Registry tag expiration feature is not available',
+      const text = findAlert().text();
+      expect(text).toContain(
+        'The Container Registry tag expiration and retention policies for this project have not been enabled.',
       );
+      expect(text).toContain('Please contact your administrator.');
+    });
+
+    describe('an admin is visiting the page', () => {
+      beforeEach(() => {
+        store.commit(SET_INITIAL_STATE, {
+          ...stringifiedFormOptions,
+          isAdmin: true,
+          adminSettingsPath: 'foo',
+        });
+      });
+
+      it('shows the admin part of the alert message', () => {
+        const sprintf = findAlert().find(GlSprintf);
+        expect(sprintf.text()).toBe('administration settings');
+        expect(sprintf.find(GlLink).attributes('href')).toBe('foo');
+      });
     });
   });
 

spec/frontend/registry/settings/store/actions_spec.js

@@ -20,7 +20,7 @@ describe('Actions Registry Store', () => {
   );
 
   describe('receiveSettingsSuccess', () => {
-    it('calls SET_SETTINGS when data is present', () => {
+    it('calls SET_SETTINGS', () => {
       testAction(
         actions.receiveSettingsSuccess,
         'foo',
@@ -29,15 +29,6 @@ describe('Actions Registry Store', () => {
         [],
       );
     });
-    it('calls SET_IS_DISABLED when data is not present', () => {
-      testAction(
-        actions.receiveSettingsSuccess,
-        null,
-        {},
-        [{ type: types.SET_IS_DISABLED, payload: true }],
-        [],
-      );
-    });
   });
 
   describe('fetchSettings', () => {

spec/frontend/registry/settings/store/getters_spec.js

@@ -29,7 +29,7 @@ describe('Getters registry settings store', () => {
     });
   });
 
-  describe('getIsDisabled', () => {
+  describe('getIsEdited', () => {
     it('returns false when original is equal to settings', () => {
       const same = { foo: 'bar' };
       expect(getters.getIsEdited({ original: same, settings: same })).toBe(false);
@@ -41,4 +41,18 @@ describe('Getters registry settings store', () => {
       );
     });
   });
+
+  describe('getIsDisabled', () => {
+    it.each`
+      original          | enableHistoricEntries | result
+      ${undefined}      | ${false}              | ${true}
+      ${{ foo: 'bar' }} | ${undefined}          | ${false}
+      ${{}}             | ${false}              | ${false}
+    `(
+      'returns $result when original is $original and enableHistoricEntries is $enableHistoricEntries',
+      ({ original, enableHistoricEntries, result }) => {
+        expect(getters.getIsDisabled({ original, enableHistoricEntries })).toBe(result);
+      },
+    );
+  });
 });

spec/frontend/registry/settings/store/mutations_spec.js

@@ -12,14 +12,19 @@ describe('Mutations Registry Store', () => {
 
   describe('SET_INITIAL_STATE', () => {
     it('should set the initial state', () => {
-      const expectedState = { ...mockState, projectId: 'foo', formOptions };
-      mutations[types.SET_INITIAL_STATE](mockState, {
+      const payload = {
         projectId: 'foo',
+        enableHistoricEntries: false,
+        adminSettingsPath: 'foo',
+        isAdmin: true,
+      };
+      const expectedState = { ...mockState, ...payload, formOptions };
+      mutations[types.SET_INITIAL_STATE](mockState, {
+        ...payload,
         ...stringifiedFormOptions,
       });
 
-      expect(mockState.projectId).toEqual(expectedState.projectId);
-      expect(mockState.formOptions).toEqual(expectedState.formOptions);
+      expect(mockState).toEqual(expectedState);
     });
   });
 
@@ -41,6 +46,13 @@ describe('Mutations Registry Store', () => {
       expect(mockState.settings).toEqual(expectedState.settings);
       expect(mockState.original).toEqual(expectedState.settings);
     });
+
+    it('should keep the default state when settings is not present', () => {
+      const originalSettings = { ...mockState.settings };
+      mutations[types.SET_SETTINGS](mockState);
+      expect(mockState.settings).toEqual(originalSettings);
+      expect(mockState.original).toEqual(undefined);
+    });
   });
 
   describe('RESET_SETTINGS', () => {
@@ -50,6 +62,13 @@ describe('Mutations Registry Store', () => {
       mutations[types.RESET_SETTINGS](mockState);
       expect(mockState.settings).toEqual(mockState.original);
     });
+
+    it('if original is undefined it should initialize to empty object', () => {
+      mockState.settings = { foo: 'bar' };
+      mockState.original = undefined;
+      mutations[types.RESET_SETTINGS](mockState);
+      expect(mockState.settings).toEqual({});
+    });
   });
 
   describe('TOGGLE_LOADING', () => {
@@ -58,11 +77,4 @@ describe('Mutations Registry Store', () => {
       expect(mockState.isLoading).toEqual(true);
     });
   });
-
-  describe('SET_IS_DISABLED', () => {
-    it('should set isDisabled', () => {
-      mutations[types.SET_IS_DISABLED](mockState, true);
-      expect(mockState.isDisabled).toEqual(true);
-    });
-  });
 });