Add latest changes from gitlab-org/gitlab@master

5 files changed, 105 insertions, 12 deletions

diff --git a/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_drawer.vue b/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_drawer.vue
index 33ae747df22..7b51ebb461b 100644
--- a/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_drawer.vue
+++ b/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_drawer.vue
@@ -38,6 +38,7 @@ import CiEnvironmentsDropdown from './ci_environments_dropdown.vue';
 import { awsTokenList } from './ci_variable_autocomplete_tokens';
 
 const trackingMixin = Tracking.mixin({ label: DRAWER_EVENT_LABEL });
+const KEY_REGEX = /^\w+$/;
 
 export const i18n = {
   addVariable: s__('CiVariables|Add variable'),
@@ -52,6 +53,10 @@ export const i18n = {
   flags: __('Flags'),
   flagsLinkTitle: FLAG_LINK_TITLE,
   key: __('Key'),
+  keyFeedback: s__("CiVariables|A variable key can only contain letters, numbers, and '_'."),
+  keyHelpText: s__(
+    'CiVariables|You can use CI/CD variables with the same name in different places, but the variables might overwrite each other. %{linkStart}What is the order of precedence for variables?%{linkEnd}',
+  ),
   maskedField: s__('CiVariables|Mask variable'),
   maskedDescription: s__(
     'CiVariables|Variable will be masked in job logs. Requires values to meet regular expression requirements.',
@@ -157,7 +162,7 @@ export default {
       return regex.test(this.variable.value);
     },
     canSubmit() {
-      return this.variable.key.length > 0 && this.isValueValid;
+      return this.variable.key.length > 0 && this.isKeyValid && this.isValueValid;
     },
     getDrawerHeaderHeight() {
       return getContentWrapperHeight();
@@ -168,6 +173,9 @@ export default {
     isExpanded() {
       return !this.variable.raw;
     },
+    isKeyValid() {
+      return KEY_REGEX.test(this.variable.key);
+    },
     isMaskedReqsMet() {
       return !this.variable.masked || this.isValueMasked;
     },
@@ -320,6 +328,9 @@ export default {
   flagLink: helpPagePath('ci/variables/index', {
     anchor: 'define-a-cicd-variable-in-the-ui',
   }),
+  variablesPrecedenceLink: helpPagePath('ci/variables/index', {
+    anchor: 'cicd-variable-precedence',
+  }),
   i18n,
   variableOptions,
   deleteModal: {
@@ -413,6 +424,7 @@ export default {
               class="gl-display-flex"
               :title="$options.i18n.flagsLinkTitle"
               :href="$options.flagLink"
+              data-testid="ci-variable-flags-docs-link"
               target="_blank"
             >
               <gl-icon name="question-o" :size="14" />
@@ -451,6 +463,23 @@ export default {
         class="gl-border-none gl-pb-0! gl-mb-n5"
         data-testid="ci-variable-key"
       />
+      <p
+        v-if="variable.key.length > 0 && !isKeyValid"
+        class="gl-pt-3! gl-pb-0! gl-mb-0 gl-text-red-500 gl-border-none"
+      >
+        {{ $options.i18n.keyFeedback }}
+      </p>
+      <p class="gl-pt-3! gl-pb-0! gl-mb-0 gl-text-secondary gl-border-none">
+        <gl-sprintf :message="$options.i18n.keyHelpText">
+          <template #link="{ content }"
+            ><gl-link
+              :href="$options.variablesPrecedenceLink"
+              data-testid="ci-variable-precedence-docs-link"
+              >{{ content }}</gl-link
+            >
+          </template>
+        </gl-sprintf>
+      </p>
       <gl-form-group
         :label="$options.i18n.value"
         label-for="ci-variable-value"
diff --git a/app/services/protected_branches/base_service.rb b/app/services/protected_branches/base_service.rb
index 0ab46bf236c..62d5e04b499 100644
--- a/app/services/protected_branches/base_service.rb
+++ b/app/services/protected_branches/base_service.rb
@@ -18,9 +18,20 @@ module ProtectedBranches
 
     def refresh_cache
       CacheService.new(@project_or_group, @current_user, @params).refresh
+      refresh_cache_for_groups_projects
     rescue StandardError => e
       Gitlab::ErrorTracking.track_exception(e)
     end
+
+    private
+
+    def refresh_cache_for_groups_projects
+      return unless @project_or_group.is_a?(Group)
+
+      @project_or_group.all_projects.find_each do |project|
+        CacheService.new(project, @current_user, @params).refresh
+      end
+    end
   end
 end
 
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index fa4221fc2a9..71dfa498a66 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -10523,6 +10523,9 @@ msgstr ""
 msgid "CiStatusText|Warning"
 msgstr ""
 
+msgid "CiVariables|A variable key can only contain letters, numbers, and '_'."
+msgstr ""
+
 msgid "CiVariables|Add variable"
 msgstr ""
 
@@ -10655,6 +10658,9 @@ msgstr ""
 msgid "CiVariables|Variables store information, like passwords and secret keys, that you can use in job scripts. Each %{entity} can define a maximum of %{limit} variables."
 msgstr ""
 
+msgid "CiVariables|You can use CI/CD variables with the same name in different places, but the variables might overwrite each other. %{linkStart}What is the order of precedence for variables?%{linkEnd}"
+msgstr ""
+
 msgid "CiVariables|You have reached the maximum number of variables available. To add new variables, you must reduce the number of defined variables."
 msgstr ""
 
diff --git a/spec/frontend/ci/ci_variable_list/components/ci_variable_drawer_spec.js b/spec/frontend/ci/ci_variable_list/components/ci_variable_drawer_spec.js
index 113e946908a..721e2b831fc 100644
--- a/spec/frontend/ci/ci_variable_list/components/ci_variable_drawer_spec.js
+++ b/spec/frontend/ci/ci_variable_list/components/ci_variable_drawer_spec.js
@@ -1,6 +1,16 @@
 import { nextTick } from 'vue';
-import { GlDrawer, GlFormCombobox, GlFormInput, GlFormSelect, GlModal } from '@gitlab/ui';
+import {
+  GlDrawer,
+  GlFormCombobox,
+  GlFormGroup,
+  GlFormInput,
+  GlFormSelect,
+  GlLink,
+  GlModal,
+  GlSprintf,
+} from '@gitlab/ui';
 import { mountExtended, shallowMountExtended } from 'helpers/vue_test_utils_helper';
+import { helpPagePath } from '~/helpers/help_page_helper';
 import CiEnvironmentsDropdown from '~/ci/ci_variable_list/components/ci_environments_dropdown.vue';
 import CiVariableDrawer from '~/ci/ci_variable_list/components/ci_variable_drawer.vue';
 import { awsTokenList } from '~/ci/ci_variable_list/components/ci_variable_autocomplete_tokens';
@@ -76,6 +86,7 @@ describe('CI Variable Drawer', () => {
   const findDrawer = () => wrapper.findComponent(GlDrawer);
   const findEnvironmentScopeDropdown = () => wrapper.findComponent(CiEnvironmentsDropdown);
   const findExpandedCheckbox = () => wrapper.findByTestId('ci-variable-expanded-checkbox');
+  const findFlagsDocsLink = () => wrapper.findByTestId('ci-variable-flags-docs-link');
   const findKeyField = () => wrapper.findComponent(GlFormCombobox);
   const findMaskedCheckbox = () => wrapper.findByTestId('ci-variable-masked-checkbox');
   const findProtectedCheckbox = () => wrapper.findByTestId('ci-variable-protected-checkbox');
@@ -83,6 +94,26 @@ describe('CI Variable Drawer', () => {
   const findValueLabel = () => wrapper.findByTestId('ci-variable-value-label');
   const findTitle = () => findDrawer().find('h2');
   const findTypeDropdown = () => wrapper.findComponent(GlFormSelect);
+  const findVariablesPrecedenceDocsLink = () =>
+    wrapper.findByTestId('ci-variable-precedence-docs-link');
+
+  describe('template', () => {
+    beforeEach(() => {
+      createComponent({ stubs: { GlFormGroup, GlLink, GlSprintf } });
+    });
+
+    it('renders docs link for variables precendece', () => {
+      expect(findVariablesPrecedenceDocsLink().attributes('href')).toBe(
+        helpPagePath('ci/variables/index', { anchor: 'cicd-variable-precedence' }),
+      );
+    });
+
+    it('renders docs link for flags', () => {
+      expect(findFlagsDocsLink().attributes('href')).toBe(
+        helpPagePath('ci/variables/index', { anchor: 'define-a-cicd-variable-in-the-ui' }),
+      );
+    });
+  });
 
   describe('validations', () => {
     describe('type dropdown', () => {
@@ -265,12 +296,22 @@ describe('CI Variable Drawer', () => {
         expect(findKeyField().props('tokenList')).toBe(awsTokenList);
       });
 
-      it('cannot submit with empty key', async () => {
-        expect(findConfirmBtn().attributes('disabled')).toBeDefined();
-
-        await findKeyField().vm.$emit('input', 'NEW_VARIABLE');
-
-        expect(findConfirmBtn().attributes('disabled')).toBeUndefined();
+      const keyFeedbackMessage = "A variable key can only contain letters, numbers, and '_'.";
+      describe.each`
+        key                      | feedbackMessage       | submitButtonDisabledState
+        ${'validKey123'}         | ${''}                 | ${undefined}
+        ${'VALID_KEY'}           | ${''}                 | ${undefined}
+        ${''}                    | ${''}                 | ${'true'}
+        ${'invalid!!key'}        | ${keyFeedbackMessage} | ${'true'}
+        ${'key with whitespace'} | ${keyFeedbackMessage} | ${'true'}
+        ${'multiline\nkey'}      | ${keyFeedbackMessage} | ${'true'}
+      `('key validation', ({ key, feedbackMessage, submitButtonDisabledState }) => {
+        it(`validates key ${key} correctly`, async () => {
+          await findKeyField().vm.$emit('input', key);
+
+          expect(findConfirmBtn().attributes('disabled')).toBe(submitButtonDisabledState);
+          expect(wrapper.text()).toContain(feedbackMessage);
+        });
       });
     });
 
diff --git a/spec/services/protected_branches/create_service_spec.rb b/spec/services/protected_branches/create_service_spec.rb
index 625aa4fa377..abfb73c147e 100644
--- a/spec/services/protected_branches/create_service_spec.rb
+++ b/spec/services/protected_branches/create_service_spec.rb
@@ -16,6 +16,8 @@ RSpec.describe ProtectedBranches::CreateService, feature_category: :compliance_m
 
     describe '#execute' do
       let(:name) { 'master' }
+      let(:group_cache_service_double) { instance_double(ProtectedBranches::CacheService) }
+      let(:project_cache_service_double) { instance_double(ProtectedBranches::CacheService) }
 
       it 'creates a new protected branch' do
         expect { service.execute }.to change(ProtectedBranch, :count).by(1)
@@ -24,8 +26,12 @@ RSpec.describe ProtectedBranches::CreateService, feature_category: :compliance_m
       end
 
       it 'refreshes the cache' do
-        expect_next_instance_of(ProtectedBranches::CacheService) do |cache_service|
-          expect(cache_service).to receive(:refresh)
+        expect(ProtectedBranches::CacheService).to receive(:new).with(entity, user, params).and_return(group_cache_service_double)
+        expect(group_cache_service_double).to receive(:refresh)
+
+        if entity.is_a?(Group)
+          expect(ProtectedBranches::CacheService).to receive(:new).with(project, user, params).and_return(project_cache_service_double)
+          expect(project_cache_service_double).to receive(:refresh)
         end
 
         service.execute
@@ -58,14 +64,14 @@ RSpec.describe ProtectedBranches::CreateService, feature_category: :compliance_m
 
   context 'with entity project' do
     let_it_be_with_reload(:entity) { create(:project) }
-
     let(:user) { entity.first_owner }
 
     it_behaves_like 'execute with entity'
   end
 
   context 'with entity group' do
-    let_it_be_with_reload(:entity) { create(:group) }
+    let_it_be_with_reload(:project) { create(:project, :in_group) }
+    let_it_be_with_reload(:entity) { project.group }
     let_it_be_with_reload(:user) { create(:user) }
 
     before do