diff --git a/.gitlab/issue_templates/Experimentation.md b/.gitlab/issue_templates/Experiment Implementation.md similarity index 96% rename from .gitlab/issue_templates/Experimentation.md rename to .gitlab/issue_templates/Experiment Implementation.md index ba7839fb941..b3883f08c25 100644 --- a/.gitlab/issue_templates/Experimentation.md +++ b/.gitlab/issue_templates/Experiment Implementation.md @@ -1,4 +1,4 @@ - + # Experiment Summary diff --git a/.gitlab/issue_templates/Experiment Successful Cleanup.md b/.gitlab/issue_templates/Experiment Successful Cleanup.md index de95ea4ab94..1dd57332b8e 100644 --- a/.gitlab/issue_templates/Experiment Successful Cleanup.md +++ b/.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" diff --git a/app/assets/javascripts/notes/mixins/discussion_navigation.js b/app/assets/javascripts/notes/mixins/discussion_navigation.js index b313d1acdf1..ad529eb99b6 100644 --- a/app/assets/javascripts/notes/mixins/discussion_navigation.js +++ b/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, }); } diff --git a/app/assets/javascripts/projects/storage_counter/components/storage_table.vue b/app/assets/javascripts/projects/storage_counter/components/storage_table.vue index 7047fd925fb..a42a9711572 100644 --- a/app/assets/javascripts/projects/storage_counter/components/storage_table.vue +++ b/app/assets/javascripts/projects/storage_counter/components/storage_table.vue @@ -1,9 +1,10 @@ + diff --git a/app/assets/javascripts/runner/components/search_tokens/status_token_config.js b/app/assets/javascripts/runner/components/search_tokens/status_token_config.js index 03dff5e61a5..9963048ae1d 100644 --- a/app/assets/javascripts/runner/components/search_tokens/status_token_config.js +++ b/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, }; diff --git a/app/assets/javascripts/vue_shared/components/confirm_danger/confirm_danger_modal.stories.js b/app/assets/javascripts/vue_shared/components/confirm_danger/confirm_danger_modal.stories.js new file mode 100644 index 00000000000..18fa297da87 --- /dev/null +++ b/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: '', + 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, +}; diff --git a/app/assets/javascripts/vue_shared/components/confirm_danger/confirm_danger_modal.vue b/app/assets/javascripts/vue_shared/components/confirm_danger/confirm_danger_modal.vue index ebb1506895e..445fb5404a2 100644 --- a/app/assets/javascripts/vue_shared/components/confirm_danger/confirm_danger_modal.vue +++ b/app/assets/javascripts/vue_shared/components/confirm_danger/confirm_danger_modal.vue @@ -1,15 +1,17 @@ @@ -71,9 +74,15 @@ export default { :action-primary="actionPrimary" @primary="$emit('confirm')" > -

+ {{ confirmDangerMessage }} -

+

{{ $options.i18n.CONFIRM_DANGER_WARNING }}

@@ -82,8 +91,13 @@ export default {

- - + + diff --git a/app/assets/javascripts/vue_shared/components/confirm_danger/constants.js b/app/assets/javascripts/vue_shared/components/confirm_danger/constants.js index ddb381e87a3..fa44a9be411 100644 --- a/app/assets/javascripts/vue_shared/components/confirm_danger/constants.js +++ b/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.', diff --git a/doc/development/documentation/styleguide/index.md b/doc/development/documentation/styleguide/index.md index 88f6227b487..d3cb9768d14 100644 --- a/doc/development/documentation/styleguide/index.md +++ b/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: diff --git a/doc/development/elasticsearch.md b/doc/development/elasticsearch.md index 946f3185423..7c67b3495ba 100644 --- a/doc/development/elasticsearch.md +++ b/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 diff --git a/spec/features/admin/admin_runners_spec.rb b/spec/features/admin/admin_runners_spec.rb index 0a78acb8545..c6810086ac4 100644 --- a/spec/features/admin/admin_runners_spec.rb +++ b/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 diff --git a/spec/frontend/notes/mixins/discussion_navigation_spec.js b/spec/frontend/notes/mixins/discussion_navigation_spec.js index 70a5ff5184a..26a072b82f8 100644 --- a/spec/frontend/notes/mixins/discussion_navigation_spec.js +++ b/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'}`, () => { - expect(store.dispatch).toHaveBeenCalledWith('diffs/scrollToFile', { - path: 'test.js', - setHash: !diffsVirtualScrolling, - }); - }); + 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: hashValue, + }); + }, + ); }); }); }); diff --git a/spec/frontend/projects/storage_counter/components/storage_table_spec.js b/spec/frontend/projects/storage_counter/components/storage_table_spec.js index 14298318fff..c9e56d8f033 100644 --- a/spec/frontend/projects/storage_counter/components/storage_table_spec.js +++ b/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`, ``) diff --git a/spec/frontend/projects/storage_counter/components/storage_type_icon_spec.js b/spec/frontend/projects/storage_counter/components/storage_type_icon_spec.js new file mode 100644 index 00000000000..01efd6f14bd --- /dev/null +++ b/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); + }, + ); + }); +});