diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-07-06 12:07:41 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-07-06 12:07:41 +0300 |
commit | d2485dbfedc4759eba5243e0d155e34494c4429b (patch) | |
tree | 0cdd51c58aef728db2e3c7e9de09976e85c062bb | |
parent | d111e00680d2b3e46a7ee37af5499407c4a93a22 (diff) |
Add latest changes from gitlab-org/gitlab@master
88 files changed, 931 insertions, 549 deletions
diff --git a/.rubocop.yml b/.rubocop.yml index 8c52516a21c..9c0fb76f68f 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -490,11 +490,6 @@ Cop/InjectEnterpriseEditionModule: Style/ReturnNil: Enabled: true -# It isn't always safe to replace `=~` with `.match?`, especially when there are -# nil values on the left hand side -Performance/RegexpMatch: - Enabled: false - Cop/ActiveRecordAssociationReload: Enabled: true Exclude: diff --git a/.rubocop_todo/performance/regexp_match.yml b/.rubocop_todo/performance/regexp_match.yml new file mode 100644 index 00000000000..c1a8036f8d1 --- /dev/null +++ b/.rubocop_todo/performance/regexp_match.yml @@ -0,0 +1,96 @@ +--- +# Cop supports --autocorrect. +Performance/RegexpMatch: + Details: grace period + Exclude: + - 'app/controllers/concerns/internal_redirect.rb' + - 'app/controllers/import/bitbucket_server_controller.rb' + - 'app/finders/ci/pipelines_finder.rb' + - 'app/helpers/application_helper.rb' + - 'app/helpers/colors_helper.rb' + - 'app/helpers/emails_helper.rb' + - 'app/models/commit_range.rb' + - 'app/models/commit_status.rb' + - 'app/models/concerns/ignorable_columns.rb' + - 'app/models/external_issue.rb' + - 'app/models/hooks/web_hook_log.rb' + - 'app/models/projects/topic.rb' + - 'app/models/repository.rb' + - 'app/models/user.rb' + - 'app/services/bulk_imports/create_service.rb' + - 'app/services/clusters/cleanup/project_namespace_service.rb' + - 'app/services/clusters/cleanup/service_account_service.rb' + - 'app/services/projects/update_remote_mirror_service.rb' + - 'app/uploaders/file_uploader.rb' + - 'app/validators/abstract_path_validator.rb' + - 'app/validators/cluster_name_validator.rb' + - 'app/validators/devise_email_validator.rb' + - 'app/validators/line_code_validator.rb' + - 'config/initializers/wikicloth_redos_patch.rb' + - 'ee/app/controllers/concerns/audit_events/enforces_valid_date_params.rb' + - 'ee/lib/ee/banzai/filter/references/vulnerability_reference_filter.rb' + - 'ee/lib/elastic/latest/git_class_proxy.rb' + - 'ee/lib/gitlab/llm/chain/utils/text_processing.rb' + - 'ee/lib/gitlab/llm/open_ai/response_modifiers/tanuki_bot.rb' + - 'ee/lib/gitlab/middleware/ip_restrictor.rb' + - 'ee/spec/spec_helper.rb' + - 'lib/api/helpers.rb' + - 'lib/api/helpers/common_helpers.rb' + - 'lib/api/validations/validators/bulk_imports.rb' + - 'lib/banzai/color_parser.rb' + - 'lib/banzai/filter/ascii_doc_sanitization_filter.rb' + - 'lib/banzai/filter/references/abstract_reference_filter.rb' + - 'lib/banzai/filter/references/reference_filter.rb' + - 'lib/bulk_imports/path_normalization.rb' + - 'lib/feature/definition.rb' + - 'lib/gitlab/authorized_keys.rb' + - 'lib/gitlab/checks/branch_check.rb' + - 'lib/gitlab/ci/build/artifacts/metadata.rb' + - 'lib/gitlab/ci/build/artifacts/metadata/entry.rb' + - 'lib/gitlab/ci/project_config/remote.rb' + - 'lib/gitlab/database/postgres_constraint.rb' + - 'lib/gitlab/database/postgres_foreign_key.rb' + - 'lib/gitlab/database/postgres_index.rb' + - 'lib/gitlab/database/postgres_partition.rb' + - 'lib/gitlab/database/postgres_partitioned_table.rb' + - 'lib/gitlab/database/reindexing/reindex_concurrently.rb' + - 'lib/gitlab/dependency_linker/base_linker.rb' + - 'lib/gitlab/dependency_linker/composer_json_linker.rb' + - 'lib/gitlab/diff/parser.rb' + - 'lib/gitlab/email/reply_parser.rb' + - 'lib/gitlab/git/gitmodules_parser.rb' + - 'lib/gitlab/metrics/samplers/threads_sampler.rb' + - 'lib/gitlab/middleware/sidekiq_web_static.rb' + - 'lib/gitlab/middleware/static.rb' + - 'lib/gitlab/url_blocker.rb' + - 'lib/tasks/gitlab/update_templates.rake' + - 'lib/uploaded_file.rb' + - 'qa/qa/flow/integrations/slack.rb' + - 'qa/qa/git/location.rb' + - 'qa/qa/resource/api_fabricator.rb' + - 'qa/qa/runtime/search.rb' + - 'qa/qa/service/cluster_provider/k3d.rb' + - 'qa/qa/specs/spec_helper.rb' + - 'qa/qa/tools/ci/ff_changes.rb' + - 'rubocop/cop/project_path_helper.rb' + - 'rubocop/cop/qa/selector_usage.rb' + - 'scripts/changed-feature-flags' + - 'scripts/failed_tests.rb' + - 'scripts/lib/glfm/parse_examples.rb' + - 'scripts/lib/glfm/update_specification.rb' + - 'scripts/lint-docs-blueprints.rb' + - 'scripts/perf/query_limiting_report.rb' + - 'scripts/qa/testcases-check' + - 'scripts/trigger-build.rb' + - 'sidekiq_cluster/cli.rb' + - 'spec/lib/gitlab/cluster/mixins/puma_cluster_spec.rb' + - 'spec/mailers/emails/in_product_marketing_spec.rb' + - 'spec/spec_helper.rb' + - 'spec/support/capybara.rb' + - 'spec/support/helpers/test_env.rb' + - 'spec/support/shared_contexts/features/integrations/integrations_shared_context.rb' + - 'spec/support/shared_examples/features/discussion_comments_shared_example.rb' + - 'spec/tooling/quality/test_level_spec.rb' + - 'tooling/danger/analytics_instrumentation.rb' + - 'tooling/danger/database_dictionary.rb' + - 'tooling/danger/specs/feature_category_suggestion.rb' diff --git a/.rubocop_todo/rspec/missing_feature_category.yml b/.rubocop_todo/rspec/missing_feature_category.yml index 0392c1feb40..bc050a40a73 100644 --- a/.rubocop_todo/rspec/missing_feature_category.yml +++ b/.rubocop_todo/rspec/missing_feature_category.yml @@ -5565,7 +5565,6 @@ RSpec/MissingFeatureCategory: - 'spec/support_specs/helpers/stub_method_calls_spec.rb' - 'spec/support_specs/matchers/be_sorted_spec.rb' - 'spec/support_specs/matchers/exceed_query_limit_helpers_spec.rb' - - 'spec/support_specs/time_travel_spec.rb' - 'spec/tasks/admin_mode_spec.rb' - 'spec/tasks/config_lint_rake_spec.rb' - 'spec/tasks/dev_rake_spec.rb' diff --git a/Gemfile.lock b/Gemfile.lock index 7cb66152e99..a2a7a3f26ae 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -16,6 +16,7 @@ PATH remote: gems/gitlab-rspec specs: gitlab-rspec (0.1.0) + activesupport (>= 6.1, < 7.1) rspec (~> 3.0) PATH diff --git a/app/assets/javascripts/batch_comments/stores/modules/batch_comments/actions.js b/app/assets/javascripts/batch_comments/stores/modules/batch_comments/actions.js index fc999cb0840..070ce38c8aa 100644 --- a/app/assets/javascripts/batch_comments/stores/modules/batch_comments/actions.js +++ b/app/assets/javascripts/batch_comments/stores/modules/batch_comments/actions.js @@ -3,6 +3,7 @@ import { createAlert } from '~/alert'; import { scrollToElement } from '~/lib/utils/common_utils'; import { __ } from '~/locale'; import { FILE_DIFF_POSITION_TYPE } from '~/diffs/constants'; +import { updateNoteErrorMessage } from '~/notes/utils'; import { CHANGES_TAB, DISCUSSION_TAB, SHOW_TAB } from '../../../constants'; import service from '../../../services/drafts_service'; import * as types from './mutation_types'; @@ -109,7 +110,7 @@ export const updateDiscussionsAfterPublish = async ({ dispatch, getters, rootGet export const updateDraft = ( { commit, getters }, - { note, noteText, resolveDiscussion, position, callback }, + { note, noteText, resolveDiscussion, position, flashContainer, callback, errorCallback }, ) => { const params = { draftId: note.id, @@ -125,11 +126,14 @@ export const updateDraft = ( .then((res) => res.data) .then((data) => commit(types.RECEIVE_DRAFT_UPDATE_SUCCESS, data)) .then(callback) - .catch(() => + .catch((e) => { createAlert({ - message: __('An error occurred while updating the comment'), - }), - ); + message: updateNoteErrorMessage(e), + parent: flashContainer, + }); + + errorCallback(); + }); }; export const scrollToDraft = ({ dispatch, rootGetters }, draft) => { diff --git a/app/assets/javascripts/ci/ci_variable_list/components/ci_environments_dropdown.vue b/app/assets/javascripts/ci/ci_variable_list/components/ci_environments_dropdown.vue index eda0e8829a8..a25f871ac92 100644 --- a/app/assets/javascripts/ci/ci_variable_list/components/ci_environments_dropdown.vue +++ b/app/assets/javascripts/ci/ci_variable_list/components/ci_environments_dropdown.vue @@ -24,6 +24,10 @@ export default { type: Array, required: true, }, + hasEnvScopeQuery: { + type: Boolean, + required: true, + }, selectedEnvironmentScope: { type: String, required: false, @@ -48,22 +52,19 @@ export default { }); }, isDropdownLoading() { - return this.areEnvironmentsLoading && this.isEnvScopeLimited && !this.isDropdownShown; + return this.areEnvironmentsLoading && this.hasEnvScopeQuery && !this.isDropdownShown; }, isDropdownSearching() { - return this.areEnvironmentsLoading && this.isEnvScopeLimited && this.isDropdownShown; - }, - isEnvScopeLimited() { - return this.glFeatures?.ciLimitEnvironmentScope; + return this.areEnvironmentsLoading && this.hasEnvScopeQuery && this.isDropdownShown; }, searchedEnvironments() { - // If FF is enabled, search query will be fired so this component will already - // receive filtered environments during the refetch. - // If FF is disabled, search the existing list of environments in the frontend - let filtered = this.isEnvScopeLimited ? this.environments : this.filteredEnvironments; + // If hasEnvScopeQuery (applies only to projects for now), search query will be fired so this + // component will already receive filtered environments during the refetch. + // Otherwise (applies to groups), search the existing list of environments in the frontend + let filtered = this.hasEnvScopeQuery ? this.environments : this.filteredEnvironments; // If there is no search term, make sure to include * - if (this.isEnvScopeLimited && !this.searchTerm) { + if (this.hasEnvScopeQuery && !this.searchTerm) { filtered = uniq([...filtered, '*']); } @@ -77,7 +78,7 @@ export default { }, shouldRenderDivider() { return ( - (this.isEnvScopeLimited || this.shouldRenderCreateButton) && !this.areEnvironmentsLoading + (this.hasEnvScopeQuery || this.shouldRenderCreateButton) && !this.areEnvironmentsLoading ); }, environmentScopeLabel() { @@ -88,7 +89,7 @@ export default { debouncedSearch: debounce(function debouncedSearch(searchTerm) { const newSearchTerm = searchTerm.trim(); this.searchTerm = newSearchTerm; - if (this.isEnvScopeLimited) { + if (this.hasEnvScopeQuery) { this.$emit('search-environment-scope', newSearchTerm); } }, 500), @@ -128,7 +129,7 @@ export default { > <template #footer> <gl-dropdown-divider v-if="shouldRenderDivider" /> - <div v-if="isEnvScopeLimited" data-testid="max-envs-notice"> + <div v-if="hasEnvScopeQuery" data-testid="max-envs-notice"> <gl-dropdown-item class="gl-list-style-none" disabled> <gl-sprintf :message="$options.i18n.maxEnvsNote" class="gl-font-sm"> <template #limit> diff --git a/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_modal.vue b/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_modal.vue index 41514d2d2f1..3af48635f3f 100644 --- a/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_modal.vue +++ b/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_modal.vue @@ -93,6 +93,10 @@ export default { required: false, default: false, }, + hasEnvScopeQuery: { + type: Boolean, + required: true, + }, mode: { type: String, required: true, @@ -147,7 +151,7 @@ export default { return !this.isTipDismissed && AWS_TOKEN_CONSTANTS.includes(this.variable.key); }, environmentsList() { - if (this.glFeatures?.ciLimitEnvironmentScope) { + if (this.hasEnvScopeQuery) { return this.environments; } @@ -385,6 +389,7 @@ export default { <ci-environments-dropdown v-if="areScopedVariablesAvailable" :are-environments-loading="areEnvironmentsLoading" + :has-env-scope-query="hasEnvScopeQuery" :selected-environment-scope="variable.environmentScope" :environments="environmentsList" @select-environment="setEnvironmentScope" diff --git a/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_settings.vue b/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_settings.vue index 26e20c690bc..b8a95f9081a 100644 --- a/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_settings.vue +++ b/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_settings.vue @@ -33,6 +33,10 @@ export default { required: false, default: false, }, + hasEnvScopeQuery: { + type: Boolean, + required: true, + }, isLoading: { type: Boolean, required: false, @@ -107,6 +111,7 @@ export default { :are-environments-loading="areEnvironmentsLoading" :are-scoped-variables-available="areScopedVariablesAvailable" :environments="environments" + :has-env-scope-query="hasEnvScopeQuery" :hide-environment-scope="hideEnvironmentScope" :variables="variables" :mode="mode" diff --git a/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_shared.vue b/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_shared.vue index f87b2769de8..9786f25ed87 100644 --- a/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_shared.vue +++ b/app/assets/javascripts/ci/ci_variable_list/components/ci_variable_shared.vue @@ -159,12 +159,13 @@ export default { return this.queryData?.environments?.query || {}; }, skip() { - return !this.queryData?.environments?.query; + return !this.hasEnvScopeQuery; }, variables() { return { + first: ENVIRONMENT_QUERY_LIMIT, fullPath: this.fullPath, - ...this.environmentQueryVariables, + search: '', }; }, update(data) { @@ -179,15 +180,8 @@ export default { areEnvironmentsLoading() { return this.$apollo.queries.environments.loading; }, - environmentQueryVariables() { - if (this.glFeatures?.ciLimitEnvironmentScope) { - return { - first: ENVIRONMENT_QUERY_LIMIT, - search: '', - }; - } - - return {}; + hasEnvScopeQuery() { + return Boolean(this.queryData?.environments?.query); }, isLoading() { return ( @@ -244,9 +238,7 @@ export default { this.variableMutation(UPDATE_MUTATION_ACTION, variable); }, async searchEnvironmentScope(searchTerm) { - if (this.glFeatures?.ciLimitEnvironmentScope) { - this.$apollo.queries.environments.refetch({ search: searchTerm }); - } + this.$apollo.queries.environments.refetch({ search: searchTerm }); }, async variableMutation(mutationAction, variable) { try { @@ -292,6 +284,7 @@ export default { :are-scoped-variables-available="areScopedVariablesAvailable" :entity="entity" :environments="environments" + :has-env-scope-query="hasEnvScopeQuery" :hide-environment-scope="hideEnvironmentScope" :is-loading="isLoading" :max-variable-limit="maxVariableLimit" diff --git a/app/assets/javascripts/design_management/pages/index.vue b/app/assets/javascripts/design_management/pages/index.vue index e7308aad785..b55610721a9 100644 --- a/app/assets/javascripts/design_management/pages/index.vue +++ b/app/assets/javascripts/design_management/pages/index.vue @@ -146,12 +146,6 @@ export default { } return 'col-12'; }, - designContentWrapperClass() { - if (this.hasDesigns) { - return 'gl-bg-gray-10 gl-border gl-border-t-0 gl-rounded-bottom-left-base gl-rounded-bottom-right-base gl-px-5'; - } - return null; - }, }, mounted() { if (this.$route.path === '/designs') { @@ -359,6 +353,7 @@ export default { <div data-testid="designs-root" class="gl-mt-4" + :class="{ 'gl-new-card': showToolbar }" @mouseenter="toggleOnPasteListener" @mouseleave="toggleOffPasteListener" > @@ -371,11 +366,7 @@ export default { > {{ uploadError }} </gl-alert> - <header - v-if="showToolbar" - class="gl-border gl-px-5 gl-py-4 gl-display-flex gl-justify-content-space-between gl-bg-white gl-rounded-top-base" - data-testid="design-toolbar-wrapper" - > + <header v-if="showToolbar" class="gl-new-card-header" data-testid="design-toolbar-wrapper"> <div class="gl-display-flex gl-justify-content-space-between gl-align-items-center gl-w-full gl-flex-wrap gl-gap-3" > @@ -427,7 +418,12 @@ export default { </div> </div> </header> - <div :class="designContentWrapperClass"> + <div + :class="{ + 'gl-mx-5': showToolbar, + 'gl-new-card-body gl-mx-3!': hasDesigns, + }" + > <gl-loading-icon v-if="isLoading" size="lg" /> <gl-alert v-else-if="error" variant="danger" :dismissible="false"> {{ $options.i18n.designLoadingError }} diff --git a/app/assets/javascripts/issues/related_merge_requests/components/related_merge_requests.vue b/app/assets/javascripts/issues/related_merge_requests/components/related_merge_requests.vue index 8490ffd33cd..de59210ff19 100644 --- a/app/assets/javascripts/issues/related_merge_requests/components/related_merge_requests.vue +++ b/app/assets/javascripts/issues/related_merge_requests/components/related_merge_requests.vue @@ -65,29 +65,29 @@ export default { <template> <div v-if="isFetchingMergeRequests || (!isFetchingMergeRequests && totalCount)"> - <div class="card card-slim gl-mt-5 gl-mb-0 gl-bg-gray-10"> - <div class="card-header gl-px-5 gl-py-4 gl-bg-white"> - <div - class="card-title gl-relative gl-display-flex gl-flex-wrap gl-align-items-center gl-line-height-20 gl-font-weight-bold gl-m-0" - > + <div class="gl-new-card"> + <div class="gl-new-card-header"> + <div class="gl-new-card-title-wrapper"> <gl-link class="anchor gl-absolute gl-text-decoration-none" href="#related-merge-requests" aria-labelledby="related-merge-requests" /> - <h3 id="related-merge-requests" class="gl-font-base gl-m-0"> + <h3 id="related-merge-requests" class="gl-new-card-title"> {{ __('Related merge requests') }} </h3> - <template v-if="totalCount"> - <gl-icon name="merge-request" class="gl-ml-3 gl-mr-2 gl-text-gray-500" /> - <span data-testid="count" class="gl-text-gray-500">{{ totalCount }}</span> - </template> - <p - v-if="hasClosingMergeRequest && !isFetchingMergeRequests" - class="gl-font-sm gl-font-weight-normal gl-flex-basis-full gl-mb-0 gl-text-gray-500" - > - {{ closingMergeRequestsText }} - </p> + <div class="gl-display-inline-flex gl-align-items-center gl-m-0"> + <template v-if="totalCount"> + <gl-icon name="merge-request" class="gl-ml-3 gl-mr-2 gl-text-gray-500" /> + <span data-testid="count" class="gl-text-gray-500">{{ totalCount }}</span> + </template> + <p + v-if="hasClosingMergeRequest && !isFetchingMergeRequests" + class="gl-font-sm gl-font-weight-normal gl-flex-basis-full gl-mb-0 gl-text-gray-500" + > + {{ closingMergeRequestsText }} + </p> + </div> </div> </div> <gl-loading-icon @@ -96,30 +96,34 @@ export default { label="Fetching related merge requests" class="gl-py-4" /> - <ul v-else class="content-list related-items-list gl-px-4! gl-py-3!"> - <li - v-for="mr in mergeRequests" - :key="mr.id" - class="list-item gl-m-0! gl-p-0! gl-border-b-0!" - > - <related-issuable-item - :id-key="mr.id" - :display-reference="mr.reference" - :title="mr.title" - :milestone="mr.milestone" - :assignees="getAssignees(mr)" - :created-at="mr.created_at" - :closed-at="mr.closed_at" - :merged-at="mr.merged_at" - :path="mr.web_url" - :state="mr.state" - :is-merge-request="true" - :pipeline-status="mr.head_pipeline && mr.head_pipeline.detailed_status" - path-id-separator="!" - class="gl-mx-n2" - /> - </li> - </ul> + <div class="gl-new-card-body"> + <div class="gl-new-card-content"> + <ul class="content-list related-items-list"> + <li + v-for="mr in mergeRequests" + :key="mr.id" + class="list-item gl-m-0! gl-p-0! gl-border-b-0!" + > + <related-issuable-item + :id-key="mr.id" + :display-reference="mr.reference" + :title="mr.title" + :milestone="mr.milestone" + :assignees="getAssignees(mr)" + :created-at="mr.created_at" + :closed-at="mr.closed_at" + :merged-at="mr.merged_at" + :path="mr.web_url" + :state="mr.state" + :is-merge-request="true" + :pipeline-status="mr.head_pipeline && mr.head_pipeline.detailed_status" + path-id-separator="!" + class="gl-mx-n2" + /> + </li> + </ul> + </div> + </div> </div> </div> </template> diff --git a/app/assets/javascripts/notes/components/comment_form.vue b/app/assets/javascripts/notes/components/comment_form.vue index 27ec8b55da7..97444bb1129 100644 --- a/app/assets/javascripts/notes/components/comment_form.vue +++ b/app/assets/javascripts/notes/components/comment_form.vue @@ -20,7 +20,7 @@ import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import * as constants from '../constants'; import eventHub from '../event_hub'; import { COMMENT_FORM } from '../i18n'; -import { getErrorMessages } from '../utils'; +import { createNoteErrorMessages } from '../utils'; import issuableStateMixin from '../mixins/issuable_state'; import CommentFieldLayout from './comment_field_layout.vue'; @@ -216,7 +216,7 @@ export default { 'toggleIssueLocalState', ]), handleSaveError({ data, status }) { - this.errors = getErrorMessages(data, status); + this.errors = createNoteErrorMessages(data, status); }, handleSaveDraft() { this.handleSave({ isDraft: true }); diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue index ab2b8cec661..0b3ba13847b 100644 --- a/app/assets/javascripts/notes/components/noteable_discussion.vue +++ b/app/assets/javascripts/notes/components/noteable_discussion.vue @@ -15,7 +15,7 @@ import { containsSensitiveToken, confirmSensitiveAction } from '~/lib/utils/secr import eventHub from '../event_hub'; import noteable from '../mixins/noteable'; import resolvable from '../mixins/resolvable'; -import { getErrorMessages } from '../utils'; +import { createNoteErrorMessages } from '../utils'; import DiffDiscussionHeader from './diff_discussion_header.vue'; import DiffWithNote from './diff_with_note.vue'; import DiscussionActions from './discussion_actions.vue'; @@ -275,7 +275,7 @@ export default { }); }, handleSaveError({ response }) { - const errorMessage = getErrorMessages(response.data, response.status)[0]; + const errorMessage = createNoteErrorMessages(response.data, response.status)[0]; createAlert({ message: errorMessage, diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue index d7cf5f74243..2a921b8c5ad 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/app/assets/javascripts/notes/components/noteable_note.vue @@ -17,8 +17,7 @@ import { containsSensitiveToken, confirmSensitiveAction } from '~/lib/utils/secr import eventHub from '../event_hub'; import noteable from '../mixins/noteable'; import resolvable from '../mixins/resolvable'; -import { renderMarkdown } from '../utils'; -import { UPDATE_COMMENT_FORM } from '../i18n'; +import { renderMarkdown, updateNoteErrorMessage } from '../utils'; import { getStartLineNumber, getEndLineNumber, @@ -316,7 +315,9 @@ export default { noteText, resolveDiscussion, position, + flashContainer: this.$el, callback: () => this.updateSuccess(), + errorCallback: () => callback(), }); if (this.isDraft) return; @@ -369,14 +370,8 @@ export default { }); }, handleUpdateError(e) { - const serverErrorMessage = e?.response?.data?.errors; - - const alertMessage = serverErrorMessage - ? sprintf(UPDATE_COMMENT_FORM.error, { reason: serverErrorMessage.toLowerCase() }, false) - : UPDATE_COMMENT_FORM.defaultError; - createAlert({ - message: alertMessage, + message: updateNoteErrorMessage(e), parent: this.$el, }); }, diff --git a/app/assets/javascripts/notes/utils.js b/app/assets/javascripts/notes/utils.js index c5859a89182..a561d26ad56 100644 --- a/app/assets/javascripts/notes/utils.js +++ b/app/assets/javascripts/notes/utils.js @@ -4,7 +4,7 @@ import { sanitize } from '~/lib/dompurify'; import { markdownConfig } from '~/lib/utils/text_utility'; import { HTTP_STATUS_UNPROCESSABLE_ENTITY } from '~/lib/utils/http_status'; import { sprintf } from '~/locale'; -import { COMMENT_FORM } from './i18n'; +import { UPDATE_COMMENT_FORM, COMMENT_FORM } from './i18n'; /** * Tracks snowplow event when User toggles timeline view @@ -23,7 +23,7 @@ export const renderMarkdown = (rawMarkdown) => { return sanitize(marked(rawMarkdown), markdownConfig); }; -export const getErrorMessages = (data, status) => { +export const createNoteErrorMessages = (data, status) => { const errors = data?.errors; if (errors && status === HTTP_STATUS_UNPROCESSABLE_ENTITY) { @@ -36,3 +36,13 @@ export const getErrorMessages = (data, status) => { return [COMMENT_FORM.GENERIC_UNSUBMITTABLE_NETWORK]; }; + +export const updateNoteErrorMessage = (e) => { + const errors = e?.response?.data?.errors; + + if (errors) { + return sprintf(UPDATE_COMMENT_FORM.error, { reason: errors.toLowerCase() }); + } + + return UPDATE_COMMENT_FORM.defaultError; +}; diff --git a/app/assets/javascripts/related_issues/components/related_issues_block.vue b/app/assets/javascripts/related_issues/components/related_issues_block.vue index f672acda062..6ccf53624cf 100644 --- a/app/assets/javascripts/related_issues/components/related_issues_block.vue +++ b/app/assets/javascripts/related_issues/components/related_issues_block.vue @@ -184,21 +184,14 @@ export default { <template> <div id="related-issues" class="related-issues-block"> <gl-card - class="gl-overflow-hidden gl-mt-5 gl-mb-0" - header-class="gl-p-0 gl-border-0" - body-class="gl-p-0 gl-bg-gray-10" + class="gl-new-card gl-overflow-hidden" + header-class="gl-new-card-header" + body-class="gl-new-card-body" + :aria-expanded="isOpen.toString()" > <template #header> - <div - :class="{ - 'gl-border-b-1': isOpen, - 'gl-border-b-0': !isOpen, - }" - class="gl-display-flex gl-justify-content-space-between gl-pl-5 gl-pr-4 gl-py-4 gl-bg-white gl-border-b-solid gl-border-b-gray-100" - > - <h3 - class="card-title h5 gl-relative gl-my-0 gl-display-flex gl-align-items-center gl-flex-grow-1 gl-line-height-24" - > + <div class="gl-new-card-title-wrapper"> + <h3 class="gl-new-card-title" data-testid="card-title"> <gl-link id="user-content-related-issues" class="anchor position-absolute gl-text-decoration-none" @@ -206,48 +199,48 @@ export default { aria-hidden="true" /> <slot name="header-text">{{ headerText }}</slot> - - <div - class="js-related-issues-header-issue-count gl-display-inline-flex gl-mx-3 gl-text-gray-500" - > - <span class="gl-display-inline-flex gl-align-items-center"> - <gl-icon :name="issuableTypeIcon" class="gl-mr-2 gl-text-gray-500" /> - {{ badgeLabel }} - </span> - </div> </h3> - <slot name="header-actions"></slot> - <gl-button - v-if="canAdmin" - size="small" - data-testid="related-issues-plus-button" - :aria-label="addIssuableButtonText" - class="gl-ml-3" - @click="addButtonClick" + <div + class="gl-new-card-count js-related-issues-header-issue-count gl-display-inline-flex gl-mx-3 gl-text-gray-500" > - <slot name="add-button-text">{{ __('Add') }}</slot> - </gl-button> - <div class="gl-pl-3 gl-ml-3 gl-border-l-1 gl-border-l-solid gl-border-l-gray-100"> - <gl-button - category="tertiary" - size="small" - :icon="toggleIcon" - :aria-label="toggleLabel" - data-testid="toggle-links" - @click="handleToggle" - /> + <span class="gl-display-inline-flex gl-align-items-center"> + <gl-icon :name="issuableTypeIcon" class="gl-mr-2 gl-text-gray-500" /> + {{ badgeLabel }} + </span> </div> </div> + <slot name="header-actions"></slot> + <gl-button + v-if="canAdmin" + size="small" + data-testid="related-issues-plus-button" + :aria-label="addIssuableButtonText" + class="gl-ml-3" + @click="addButtonClick" + > + <slot name="add-button-text">{{ __('Add') }}</slot> + </gl-button> + <div class="gl-pl-3 gl-ml-3 gl-border-l-1 gl-border-l-solid gl-border-l-gray-100"> + <gl-button + category="tertiary" + size="small" + :icon="toggleIcon" + :aria-label="toggleLabel" + data-testid="toggle-links" + @click="handleToggle" + /> + </div> </template> <div v-if="isOpen" - class="linked-issues-card-body gl-py-3 gl-px-4 gl-bg-gray-10" + class="linked-issues-card-body gl-new-card-content" data-testid="related-issues-body" > <div v-if="isFormVisible" - class="js-add-related-issues-form-area card-body bg-white gl-mt-2 gl-border-1 gl-border-solid gl-border-gray-100 gl-rounded-base" + class="js-add-related-issues-form-area gl-new-card-add-form" :class="{ 'gl-mb-5': shouldShowTokenBody, 'gl-show-field-errors': hasError }" + data-testid="add-item-form" > <add-issuable-form :show-categorized-issues="showCategorizedIssues" @@ -290,7 +283,7 @@ export default { /> </template> <div v-if="!shouldShowTokenBody && !isFormVisible" data-testid="related-items-empty"> - <p class="gl-p-2 gl-mb-0 gl-text-gray-500"> + <p class="gl-new-card-empty"> {{ emptyStateMessage }} <gl-link v-if="hasHelpPath" diff --git a/app/assets/javascripts/vue_shared/components/source_viewer/source_viewer_new.vue b/app/assets/javascripts/vue_shared/components/source_viewer/source_viewer_new.vue index 4d4d15a12b4..8e4c438719e 100644 --- a/app/assets/javascripts/vue_shared/components/source_viewer/source_viewer_new.vue +++ b/app/assets/javascripts/vue_shared/components/source_viewer/source_viewer_new.vue @@ -2,6 +2,7 @@ import SafeHtml from '~/vue_shared/directives/safe_html'; import Tracking from '~/tracking'; import addBlobLinksTracking from '~/blob/blob_links_tracking'; +import LineHighlighter from '~/blob/line_highlighter'; import { EVENT_ACTION, EVENT_LABEL_VIEWER } from './constants'; import Chunk from './components/chunk_new.vue'; @@ -30,6 +31,11 @@ export default { default: () => [], }, }, + data() { + return { + lineHighlighter: new LineHighlighter(), + }; + }, created() { this.track(EVENT_ACTION, { label: EVENT_LABEL_VIEWER, property: this.blob.language }); addBlobLinksTracking(); diff --git a/app/assets/javascripts/work_items/components/widget_wrapper.vue b/app/assets/javascripts/work_items/components/widget_wrapper.vue index 6ae30e9b084..f343f787358 100644 --- a/app/assets/javascripts/work_items/components/widget_wrapper.vue +++ b/app/assets/javascripts/work_items/components/widget_wrapper.vue @@ -27,6 +27,9 @@ export default { toggleLabel() { return this.isOpen ? __('Collapse') : __('Expand'); }, + isOpenString() { + return this.isOpen ? 'true' : 'false'; + }, }, methods: { hide() { @@ -43,18 +46,10 @@ export default { </script> <template> - <div - id="tasks" - class="gl-rounded-base gl-border-1 gl-border-solid gl-border-gray-100 gl-bg-gray-10 gl-mt-5" - > - <div - class="gl-pl-5 gl-pr-4 gl-py-4 gl-display-flex gl-justify-content-space-between gl-bg-white gl-rounded-base" - :class="{ - 'gl-border-b-1 gl-border-b-solid gl-border-b-gray-100 gl-rounded-bottom-left-none! gl-rounded-bottom-right-none!': isOpen, - }" - > - <div class="gl-display-flex gl-flex-grow-1"> - <h3 class="card-title h5 gl-m-0 gl-relative gl-line-height-24"> + <div id="tasks" class="gl-new-card" :aria-expanded="isOpenString"> + <div class="gl-new-card-header"> + <div class="gl-new-card-title-wrapper"> + <h3 class="gl-new-card-title"> <gl-link id="user-content-tasks-links" class="anchor position-absolute gl-text-decoration-none" @@ -66,7 +61,7 @@ export default { <slot name="header-suffix"></slot> </div> <slot name="header-right"></slot> - <div class="gl-border-l-1 gl-border-l-solid gl-border-l-gray-100 gl-pl-3 gl-ml-3"> + <div class="gl-new-card-toggle"> <gl-button category="tertiary" size="small" @@ -80,12 +75,7 @@ export default { <gl-alert v-if="error" variant="danger" @dismiss="$emit('dismissAlert')"> {{ error }} </gl-alert> - <div - v-if="isOpen" - class="gl-bg-gray-10 gl-rounded-bottom-left-base gl-rounded-bottom-right-base" - :class="{ 'gl-p-3': !error }" - data-testid="widget-body" - > + <div v-if="isOpen" class="gl-new-card-body" :class="{ error: error }" data-testid="widget-body"> <slot name="body"></slot> </div> </div> diff --git a/app/assets/javascripts/work_items/components/work_item_links/work_item_link_child.vue b/app/assets/javascripts/work_items/components/work_item_links/work_item_link_child.vue index 401c8a53eb0..ec44a654e89 100644 --- a/app/assets/javascripts/work_items/components/work_item_links/work_item_link_child.vue +++ b/app/assets/javascripts/work_items/components/work_item_links/work_item_link_child.vue @@ -271,7 +271,7 @@ export default { @click="toggleItem" /> <div - class="item-body work-item-link-child gl-relative gl-display-flex gl-flex-grow-1 gl-overflow-break-word gl-min-w-0 gl-pl-3 gl-pr-2 gl-py-2 gl-rounded-base" + class="item-body work-item-link-child gl-relative gl-display-flex gl-flex-grow-1 gl-overflow-break-word gl-min-w-0 gl-pl-3 gl-pr-2 gl-py-2 gl-mx-n2 gl-rounded-base" data-testid="links-child" > <div class="item-contents gl-display-flex gl-flex-grow-1 gl-flex-wrap gl-min-w-0"> diff --git a/app/assets/javascripts/work_items/components/work_item_links/work_item_links.vue b/app/assets/javascripts/work_items/components/work_item_links/work_item_links.vue index 19836e622af..9b3b4df5db6 100644 --- a/app/assets/javascripts/work_items/components/work_item_links/work_item_links.vue +++ b/app/assets/javascripts/work_items/components/work_item_links/work_item_links.vue @@ -236,52 +236,53 @@ export default { </gl-dropdown> </template> <template #body> - <gl-loading-icon v-if="isLoading" color="dark" class="gl-my-2" /> - - <template v-else> - <div v-if="isChildrenEmpty && !isShownAddForm && !error" data-testid="links-empty"> - <p class="gl-px-3 gl-py-2 gl-mb-0 gl-text-gray-500"> - {{ $options.i18n.emptyStateMessage }} - </p> - </div> - <work-item-links-form - v-if="isShownAddForm" - ref="wiLinksForm" - data-testid="add-links-form" - :issuable-gid="issuableGid" - :work-item-iid="iid" - :children-ids="childrenIds" - :parent-confidential="confidential" - :parent-iteration="issuableIteration" - :parent-milestone="issuableMilestone" - :form-type="formType" - :parent-work-item-type="workItem.workItemType.name" - @cancel="hideAddForm" - /> - <work-item-children-wrapper - :children="children" - :can-update="canUpdate" - :work-item-id="issuableGid" - :work-item-iid="iid" - @error="error = $event" - @show-modal="openChild" - /> - <work-item-detail-modal - ref="modal" - :work-item-id="activeChild.id" - :work-item-iid="activeChild.iid" - @close="closeModal" - @workItemDeleted="handleWorkItemDeleted(activeChild)" - @openReportAbuse="openReportAbuseDrawer" - /> - <abuse-category-selector - v-if="isReportDrawerOpen && reportAbusePath" - :reported-user-id="reportedUserId" - :reported-from-url="reportedUrl" - :show-drawer="isReportDrawerOpen" - @close-drawer="toggleReportAbuseDrawer(false)" - /> - </template> + <div class="gl-new-card-content"> + <gl-loading-icon v-if="isLoading" color="dark" class="gl-my-2" /> + <template v-else> + <div v-if="isChildrenEmpty && !isShownAddForm && !error" data-testid="links-empty"> + <p class="gl-new-card-empty"> + {{ $options.i18n.emptyStateMessage }} + </p> + </div> + <work-item-links-form + v-if="isShownAddForm" + ref="wiLinksForm" + data-testid="add-links-form" + :issuable-gid="issuableGid" + :work-item-iid="iid" + :children-ids="childrenIds" + :parent-confidential="confidential" + :parent-iteration="issuableIteration" + :parent-milestone="issuableMilestone" + :form-type="formType" + :parent-work-item-type="workItem.workItemType.name" + @cancel="hideAddForm" + /> + <work-item-children-wrapper + :children="children" + :can-update="canUpdate" + :work-item-id="issuableGid" + :work-item-iid="iid" + @error="error = $event" + @show-modal="openChild" + /> + <work-item-detail-modal + ref="modal" + :work-item-id="activeChild.id" + :work-item-iid="activeChild.iid" + @close="closeModal" + @workItemDeleted="handleWorkItemDeleted(activeChild)" + @openReportAbuse="openReportAbuseDrawer" + /> + <abuse-category-selector + v-if="isReportDrawerOpen && reportAbusePath" + :reported-user-id="reportedUserId" + :reported-from-url="reportedUrl" + :show-drawer="isReportDrawerOpen" + @close-drawer="toggleReportAbuseDrawer(false)" + /> + </template> + </div> </template> </widget-wrapper> </template> diff --git a/app/assets/javascripts/work_items/components/work_item_links/work_item_links_form.vue b/app/assets/javascripts/work_items/components/work_item_links/work_item_links_form.vue index 289a48b5eaf..db649913602 100644 --- a/app/assets/javascripts/work_items/components/work_item_links/work_item_links_form.vue +++ b/app/assets/javascripts/work_items/components/work_item_links/work_item_links_form.vue @@ -347,7 +347,8 @@ export default { <template> <gl-form - class="gl-bg-white gl-mt-1 gl-mb-3 gl-p-4 gl-border gl-border-gray-100 gl-rounded-base" + class="gl-new-card-add-form" + data-testid="add-item-form" @submit.prevent="addOrCreateMethod" > <gl-alert v-if="error" variant="danger" class="gl-mb-3" @dismiss="unsetError"> diff --git a/app/assets/javascripts/work_items/components/work_item_links/work_item_tree.vue b/app/assets/javascripts/work_items/components/work_item_links/work_item_tree.vue index 44e8dac79c4..83f3c391769 100644 --- a/app/assets/javascripts/work_items/components/work_item_links/work_item_tree.vue +++ b/app/assets/javascripts/work_items/components/work_item_links/work_item_tree.vue @@ -127,9 +127,11 @@ export default { </template> <template #body> <div v-if="!isShownAddForm && children.length === 0" data-testid="tree-empty"> - <p class="gl-mb-0 gl-py-2 gl-ml-3 gl-text-gray-500"> - {{ $options.WORK_ITEMS_TREE_TEXT_MAP[workItemType].empty }} - </p> + <div class="gl-new-card-content"> + <p class="gl-new-card-empty"> + {{ $options.WORK_ITEMS_TREE_TEXT_MAP[workItemType].empty }} + </p> + </div> </div> <work-item-links-form v-if="isShownAddForm" diff --git a/app/assets/stylesheets/framework.scss b/app/assets/stylesheets/framework.scss index e60353578b0..3957f82f658 100644 --- a/app/assets/stylesheets/framework.scss +++ b/app/assets/stylesheets/framework.scss @@ -64,3 +64,4 @@ @import 'framework/card'; @import 'framework/source_editor'; @import 'framework/diffs'; +@import 'framework/new_card'; diff --git a/app/assets/stylesheets/framework/new_card.scss b/app/assets/stylesheets/framework/new_card.scss new file mode 100644 index 00000000000..e6a4165980c --- /dev/null +++ b/app/assets/stylesheets/framework/new_card.scss @@ -0,0 +1,94 @@ +.gl-new-card { + @include gl-mt-5; + @include gl-bg-gray-10; + @include gl-border-1; + @include gl-border-solid; + @include gl-border-gray-100; + @include gl-rounded-base; + + &-header { + @include gl-pl-5; + @include gl-pr-4; + @include gl-py-4; + @include gl-display-flex; + @include gl-justify-content-space-between; + @include gl-bg-white; + @include gl-border-b-1; + @include gl-border-b-solid; + @include gl-border-b-gray-100; + @include gl-rounded-top-base; + } + + &[aria-expanded=false] &-header { + @include gl-border-bottom-0; + @include gl-rounded-base; + } + + &-title-wrapper { + @include gl-display-flex; + @include gl-flex-grow-1; + } + + &-title { + @include gl-font-base; + @include gl-font-weight-bold; + @include gl-relative; + @include gl-m-0; + @include gl-line-height-24; + } + + &-title-lg { + @include gl-font-lg; + } + + &-count { + @include gl-mx-3; + @include gl-font-base; + @include gl-font-weight-bold; + @include gl-text-gray-500; + } + + &-description { + @include gl-text-gray-500; + @include gl-mt-1; + } + + &-toggle { + @include gl-pl-3; + @include gl-ml-3; + @include gl-border-l-1; + @include gl-border-l-solid; + @include gl-border-l-gray-100; + } + + &-body { + @include gl-rounded-bottom-base; + @include gl-px-3; + @include gl-py-0; + } + + &-content { + @include gl-px-2; + @include gl-py-3; + } + + &-empty { + @include gl-p-2; + @include gl-mb-0; + @include gl-text-gray-500; + } + + &-footer { + @include gl-bg-white; + } + + &-add-form { + @include gl-p-4; + @include gl-my-2; + @include gl-bg-white; + @include gl-border-1; + @include gl-border-solid; + @include gl-border-gray-100; + @include gl-rounded-base; + } +} diff --git a/app/controllers/projects/merge_requests/drafts_controller.rb b/app/controllers/projects/merge_requests/drafts_controller.rb index 33371089b88..74c495261a3 100644 --- a/app/controllers/projects/merge_requests/drafts_controller.rb +++ b/app/controllers/projects/merge_requests/drafts_controller.rb @@ -38,11 +38,12 @@ class Projects::MergeRequests::DraftsController < Projects::MergeRequests::Appli end def update - draft_note.update!(draft_note_params) - - prepare_notes_for_rendering(draft_note) - - render json: DraftNoteSerializer.new(current_user: current_user).represent(draft_note) + if draft_note.update(draft_note_params) + prepare_notes_for_rendering(draft_note) + render json: DraftNoteSerializer.new(current_user: current_user).represent(draft_note) + else + render json: { errors: draft_note.errors.full_messages.to_sentence }, status: :unprocessable_entity + end end def destroy diff --git a/app/controllers/projects/settings/ci_cd_controller.rb b/app/controllers/projects/settings/ci_cd_controller.rb index ff1b6fa8d3d..0f4893fdb79 100644 --- a/app/controllers/projects/settings/ci_cd_controller.rb +++ b/app/controllers/projects/settings/ci_cd_controller.rb @@ -14,7 +14,6 @@ module Projects before_action do push_frontend_feature_flag(:ci_variables_pages, current_user) - push_frontend_feature_flag(:ci_limit_environment_scope, @project) push_frontend_feature_flag(:frozen_outbound_job_token_scopes, @project) push_frontend_feature_flag(:frozen_outbound_job_token_scopes_override, @project) end diff --git a/app/models/merge_request/metrics.rb b/app/models/merge_request/metrics.rb index 70216144035..a13cb353c7b 100644 --- a/app/models/merge_request/metrics.rb +++ b/app/models/merge_request/metrics.rb @@ -13,7 +13,7 @@ class MergeRequest::Metrics < ApplicationRecord before_save :ensure_target_project_id scope :merged_after, ->(date) { where(arel_table[:merged_at].gteq(date)) } - scope :merged_before, ->(date) { where(arel_table[:merged_at].lteq(date)) } + scope :merged_before, ->(date) { where(arel_table[:merged_at].lteq(date.is_a?(Time) ? date.end_of_day : date)) } scope :with_valid_time_to_merge, -> { where(arel_table[:merged_at].gt(arel_table[:created_at])) } scope :by_target_project, ->(project) { where(target_project_id: project) } diff --git a/app/models/namespaces/traversal/linear.rb b/app/models/namespaces/traversal/linear.rb index 469483931f0..6a4131186a1 100644 --- a/app/models/namespaces/traversal/linear.rb +++ b/app/models/namespaces/traversal/linear.rb @@ -96,13 +96,6 @@ module Namespaces traversal_ids.present? end - def use_traversal_ids_for_ancestors? - return false unless use_traversal_ids? - return false unless Feature.enabled?(:use_traversal_ids_for_ancestors, root_ancestor) - - traversal_ids.present? - end - def use_traversal_ids_for_ancestors_upto? return false unless use_traversal_ids? return false unless Feature.enabled?(:use_traversal_ids_for_ancestors_upto, root_ancestor) @@ -149,7 +142,7 @@ module Namespaces end def ancestors(hierarchy_order: nil) - return super unless use_traversal_ids_for_ancestors? + return super unless use_traversal_ids? return self.class.none if parent_id.blank? @@ -157,7 +150,7 @@ module Namespaces end def ancestor_ids(hierarchy_order: nil) - return super unless use_traversal_ids_for_ancestors? + return super unless use_traversal_ids? hierarchy_order == :desc ? traversal_ids[0..-2] : traversal_ids[0..-2].reverse end @@ -191,7 +184,7 @@ module Namespaces end def self_and_ancestors(hierarchy_order: nil) - return super unless use_traversal_ids_for_ancestors? + return super unless use_traversal_ids? return self.class.where(id: id) if parent_id.blank? @@ -199,7 +192,7 @@ module Namespaces end def self_and_ancestor_ids(hierarchy_order: nil) - return super unless use_traversal_ids_for_ancestors? + return super unless use_traversal_ids? hierarchy_order == :desc ? traversal_ids : traversal_ids.reverse end diff --git a/app/models/namespaces/traversal/linear_scopes.rb b/app/models/namespaces/traversal/linear_scopes.rb index c50d3dd1de6..52eb0d47593 100644 --- a/app/models/namespaces/traversal/linear_scopes.rb +++ b/app/models/namespaces/traversal/linear_scopes.rb @@ -18,7 +18,7 @@ module Namespaces end def roots - return super unless use_traversal_ids_roots? + return super unless use_traversal_ids? root_ids = all.select("#{quoted_table_name}.traversal_ids[1]").distinct unscoped.where(id: root_ids) @@ -78,11 +78,6 @@ module Namespaces Feature.enabled?(:use_traversal_ids) end - def use_traversal_ids_roots? - Feature.enabled?(:use_traversal_ids_roots) && - use_traversal_ids? - end - def use_traversal_ids_for_descendants_scopes? Feature.enabled?(:use_traversal_ids_for_descendants_scopes) && use_traversal_ids? diff --git a/app/views/projects/branches/_panel.html.haml b/app/views/projects/branches/_panel.html.haml index a632e29d34f..c01e3677c19 100644 --- a/app/views/projects/branches/_panel.html.haml +++ b/app/views/projects/branches/_panel.html.haml @@ -7,9 +7,9 @@ - return unless branches.any? -= render Pajamas::CardComponent.new(card_options: {class: 'gl-mt-5 gl-bg-gray-10'}, header_options: {class: 'gl-px-5 gl-py-4 gl-bg-white'}, body_options: {class: 'gl-px-3 gl-py-0'}, footer_options: {class: 'gl-bg-white'}) do |c| += render Pajamas::CardComponent.new(card_options: { class: 'gl-new-card' }, header_options: { class: 'gl-new-card-header' }, body_options: { class: 'gl-new-card-body' }, footer_options: { class: 'gl-new-card-footer' }) do |c| - c.with_header do - %h3.card-title.h5.gl-line-height-24.gl-m-0 + %h3.gl-new-card-title.h5 = panel_title - c.with_body do %ul.content-list.branches-list.all-branches{ data: { qa_selector: 'all_branches_container' } } diff --git a/config/feature_flags/development/ci_limit_environment_scope.yml b/config/feature_flags/development/ci_limit_environment_scope.yml deleted file mode 100644 index 0beed23a151..00000000000 --- a/config/feature_flags/development/ci_limit_environment_scope.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: ci_limit_environment_scope -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/113171 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/395003 -milestone: '15.10' -type: development -group: group::pipeline security -default_enabled: false diff --git a/config/feature_flags/development/use_traversal_ids_for_ancestors.yml b/config/feature_flags/development/use_traversal_ids_for_ancestors.yml deleted file mode 100644 index 64ba5b17513..00000000000 --- a/config/feature_flags/development/use_traversal_ids_for_ancestors.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: use_traversal_ids_for_ancestors -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/57137 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/334952 -milestone: '13.12' -type: development -group: group::tenant scale -default_enabled: true diff --git a/config/feature_flags/development/use_traversal_ids_roots.yml b/config/feature_flags/development/use_traversal_ids_roots.yml deleted file mode 100644 index d1f4cec7517..00000000000 --- a/config/feature_flags/development/use_traversal_ids_roots.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: use_traversal_ids_roots -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74148 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/345438 -milestone: '14.5' -type: development -group: group::tenant scale -default_enabled: true diff --git a/config/metrics/objects_schemas/batched_background_migrations_metric.json b/config/metrics/objects_schemas/batched_background_migrations_metric.json new file mode 100644 index 00000000000..968e804f600 --- /dev/null +++ b/config/metrics/objects_schemas/batched_background_migrations_metric.json @@ -0,0 +1,18 @@ +{ + "type": "array", + "items": { + "type": [ + { + "type": "object", + "properties": { + "job_class_name": { + "type": "string" + }, + "elapsed_time": { + "type": "integer" + } + } + } + ] + } +} diff --git a/config/metrics/settings/20230602180038_batched_background_migrations_metric.yml b/config/metrics/settings/20230602180038_batched_background_migrations_metric.yml new file mode 100644 index 00000000000..aa96cb8da50 --- /dev/null +++ b/config/metrics/settings/20230602180038_batched_background_migrations_metric.yml @@ -0,0 +1,24 @@ +--- +key_path: batched_background_migrations_metric +name: "batched_background_migrations" +description: "Tracks the execution time of batched background migrations" +product_section: enablement +product_stage: data_stores +product_group: database +value_type: object +status: active +milestone: "16.2" +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/122510 +time_frame: 7d +data_source: database +data_category: optional +instrumentation_class: BatchedBackgroundMigrationsMetric +performance_indicator_type: [] +distribution: +- ce +- ee +tier: +- free +- premium +- ultimate +value_json_schema: "config/metrics/objects_schemas/batched_background_migrations_metric.json" diff --git a/db/post_migrate/20230704062132_replace_p_ci_builds_metadata_foreign_key_v5.rb b/db/post_migrate/20230704062132_replace_p_ci_builds_metadata_foreign_key_v5.rb new file mode 100644 index 00000000000..c535c54b022 --- /dev/null +++ b/db/post_migrate/20230704062132_replace_p_ci_builds_metadata_foreign_key_v5.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class ReplacePCiBuildsMetadataForeignKeyV5 < Gitlab::Database::Migration[2.1] + include Gitlab::Database::PartitioningMigrationHelpers + + disable_ddl_transaction! + + def up + add_concurrent_partitioned_foreign_key :p_ci_builds_metadata, :p_ci_builds, + name: :temp_fk_e20479742e_p, + column: [:partition_id, :build_id], + target_column: [:partition_id, :id], + on_update: :cascade, + on_delete: :cascade, + validate: true, + reverse_lock_order: true + end + + def down + with_lock_retries do + remove_foreign_key_if_exists :p_ci_builds_metadata, :p_ci_builds, + name: :temp_fk_e20479742e_p, + reverse_lock_order: true + end + end +end diff --git a/db/post_migrate/20230704062136_replace_p_ci_runner_machine_builds_foreign_key_v4.rb b/db/post_migrate/20230704062136_replace_p_ci_runner_machine_builds_foreign_key_v4.rb new file mode 100644 index 00000000000..6cb8440bee8 --- /dev/null +++ b/db/post_migrate/20230704062136_replace_p_ci_runner_machine_builds_foreign_key_v4.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class ReplacePCiRunnerMachineBuildsForeignKeyV4 < Gitlab::Database::Migration[2.1] + include Gitlab::Database::PartitioningMigrationHelpers + + disable_ddl_transaction! + + def up + add_concurrent_partitioned_foreign_key :p_ci_runner_machine_builds, :p_ci_builds, + name: :temp_fk_bb490f12fe_p, + column: [:partition_id, :build_id], + target_column: [:partition_id, :id], + on_update: :cascade, + on_delete: :cascade, + validate: true, + reverse_lock_order: true + end + + def down + with_lock_retries do + remove_foreign_key_if_exists :p_ci_runner_machine_builds, :p_ci_builds, + name: :temp_fk_bb490f12fe_p, + reverse_lock_order: true + end + end +end diff --git a/db/schema_migrations/20230704062132 b/db/schema_migrations/20230704062132 new file mode 100644 index 00000000000..807788250b0 --- /dev/null +++ b/db/schema_migrations/20230704062132 @@ -0,0 +1 @@ +b103d237ee15e12602d656dca33abde5e6849ac4e81df606ba5578db9023f890
\ No newline at end of file diff --git a/db/schema_migrations/20230704062136 b/db/schema_migrations/20230704062136 new file mode 100644 index 00000000000..62964934621 --- /dev/null +++ b/db/schema_migrations/20230704062136 @@ -0,0 +1 @@ +74a1d7edb1319534d2853fc94726dae0e28944395c2fb30e0fc4597eb5ba1330
\ No newline at end of file diff --git a/doc/administration/integration/plantuml.md b/doc/administration/integration/plantuml.md index 58833c498eb..f05197d45e7 100644 --- a/doc/administration/integration/plantuml.md +++ b/doc/administration/integration/plantuml.md @@ -282,7 +282,7 @@ The Tomcat service should restart. After the restart is complete, the PlantUML integration is ready and listening for requests on port `8005`: `http://localhost:8005/plantuml` -To test if the PlantUML server is working, run `curl --location --verbose http://localhost:8005/plantuml/`. +To test if the PlantUML server is working, run `curl --location --verbose "http://localhost:8005/plantuml/"`. To change the Tomcat defaults, edit the `/opt/tomcat/conf/server.xml` file. @@ -342,4 +342,4 @@ these steps: - `deflate` is the default encoding type for PlantUML. To use a different encoding type, PlantUML integration [requires a header prefix in the URL](https://plantuml.com/text-encoding) - to distinguish different encoding types. + to distinguish different encoding types. diff --git a/doc/administration/package_information/defaults.md b/doc/administration/package_information/defaults.md index ac183afdc2f..0f6ae78a2af 100644 --- a/doc/administration/package_information/defaults.md +++ b/doc/administration/package_information/defaults.md @@ -47,7 +47,7 @@ by default: | Mattermost | No | Port | X | 8065 | | Mattermost | No | Port | X | 80 or 443 | | PgBouncer | No | Port | X | 6432 | -| Consul | No | Port | X | 8300, 8301(UDP), 8500, 8600[^Consul-notes] | +| Consul | No | Port | X | 8300, 8301(TCP and UDP), 8500, 8600[^Consul-notes] | | Patroni | No | Port | X | 8008 | | GitLab KAS | Yes | Port | X | 8150 | | Gitaly | Yes | Socket | Port (8075) | 8075 or 9999 (TLS) | diff --git a/doc/development/documentation/styleguide/index.md b/doc/development/documentation/styleguide/index.md index 305171da974..459c23bc0ea 100644 --- a/doc/development/documentation/styleguide/index.md +++ b/doc/development/documentation/styleguide/index.md @@ -133,7 +133,7 @@ the team is happy to review and improve upon your content. Review the [Documentation guidelines](index.md) before you begin your first documentation MR. Maintaining a knowledge base separate from the documentation would -be against the documentation-first methodology, because the content would overlap with +be against the documentation-first methodology because the content would overlap with the documentation. ## Writing for localization @@ -826,7 +826,7 @@ For example: You can expand on this text by using phrases like `For more information about this feature, see...` -Do not to use the following constructions: +Do not use the following constructions: - `Learn more about...` - `To read more...`. @@ -883,7 +883,7 @@ If you must use one of these links: - If the link is to a confidential issue, mention that the issue is visible only to GitLab team members, as in the first example. - If the link requires a specific role or permissions, mention that information, as in the second example. -- Put the link in backticks, so that it does not cause link checkers to fail. +- Put the link in backticks so that it does not cause link checkers to fail. Examples: @@ -914,7 +914,7 @@ document to ensure it links to the most recent version of the file. ## Navigation -When documenting how to navigate through the GitLab UI: +When documenting how to navigate the GitLab UI: - Always use location, then action. - From the **Visibility** dropdown list (location), select **Public** (action). diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md index f4829a1e239..7785819d1e0 100644 --- a/doc/development/testing_guide/best_practices.md +++ b/doc/development/testing_guide/best_practices.md @@ -1000,7 +1000,7 @@ describe 'specs which require time to be frozen to a specific date and/or time', end ``` -[Under the hood](https://gitlab.com/gitlab-org/gitlab/-/blob/master/spec/support/time_travel.rb), these helpers use the `around(:each)` hook and the block syntax of the +[Under the hood](https://gitlab.com/gitlab-org/gitlab/-/blob/master/gems/gitlab-rspec/lib/gitlab/rspec/configurations/time_travel.rb), these helpers use the `around(:each)` hook and the block syntax of the [`ActiveSupport::Testing::TimeHelpers`](https://api.rubyonrails.org/classes/ActiveSupport/Testing/TimeHelpers.html) methods: diff --git a/doc/integration/kerberos.md b/doc/integration/kerberos.md index f3ea7829ace..8215842652f 100644 --- a/doc/integration/kerberos.md +++ b/doc/integration/kerberos.md @@ -431,6 +431,11 @@ If you're experiencing this error, ensure there is connectivity between the client machine and the Kerberos server - this is a prerequisite! Traffic may be blocked by a firewall, or the DNS records may be incorrect. +#### `GitLab DNS record is a CNAME record` error + +Kerberos fails with this error when GitLab is referenced with a `CNAME` record. +To resolve this issue, ensure the DNS record for GitLab is an `A` record. + #### Mismatched forward and reverse DNS records for GitLab instance hostname Another failure mode occurs when the forward and reverse DNS records for the diff --git a/doc/topics/git/lfs/index.md b/doc/topics/git/lfs/index.md index 5f3458821dd..97e729aee55 100644 --- a/doc/topics/git/lfs/index.md +++ b/doc/topics/git/lfs/index.md @@ -33,7 +33,7 @@ Prerequisites: To do this: 1. On the left sidebar, at the top, select **Search GitLab** (**{search}**) to find your project. -1. Select **Settings > CI/CD**. +1. Select **Settings > General**. 1. Expand the **Visibility, project features, permissions** section. 1. Turn on the **Git Large File Storage (LFS)** toggle. 1. Select **Save changes**. diff --git a/doc/user/profile/achievements.md b/doc/user/profile/achievements.md index 31ce72e8423..5e48625bdd6 100644 --- a/doc/user/profile/achievements.md +++ b/doc/user/profile/achievements.md @@ -112,7 +112,7 @@ mutation achievementsCreate($file: Upload!) { To supply the avatar file, call the mutation using `curl`: ```shell -curl 'https://gitlab.com/api/graphql' \ +curl "https://gitlab.com/api/graphql" \ -H "Authorization: Bearer <your-pat-token>" \ -H "Content-Type: multipart/form-data" \ -F operations='{ "query": "mutation ($file: Upload!) { achievementsCreate(input: { namespaceId: \"gid://gitlab/Namespace/<namespace-id>\", name: \"<name>\", description: \"<description>\", avatar: $file }) { achievement { id name description avatarUrl } } }", "variables": { "file": null } }' \ diff --git a/gems/gitlab-rspec/.rubocop.yml b/gems/gitlab-rspec/.rubocop.yml index 8c670b439d3..a2bdc5735c4 100644 --- a/gems/gitlab-rspec/.rubocop.yml +++ b/gems/gitlab-rspec/.rubocop.yml @@ -1,2 +1,14 @@ inherit_from: - ../config/rubocop.yml + +RSpec/InstanceVariable: + Exclude: + - spec/**/*.rb + +Gitlab/ChangeTimezone: + Exclude: + - spec/gitlab/rspec/time_travel_spec.rb + +# FIXME +Gitlab/RSpec/AvoidSetup: + Enabled: false diff --git a/gems/gitlab-rspec/Gemfile.lock b/gems/gitlab-rspec/Gemfile.lock index 56c4b01e764..dcdb4dd009e 100644 --- a/gems/gitlab-rspec/Gemfile.lock +++ b/gems/gitlab-rspec/Gemfile.lock @@ -2,6 +2,7 @@ PATH remote: . specs: gitlab-rspec (0.1.0) + activesupport (>= 6.1, < 7.1) rspec (~> 3.0) GEM diff --git a/gems/gitlab-rspec/gitlab-rspec.gemspec b/gems/gitlab-rspec/gitlab-rspec.gemspec index 60bc84fbe36..c2c5b6c60b7 100644 --- a/gems/gitlab-rspec/gitlab-rspec.gemspec +++ b/gems/gitlab-rspec/gitlab-rspec.gemspec @@ -18,6 +18,7 @@ Gem::Specification.new do |spec| spec.files = Dir["lib/**/*.rb"] spec.require_paths = ["lib"] + spec.add_runtime_dependency "activesupport", ">= 6.1", "< 7.1" spec.add_runtime_dependency "rspec", "~> 3.0" spec.add_development_dependency "factory_bot_rails", "~> 6.2.0" diff --git a/gems/gitlab-rspec/lib/gitlab/rspec/all.rb b/gems/gitlab-rspec/lib/gitlab/rspec/all.rb index 47beb70d23e..091d2ba0287 100644 --- a/gems/gitlab-rspec/lib/gitlab/rspec/all.rb +++ b/gems/gitlab-rspec/lib/gitlab/rspec/all.rb @@ -2,3 +2,7 @@ require_relative "../rspec" require_relative "stub_env" + +require_relative "configurations/time_travel" + +Gitlab::Rspec::Configurations::TimeTravel.configure! diff --git a/gems/gitlab-rspec/lib/gitlab/rspec/configurations/time_travel.rb b/gems/gitlab-rspec/lib/gitlab/rspec/configurations/time_travel.rb new file mode 100644 index 00000000000..b30aa1cde0d --- /dev/null +++ b/gems/gitlab-rspec/lib/gitlab/rspec/configurations/time_travel.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require 'active_support/all' +require 'active_support/testing/time_helpers' + +module Gitlab + module Rspec + module Configurations + class TimeTravel + def self.configure! + RSpec.configure do |config| + config.include ActiveSupport::Testing::TimeHelpers + + config.around(:example, :freeze_time) do |example| + freeze_time { example.run } + end + + config.around(:example, :time_travel_to) do |example| + date_or_time = example.metadata[:time_travel_to] + + unless date_or_time.respond_to?(:to_time) && date_or_time.to_time.present? + raise 'The time_travel_to RSpec metadata must have a Date or Time value.' + end + + travel_to(date_or_time) { example.run } + end + end + end + end + end + end +end diff --git a/spec/support_specs/time_travel_spec.rb b/gems/gitlab-rspec/spec/gitlab/rspec/time_travel_spec.rb index 8fa51c0c1f0..79804a99f70 100644 --- a/spec/support_specs/time_travel_spec.rb +++ b/gems/gitlab-rspec/spec/gitlab/rspec/time_travel_spec.rb @@ -1,8 +1,15 @@ # frozen_string_literal: true -require 'spec_helper' - RSpec.describe 'time travel' do + before(:all) do + @original_time_zone = Time.zone + Time.zone = 'Eastern Time (US & Canada)' + end + + after(:all) do + Time.zone = @original_time_zone + end + describe ':freeze_time' do it 'freezes time around a spec example', :freeze_time do expect { sleep 0.1 }.not_to change { Time.now.to_f } diff --git a/lefthook.yml b/lefthook.yml index 880e5ff8136..7a3c7e5d174 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -61,11 +61,10 @@ pre-push: glob: 'doc/*.md' run: 'if [ $VALE_WARNINGS ]; then minWarnings=warning; else minWarnings=error; fi; if command -v vale > /dev/null 2>&1; then if ! vale --config .vale.ini --minAlertLevel $minWarnings {files}; then echo "ERROR: Fix any linting errors and make sure you are using the latest version of Vale."; exit 1; fi; else echo "ERROR: Vale not found. For more information, see https://docs.errata.ai/vale/install."; exit 1; fi' gettext: - skip: true # This is disabled by default. You can enable this check by adding skip: false in lefthook-local.yml https://github.com/evilmartians/lefthook/blob/master/docs/configuration.md#skip tags: backend frontend view haml files: git diff --name-only --diff-filter=d $(git merge-base origin/master HEAD)..HEAD | while read file;do git diff --unified=1 $(git merge-base origin/master HEAD)..HEAD $file | grep -Fqe '_(' && echo $file;done; true glob: '*.{haml,rb,js,vue}' - run: bin/rake gettext:updated_check + run: tooling/bin/gettext_extractor /dev/stdout --silent | diff - locale/gitlab.pot docs-metadata: # See https://docs.gitlab.com/ee/development/documentation/#metadata tags: documentation style files: git diff --name-only --diff-filter=d $(git merge-base origin/master HEAD)..HEAD @@ -136,3 +135,8 @@ auto-fix: files: git diff --name-only --diff-filter=d $(git merge-base origin/master HEAD)..HEAD glob: '*.{rb,rake}' run: REVEAL_RUBOCOP_TODO=0 bundle exec rubocop --parallel --autocorrect --force-exclusion {files} + gettext: + tags: backend frontend view haml + files: git diff --name-only --diff-filter=d $(git merge-base origin/master HEAD)..HEAD | while read file;do git diff --unified=1 $(git merge-base origin/master HEAD)..HEAD $file | grep -Fqe '_(' && echo $file;done; true + glob: '*.{haml,rb,js,vue}' + run: tooling/bin/gettext_extractor locale/gitlab.pot diff --git a/lib/gitlab/seeders/ci/runner/runner_fleet_pipeline_seeder.rb b/lib/gitlab/seeders/ci/runner/runner_fleet_pipeline_seeder.rb index c77db02061c..2cd9afc5bdc 100644 --- a/lib/gitlab/seeders/ci/runner/runner_fleet_pipeline_seeder.rb +++ b/lib/gitlab/seeders/ci/runner/runner_fleet_pipeline_seeder.rb @@ -169,7 +169,10 @@ module Gitlab } logger.info(message: 'Creating build', **build_attrs) - ::Ci::Build.new(importing: true, **build_attrs).tap(&:save!) + ::Ci::Build.transaction do + build = ::Ci::Build.new(importing: true, **build_attrs).tap(&:save!) + ::Ci::RunningBuild.upsert_shared_runner_build!(build) if build.running? && build.shared_runner_build? + end end def random_pipeline_status diff --git a/lib/gitlab/usage/metrics/instrumentations/batched_background_migrations_metric.rb b/lib/gitlab/usage/metrics/instrumentations/batched_background_migrations_metric.rb new file mode 100644 index 00000000000..f5529b96678 --- /dev/null +++ b/lib/gitlab/usage/metrics/instrumentations/batched_background_migrations_metric.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module Gitlab + module Usage + module Metrics + module Instrumentations + class BatchedBackgroundMigrationsMetric < DatabaseMetric + relation { Gitlab::Database::BackgroundMigration::BatchedMigration.with_status(:finished) } + + timestamp_column(:finished_at) + + operation :count + + def value + relation.map do |batched_migration| + { + job_class_name: batched_migration.job_class_name, + elapsed_time: batched_migration.finished_at.to_i - batched_migration.started_at.to_i + } + end + end + end + end + end + end +end diff --git a/lib/tasks/gitlab/shell.rake b/lib/tasks/gitlab/shell.rake index a5dcb23450f..8b305d68c68 100644 --- a/lib/tasks/gitlab/shell.rake +++ b/lib/tasks/gitlab/shell.rake @@ -44,7 +44,7 @@ namespace :gitlab do desc "GitLab | Shell | Setup gitlab-shell" task setup: :gitlab_environment do - setup + setup_gitlab_shell end desc "GitLab | Shell | Build missing projects" @@ -63,10 +63,13 @@ namespace :gitlab do end end - def setup - warn_user_is_not_gitlab + def setup_gitlab_shell + unless Gitlab::CurrentSettings.authorized_keys_enabled? + puts 'The "Write to authorized_keys" setting is disabled. Skipping rebuilding the authorized_keys file...' + return + end - ensure_write_to_authorized_keys_is_enabled + warn_user_is_not_gitlab unless ENV['force'] == 'yes' puts "This task will now rebuild the authorized_keys file." @@ -89,44 +92,4 @@ namespace :gitlab do puts "Quitting...".color(:red) exit 1 end - - def ensure_write_to_authorized_keys_is_enabled - return if Gitlab::CurrentSettings.authorized_keys_enabled? - - puts authorized_keys_is_disabled_warning - - unless ENV['force'] == 'yes' - puts 'Do you want to permanently enable the "Write to authorized_keys file" setting now?' - ask_to_continue - end - - puts 'Enabling the "Write to authorized_keys file" setting...' - Gitlab::CurrentSettings.update!(authorized_keys_enabled: true) - - puts 'Successfully enabled "Write to authorized_keys file"!' - puts '' - end - - def authorized_keys_is_disabled_warning - <<-MSG.strip_heredoc - WARNING - - The "Write to authorized_keys file" setting is disabled, which prevents - the file from being rebuilt! - - It should be enabled for most GitLab installations. Large installations - may wish to disable it as part of speeding up SSH operations. - - See https://docs.gitlab.com/ee/administration/operations/fast_ssh_key_lookup.html - - If you did not intentionally disable this option in Admin Area > Settings, - then you may have been affected by the 9.3.0 bug in which the new setting - was disabled by default. - - https://gitlab.com/gitlab-org/gitlab/issues/2738 - - It was reverted in 9.3.1 and fixed in 9.3.3, however, if Settings were - saved while the setting was unchecked, then it is still disabled. - MSG - end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index c678a93ee61..a2b7debdce3 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4863,6 +4863,9 @@ msgstr "" msgid "An error occurred while creating the %{issuableType}. Please try again." msgstr "" +msgid "An error occurred while creating the issue. Please try again." +msgstr "" + msgid "An error occurred while decoding the file." msgstr "" @@ -5138,9 +5141,6 @@ msgstr "" msgid "An error occurred while updating labels." msgstr "" -msgid "An error occurred while updating the comment" -msgstr "" - msgid "An error occurred while updating the configuration." msgstr "" diff --git a/qa/qa/page/base.rb b/qa/qa/page/base.rb index 6c01fa9f344..6aadf7e1fd0 100644 --- a/qa/qa/page/base.rb +++ b/qa/qa/page/base.rb @@ -4,6 +4,11 @@ require 'capybara/dsl' module QA module Page + # Page base class + # + # @!method self.perform + # Perform action on the page + # @yieldparam [self] instance of page object class Base # Generic matcher for common css selectors like: # - class name '.someclass' diff --git a/qa/qa/resource/issuable.rb b/qa/qa/resource/issuable.rb index 5aee27b46d4..508f412711a 100644 --- a/qa/qa/resource/issuable.rb +++ b/qa/qa/resource/issuable.rb @@ -5,7 +5,7 @@ module QA class Issuable < Base using Rainbow - # Commentes (notes) path + # Comments (notes) path # # @return [String] def api_comments_path diff --git a/qa/qa/resource/merge_request.rb b/qa/qa/resource/merge_request.rb index 0d1a0bb2cd4..93e8c4f3be6 100644 --- a/qa/qa/resource/merge_request.rb +++ b/qa/qa/resource/merge_request.rb @@ -45,7 +45,7 @@ module QA resource.project = project resource.api_client = api_client resource.commit_message = 'This is a test commit' - resource.add_files([{ 'file_path': "file-#{SecureRandom.hex(8)}.txt", 'content': 'MR init' }]) + resource.add_files([{ file_path: "file-#{SecureRandom.hex(8)}.txt", content: 'MR init' }]) resource.branch = target_branch resource.start_branch = project.default_branch if target_branch != project.default_branch @@ -60,7 +60,7 @@ module QA resource.branch = source_branch resource.start_branch = target_branch - files = [{ 'file_path': file_name, 'content': file_content }] + files = [{ file_path: file_name, content: file_content }] update_existing_file ? resource.update_files(files) : resource.add_files(files) end end @@ -139,7 +139,8 @@ module QA source_branch: source_branch, target_branch: target_branch, title: title, - reviewer_ids: reviewer_ids + reviewer_ids: reviewer_ids, + labels: labels.join(",") } end diff --git a/spec/controllers/projects/merge_requests/drafts_controller_spec.rb b/spec/controllers/projects/merge_requests/drafts_controller_spec.rb index fea176dfa5d..68fbeb00b67 100644 --- a/spec/controllers/projects/merge_requests/drafts_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/drafts_controller_spec.rb @@ -239,6 +239,30 @@ RSpec.describe Projects::MergeRequests::DraftsController, feature_category: :cod expect(draft.note).to eq('This is an updated unpublished comment') expect(json_response['note_html']).not_to be_empty end + + context 'when the draft note is invalid' do + before do + errors = ActiveModel::Errors.new(draft) + errors.add(:base, 'Error 1') + errors.add(:base, 'Error 2') + + allow_next_found_instance_of(DraftNote) do |instance| + allow(instance).to receive(:update).and_return(false) + allow(instance).to receive(:errors).and_return(errors) + end + end + + it 'does not update the draft' do + expect { update_draft_note }.not_to change { draft.reload.note } + end + + it 'returns status 422', :aggregate_failures do + update_draft_note + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(response.body).to eq('{"errors":"Error 1 and Error 2"}') + end + end end describe 'POST #publish' do diff --git a/spec/frontend/batch_comments/components/draft_note_spec.js b/spec/frontend/batch_comments/components/draft_note_spec.js index 28149c630c9..b6042b4aa81 100644 --- a/spec/frontend/batch_comments/components/draft_note_spec.js +++ b/spec/frontend/batch_comments/components/draft_note_spec.js @@ -69,6 +69,9 @@ describe('Batch comments draft note component', () => { note: draft, noteText: 'a', resolveDiscussion: false, + callback: jest.fn(), + parentElement: wrapper.vm.$el, + errorCallback: jest.fn(), }; findNoteableNote().vm.$emit('handleUpdateNote', formData); diff --git a/spec/frontend/batch_comments/stores/modules/batch_comments/actions_spec.js b/spec/frontend/batch_comments/stores/modules/batch_comments/actions_spec.js index 7b769ea0a48..824b2a296c6 100644 --- a/spec/frontend/batch_comments/stores/modules/batch_comments/actions_spec.js +++ b/spec/frontend/batch_comments/stores/modules/batch_comments/actions_spec.js @@ -1,10 +1,15 @@ import MockAdapter from 'axios-mock-adapter'; import { TEST_HOST } from 'helpers/test_constants'; import testAction from 'helpers/vuex_action_helper'; +import { sprintf } from '~/locale'; +import { createAlert } from '~/alert'; import service from '~/batch_comments/services/drafts_service'; import * as actions from '~/batch_comments/stores/modules/batch_comments/actions'; import axios from '~/lib/utils/axios_utils'; import { HTTP_STATUS_INTERNAL_SERVER_ERROR, HTTP_STATUS_OK } from '~/lib/utils/http_status'; +import { UPDATE_COMMENT_FORM } from '~/notes/i18n'; + +jest.mock('~/alert'); describe('Batch comments store actions', () => { let res = {}; @@ -263,6 +268,28 @@ describe('Batch comments store actions', () => { expect(service.update.mock.calls[0][1].position).toBe(expectation); }); }); + + describe('when updating a draft returns an error', () => { + const errorCallback = jest.fn(); + const flashContainer = null; + const error = 'server error'; + + beforeEach(async () => { + service.update.mockRejectedValue({ response: { data: { errors: error } } }); + await actions.updateDraft(context, { ...params, flashContainer, errorCallback }); + }); + + it('renders an error message', () => { + expect(createAlert).toHaveBeenCalledWith({ + message: sprintf(UPDATE_COMMENT_FORM.error, { reason: error }), + parent: flashContainer, + }); + }); + + it('calls errorCallback', () => { + expect(errorCallback).toHaveBeenCalledTimes(1); + }); + }); }); describe('expandAllDiscussions', () => { diff --git a/spec/frontend/ci/ci_variable_list/components/ci_environments_dropdown_spec.js b/spec/frontend/ci/ci_variable_list/components/ci_environments_dropdown_spec.js index 415a1d0bfa3..64227872af3 100644 --- a/spec/frontend/ci/ci_variable_list/components/ci_environments_dropdown_spec.js +++ b/spec/frontend/ci/ci_variable_list/components/ci_environments_dropdown_spec.js @@ -16,6 +16,7 @@ describe('Ci environments dropdown', () => { const defaultProps = { areEnvironmentsLoading: false, environments: envs, + hasEnvScopeQuery: false, selectedEnvironmentScope: '', }; @@ -28,17 +29,12 @@ describe('Ci environments dropdown', () => { const findDropdownDivider = () => wrapper.findComponent(GlDropdownDivider); const findMaxEnvNote = () => wrapper.findByTestId('max-envs-notice'); - const createComponent = ({ props = {}, searchTerm = '', enableFeatureFlag = false } = {}) => { + const createComponent = ({ props = {}, searchTerm = '' } = {}) => { wrapper = mountExtended(CiEnvironmentsDropdown, { propsData: { ...defaultProps, ...props, }, - provide: { - glFeatures: { - ciLimitEnvironmentScope: enableFeatureFlag, - }, - }, }); findListbox().vm.$emit('search', searchTerm); @@ -62,14 +58,14 @@ describe('Ci environments dropdown', () => { describe('Search term is empty', () => { describe.each` - featureFlag | flagStatus | defaultEnvStatus | firstItemValue | envIndices - ${true} | ${'enabled'} | ${'prepends'} | ${'*'} | ${[1, 2, 3]} - ${false} | ${'disabled'} | ${'does not prepend'} | ${envs[0]} | ${[0, 1, 2]} + hasEnvScopeQuery | status | defaultEnvStatus | firstItemValue | envIndices + ${true} | ${'exists'} | ${'prepends'} | ${'*'} | ${[1, 2, 3]} + ${false} | ${'does not exist'} | ${'does not prepend'} | ${envs[0]} | ${[0, 1, 2]} `( - 'when ciLimitEnvironmentScope feature flag is $flagStatus', - ({ featureFlag, defaultEnvStatus, firstItemValue, envIndices }) => { + 'when query for fetching environment scope $status', + ({ defaultEnvStatus, firstItemValue, hasEnvScopeQuery, envIndices }) => { beforeEach(() => { - createComponent({ props: { environments: envs }, enableFeatureFlag: featureFlag }); + createComponent({ props: { environments: envs, hasEnvScopeQuery } }); }); it(`${defaultEnvStatus} * in listbox`, () => { @@ -102,7 +98,7 @@ describe('Ci environments dropdown', () => { }); }); - describe('When ciLimitEnvironmentScope feature flag is disabled', () => { + describe('when environments are not fetched via graphql', () => { const currentEnv = envs[2]; beforeEach(() => { @@ -129,11 +125,11 @@ describe('Ci environments dropdown', () => { }); }); - describe('When ciLimitEnvironmentScope feature flag is enabled', () => { + describe('when fetching environments via graphql', () => { const currentEnv = envs[2]; beforeEach(() => { - createComponent({ enableFeatureFlag: true }); + createComponent({ props: { hasEnvScopeQuery: true } }); }); it('renders dropdown divider', () => { @@ -147,7 +143,7 @@ describe('Ci environments dropdown', () => { }); it('renders dropdown loading icon while fetch query is loading', () => { - createComponent({ enableFeatureFlag: true, props: { areEnvironmentsLoading: true } }); + createComponent({ props: { areEnvironmentsLoading: true, hasEnvScopeQuery: true } }); expect(findListbox().props('loading')).toBe(true); expect(findListbox().props('searching')).toBe(false); @@ -155,7 +151,7 @@ describe('Ci environments dropdown', () => { }); it('renders search loading icon while search query is loading and dropdown is open', async () => { - createComponent({ enableFeatureFlag: true, props: { areEnvironmentsLoading: true } }); + createComponent({ props: { areEnvironmentsLoading: true, hasEnvScopeQuery: true } }); await findListbox().vm.$emit('shown'); expect(findListbox().props('loading')).toBe(false); diff --git a/spec/frontend/ci/ci_variable_list/components/ci_variable_modal_spec.js b/spec/frontend/ci/ci_variable_list/components/ci_variable_modal_spec.js index e9484cfce57..d843646df16 100644 --- a/spec/frontend/ci/ci_variable_list/components/ci_variable_modal_spec.js +++ b/spec/frontend/ci/ci_variable_list/components/ci_variable_modal_spec.js @@ -48,6 +48,7 @@ describe('Ci variable modal', () => { areScopedVariablesAvailable: true, environments: [], hideEnvironmentScope: false, + hasEnvScopeQuery: false, mode: ADD_VARIABLE_ACTION, selectedVariable: {}, variables: [], @@ -349,14 +350,14 @@ describe('Ci variable modal', () => { expect(link.attributes('href')).toBe(defaultProvide.environmentScopeLink); }); - describe('when feature flag is enabled', () => { + describe('when query for envioronment scope exists', () => { beforeEach(() => { createComponent({ props: { environments: mockEnvs, + hasEnvScopeQuery: true, variables: mockVariablesWithUniqueScopes(projectString), }, - provide: { glFeatures: { ciLimitEnvironmentScope: true } }, }); }); diff --git a/spec/frontend/ci/ci_variable_list/components/ci_variable_settings_spec.js b/spec/frontend/ci/ci_variable_list/components/ci_variable_settings_spec.js index 12ca9a78369..d72cfc5fc14 100644 --- a/spec/frontend/ci/ci_variable_list/components/ci_variable_settings_spec.js +++ b/spec/frontend/ci/ci_variable_list/components/ci_variable_settings_spec.js @@ -21,6 +21,7 @@ describe('Ci variable table', () => { environments: mapEnvironmentNames(mockEnvs), hideEnvironmentScope: false, isLoading: false, + hasEnvScopeQuery: false, maxVariableLimit: 5, pageInfo: { after: '' }, variables: mockVariablesWithScopes(projectString), @@ -60,6 +61,7 @@ describe('Ci variable table', () => { areEnvironmentsLoading: defaultProps.areEnvironmentsLoading, areScopedVariablesAvailable: defaultProps.areScopedVariablesAvailable, environments: defaultProps.environments, + hasEnvScopeQuery: defaultProps.hasEnvScopeQuery, hideEnvironmentScope: defaultProps.hideEnvironmentScope, variables: defaultProps.variables, mode: ADD_VARIABLE_ACTION, diff --git a/spec/frontend/ci/ci_variable_list/components/ci_variable_shared_spec.js b/spec/frontend/ci/ci_variable_list/components/ci_variable_shared_spec.js index 2f4d48aacfc..6fa1915f3c1 100644 --- a/spec/frontend/ci/ci_variable_list/components/ci_variable_shared_spec.js +++ b/spec/frontend/ci/ci_variable_list/components/ci_variable_shared_spec.js @@ -52,6 +52,7 @@ const mockProvide = { const defaultProps = { areScopedVariablesAvailable: true, + hasEnvScopeQuery: false, pageInfo: {}, hideEnvironmentScope: false, refetchAfterMutation: false, @@ -219,16 +220,12 @@ describe('Ci Variable Shared Component', () => { expect(mockEnvironments).toHaveBeenCalled(); }); - describe('when Limit Environment Scope FF is enabled', () => { + // applies only to project-level CI variables + describe('when environment scope is limited', () => { beforeEach(async () => { await createComponentWithApollo({ props: { ...createProjectProps() }, - provide: { - glFeatures: { - ciLimitEnvironmentScope: true, - ciVariablesPages: isVariablePagesEnabled, - }, - }, + provide: pagesFeatureFlagProvide, }); }); @@ -258,27 +255,6 @@ describe('Ci Variable Shared Component', () => { expect(findLoadingIcon().exists()).toBe(false); }); }); - - describe('when Limit Environment Scope FF is disabled', () => { - beforeEach(async () => { - await createComponentWithApollo({ - props: { ...createProjectProps() }, - provide: pagesFeatureFlagProvide, - }); - }); - - it('initial query is called with the correct variables', () => { - expect(mockEnvironments).toHaveBeenCalledWith({ fullPath: '/namespace/project/' }); - }); - - it(`does not refetch environments when search term is present`, async () => { - expect(mockEnvironments).toHaveBeenCalledTimes(1); - - await findCiSettings().vm.$emit('search-environment-scope', 'staging'); - - expect(mockEnvironments).toHaveBeenCalledTimes(1); - }); - }); }); describe("when there isn't an environment key in queryData", () => { @@ -538,6 +514,7 @@ describe('Ci Variable Shared Component', () => { areEnvironmentsLoading: false, areScopedVariablesAvailable: wrapper.props().areScopedVariablesAvailable, hideEnvironmentScope: defaultProps.hideEnvironmentScope, + hasEnvScopeQuery: props.hasEnvScopeQuery, pageInfo: defaultProps.pageInfo, isLoading: false, maxVariableLimit, diff --git a/spec/frontend/ci/ci_variable_list/mocks.js b/spec/frontend/ci/ci_variable_list/mocks.js index 9c9c99ad5ea..41dfc0ebfda 100644 --- a/spec/frontend/ci/ci_variable_list/mocks.js +++ b/spec/frontend/ci/ci_variable_list/mocks.js @@ -189,6 +189,7 @@ export const createProjectProps = () => { componentName: 'ProjectVariable', entity: 'project', fullPath: '/namespace/project/', + hasEnvScopeQuery: true, id: 'gid://gitlab/Project/20', mutationData: { [ADD_MUTATION_ACTION]: addProjectVariable, @@ -213,6 +214,7 @@ export const createGroupProps = () => { componentName: 'GroupVariable', entity: 'group', fullPath: '/my-group', + hasEnvScopeQuery: false, id: 'gid://gitlab/Group/20', mutationData: { [ADD_MUTATION_ACTION]: addGroupVariable, @@ -231,6 +233,7 @@ export const createGroupProps = () => { export const createInstanceProps = () => { return { componentName: 'InstanceVariable', + hasEnvScopeQuery: false, entity: '', mutationData: { [ADD_MUTATION_ACTION]: addAdminVariable, diff --git a/spec/frontend/issuable/related_issues/components/related_issues_block_spec.js b/spec/frontend/issuable/related_issues/components/related_issues_block_spec.js index e97c0312181..fb154e01c7a 100644 --- a/spec/frontend/issuable/related_issues/components/related_issues_block_spec.js +++ b/spec/frontend/issuable/related_issues/components/related_issues_block_spec.js @@ -76,7 +76,7 @@ describe('RelatedIssuesBlock', () => { helpPath: '/help/user/project/issues/related_issues', }); - expect(wrapper.find('.card-title').text()).toContain(titleText); + expect(wrapper.findByTestId('card-title').text()).toContain(titleText); expect(findIssueCountBadgeAddButton().attributes('aria-label')).toBe(addButtonText); }, ); @@ -99,7 +99,7 @@ describe('RelatedIssuesBlock', () => { slots: { 'header-text': headerText }, }); - expect(wrapper.find('.card-title').html()).toContain(headerText); + expect(wrapper.findByTestId('card-title').html()).toContain(headerText); }); }); diff --git a/spec/frontend/issuable/related_issues/components/related_issues_root_spec.js b/spec/frontend/issuable/related_issues/components/related_issues_root_spec.js index b119c836411..b2477dae52a 100644 --- a/spec/frontend/issuable/related_issues/components/related_issues_root_spec.js +++ b/spec/frontend/issuable/related_issues/components/related_issues_root_spec.js @@ -1,6 +1,5 @@ import { mount } from '@vue/test-utils'; import MockAdapter from 'axios-mock-adapter'; -import { nextTick } from 'vue'; import waitForPromises from 'helpers/wait_for_promises'; import { defaultProps, @@ -17,7 +16,6 @@ import { import { linkedIssueTypesMap } from '~/related_issues/constants'; import RelatedIssuesBlock from '~/related_issues/components/related_issues_block.vue'; import RelatedIssuesRoot from '~/related_issues/components/related_issues_root.vue'; -import relatedIssuesService from '~/related_issues/services/related_issues_service'; jest.mock('~/alert'); @@ -58,11 +56,8 @@ describe('RelatedIssuesRoot', () => { describe('when "relatedIssueRemoveRequest" event is emitted', () => { describe('when emitted value is a numerical issue', () => { beforeEach(async () => { - jest - .spyOn(relatedIssuesService.prototype, 'fetchRelatedIssues') - .mockReturnValue(Promise.reject()); + mock.onGet(defaultProps.endpoint).reply(HTTP_STATUS_OK, [issuable1]); await createComponent(); - wrapper.vm.store.setRelatedIssues([issuable1]); }); it('removes related issue on API success', async () => { @@ -91,8 +86,7 @@ describe('RelatedIssuesRoot', () => { const workItem = `gid://gitlab/WorkItem/${issuable1.id}`; createComponent({ data: { state: { relatedIssues: [issuable1] } } }); - findRelatedIssuesBlock().vm.$emit('relatedIssueRemoveRequest', workItem); - await nextTick(); + await findRelatedIssuesBlock().vm.$emit('relatedIssueRemoveRequest', workItem); expect(findRelatedIssuesBlock().props('relatedIssues')).toEqual([]); }); @@ -103,8 +97,7 @@ describe('RelatedIssuesRoot', () => { it('toggles related issues form to visible from hidden', async () => { createComponent(); - findRelatedIssuesBlock().vm.$emit('toggleAddRelatedIssuesForm'); - await nextTick(); + await findRelatedIssuesBlock().vm.$emit('toggleAddRelatedIssuesForm'); expect(findRelatedIssuesBlock().props('isFormVisible')).toBe(true); }); @@ -112,24 +105,25 @@ describe('RelatedIssuesRoot', () => { it('toggles related issues form to hidden from visible', async () => { createComponent({ data: { isFormVisible: true } }); - findRelatedIssuesBlock().vm.$emit('toggleAddRelatedIssuesForm'); - await nextTick(); + await findRelatedIssuesBlock().vm.$emit('toggleAddRelatedIssuesForm'); expect(findRelatedIssuesBlock().props('isFormVisible')).toBe(false); }); }); describe('when "pendingIssuableRemoveRequest" event is emitted', () => { - beforeEach(() => { + beforeEach(async () => { createComponent(); - wrapper.vm.store.setPendingReferences([issuable1.reference]); + await findRelatedIssuesBlock().vm.$emit('addIssuableFormInput', { + untouchedRawReferences: [issuable1.reference], + touchedReference: '', + }); }); it('removes pending related issue', async () => { expect(findRelatedIssuesBlock().props('pendingReferences')).toHaveLength(1); - findRelatedIssuesBlock().vm.$emit('pendingIssuableRemoveRequest', 0); - await nextTick(); + await findRelatedIssuesBlock().vm.$emit('pendingIssuableRemoveRequest', 0); expect(findRelatedIssuesBlock().props('pendingReferences')).toHaveLength(0); }); @@ -137,33 +131,24 @@ describe('RelatedIssuesRoot', () => { describe('when "addIssuableFormSubmit" event is emitted', () => { beforeEach(async () => { - jest - .spyOn(relatedIssuesService.prototype, 'fetchRelatedIssues') - .mockReturnValue(Promise.reject()); await createComponent(); - jest.spyOn(wrapper.vm, 'processAllReferences'); - jest.spyOn(wrapper.vm.service, 'addRelatedIssues'); createAlert.mockClear(); }); - it('processes references before submitting', () => { + it('processes references before submitting', async () => { const input = '#123'; const linkedIssueType = linkedIssueTypesMap.RELATES_TO; const emitObj = { pendingReferences: input, linkedIssueType, }; - - findRelatedIssuesBlock().vm.$emit('addIssuableFormSubmit', emitObj); - - expect(wrapper.vm.processAllReferences).toHaveBeenCalledWith(input); - expect(wrapper.vm.service.addRelatedIssues).toHaveBeenCalledWith([input], linkedIssueType); + await findRelatedIssuesBlock().vm.$emit('addIssuableFormSubmit', emitObj); + expect(findRelatedIssuesBlock().props('pendingReferences')).toEqual([input]); }); - it('submits zero pending issues as related issue', () => { - wrapper.vm.store.setPendingReferences([]); - - findRelatedIssuesBlock().vm.$emit('addIssuableFormSubmit', {}); + it('submits zero pending issues as related issue', async () => { + await findRelatedIssuesBlock().vm.$emit('addIssuableFormSubmit', {}); + await waitForPromises(); expect(findRelatedIssuesBlock().props('pendingReferences')).toHaveLength(0); expect(findRelatedIssuesBlock().props('relatedIssues')).toHaveLength(0); @@ -177,9 +162,11 @@ describe('RelatedIssuesRoot', () => { status: 'success', }, }); - wrapper.vm.store.setPendingReferences([issuable1.reference]); - - findRelatedIssuesBlock().vm.$emit('addIssuableFormSubmit', {}); + await findRelatedIssuesBlock().vm.$emit('addIssuableFormInput', { + untouchedRawReferences: [issuable1], + touchedReference: '', + }); + await findRelatedIssuesBlock().vm.$emit('addIssuableFormSubmit', {}); await waitForPromises(); expect(findRelatedIssuesBlock().props('pendingReferences')).toHaveLength(0); @@ -196,9 +183,11 @@ describe('RelatedIssuesRoot', () => { status: 'success', }, }); - wrapper.vm.store.setPendingReferences([issuable1.reference, issuable2.reference]); - - findRelatedIssuesBlock().vm.$emit('addIssuableFormSubmit', {}); + await findRelatedIssuesBlock().vm.$emit('addIssuableFormInput', { + untouchedRawReferences: [issuable1.reference, issuable2.reference], + touchedReference: '', + }); + await findRelatedIssuesBlock().vm.$emit('addIssuableFormSubmit', {}); await waitForPromises(); expect(findRelatedIssuesBlock().props('pendingReferences')).toHaveLength(0); @@ -212,12 +201,15 @@ describe('RelatedIssuesRoot', () => { const input = '#123'; const message = 'error'; mock.onPost(defaultProps.endpoint).reply(HTTP_STATUS_CONFLICT, { message }); - wrapper.vm.store.setPendingReferences([issuable1.reference, issuable2.reference]); + await findRelatedIssuesBlock().vm.$emit('addIssuableFormInput', { + untouchedRawReferences: [issuable1.reference, issuable2.reference], + touchedReference: '', + }); expect(findRelatedIssuesBlock().props('hasError')).toBe(false); expect(findRelatedIssuesBlock().props('itemAddFailureMessage')).toBe(null); - findRelatedIssuesBlock().vm.$emit('addIssuableFormSubmit', input); + await findRelatedIssuesBlock().vm.$emit('addIssuableFormSubmit', input); await waitForPromises(); expect(findRelatedIssuesBlock().props('hasError')).toBe(true); @@ -229,8 +221,7 @@ describe('RelatedIssuesRoot', () => { beforeEach(() => createComponent({ data: { isFormVisible: true, inputValue: 'foo' } })); it('hides form and resets input', async () => { - findRelatedIssuesBlock().vm.$emit('addIssuableFormCancel'); - await nextTick(); + await findRelatedIssuesBlock().vm.$emit('addIssuableFormCancel'); expect(findRelatedIssuesBlock().props('isFormVisible')).toBe(false); expect(findRelatedIssuesBlock().props('inputValue')).toBe(''); @@ -243,11 +234,10 @@ describe('RelatedIssuesRoot', () => { const input = '#123 '; createComponent(); - findRelatedIssuesBlock().vm.$emit('addIssuableFormInput', { + await findRelatedIssuesBlock().vm.$emit('addIssuableFormInput', { untouchedRawReferences: [input.trim()], touchedReference: input, }); - await nextTick(); expect(findRelatedIssuesBlock().props('pendingReferences')).toEqual([input.trim()]); }); @@ -256,11 +246,10 @@ describe('RelatedIssuesRoot', () => { const input = 'asdf/qwer#444 '; createComponent(); - findRelatedIssuesBlock().vm.$emit('addIssuableFormInput', { + await findRelatedIssuesBlock().vm.$emit('addIssuableFormInput', { untouchedRawReferences: [input.trim()], touchedReference: input, }); - await nextTick(); expect(findRelatedIssuesBlock().props('pendingReferences')).toEqual([input.trim()]); }); @@ -270,11 +259,10 @@ describe('RelatedIssuesRoot', () => { const input = `${link} `; createComponent(); - findRelatedIssuesBlock().vm.$emit('addIssuableFormInput', { + await findRelatedIssuesBlock().vm.$emit('addIssuableFormInput', { untouchedRawReferences: [input.trim()], touchedReference: input, }); - await nextTick(); expect(findRelatedIssuesBlock().props('pendingReferences')).toEqual([link]); }); @@ -283,11 +271,10 @@ describe('RelatedIssuesRoot', () => { const input = 'asdf/qwer#444 #12 '; createComponent(); - findRelatedIssuesBlock().vm.$emit('addIssuableFormInput', { + await findRelatedIssuesBlock().vm.$emit('addIssuableFormInput', { untouchedRawReferences: input.trim().split(/\s/), touchedReference: '2', }); - await nextTick(); expect(findRelatedIssuesBlock().props('pendingReferences')).toEqual([ 'asdf/qwer#444', @@ -299,11 +286,10 @@ describe('RelatedIssuesRoot', () => { const input = 'something random '; createComponent(); - findRelatedIssuesBlock().vm.$emit('addIssuableFormInput', { + await findRelatedIssuesBlock().vm.$emit('addIssuableFormInput', { untouchedRawReferences: input.trim().split(/\s/), touchedReference: '2', }); - await nextTick(); expect(findRelatedIssuesBlock().props('pendingReferences')).toEqual([ 'something', @@ -317,11 +303,10 @@ describe('RelatedIssuesRoot', () => { const input = '23'; createComponent({ props: { pathIdSeparator } }); - findRelatedIssuesBlock().vm.$emit('addIssuableFormInput', { + await findRelatedIssuesBlock().vm.$emit('addIssuableFormInput', { untouchedRawReferences: input.trim().split(/\s/), touchedReference: input, }); - await nextTick(); expect(findRelatedIssuesBlock().props('inputValue')).toBe(`${pathIdSeparator}${input}`); }, @@ -331,15 +316,13 @@ describe('RelatedIssuesRoot', () => { describe('when "addIssuableFormBlur" event is emitted', () => { beforeEach(() => { createComponent(); - jest.spyOn(wrapper.vm, 'processAllReferences').mockImplementation(() => {}); }); - it('adds any references to pending when blurring', () => { + it('adds any references to pending when blurring', async () => { const input = '#123'; - - findRelatedIssuesBlock().vm.$emit('addIssuableFormBlur', input); - - expect(wrapper.vm.processAllReferences).toHaveBeenCalledWith(input); + expect(findRelatedIssuesBlock().props('pendingReferences')).toEqual([]); + await findRelatedIssuesBlock().vm.$emit('addIssuableFormBlur', input); + expect(findRelatedIssuesBlock().props('pendingReferences')).toEqual([input]); }); }); }); diff --git a/spec/frontend/notes/components/noteable_note_spec.js b/spec/frontend/notes/components/noteable_note_spec.js index 0a01e76c2bd..059972df56b 100644 --- a/spec/frontend/notes/components/noteable_note_spec.js +++ b/spec/frontend/notes/components/noteable_note_spec.js @@ -385,10 +385,24 @@ describe('issue_note', () => { afterEach(() => updateNote.mockReset()); - it('responds to handleFormUpdate', () => { + it('emits handleUpdateNote', () => { + const updatedNote = { ...note, note_html: `<p dir="auto">${params.noteText}</p>\n` }; + findNoteBody().vm.$emit('handleFormUpdate', params); expect(wrapper.emitted('handleUpdateNote')).toHaveLength(1); + + expect(wrapper.emitted('handleUpdateNote')[0]).toEqual([ + { + note: updatedNote, + noteText: params.noteText, + resolveDiscussion: params.resolveDiscussion, + position: {}, + flashContainer: wrapper.vm.$el, + callback: expect.any(Function), + errorCallback: expect.any(Function), + }, + ]); }); it('updates note content', async () => { diff --git a/spec/frontend/notes/utils_spec.js b/spec/frontend/notes/utils_spec.js index 0882e0a5759..3607c3c546c 100644 --- a/spec/frontend/notes/utils_spec.js +++ b/spec/frontend/notes/utils_spec.js @@ -1,12 +1,12 @@ import { sprintf } from '~/locale'; -import { getErrorMessages } from '~/notes/utils'; +import { createNoteErrorMessages, updateNoteErrorMessage } from '~/notes/utils'; import { HTTP_STATUS_UNPROCESSABLE_ENTITY, HTTP_STATUS_BAD_REQUEST } from '~/lib/utils/http_status'; -import { COMMENT_FORM } from '~/notes/i18n'; +import { COMMENT_FORM, UPDATE_COMMENT_FORM } from '~/notes/i18n'; -describe('getErrorMessages', () => { +describe('createNoteErrorMessages', () => { describe('when http status is not HTTP_STATUS_UNPROCESSABLE_ENTITY', () => { it('returns generic error', () => { - const errorMessages = getErrorMessages( + const errorMessages = createNoteErrorMessages( { errors: ['unknown error'] }, HTTP_STATUS_BAD_REQUEST, ); @@ -17,7 +17,7 @@ describe('getErrorMessages', () => { describe('when http status is HTTP_STATUS_UNPROCESSABLE_ENTITY', () => { it('returns all errors', () => { - const errorMessages = getErrorMessages( + const errorMessages = createNoteErrorMessages( { errors: 'error 1 and error 2' }, HTTP_STATUS_UNPROCESSABLE_ENTITY, ); @@ -29,7 +29,7 @@ describe('getErrorMessages', () => { describe('when response contains commands_only errors', () => { it('only returns commands_only errors', () => { - const errorMessages = getErrorMessages( + const errorMessages = createNoteErrorMessages( { errors: { commands_only: ['commands_only error 1', 'commands_only error 2'], @@ -44,3 +44,22 @@ describe('getErrorMessages', () => { }); }); }); + +describe('updateNoteErrorMessage', () => { + describe('with server error', () => { + it('returns error message with server error', () => { + const error = 'error 1 and error 2'; + const errorMessage = updateNoteErrorMessage({ response: { data: { errors: error } } }); + + expect(errorMessage).toEqual(sprintf(UPDATE_COMMENT_FORM.error, { reason: error })); + }); + }); + + describe('without server error', () => { + it('returns generic error message', () => { + const errorMessage = updateNoteErrorMessage(null); + + expect(errorMessage).toEqual(UPDATE_COMMENT_FORM.defaultError); + }); + }); +}); diff --git a/spec/frontend/vue_shared/components/source_viewer/source_viewer_new_spec.js b/spec/frontend/vue_shared/components/source_viewer/source_viewer_new_spec.js index 715234e56fd..6b711b6b6b2 100644 --- a/spec/frontend/vue_shared/components/source_viewer/source_viewer_new_spec.js +++ b/spec/frontend/vue_shared/components/source_viewer/source_viewer_new_spec.js @@ -3,9 +3,11 @@ import SourceViewer from '~/vue_shared/components/source_viewer/source_viewer_ne import Chunk from '~/vue_shared/components/source_viewer/components/chunk_new.vue'; import { EVENT_ACTION, EVENT_LABEL_VIEWER } from '~/vue_shared/components/source_viewer/constants'; import Tracking from '~/tracking'; +import LineHighlighter from '~/blob/line_highlighter'; import addBlobLinksTracking from '~/blob/blob_links_tracking'; import { BLOB_DATA_MOCK, CHUNK_1, CHUNK_2, LANGUAGE_MOCK } from './mock_data'; +jest.mock('~/blob/line_highlighter'); jest.mock('~/blob/blob_links_tracking'); describe('Source Viewer component', () => { @@ -25,6 +27,10 @@ describe('Source Viewer component', () => { return createComponent(); }); + it('instantiates the lineHighlighter class', () => { + expect(LineHighlighter).toHaveBeenCalled(); + }); + describe('event tracking', () => { it('fires a tracking event when the component is created', () => { const eventData = { label: EVENT_LABEL_VIEWER, property: LANGUAGE_MOCK }; diff --git a/spec/lib/gitlab/usage/metrics/instrumentations/batched_background_migrations_metric_spec.rb b/spec/lib/gitlab/usage/metrics/instrumentations/batched_background_migrations_metric_spec.rb new file mode 100644 index 00000000000..f7b15539400 --- /dev/null +++ b/spec/lib/gitlab/usage/metrics/instrumentations/batched_background_migrations_metric_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Usage::Metrics::Instrumentations::BatchedBackgroundMigrationsMetric, feature_category: :database do + let(:expected_value) do + [ + { + job_class_name: 'test', + elapsed_time: 2.days.to_i + } + ] + end + + let_it_be(:active_migration) { create(:batched_background_migration, :active) } + let_it_be(:finished_migration) do + create(:batched_background_migration, :finished, job_class_name: 'test', started_at: 5.days.ago, + finished_at: 3.days.ago) + end + + let_it_be(:old_finished_migration) do + create(:batched_background_migration, :finished, job_class_name: 'old_test', started_at: 100.days.ago, + finished_at: 99.days.ago) + end + + it_behaves_like 'a correct instrumented metric value', { time_frame: '7d' } +end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index b0ff070e4a6..50e2ded695c 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -316,8 +316,7 @@ RSpec.describe Ci::Runner, type: :model, feature_category: :runner do context 'when use_traversal_ids* are disabled' do before do stub_feature_flags( - use_traversal_ids: false, - use_traversal_ids_for_ancestors: false + use_traversal_ids: false ) end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index cf6ad80b838..8385468756f 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -737,14 +737,6 @@ RSpec.describe Group, feature_category: :groups_and_projects do it 'hierarchy order' do expect(group.ancestors(hierarchy_order: :asc).to_sql).to include 'ORDER BY "depth" ASC' end - - context 'ancestor linear queries feature flag disabled' do - before do - stub_feature_flags(use_traversal_ids_for_ancestors: false) - end - - it { expect(group.ancestors.to_sql).not_to include 'traversal_ids <@' } - end end describe '#ancestors_upto' do diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 7aae8f766ea..0b9b5a1df47 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -1609,38 +1609,6 @@ RSpec.describe Namespace, feature_category: :groups_and_projects do end end - describe '#use_traversal_ids_for_ancestors?' do - let_it_be(:namespace, reload: true) { create(:namespace) } - - subject { namespace.use_traversal_ids_for_ancestors? } - - context 'when use_traversal_ids_for_ancestors? feature flag is true' do - before do - stub_feature_flags(use_traversal_ids_for_ancestors: true) - end - - it { is_expected.to eq true } - - it_behaves_like 'disabled feature flag when traversal_ids is blank' - end - - context 'when use_traversal_ids_for_ancestors? feature flag is false' do - before do - stub_feature_flags(use_traversal_ids_for_ancestors: false) - end - - it { is_expected.to eq false } - end - - context 'when use_traversal_ids? feature flag is false' do - before do - stub_feature_flags(use_traversal_ids: false) - end - - it { is_expected.to eq false } - end - end - describe '#use_traversal_ids_for_ancestors_upto?' do let_it_be(:namespace, reload: true) { create(:namespace) } diff --git a/spec/services/ci/process_sync_events_service_spec.rb b/spec/services/ci/process_sync_events_service_spec.rb index 84b6d7d96f6..ff9bcd0f8e9 100644 --- a/spec/services/ci/process_sync_events_service_spec.rb +++ b/spec/services/ci/process_sync_events_service_spec.rb @@ -145,9 +145,9 @@ RSpec.describe Ci::ProcessSyncEventsService, feature_category: :continuous_integ end end - context 'when the FFs use_traversal_ids and use_traversal_ids_for_ancestors are disabled' do + context 'when the use_traversal_ids FF is disabled' do before do - stub_feature_flags(use_traversal_ids: false, use_traversal_ids_for_ancestors: false) + stub_feature_flags(use_traversal_ids: false) end it_behaves_like 'event consuming' diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml index 0dda40d30f8..0f91ff5f12c 100644 --- a/spec/support/rspec_order_todo.yml +++ b/spec/support/rspec_order_todo.yml @@ -9680,7 +9680,6 @@ - './spec/support_specs/helpers/stub_method_calls_spec.rb' - './spec/support_specs/matchers/be_sorted_spec.rb' - './spec/support_specs/matchers/exceed_query_limit_helpers_spec.rb' -- './spec/support_specs/time_travel_spec.rb' - './spec/tasks/admin_mode_spec.rb' - './spec/tasks/cache/clear/redis_spec.rb' - './spec/tasks/config_lint_spec.rb' diff --git a/spec/support/shared_examples/features/cascading_settings_shared_examples.rb b/spec/support/shared_examples/features/cascading_settings_shared_examples.rb index cb80751ff49..2bda352c11f 100644 --- a/spec/support/shared_examples/features/cascading_settings_shared_examples.rb +++ b/spec/support/shared_examples/features/cascading_settings_shared_examples.rb @@ -24,14 +24,6 @@ RSpec.shared_examples 'a cascading setting' do include_examples 'subgroup settings are disabled' - context 'when use_traversal_ids_for_ancestors is disabled' do - before do - stub_feature_flags(use_traversal_ids_for_ancestors: false) - end - - include_examples 'subgroup settings are disabled' - end - it 'does not show enforcement checkbox in subgroups' do visit subgroup_path diff --git a/spec/support/shared_examples/namespaces/traversal_scope_examples.rb b/spec/support/shared_examples/namespaces/traversal_scope_examples.rb index 0c4e5ce51fc..b308295b5fb 100644 --- a/spec/support/shared_examples/namespaces/traversal_scope_examples.rb +++ b/spec/support/shared_examples/namespaces/traversal_scope_examples.rb @@ -70,10 +70,9 @@ RSpec.shared_examples 'namespace traversal scopes' do end describe '.roots' do - context "use_traversal_ids_roots feature flag is true" do + context "use_traversal_ids feature flag is true" do before do stub_feature_flags(use_traversal_ids: true) - stub_feature_flags(use_traversal_ids_roots: true) end it_behaves_like '.roots' @@ -83,9 +82,9 @@ RSpec.shared_examples 'namespace traversal scopes' do end end - context "use_traversal_ids_roots feature flag is false" do + context "use_traversal_ids feature flag is false" do before do - stub_feature_flags(use_traversal_ids_roots: false) + stub_feature_flags(use_traversal_ids: false) end it_behaves_like '.roots' diff --git a/spec/support/time_travel.rb b/spec/support/time_travel.rb deleted file mode 100644 index 9dfbfd20524..00000000000 --- a/spec/support/time_travel.rb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -require 'active_support/testing/time_helpers' - -RSpec.configure do |config| - config.include ActiveSupport::Testing::TimeHelpers - - config.around(:example, :freeze_time) do |example| - freeze_time { example.run } - end - - config.around(:example, :time_travel_to) do |example| - date_or_time = example.metadata[:time_travel_to] - - unless date_or_time.respond_to?(:to_time) && date_or_time.to_time.present? - raise 'The time_travel_to RSpec metadata must have a Date or Time value.' - end - - travel_to(date_or_time) { example.run } - end -end diff --git a/spec/tasks/gitlab/shell_rake_spec.rb b/spec/tasks/gitlab/shell_rake_spec.rb index 195859eac70..30f512205f9 100644 --- a/spec/tasks/gitlab/shell_rake_spec.rb +++ b/spec/tasks/gitlab/shell_rake_spec.rb @@ -24,21 +24,75 @@ RSpec.describe 'gitlab:shell rake tasks', :silence_stdout do end describe 'setup task' do - it 'writes authorized keys into the file' do - allow(Gitlab::CurrentSettings).to receive(:authorized_keys_enabled?).and_return(true) - stub_env('force', 'yes') + let!(:auth_key) { create(:key) } + let!(:auth_and_signing_key) { create(:key, usage_type: :auth_and_signing) } - auth_key = create(:key) - auth_and_signing_key = create(:key, usage_type: :auth_and_signing) + before do create(:key, usage_type: :signing) - expect_next_instance_of(Gitlab::AuthorizedKeys) do |instance| - expect(instance).to receive(:batch_add_keys).once do |keys| - expect(keys).to match_array([auth_key, auth_and_signing_key]) + allow(Gitlab::CurrentSettings).to receive(:authorized_keys_enabled?).and_return(write_to_authorized_keys) + end + + context 'when "Write to authorized keys" is enabled' do + let(:write_to_authorized_keys) { true } + + before do + stub_env('force', force) + end + + context 'when "force" is not set' do + let(:force) { nil } + + context 'when the user answers "yes"' do + it 'writes authorized keys into the file' do + allow(main_object).to receive(:ask_to_continue) + + expect_next_instance_of(Gitlab::AuthorizedKeys) do |instance| + expect(instance).to receive(:batch_add_keys).once do |keys| + expect(keys).to match_array([auth_key, auth_and_signing_key]) + end + end + + run_rake_task('gitlab:shell:setup') + end + end + + context 'when the user answers "no"' do + it 'does not write authorized keys into the file' do + allow(main_object).to receive(:ask_to_continue).and_raise(Gitlab::TaskAbortedByUserError) + + expect(Gitlab::AuthorizedKeys).not_to receive(:new) + + expect do + run_rake_task('gitlab:shell:setup') + end.to raise_error(SystemExit) + end + end + end + + context 'when "force" is set to "yes"' do + let(:force) { 'yes' } + + it 'writes authorized keys into the file' do + expect_next_instance_of(Gitlab::AuthorizedKeys) do |instance| + expect(instance).to receive(:batch_add_keys).once do |keys| + expect(keys).to match_array([auth_key, auth_and_signing_key]) + end + end + + run_rake_task('gitlab:shell:setup') end end + end + + context 'when "Write to authorized keys" is disabled' do + let(:write_to_authorized_keys) { false } - run_rake_task('gitlab:shell:setup') + it 'does not write authorized keys into the file' do + expect(Gitlab::AuthorizedKeys).not_to receive(:new) + + run_rake_task('gitlab:shell:setup') + end end end end diff --git a/spec/tooling/rspec_flaky/flaky_example_spec.rb b/spec/tooling/rspec_flaky/flaky_example_spec.rb index 68e40bc1050..c100756d734 100644 --- a/spec/tooling/rspec_flaky/flaky_example_spec.rb +++ b/spec/tooling/rspec_flaky/flaky_example_spec.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require 'gitlab/rspec/all' -require_relative '../../support/time_travel' require_relative '../../../tooling/rspec_flaky/flaky_example' diff --git a/spec/tooling/rspec_flaky/flaky_examples_collection_spec.rb b/spec/tooling/rspec_flaky/flaky_examples_collection_spec.rb index 9d75c97febe..8304bcb426e 100644 --- a/spec/tooling/rspec_flaky/flaky_examples_collection_spec.rb +++ b/spec/tooling/rspec_flaky/flaky_examples_collection_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require_relative '../../support/time_travel' +require 'gitlab/rspec/all' require_relative '../../../tooling/rspec_flaky/flaky_examples_collection' diff --git a/spec/tooling/rspec_flaky/listener_spec.rb b/spec/tooling/rspec_flaky/listener_spec.rb index c0b71403347..d1ed84dad76 100644 --- a/spec/tooling/rspec_flaky/listener_spec.rb +++ b/spec/tooling/rspec_flaky/listener_spec.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require 'gitlab/rspec/all' -require_relative '../../support/time_travel' require_relative '../../../tooling/rspec_flaky/listener' diff --git a/spec/tooling/rspec_flaky/report_spec.rb b/spec/tooling/rspec_flaky/report_spec.rb index e7365c1e150..543a69cd8ee 100644 --- a/spec/tooling/rspec_flaky/report_spec.rb +++ b/spec/tooling/rspec_flaky/report_spec.rb @@ -1,8 +1,7 @@ # frozen_string_literal: true require 'tempfile' - -require_relative '../../support/time_travel' +require 'gitlab/rspec/all' require_relative '../../../tooling/rspec_flaky/report' diff --git a/tooling/bin/gettext_extractor b/tooling/bin/gettext_extractor index 39f029616df..c430e89a9d3 100755 --- a/tooling/bin/gettext_extractor +++ b/tooling/bin/gettext_extractor @@ -4,6 +4,7 @@ require_relative '../lib/tooling/gettext_extractor' pot_file = ARGV.shift +silent = '--silent' in ARGV if !pot_file || !Dir.exist?(File.dirname(pot_file)) abort <<~MSG @@ -12,9 +13,11 @@ if !pot_file || !Dir.exist?(File.dirname(pot_file)) MSG end -puts <<~MSG - Extracting translatable strings from source files... -MSG +unless silent + puts <<~MSG + Extracting translatable strings from source files... + MSG +end root_dir = File.expand_path('../../', __dir__) @@ -24,6 +27,8 @@ extractor = Tooling::GettextExtractor.new( File.write(pot_file, extractor.generate_pot) -puts <<~MSG - All done. Please commit the changes to `#{pot_file}`. -MSG +unless silent + puts <<~MSG + All done. Please commit the changes to `#{pot_file}`. + MSG +end |