Add latest changes from gitlab-org/gitlab@master

.gitlab/issue_templates/Experiment Implementation.md

@@ -1,4 +1,4 @@
-<!-- Title suggestion: Experiment: [description] -->
+<!-- Title suggestion: Experiment Implementation: [description] -->
 
 # Experiment Summary
 <!-- Quick rundown of what is being done -->

.gitlab/issue_templates/Experiment Successful Cleanup.md

@@ -16,6 +16,6 @@ The changes need to become an official part of the product.
 - [ ] Optional: Migrate experiment to a default enabled [feature flag](https://docs.gitlab.com/ee/development/feature_flags) for one milestone and add a changelog. Converting to a feature flag can be skipped at the ICs discretion if risk is deemed low with consideration to both SaaS and (if applicable) self managed
 - [ ] In the next milestone, [remove the feature flag](https://docs.gitlab.com/ee/development/feature_flags/controls.html#cleaning-up) if applicable
 - [ ] After the flag removal is deployed, [clean up the feature/experiment feature flags](https://docs.gitlab.com/ee/development/feature_flags/controls.html#cleaning-up) by running chatops command in `#production` channel
-- [ ] Ensure the corresponding [Experiment Tracking](https://gitlab.com/groups/gitlab-org/-/boards/1352542?label_name[]=devops%3A%3Agrowth&label_name[]=growth%20experiment&label_name[]=experiment%20tracking) issue is updated
+- [ ] Ensure the corresponding [Experiment Rollout](https://gitlab.com/groups/gitlab-org/-/boards/1352542?label_name[]=devops%3A%3Agrowth&label_name[]=growth%20experiment&label_name[]=experiment-rollout) issue is updated
 
 /label ~"type::maintenance" ~"workflow::scheduling" ~"growth experiment" ~"feature flag"

app/assets/javascripts/notes/mixins/discussion_navigation.js

@@ -1,5 +1,6 @@
 import { mapGetters, mapActions, mapState } from 'vuex';
 import { scrollToElementWithContext, scrollToElement } from '~/lib/utils/common_utils';
+import { updateHistory } from '../../lib/utils/url_utility';
 import eventHub from '../event_hub';
 
 const isDiffsVirtualScrollingEnabled = () => window.gon?.features?.diffsVirtualScrolling;
@@ -22,13 +23,43 @@ function scrollTo(selector, { withoutContext = false } = {}) {
   return false;
 }
 
+function updateUrlWithNoteId(noteId) {
+  const newHistoryEntry = {
+    state: null,
+    title: window.title,
+    url: `#note_${noteId}`,
+    replace: true,
+  };
+
+  if (noteId && isDiffsVirtualScrollingEnabled()) {
+    // Temporarily mask the ID to avoid the browser default
+    //    scrolling taking over which is broken with virtual
+    //    scrolling enabled.
+    const note = document.querySelector(`#note_${noteId}`);
+    note?.setAttribute('id', `masked::${note.id}`);
+
+    // Update the hash now that the ID "doesn't exist" in the page
+    updateHistory(newHistoryEntry);
+
+    // Unmask the note's ID
+    note?.setAttribute('id', `note_${noteId}`);
+  } else if (noteId) {
+    updateHistory(newHistoryEntry);
+  }
+}
+
 /**
  * @param {object} self Component instance with mixin applied
  * @param {string} id Discussion id we are jumping to
  */
-function diffsJump({ expandDiscussion }, id) {
+function diffsJump({ expandDiscussion }, id, firstNoteId) {
   const selector = `ul.notes[data-discussion-id="${id}"]`;
-  eventHub.$once('scrollToDiscussion', () => scrollTo(selector));
+
+  eventHub.$once('scrollToDiscussion', () => {
+    scrollTo(selector);
+    // Wait for the discussion scroll before updating to the more specific ID
+    setTimeout(() => updateUrlWithNoteId(firstNoteId), 0);
+  });
   expandDiscussion({ discussionId: id });
 }
 
@@ -60,12 +91,13 @@ function switchToDiscussionsTabAndJumpTo(self, id) {
  * @param {object} discussion Discussion we are jumping to
  */
 function jumpToDiscussion(self, discussion) {
-  const { id, diff_discussion: isDiffDiscussion } = discussion;
+  const { id, diff_discussion: isDiffDiscussion, notes } = discussion;
+  const firstNoteId = notes?.[0]?.id;
   if (id) {
     const activeTab = window.mrTabs.currentAction;
 
     if (activeTab === 'diffs' && isDiffDiscussion) {
-      diffsJump(self, id);
+      diffsJump(self, id, firstNoteId);
     } else if (activeTab === 'show') {
       discussionJump(self, id);
     } else {
@@ -83,6 +115,7 @@ function handleDiscussionJump(self, fn, discussionId = self.currentDiscussionId)
   const isDiffView = window.mrTabs.currentAction === 'diffs';
   const targetId = fn(discussionId, isDiffView);
   const discussion = self.getDiscussion(targetId);
+  const setHash = !isDiffView && !isDiffsVirtualScrollingEnabled();
   const discussionFilePath = discussion?.diff_file?.file_path;
 
   if (isDiffsVirtualScrollingEnabled()) {
@@ -92,7 +125,7 @@ function handleDiscussionJump(self, fn, discussionId = self.currentDiscussionId)
   if (discussionFilePath) {
     self.scrollToFile({
       path: discussionFilePath,
-      setHash: !isDiffsVirtualScrollingEnabled(),
+      setHash,
     });
   }
 

app/assets/javascripts/projects/storage_counter/components/storage_table.vue

@@ -1,9 +1,10 @@
 <script>
-import { GlLink, GlIcon, GlTable, GlSprintf } from '@gitlab/ui';
+import { GlLink, GlIcon, GlTableLite as GlTable, GlSprintf } from '@gitlab/ui';
 import { numberToHumanSize } from '~/lib/utils/number_utils';
 import { thWidthClass } from '~/lib/utils/table_utility';
 import { sprintf } from '~/locale';
 import { PROJECT_TABLE_LABELS, HELP_LINK_ARIA_LABEL } from '../constants';
+import StorageTypeIcon from './storage_type_icon.vue';
 
 export default {
   name: 'StorageTable',
@@ -12,6 +13,7 @@ export default {
     GlIcon,
     GlTable,
     GlSprintf,
+    StorageTypeIcon,
   },
   props: {
     storageTypes: {
@@ -48,6 +50,12 @@ export default {
 <template>
   <gl-table :items="storageTypes" :fields="$options.projectTableFields">
     <template #cell(storageType)="{ item }">
+      <div class="gl-display-flex gl-flex-direction-row">
+        <storage-type-icon
+          :name="item.storageType.id"
+          :data-testid="`${item.storageType.id}-icon`"
+        />
+        <div>
           <p class="gl-font-weight-bold gl-mb-0" :data-testid="`${item.storageType.id}-name`">
             {{ item.storageType.name }}
             <gl-link
@@ -73,6 +81,8 @@ export default {
               </template>
             </gl-sprintf>
           </p>
+        </div>
+      </div>
     </template>
   </gl-table>
 </template>

app/assets/javascripts/projects/storage_counter/components/storage_type_icon.vue

@@ -0,0 +1,35 @@
+<script>
+import { GlIcon } from '@gitlab/ui';
+
+export default {
+  components: { GlIcon },
+  props: {
+    name: {
+      type: String,
+      required: false,
+      default: '',
+    },
+  },
+  methods: {
+    iconName(storageTypeName) {
+      const defaultStorageTypeIcon = 'disk';
+      const storageTypeIconMap = {
+        lfsObjectsSize: 'doc-image',
+        snippetsSize: 'snippet',
+        uploadsSize: 'upload',
+        repositorySize: 'infrastructure-registry',
+        packagesSize: 'package',
+      };
+
+      return storageTypeIconMap[`${storageTypeName}`] ?? defaultStorageTypeIcon;
+    },
+  },
+};
+</script>
+<template>
+  <span
+    class="gl-display-inline-flex gl-align-items-flex-start gl-justify-content-center gl-min-w-8 gl-pr-2 gl-pt-1"
+  >
+    <gl-icon :name="iconName(name)" :size="16" class="gl-mt-1" />
+  </span>
+</template>

app/assets/javascripts/runner/components/search_tokens/status_token_config.js

@@ -10,23 +10,29 @@ import {
   PARAM_KEY_STATUS,
 } from '../../constants';
 
+const options = [
+  { value: STATUS_ACTIVE, title: s__('Runners|Active') },
+  { value: STATUS_PAUSED, title: s__('Runners|Paused') },
+  { value: STATUS_ONLINE, title: s__('Runners|Online') },
+  { value: STATUS_OFFLINE, title: s__('Runners|Offline') },
+  { value: STATUS_NOT_CONNECTED, title: s__('Runners|Not connected') },
+];
+
 export const statusTokenConfig = {
   icon: 'status',
   title: __('Status'),
   type: PARAM_KEY_STATUS,
   token: BaseToken,
   unique: true,
-  options: [
-    { value: STATUS_ACTIVE, title: s__('Runners|Active') },
-    { value: STATUS_PAUSED, title: s__('Runners|Paused') },
-    { value: STATUS_ONLINE, title: s__('Runners|Online') },
-    { value: STATUS_OFFLINE, title: s__('Runners|Offline') },
-
-    // Added extra quotes in this title to avoid splitting this value:
+  options: options.map(({ value, title }) => ({
+    value,
+    // Replace whitespace with a special character to avoid
+    // splitting this value.
+    // Replacing in each option, as translations may also
+    // contain spaces!
+    // see: https://gitlab.com/gitlab-org/gitlab/-/issues/344142
     // see: https://gitlab.com/gitlab-org/gitlab-ui/-/issues/1438
-    { value: STATUS_NOT_CONNECTED, title: `"${s__('Runners|Not connected')}"` },
-  ],
-  // TODO In principle we could support more complex search rules,
-  // this can be added to a separate issue.
+    title: title.replace(' ', '\u00a0'),
+  })),
   operators: OPERATOR_IS_ONLY,
 };

app/assets/javascripts/vue_shared/components/confirm_danger/confirm_danger_modal.stories.js

@@ -0,0 +1,28 @@
+/* eslint-disable @gitlab/require-i18n-strings */
+import ConfirmDanger from './confirm_danger.vue';
+
+export default {
+  component: ConfirmDanger,
+  title: 'vue_shared/components/modals/confirm_danger_modal',
+};
+
+const Template = (args, { argTypes }) => ({
+  components: { ConfirmDanger },
+  props: Object.keys(argTypes),
+  template: '<confirm-danger v-bind="$props" />',
+  provide: {
+    confirmDangerMessage: 'You require more Vespene Gas',
+  },
+});
+
+export const Default = Template.bind({});
+Default.args = {
+  phrase: 'You must construct additional pylons',
+  buttonText: 'Confirm button text',
+};
+
+export const Disabled = Template.bind({});
+Disabled.args = {
+  ...Default.args,
+  disabled: true,
+};

app/assets/javascripts/vue_shared/components/confirm_danger/confirm_danger_modal.vue

@@ -1,15 +1,17 @@
 <script>
-import { GlModal, GlFormGroup, GlFormInput, GlSprintf } from '@gitlab/ui';
+import { GlAlert, GlModal, GlFormGroup, GlFormInput, GlSprintf } from '@gitlab/ui';
 import {
   CONFIRM_DANGER_MODAL_BUTTON,
   CONFIRM_DANGER_MODAL_TITLE,
   CONFIRM_DANGER_PHRASE_TEXT,
   CONFIRM_DANGER_WARNING,
+  CONFIRM_DANGER_MODAL_ERROR,
 } from './constants';
 
 export default {
   name: 'ConfirmDangerModal',
   components: {
+    GlAlert,
     GlModal,
     GlFormGroup,
     GlFormInput,
@@ -38,8 +40,8 @@ export default {
   },
   computed: {
     isValid() {
-      return (
-        this.confirmationPhrase.length && this.equalString(this.confirmationPhrase, this.phrase)
+      return Boolean(
+        this.confirmationPhrase.length && this.equalString(this.confirmationPhrase, this.phrase),
       );
     },
     actionPrimary() {
@@ -59,6 +61,7 @@ export default {
     CONFIRM_DANGER_MODAL_TITLE,
     CONFIRM_DANGER_WARNING,
     CONFIRM_DANGER_PHRASE_TEXT,
+    CONFIRM_DANGER_MODAL_ERROR,
   },
 };
 </script>
@@ -71,9 +74,15 @@ export default {
     :action-primary="actionPrimary"
     @primary="$emit('confirm')"
   >
-    <p v-if="confirmDangerMessage" class="text-danger" data-testid="confirm-danger-message">
+    <gl-alert
+      v-if="confirmDangerMessage"
+      variant="danger"
+      data-testid="confirm-danger-message"
+      :dismissible="false"
+      class="gl-mb-4"
+    >
       {{ confirmDangerMessage }}
-    </p>
+    </gl-alert>
     <p data-testid="confirm-danger-warning">{{ $options.i18n.CONFIRM_DANGER_WARNING }}</p>
     <p data-testid="confirm-danger-phrase">
       <gl-sprintf :message="$options.i18n.CONFIRM_DANGER_PHRASE_TEXT">
@@ -82,8 +91,13 @@ export default {
         </template>
       </gl-sprintf>
     </p>
-    <gl-form-group class="form-control" :state="isValid">
-      <gl-form-input v-model="confirmationPhrase" data-testid="confirm-danger-input" type="text" />
+    <gl-form-group :state="isValid" :invalid-feedback="$options.i18n.CONFIRM_DANGER_MODAL_ERROR">
+      <gl-form-input
+        v-model="confirmationPhrase"
+        class="form-control"
+        data-testid="confirm-danger-input"
+        type="text"
+      />
     </gl-form-group>
   </gl-modal>
 </template>

app/assets/javascripts/vue_shared/components/confirm_danger/constants.js

@@ -2,6 +2,7 @@ import { __ } from '~/locale';
 
 export const CONFIRM_DANGER_MODAL_ID = 'confirm-danger-modal';
 export const CONFIRM_DANGER_MODAL_TITLE = __('Confirmation required');
+export const CONFIRM_DANGER_MODAL_ERROR = __('Confirmation required');
 export const CONFIRM_DANGER_MODAL_BUTTON = __('Confirm');
 export const CONFIRM_DANGER_WARNING = __(
   'This action can lead to data loss. To prevent accidental actions we ask you to confirm your intention.',

doc/development/documentation/styleguide/index.md

@@ -317,8 +317,8 @@ create an issue or an MR to propose a change to the user interface text.
 
 #### Feature names
 
-- *Feature names are typically lowercase*.
-- *Some features are capitalized*, typically nouns naming GitLab-specific
+- Feature names are typically lowercase.
+- Some features are capitalized, typically nouns that name GitLab-specific
   capabilities or tools.
 
 See the [word list](word_list.md) for details.
@@ -404,13 +404,13 @@ Some contractions, however, should be avoided:
 
   | Do                                       | Don't                                   |
   |------------------------------------------|-----------------------------------------|
-  | Do *not* install X with Y.               | *Don't* install X with Y.               |
+  | Do not install X with Y.               | Don't install X with Y.               |
 
 - Do not use contractions in reference documentation. For example:
 
   | Do                                       | Don't                                   |
   |------------------------------------------|-----------------------------------------|
-  | Do *not* set a limit greater than 1000.  | *Don't* set a limit greater than 1000.  |
+  | Do not set a limit greater than 1000.  | Don't set a limit greater than 1000.  |
   | For `parameter1`, the default is 10.     | For `parameter1`, the default's 10.     |
 
 - Avoid contractions in error messages. Examples:
@@ -701,7 +701,7 @@ that's best described by a matrix, tables are the best choice.
 
 To keep tables accessible and scannable, tables should not have any
 empty cells. If there is no otherwise meaningful value for a cell, consider entering
-*N/A* (for 'not applicable') or *none*.
+**N/A** for 'not applicable' or **None**.
 
 To help tables be easier to maintain, consider adding additional spaces to the
 column widths to make them consistent. For example:

doc/development/elasticsearch.md

@@ -250,6 +250,11 @@ the migration runs and set it back to that value when the migration is completed
   will halt the migration if the storage required is not available when the migration runs. The migration must provide
   the space required in bytes by defining a `space_required_bytes` method.
 
+- `retry_on_failure` - Enable the retry on failure feature. By default, it retries
+  the migration 30 times. After it runs out of retries, the migration is marked as halted.
+  To customize the number of retries, pass the `max_attempts` argument:
+  `retry_on_failure max_attempts: 10`
+
 ```ruby
 # frozen_string_literal: true
 
@@ -259,6 +264,7 @@ class BatchedMigrationName < Elastic::Migration
   throttle_delay 10.minutes
   pause_indexing!
   space_requirements!
+  retry_on_failure
 
   # ...
 end

spec/features/admin/admin_runners_spec.rb

@@ -137,6 +137,19 @@ RSpec.describe "Admin Runners" do
           expect(page).not_to have_content 'runner-b-1'
           expect(page).not_to have_content 'runner-a-2'
         end
+
+        it 'shows correct runner when type is selected and search term is entered' do
+          create(:ci_runner, :instance, description: 'runner-connected', contacted_at: Time.now)
+          create(:ci_runner, :instance, description: 'runner-not-connected', contacted_at: nil)
+
+          visit admin_runners_path
+
+          # use the string "Not" to avoid using space and trigger an early selection
+          input_filtered_search_filter_is_only('Status', 'Not')
+
+          expect(page).not_to have_content 'runner-connected'
+          expect(page).to have_content 'runner-not-connected'
+        end
       end
 
       describe 'filter by type' do

spec/frontend/notes/mixins/discussion_navigation_spec.js

@@ -226,16 +226,13 @@ describe('Discussion navigation mixin', () => {
       ${false}
       ${true}
     `('virtual scrolling feature is $diffsVirtualScrolling', ({ diffsVirtualScrolling }) => {
-      beforeEach(async () => {
+      beforeEach(() => {
         window.gon = { features: { diffsVirtualScrolling } };
 
         jest.spyOn(store, 'dispatch');
 
         store.state.notes.currentDiscussionId = 'a';
         window.location.hash = 'test';
-        wrapper.vm.jumpToNextDiscussion();
-
-        await nextTick();
       });
 
       afterEach(() => {
@@ -243,16 +240,34 @@ describe('Discussion navigation mixin', () => {
         window.location.hash = '';
       });
 
-      it('resets location hash if diffsVirtualScrolling flag is true', () => {
+      it('resets location hash if diffsVirtualScrolling flag is true', async () => {
+        wrapper.vm.jumpToNextDiscussion();
+
+        await nextTick();
+
         expect(window.location.hash).toBe(diffsVirtualScrolling ? '' : '#test');
       });
 
-      it(`calls scrollToFile with setHash as ${diffsVirtualScrolling ? 'false' : 'true'}`, () => {
+      it.each`
+        tabValue   | hashValue
+        ${'diffs'} | ${false}
+        ${'show'}  | ${!diffsVirtualScrolling}
+        ${'other'} | ${!diffsVirtualScrolling}
+      `(
+        'calls scrollToFile with setHash as $hashValue when the tab is $tabValue',
+        async ({ hashValue, tabValue }) => {
+          window.mrTabs.currentAction = tabValue;
+
+          wrapper.vm.jumpToNextDiscussion();
+
+          await nextTick();
+
           expect(store.dispatch).toHaveBeenCalledWith('diffs/scrollToFile', {
             path: 'test.js',
-          setHash: !diffsVirtualScrolling,
-        });
-      });
+            setHash: hashValue,
+          });
+        },
+      );
     });
   });
 });

spec/frontend/projects/storage_counter/components/storage_table_spec.js

@@ -1,4 +1,4 @@
-import { GlTable } from '@gitlab/ui';
+import { GlTableLite } from '@gitlab/ui';
 import { mount } from '@vue/test-utils';
 import { extendedWrapper } from 'helpers/vue_test_utils_helper';
 import StorageTable from '~/projects/storage_counter/components/storage_table.vue';
@@ -22,7 +22,7 @@ describe('StorageTable', () => {
     );
   };
 
-  const findTable = () => wrapper.findComponent(GlTable);
+  const findTable = () => wrapper.findComponent(GlTableLite);
 
   beforeEach(() => {
     createComponent();
@@ -37,6 +37,7 @@ describe('StorageTable', () => {
       ({ storageType: { id, name, description } }) => {
         expect(wrapper.findByTestId(`${id}-name`).text()).toBe(name);
         expect(wrapper.findByTestId(`${id}-description`).text()).toBe(description);
+        expect(wrapper.findByTestId(`${id}-icon`).props('name')).toBe(id);
         expect(wrapper.findByTestId(`${id}-help-link`).attributes('href')).toBe(
           defaultProvideValues.helpLinks[id.replace(`Size`, `HelpPagePath`)]
             .replace(`Size`, ``)

spec/frontend/projects/storage_counter/components/storage_type_icon_spec.js

@@ -0,0 +1,41 @@
+import { mount } from '@vue/test-utils';
+import { GlIcon } from '@gitlab/ui';
+import StorageTypeIcon from '~/projects/storage_counter/components/storage_type_icon.vue';
+
+describe('StorageTypeIcon', () => {
+  let wrapper;
+
+  const createComponent = (props = {}) => {
+    wrapper = mount(StorageTypeIcon, {
+      propsData: {
+        ...props,
+      },
+    });
+  };
+
+  const findGlIcon = () => wrapper.findComponent(GlIcon);
+
+  describe('rendering icon', () => {
+    afterEach(() => {
+      wrapper.destroy();
+    });
+
+    it.each`
+      expected                     | provided
+      ${'doc-image'}               | ${'lfsObjectsSize'}
+      ${'snippet'}                 | ${'snippetsSize'}
+      ${'infrastructure-registry'} | ${'repositorySize'}
+      ${'package'}                 | ${'packagesSize'}
+      ${'upload'}                  | ${'uploadsSize'}
+      ${'disk'}                    | ${'wikiSize'}
+      ${'disk'}                    | ${'anything-else'}
+    `(
+      'renders icon with name of $expected when name prop is $provided',
+      ({ expected, provided }) => {
+        createComponent({ name: provided });
+
+        expect(findGlIcon().props('name')).toBe(expected);
+      },
+    );
+  });
+});