diff options
48 files changed, 700 insertions, 620 deletions
diff --git a/app/assets/javascripts/behaviors/shortcuts.js b/app/assets/javascripts/behaviors/shortcuts.js index 7987a533ae5..7352be0dbd5 100644 --- a/app/assets/javascripts/behaviors/shortcuts.js +++ b/app/assets/javascripts/behaviors/shortcuts.js @@ -1,5 +1,3 @@ -import Shortcuts from './shortcuts/shortcuts'; - export default function initPageShortcuts() { const { page } = document.body.dataset; const pagesWithCustomShortcuts = [ @@ -29,7 +27,9 @@ export default function initPageShortcuts() { // the pages above have their own shortcuts sub-classes instantiated elsewhere // TODO: replace this whitelist with something more automated/maintainable if (page && !pagesWithCustomShortcuts.includes(page)) { - return new Shortcuts(); + import(/* webpackChunkName: 'shortcutsBundle' */ './shortcuts/shortcuts') + .then(({ default: Shortcuts }) => new Shortcuts()) + .catch(() => {}); } return false; } diff --git a/app/assets/javascripts/design_management/components/design_notes/design_discussion.vue b/app/assets/javascripts/design_management/components/design_notes/design_discussion.vue index d28635db601..f87bd695560 100644 --- a/app/assets/javascripts/design_management/components/design_notes/design_discussion.vue +++ b/app/assets/javascripts/design_management/components/design_notes/design_discussion.vue @@ -2,18 +2,19 @@ import { ApolloMutation } from 'vue-apollo'; import { GlTooltipDirective, GlIcon, GlLoadingIcon, GlLink } from '@gitlab/ui'; import { s__ } from '~/locale'; +import createFlash from '~/flash'; import ReplyPlaceholder from '~/notes/components/discussion_reply_placeholder.vue'; import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; import allVersionsMixin from '../../mixins/all_versions'; import createNoteMutation from '../../graphql/mutations/create_note.mutation.graphql'; import toggleResolveDiscussionMutation from '../../graphql/mutations/toggle_resolve_discussion.mutation.graphql'; -import getDesignQuery from '../../graphql/queries/get_design.query.graphql'; import activeDiscussionQuery from '../../graphql/queries/active_discussion.query.graphql'; import DesignNote from './design_note.vue'; import DesignReplyForm from './design_reply_form.vue'; -import { updateStoreAfterAddDiscussionComment } from '../../utils/cache_update'; import { ACTIVE_DISCUSSION_SOURCE_TYPES } from '../../constants'; import ToggleRepliesWidget from './toggle_replies_widget.vue'; +import { hasErrors } from '../../utils/cache_update'; +import { ADD_DISCUSSION_COMMENT_ERROR } from '../../utils/error_messages'; export default { components: { @@ -136,21 +137,10 @@ export default { }, }, methods: { - addDiscussionComment( - store, - { - data: { createNote }, - }, - ) { - updateStoreAfterAddDiscussionComment( - store, - createNote, - getDesignQuery, - this.designVariables, - this.discussion.id, - ); - }, - onDone() { + onDone({ data: { createNote } }) { + if (hasErrors(createNote)) { + createFlash({ message: ADD_DISCUSSION_COMMENT_ERROR }); + } this.discussionComment = ''; this.hideForm(); if (this.shouldChangeResolvedStatus) { @@ -278,7 +268,6 @@ export default { :variables="{ input: mutationPayload, }" - :update="addDiscussionComment" @done="onDone" @error="onCreateNoteError" > diff --git a/app/assets/javascripts/design_management/utils/cache_update.js b/app/assets/javascripts/design_management/utils/cache_update.js index 7f41b7da369..dce33298efb 100644 --- a/app/assets/javascripts/design_management/utils/cache_update.js +++ b/app/assets/javascripts/design_management/utils/cache_update.js @@ -7,15 +7,11 @@ import { extractCurrentDiscussion, extractDesign, extractDesigns } from './desig import { ADD_IMAGE_DIFF_NOTE_ERROR, UPDATE_IMAGE_DIFF_NOTE_ERROR, - ADD_DISCUSSION_COMMENT_ERROR, designDeletionError, } from './error_messages'; const designsOf = data => data.project.issue.designCollection.designs; -const isParticipating = (design, username) => - design.issue.participants.nodes.some(participant => participant.username === username); - const deleteDesignsFromStore = (store, query, selectedDesigns) => { const sourceData = store.readQuery(query); @@ -57,36 +53,6 @@ const addNewVersionToStore = (store, query, version) => { }); }; -const addDiscussionCommentToStore = (store, createNote, query, queryVariables, discussionId) => { - const sourceData = store.readQuery({ - query, - variables: queryVariables, - }); - - const newParticipant = { - __typename: 'User', - ...createNote.note.author, - }; - - const data = produce(sourceData, draftData => { - const design = extractDesign(draftData); - const currentDiscussion = extractCurrentDiscussion(design.discussions, discussionId); - currentDiscussion.notes.nodes = [...currentDiscussion.notes.nodes, createNote.note]; - - if (!isParticipating(design, createNote.note.author.username)) { - design.issue.participants.nodes = [...design.issue.participants.nodes, newParticipant]; - } - - design.notesCount += 1; - }); - - store.writeQuery({ - query, - variables: queryVariables, - data, - }); -}; - const addImageDiffNoteToStore = (store, createImageDiffNote, query, variables) => { const sourceData = store.readQuery({ query, @@ -246,20 +212,6 @@ export const updateStoreAfterDesignsDelete = (store, data, query, designs) => { } }; -export const updateStoreAfterAddDiscussionComment = ( - store, - data, - query, - queryVariables, - discussionId, -) => { - if (hasErrors(data)) { - onError(data, ADD_DISCUSSION_COMMENT_ERROR); - } else { - addDiscussionCommentToStore(store, data, query, queryVariables, discussionId); - } -}; - export const updateStoreAfterAddImageDiffNote = (store, data, query, queryVariables) => { if (hasErrors(data)) { onError(data, ADD_IMAGE_DIFF_NOTE_ERROR); diff --git a/app/assets/javascripts/ide/components/jobs/detail.vue b/app/assets/javascripts/ide/components/jobs/detail.vue index 992b15f2fef..11033a5cc88 100644 --- a/app/assets/javascripts/ide/components/jobs/detail.vue +++ b/app/assets/javascripts/ide/components/jobs/detail.vue @@ -40,10 +40,10 @@ export default { }, }, mounted() { - this.getTrace(); + this.getLogs(); }, methods: { - ...mapActions('pipelines', ['fetchJobTrace', 'setDetailJob']), + ...mapActions('pipelines', ['fetchJobLogs', 'setDetailJob']), scrollDown() { if (this.$refs.buildTrace) { this.$refs.buildTrace.scrollTo(0, this.$refs.buildTrace.scrollHeight); @@ -66,8 +66,8 @@ export default { this.scrollPos = ''; } }), - getTrace() { - return this.fetchJobTrace().then(() => this.scrollDown()); + getLogs() { + return this.fetchJobLogs().then(() => this.scrollDown()); }, }, }; diff --git a/app/assets/javascripts/ide/stores/modules/pipelines/actions.js b/app/assets/javascripts/ide/stores/modules/pipelines/actions.js index 86b889546b0..99bd08ee876 100644 --- a/app/assets/javascripts/ide/stores/modules/pipelines/actions.js +++ b/app/assets/javascripts/ide/stores/modules/pipelines/actions.js @@ -118,31 +118,31 @@ export const setDetailJob = ({ commit, dispatch }, job) => { }); }; -export const requestJobTrace = ({ commit }) => commit(types.REQUEST_JOB_TRACE); -export const receiveJobTraceError = ({ commit, dispatch }) => { +export const requestJobLogs = ({ commit }) => commit(types.REQUEST_JOB_LOGS); +export const receiveJobLogsError = ({ commit, dispatch }) => { dispatch( 'setErrorMessage', { - text: __('An error occurred while fetching the job trace.'), + text: __('An error occurred while fetching the job logs.'), action: () => - dispatch('fetchJobTrace').then(() => dispatch('setErrorMessage', null, { root: true })), + dispatch('fetchJobLogs').then(() => dispatch('setErrorMessage', null, { root: true })), actionText: __('Please try again'), actionPayload: null, }, { root: true }, ); - commit(types.RECEIVE_JOB_TRACE_ERROR); + commit(types.RECEIVE_JOB_LOGS_ERROR); }; -export const receiveJobTraceSuccess = ({ commit }, data) => - commit(types.RECEIVE_JOB_TRACE_SUCCESS, data); +export const receiveJobLogsSuccess = ({ commit }, data) => + commit(types.RECEIVE_JOB_LOGS_SUCCESS, data); -export const fetchJobTrace = ({ dispatch, state }) => { - dispatch('requestJobTrace'); +export const fetchJobLogs = ({ dispatch, state }) => { + dispatch('requestJobLogs'); return axios .get(`${state.detailJob.path}/trace`, { params: { format: 'json' } }) - .then(({ data }) => dispatch('receiveJobTraceSuccess', data)) - .catch(() => dispatch('receiveJobTraceError')); + .then(({ data }) => dispatch('receiveJobLogsSuccess', data)) + .catch(() => dispatch('receiveJobLogsError')); }; export const resetLatestPipeline = ({ commit }) => { diff --git a/app/assets/javascripts/ide/stores/modules/pipelines/mutation_types.js b/app/assets/javascripts/ide/stores/modules/pipelines/mutation_types.js index f4c36b9d96f..fea3055e0fe 100644 --- a/app/assets/javascripts/ide/stores/modules/pipelines/mutation_types.js +++ b/app/assets/javascripts/ide/stores/modules/pipelines/mutation_types.js @@ -10,6 +10,6 @@ export const TOGGLE_STAGE_COLLAPSE = 'TOGGLE_STAGE_COLLAPSE'; export const SET_DETAIL_JOB = 'SET_DETAIL_JOB'; -export const REQUEST_JOB_TRACE = 'REQUEST_JOB_TRACE'; -export const RECEIVE_JOB_TRACE_ERROR = 'RECEIVE_JOB_TRACE_ERROR'; -export const RECEIVE_JOB_TRACE_SUCCESS = 'RECEIVE_JOB_TRACE_SUCCESS'; +export const REQUEST_JOB_LOGS = 'REQUEST_JOB_LOGS'; +export const RECEIVE_JOB_LOGS_ERROR = 'RECEIVE_JOB_LOGS_ERROR'; +export const RECEIVE_JOB_LOGS_SUCCESS = 'RECEIVE_JOB_LOGS_SUCCESS'; diff --git a/app/assets/javascripts/ide/stores/modules/pipelines/mutations.js b/app/assets/javascripts/ide/stores/modules/pipelines/mutations.js index eaaa82cb339..3a3cb4a7cb2 100644 --- a/app/assets/javascripts/ide/stores/modules/pipelines/mutations.js +++ b/app/assets/javascripts/ide/stores/modules/pipelines/mutations.js @@ -66,13 +66,13 @@ export default { [types.SET_DETAIL_JOB](state, job) { state.detailJob = { ...job }; }, - [types.REQUEST_JOB_TRACE](state) { + [types.REQUEST_JOB_LOGS](state) { state.detailJob.isLoading = true; }, - [types.RECEIVE_JOB_TRACE_ERROR](state) { + [types.RECEIVE_JOB_LOGS_ERROR](state) { state.detailJob.isLoading = false; }, - [types.RECEIVE_JOB_TRACE_SUCCESS](state, data) { + [types.RECEIVE_JOB_LOGS_SUCCESS](state, data) { state.detailJob.isLoading = false; state.detailJob.output = data.html; }, diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index 56a91fa0305..405b8fe9c9e 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -2,6 +2,7 @@ module MergeRequests class RefreshService < MergeRequests::BaseService + include Gitlab::Utils::StrongMemoize attr_reader :push def execute(oldrev, newrev, ref) @@ -23,25 +24,37 @@ module MergeRequests post_merge_manually_merged link_forks_lfs_objects reload_merge_requests - outdate_suggestions - refresh_pipelines_on_merge_requests - abort_auto_merges + + merge_requests_for_source_branch.each do |mr| + outdate_suggestions(mr) + refresh_pipelines_on_merge_requests(mr) + abort_auto_merges(mr) + mark_pending_todos_done(mr) + end + abort_ff_merge_requests_with_when_pipeline_succeeds - mark_pending_todos_done cache_merge_requests_closing_issues - # Leave a system note if a branch was deleted/added - if @push.branch_added? || @push.branch_removed? - comment_mr_branch_presence_changed - end + merge_requests_for_source_branch.each do |mr| + # Leave a system note if a branch was deleted/added + if branch_added_or_removed? + comment_mr_branch_presence_changed(mr) + end - notify_about_push - mark_mr_as_wip_from_commits - execute_mr_web_hooks + notify_about_push(mr) + mark_mr_as_wip_from_commits(mr) + execute_mr_web_hooks(mr) + end true end + def branch_added_or_removed? + strong_memoize(:branch_added_or_removed) do + @push.branch_added? || @push.branch_removed? + end + end + def close_upon_missing_source_branch_ref # MergeRequest#reload_diff ignores not opened MRs. This means it won't # create an `empty` diff for `closed` MRs without a source branch, keeping @@ -140,25 +153,22 @@ module MergeRequests merge_request.source_branch == @push.branch_name end - def outdate_suggestions - outdate_service = Suggestions::OutdateService.new + def outdate_suggestions(merge_request) + outdate_service.execute(merge_request) + end - merge_requests_for_source_branch.each do |merge_request| - outdate_service.execute(merge_request) - end + def outdate_service + @outdate_service ||= Suggestions::OutdateService.new end - def refresh_pipelines_on_merge_requests - merge_requests_for_source_branch.each do |merge_request| - create_pipeline_for(merge_request, current_user) - UpdateHeadPipelineForMergeRequestWorker.perform_async(merge_request.id) - end + def refresh_pipelines_on_merge_requests(merge_request) + create_pipeline_for(merge_request, current_user) + + UpdateHeadPipelineForMergeRequestWorker.perform_async(merge_request.id) end - def abort_auto_merges - merge_requests_for_source_branch.each do |merge_request| - abort_auto_merge(merge_request, 'source branch was updated') - end + def abort_auto_merges(merge_request) + abort_auto_merge(merge_request, 'source branch was updated') end def abort_ff_merge_requests_with_when_pipeline_succeeds @@ -187,10 +197,8 @@ module MergeRequests .with_auto_merge_enabled end - def mark_pending_todos_done - merge_requests_for_source_branch.each do |merge_request| - todo_service.merge_request_push(merge_request, @current_user) - end + def mark_pending_todos_done(merge_request) + todo_service.merge_request_push(merge_request, @current_user) end def find_new_commits @@ -218,62 +226,54 @@ module MergeRequests end # Add comment about branches being deleted or added to merge requests - def comment_mr_branch_presence_changed + def comment_mr_branch_presence_changed(merge_request) presence = @push.branch_added? ? :add : :delete - merge_requests_for_source_branch.each do |merge_request| - SystemNoteService.change_branch_presence( - merge_request, merge_request.project, @current_user, - :source, @push.branch_name, presence) - end + SystemNoteService.change_branch_presence( + merge_request, merge_request.project, @current_user, + :source, @push.branch_name, presence) end # Add comment about pushing new commits to merge requests and send nofitication emails - def notify_about_push + def notify_about_push(merge_request) return unless @commits.present? - merge_requests_for_source_branch.each do |merge_request| - mr_commit_ids = Set.new(merge_request.commit_shas) + mr_commit_ids = Set.new(merge_request.commit_shas) - new_commits, existing_commits = @commits.partition do |commit| - mr_commit_ids.include?(commit.id) - end + new_commits, existing_commits = @commits.partition do |commit| + mr_commit_ids.include?(commit.id) + end - SystemNoteService.add_commits(merge_request, merge_request.project, - @current_user, new_commits, - existing_commits, @push.oldrev) + SystemNoteService.add_commits(merge_request, merge_request.project, + @current_user, new_commits, + existing_commits, @push.oldrev) - notification_service.push_to_merge_request(merge_request, @current_user, new_commits: new_commits, existing_commits: existing_commits) - end + notification_service.push_to_merge_request(merge_request, @current_user, new_commits: new_commits, existing_commits: existing_commits) end - def mark_mr_as_wip_from_commits + def mark_mr_as_wip_from_commits(merge_request) return unless @commits.present? - merge_requests_for_source_branch.each do |merge_request| - commit_shas = merge_request.commit_shas + commit_shas = merge_request.commit_shas - wip_commit = @commits.detect do |commit| - commit.work_in_progress? && commit_shas.include?(commit.sha) - end + wip_commit = @commits.detect do |commit| + commit.work_in_progress? && commit_shas.include?(commit.sha) + end - if wip_commit && !merge_request.work_in_progress? - merge_request.update(title: merge_request.wip_title) - SystemNoteService.add_merge_request_wip_from_commit( - merge_request, - merge_request.project, - @current_user, - wip_commit - ) - end + if wip_commit && !merge_request.work_in_progress? + merge_request.update(title: merge_request.wip_title) + SystemNoteService.add_merge_request_wip_from_commit( + merge_request, + merge_request.project, + @current_user, + wip_commit + ) end end # Call merge request webhook with update branches - def execute_mr_web_hooks - merge_requests_for_source_branch.each do |merge_request| - execute_hooks(merge_request, 'update', old_rev: @push.oldrev) - end + def execute_mr_web_hooks(merge_request) + execute_hooks(merge_request, 'update', old_rev: @push.oldrev) end # If the merge requests closes any issues, save this information in the diff --git a/changelogs/unreleased/238969-Web-IDE-Rename-Job-Trace-To-Job-Logs.yml b/changelogs/unreleased/238969-Web-IDE-Rename-Job-Trace-To-Job-Logs.yml new file mode 100644 index 00000000000..3afdfa77733 --- /dev/null +++ b/changelogs/unreleased/238969-Web-IDE-Rename-Job-Trace-To-Job-Logs.yml @@ -0,0 +1,5 @@ +--- +title: Rename job trace to job logs in IDE code +merge_request: 41522 +author: Kev @KevSlashNull +type: other diff --git a/changelogs/unreleased/245247-check-for-usage-ping-enabled-when-tracking-using-redis-hll.yml b/changelogs/unreleased/245247-check-for-usage-ping-enabled-when-tracking-using-redis-hll.yml new file mode 100644 index 00000000000..76d70b52803 --- /dev/null +++ b/changelogs/unreleased/245247-check-for-usage-ping-enabled-when-tracking-using-redis-hll.yml @@ -0,0 +1,5 @@ +--- +title: Check if usage ping enabled for all tracking using Redis HLL +merge_request: 41562 +author: +type: added diff --git a/changelogs/unreleased/246532-on-design-discussion-comment-is-added-twice.yml b/changelogs/unreleased/246532-on-design-discussion-comment-is-added-twice.yml new file mode 100644 index 00000000000..8c11a731541 --- /dev/null +++ b/changelogs/unreleased/246532-on-design-discussion-comment-is-added-twice.yml @@ -0,0 +1,5 @@ +--- +title: Resolve design discussion bug where a comment is added twice +merge_request: 41687 +author: +type: fixed diff --git a/changelogs/unreleased/hide-latest-versions-from-ui.yml b/changelogs/unreleased/hide-latest-versions-from-ui.yml new file mode 100644 index 00000000000..27d276df7f9 --- /dev/null +++ b/changelogs/unreleased/hide-latest-versions-from-ui.yml @@ -0,0 +1,5 @@ +--- +title: Hide the latest version of templates from the template selector +merge_request: 40937 +author: +type: other diff --git a/changelogs/unreleased/refactor-deployment-templates.yml b/changelogs/unreleased/refactor-deployment-templates.yml new file mode 100644 index 00000000000..33a5fb8930e --- /dev/null +++ b/changelogs/unreleased/refactor-deployment-templates.yml @@ -0,0 +1,5 @@ +--- +title: Move Jobs/Deploy/ECS.gitlab-ci.yml to the top level of AutoDevOps template +merge_request: 41096 +author: +type: fixed diff --git a/changelogs/unreleased/refresh-service-optimisations.yml b/changelogs/unreleased/refresh-service-optimisations.yml new file mode 100644 index 00000000000..f5b4d473453 --- /dev/null +++ b/changelogs/unreleased/refresh-service-optimisations.yml @@ -0,0 +1,5 @@ +--- +title: Reduce MergeRequest::RefreshService loops +merge_request: 40135 +author: +type: performance diff --git a/doc/.vale/gitlab/ContractionsDiscard.yml b/doc/.vale/gitlab/ContractionsDiscard.yml deleted file mode 100644 index 698fda86b5b..00000000000 --- a/doc/.vale/gitlab/ContractionsDiscard.yml +++ /dev/null @@ -1,32 +0,0 @@ ---- -# Suggestion: gitlab.ContractionsDiscard -# -# Suggests a list of agreed-upon contractions to discard. -# -# For a list of all options, see https://errata-ai.gitbook.io/vale/getting-started/styles -extends: substitution -message: 'Use "%s" instead of "%s", for a friendly, informal tone.' -link: https://docs.gitlab.com/ee/development/documentation/styleguide.html#language -level: suggestion -nonword: false -ignorecase: true -swap: - - # Uncommon contractions are not ok - aren't: are not - couldn't: could not - didn't: did not - doesn't: does not - hasn't: has not - how's: how is - isn't: is not - shouldn't: should not - they're: they are - wasn't: was not - weren't: were not - we've: we have - what's: what is - when's: when is - where's: where is - who's: who is - why's: why is diff --git a/doc/.vale/gitlab/ContractionsKeep.yml b/doc/.vale/gitlab/ContractionsKeep.yml deleted file mode 100644 index eeaf65e0829..00000000000 --- a/doc/.vale/gitlab/ContractionsKeep.yml +++ /dev/null @@ -1,25 +0,0 @@ ---- -# Suggestion: gitlab.ContractionsKeep -# -# Suggests a list of agreed-upon contractions to keep. -# -# For a list of all options, see https://errata-ai.gitbook.io/vale/getting-started/styles -extends: substitution -message: 'Use "%s" instead of "%s", for a friendly, informal tone.' -link: https://docs.gitlab.com/ee/development/documentation/styleguide.html#language -level: suggestion -nonword: false -ignorecase: true -swap: - - # Common contractions are ok - it is: it's - can not: can't - cannot: can't - do not: don't - have not: haven't - that is: that's - we are: we're - would not: wouldn't - you are: you're - you have: you've diff --git a/doc/administration/operations/puma.md b/doc/administration/operations/puma.md index 1607515dcfc..f5b09d7a978 100644 --- a/doc/administration/operations/puma.md +++ b/doc/administration/operations/puma.md @@ -58,7 +58,6 @@ plays an important role in your deployment, we suggest you benchmark to find the optimal configuration: - The safest option is to start with single-threaded Puma. When working with -Rugged, single-threaded Puma does work the same as Unicorn. - -- To force Rugged auto detect with multi-threaded Puma, you can use [feature -flags](../../development/gitaly.md#legacy-rugged-code). + Rugged, single-threaded Puma works the same as Unicorn. +- To force Rugged to be used with multi-threaded Puma, you can use + [feature flags](../../development/gitaly.md#legacy-rugged-code). diff --git a/doc/api/deployments.md b/doc/api/deployments.md index 426b3e10ecf..b0de972160b 100644 --- a/doc/api/deployments.md +++ b/doc/api/deployments.md @@ -15,15 +15,15 @@ Get a list of deployments in a project. GET /projects/:id/deployments ``` -| Attribute | Type | Required | Description | -|-----------|---------|----------|---------------------| -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | -| `order_by`| string | no | Return deployments ordered by `id` or `iid` or `created_at` or `updated_at` or `ref` fields. Default is `id` | -| `sort` | string | no | Return deployments sorted in `asc` or `desc` order. Default is `asc` | -| `updated_after` | datetime | no | Return deployments updated after the specified date | -| `updated_before` | datetime | no | Return deployments updated before the specified date | -| `environment` | string | no | The name of the environment to filter deployments by | -| `status` | string | no | The status to filter deployments by | +| Attribute | Type | Required | Description | +|------------------|----------------|----------|-----------------------------------------------------------------------------------------------------------------| +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | +| `order_by` | string | no | Return deployments ordered by `id` or `iid` or `created_at` or `updated_at` or `ref` fields. Default is `id` | +| `sort` | string | no | Return deployments sorted in `asc` or `desc` order. Default is `asc` | +| `updated_after` | datetime | no | Return deployments updated after the specified date | +| `updated_before` | datetime | no | Return deployments updated before the specified date | +| `environment` | string | no | The [name of the environment](../ci/environments/index.md#defining-environments) to filter deployments by | +| `status` | string | no | The status to filter deployments by | The status attribute can be one of the following values: @@ -278,14 +278,14 @@ Example of response POST /projects/:id/deployments ``` -| Attribute | Type | Required | Description | -|------------------|----------------|----------|---------------------| -| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | -| `environment` | string | yes | The name of the environment to create the deployment for | -| `sha` | string | yes | The SHA of the commit that is deployed | -| `ref` | string | yes | The name of the branch or tag that is deployed | -| `tag` | boolean | yes | A boolean that indicates if the deployed ref is a tag (true) or not (false) | -| `status` | string | yes | The status of the deployment | +| Attribute | Type | Required | Description | +|---------------|----------------|----------|-----------------------------------------------------------------------------------------------------------------| +| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user | +| `environment` | string | yes | The [name of the environment](../ci/environments/index.md#defining-environments) to create the deployment for | +| `sha` | string | yes | The SHA of the commit that is deployed | +| `ref` | string | yes | The name of the branch or tag that is deployed | +| `tag` | boolean | yes | A boolean that indicates if the deployed ref is a tag (true) or not (false) | +| `status` | string | yes | The status of the deployment | The status can be one of the following values: diff --git a/doc/development/cicd/templates.md b/doc/development/cicd/templates.md index efcc7b363ff..891555d93fc 100644 --- a/doc/development/cicd/templates.md +++ b/doc/development/cicd/templates.md @@ -16,7 +16,7 @@ All template files reside in the `lib/gitlab/ci/templates` directory, and are ca | Sub-directory | Content | [Selectable in UI](#make-sure-the-new-template-can-be-selected-in-ui) | |----------------|--------------------------------------------------------------|-----------------------------------------------------------------------| | `/AWS/*` | Cloud Deployment (AWS) related jobs | No | -| `/Jobs/*` | Auto DevOps related jobs | Yes | +| `/Jobs/*` | Auto DevOps related jobs | No | | `/Pages/*` | Static site generators for GitLab Pages (for example Jekyll) | Yes | | `/Security/*` | Security related jobs | Yes | | `/Verify/*` | Verify/testing related jobs | Yes | diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md index b099d589b0f..27ef0a595da 100644 --- a/doc/development/documentation/styleguide.md +++ b/doc/development/documentation/styleguide.md @@ -540,35 +540,10 @@ tenses, words, and phrases: ### Contractions -- Use common contractions when it helps create a friendly and informal tone, - especially in tutorials, instructional documentation, and - [user interfaces](https://design.gitlab.com/content/punctuation/#contractions). - (Tested in [`Contractions.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/doc/.vale/gitlab/Contractions.yml).) - - <!-- vale gitlab.ContractionsKeep = NO --> - <!-- vale gitlab.ContractionsDiscard = NO --> - <!-- vale gitlab.FutureTense = NO --> - | Do | Don't | - |----------|-----------| - | it's | it is | - | can't | cannot | - | wouldn't | would not | - | you're | you are | - | you've | you have | - | haven't | have not | - | don't | do not | - | we're | we are | - | that's | that is | - | won't | will not | - -- Avoid less common contractions: - - | Do | Don't | - |--------------|-------------| - | he would | he'd | - | it will | it'll | - | should have | should've | - | there would | there'd | +Contractions can create a friendly and informal tone, especially in tutorials, instructional +documentation, and [user interfaces](https://design.gitlab.com/content/punctuation/#contractions). + +Some contractions should be avoided: - Do not use contractions with a proper noun and a verb. For example: @@ -580,13 +555,13 @@ tenses, words, and phrases: | Do | Don't | |-----------------------------|----------------------------| - | Do *not* install X with Y | *Don't* install X with Y | + | Do **not** install X with Y | **Don't** install X with Y | - Do not use contractions in reference documentation. For example: | Do | Don't | |------------------------------------------|----------------------------------------| - | Do *not* set a limit greater than 1000 | *Don't* set a limit greater than 1000 | + | Do **not** set a limit greater than 1000 | **Don't** set a limit greater than 1000 | | For `parameter1`, the default is 10 | For `parameter1`, the default's 10 | - Avoid contractions in error messages. Examples: @@ -596,10 +571,6 @@ tenses, words, and phrases: | Requests to localhost are not allowed | Requests to localhost aren't allowed | | Specified URL cannot be used | Specified URL can't be used | - <!-- vale gitlab.ContractionsKeep = YES --> - <!-- vale gitlab.ContractionsDiscard = YES --> - <!-- vale gitlab.FutureTense = YES --> - ## Text - [Write in Markdown](#markdown). diff --git a/doc/development/fe_guide/graphql.md b/doc/development/fe_guide/graphql.md index f5e16d377f1..a63cb95fafd 100644 --- a/doc/development/fe_guide/graphql.md +++ b/doc/development/fe_guide/graphql.md @@ -602,6 +602,174 @@ it('calls mutation on submitting form ', () => { }); ``` +### Testing with mocked Apollo Client + +To test the logic of Apollo cache updates, we might want to mock an Apollo Client in our unit tests. To separate tests with mocked client from 'usual' unit tests, it's recommended to create an additional component factory. This way we only create Apollo Client instance when it's necessary: + +```javascript +function createComponent() {...} + +function createComponentWithApollo() {...} +``` + +We use [`mock-apollo-client`](https://www.npmjs.com/package/mock-apollo-client) library to mock Apollo client in tests. + +```javascript +import { createMockClient } from 'mock-apollo-client'; +``` + +Then we need to inject `VueApollo` to Vue local instance (`localVue.use()` can also be called within `createComponentWithApollo()`) + +```javascript +import VueApollo from 'vue-apollo'; +import { createLocalVue } from '@vue/test-utils'; + +const localVue = createLocalVue(); +localVue.use(VueApollo); +``` + +After this, on the global `describe`, we should create a variable for `fakeApollo`: + +```javascript +describe('Some component with Apollo mock', () => { + let wrapper; + let fakeApollo +}) +``` + +Within component factory, we need to define an array of _handlers_ for every query or mutation: + +```javascript +import getDesignListQuery from '~/design_management/graphql/queries/get_design_list.query.graphql'; +import permissionsQuery from '~/design_management/graphql/queries/design_permissions.query.graphql'; +import moveDesignMutation from '~/design_management/graphql/mutations/move_design.mutation.graphql'; + +describe('Some component with Apollo mock', () => { + let wrapper; + let fakeApollo; + + function createComponentWithApollo() { + const requestHandlers = [ + [getDesignListQuery, jest.fn().mockResolvedValue(designListQueryResponse)], + [permissionsQuery, jest.fn().mockResolvedValue(permissionsQueryResponse)], + ]; + } +}) +``` + +After this, we need to create a mock Apollo Client instance using a helper: + +```javascript +import createMockApollo from 'jest/helpers/mock_apollo_helper'; + +describe('Some component with Apollo mock', () => { + let wrapper; + let fakeApollo; + + function createComponentWithApollo() { + const requestHandlers = [ + [getDesignListQuery, jest.fn().mockResolvedValue(designListQueryResponse)], + [permissionsQuery, jest.fn().mockResolvedValue(permissionsQueryResponse)], + ]; + + fakeApollo = createMockApollo(requestHandlers); + wrapper = shallowMount(Index, { + localVue, + apolloProvider: fakeApollo, + }); + } +}) +``` + +NOTE: **Note:** +When mocking resolved values, make sure the structure of the response is the same as actual API response: i.e. root property should be `data` for example + +When testing queries, please keep in mind they are promises, so they need to be _resolved_ to render a result. Without resolving, we can check the `loading` state of the query: + +```javascript +it('renders a loading state', () => { + createComponentWithApollo(); + + expect(wrapper.find(LoadingSpinner).exists()).toBe(true) +}); + +it('renders designs list', async () => { + createComponentWithApollo(); + + jest.runOnlyPendingTimers(); + await wrapper.vm.$nextTick(); + + expect(findDesigns()).toHaveLength(3); +}); +``` + +If we need to test a query error, we need to mock a rejected value as request handler: + +```javascript +function createComponentWithApollo() { + ... + const requestHandlers = [ + [getDesignListQuery, jest.fn().mockRejectedValue(new Error('GraphQL error')], + ]; + ... +} +... + +it('renders error if query fails', async () => { + createComponent() + + jest.runOnlyPendingTimers(); + await wrapper.vm.$nextTick(); + + expect(wrapper.find('.test-error').exists()).toBe(true) +}) +``` + +Request handlers can also be passed to component factory as a parameter. + +Mutations could be tested the same way with a few additional `nextTick`s to get the updated result: + +```javascript +function createComponentWithApollo({ + moveHandler = jest.fn().mockResolvedValue(moveDesignMutationResponse), +}) { + moveDesignHandler = moveHandler; + + const requestHandlers = [ + [getDesignListQuery, jest.fn().mockResolvedValue(designListQueryResponse)], + [permissionsQuery, jest.fn().mockResolvedValue(permissionsQueryResponse)], + [moveDesignMutation, moveDesignHandler], + ]; + + fakeApollo = createMockApollo(requestHandlers); + wrapper = shallowMount(Index, { + localVue, + apolloProvider: fakeApollo, + }); +} +... +it('calls a mutation with correct parameters and reorders designs', async () => { + createComponentWithApollo({}); + + wrapper.find(VueDraggable).vm.$emit('change', { + moved: { + newIndex: 0, + element: designToMove, + }, + }); + + expect(moveDesignHandler).toHaveBeenCalled(); + + await wrapper.vm.$nextTick(); + + expect( + findDesigns() + .at(0) + .props('id'), + ).toBe('2'); +}); +``` + ## Handling errors GitLab's GraphQL mutations currently have two distinct error modes: [Top-level](#top-level-errors) and [errors-as-data](#errors-as-data). diff --git a/doc/development/licensed_feature_availability.md b/doc/development/licensed_feature_availability.md index cf479544eea..62f5bb7e029 100644 --- a/doc/development/licensed_feature_availability.md +++ b/doc/development/licensed_feature_availability.md @@ -35,7 +35,3 @@ the instance license. ```ruby License.feature_available?(:feature_symbol) ``` - -## Enabling promo features on GitLab.com - -A paid feature can be made available to everyone on GitLab.com by enabling the feature flag `"promo_#{feature}"`. diff --git a/doc/user/application_security/sast/index.md b/doc/user/application_security/sast/index.md index db5f0d1635d..d14efb3e6a5 100644 --- a/doc/user/application_security/sast/index.md +++ b/doc/user/application_security/sast/index.md @@ -106,6 +106,7 @@ as shown in the following table: | [Presentation of JSON Report in Merge Request](#overview) | **{dotted-circle}** | **{check-circle}** | | [Interaction with Vulnerabilities](#interacting-with-the-vulnerabilities) | **{dotted-circle}** | **{check-circle}** | | [Access to Security Dashboard](#security-dashboard) | **{dotted-circle}** | **{check-circle}** | +| [Configure SAST in the UI](#configure-sast-in-the-ui) | **{dotted-circle}** | **{check-circle}** | ## Contribute your scanner @@ -142,7 +143,7 @@ The results are saved as a that you can later download and analyze. Due to implementation limitations, we always take the latest SAST artifact available. -### Configure SAST in the UI +### Configure SAST in the UI **(ULTIMATE)** > - [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/3659) in GitLab Ultimate 13.3. > - [Improved](https://gitlab.com/gitlab-org/gitlab/-/issues/232862) in GitLab Ultimate 13.4. diff --git a/doc/user/project/code_owners.md b/doc/user/project/code_owners.md index 8c7de16a7d6..9b19de44026 100644 --- a/doc/user/project/code_owners.md +++ b/doc/user/project/code_owners.md @@ -9,7 +9,6 @@ type: reference > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/6916) in [GitLab Starter](https://about.gitlab.com/pricing/) 11.3. -> - [Support for group namespaces](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/53182) added in GitLab Starter 12.1. > - Code Owners for Merge Request approvals was [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/4418) in [GitLab Premium](https://about.gitlab.com/pricing/) 11.9. ## Introduction @@ -108,40 +107,53 @@ in the `.gitignore` file followed by one or more of: - A user's `@username`. - A user's email address. - The `@name` of one or more groups that should be owners of the file. +- Lines starting with `#` are escaped. -Groups must be added as [members of the project](members/index.md), -or they will be ignored. +The order in which the paths are defined is significant: the last pattern that +matches a given path will be used to find the code owners. -Starting in [GitLab 13.0](https://gitlab.com/gitlab-org/gitlab/-/issues/32432), -you can additionally specify groups or subgroups from the project's upper group -hierarchy as potential code owners, without having to invite them specifically -to the project. Groups outside the project's hierarchy or children beneath the -hierarchy must still be explicitly invited to the project in order to show as -Code Owners. +### Groups as Code Owners -For example, consider the following hierarchy for the example project -`example_project`: +> - [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/53182) in GitLab Starter 12.1. +> - Group and subgroup hierarchy support was [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/32432) in [GitLab Starter](https://about.gitlab.com/pricing/) 13.0. -```plaintext -group >> sub-group >> sub-subgroup >> example_project >> file.md -``` +Groups and subgroups members are inherited as eligible Code Owners to a +project, as long as the hierarchy is respected. + +For example, consider a given group called "Group X" (slug `group-x`) and a +"Subgroup Y" (slug `group-x/subgroup-y`) that belongs to the Group X, and +suppose you have a project called "Project A" within the group and a +"Project B" within the subgroup. -Any of the following groups would be eligible to be specified as code owners: +The eligible Code Owners to Project B are both the members of the Group X and +the Subgroup Y. And the eligible Code Owners to the Project A are just the +members of the Group X, given that Project A doesn't belong to the Subgroup Y: -- `@group` -- `@group/sub-group` -- `@group/sub-group/sub-subgroup` + -In addition, any groups that have been invited to the project using the -**Members** tool will also be recognized as eligible code owners. +But you have the option to [invite](members/share_project_with_groups.md) +the Subgroup Y to the Project A so that their members also become eligible +Code Owners: -The order in which the paths are defined is significant: the last -pattern that matches a given path will be used to find the code -owners. + -Starting a line with a `#` indicates a comment. This needs to be -escaped using `\#` to address files for which the name starts with a -`#`. +Once invited, any member (`@user`) of the group or subgroup can be set +as Code Owner to files of the Project A or B, as well as the entire Group X +(`@group-x`) or Subgroup Y (`@group-x/subgroup-y`), as exemplified below: + +```plaintext +# A member of the group or subgroup as Code Owner to a file +file.md @user + +# All group members as Code Owners to a file +file.md @group-x + +# All subgroup members as Code Owners to a file +file.md @group-x/subgroup-y + +# All group and subgroup members as Code Owners to a file +file.md @group-x @group-x/subgroup-y +``` ### Code Owners Sections **(PREMIUM)** diff --git a/doc/user/project/img/code_owners_invite_members_v13_4.png b/doc/user/project/img/code_owners_invite_members_v13_4.png Binary files differnew file mode 100644 index 00000000000..852a5f68b36 --- /dev/null +++ b/doc/user/project/img/code_owners_invite_members_v13_4.png diff --git a/doc/user/project/img/code_owners_members_v13_4.png b/doc/user/project/img/code_owners_members_v13_4.png Binary files differnew file mode 100644 index 00000000000..e37fe72cd6e --- /dev/null +++ b/doc/user/project/img/code_owners_members_v13_4.png diff --git a/lib/api/project_snippets.rb b/lib/api/project_snippets.rb index bed86e9990d..05765d1e6ac 100644 --- a/lib/api/project_snippets.rb +++ b/lib/api/project_snippets.rb @@ -84,14 +84,17 @@ module API end params do requires :snippet_id, type: Integer, desc: 'The ID of a project snippet' - optional :title, type: String, allow_blank: false, desc: 'The title of the snippet' - optional :file_name, type: String, desc: 'The file name of the snippet' optional :content, type: String, allow_blank: false, desc: 'The content of the snippet' optional :description, type: String, desc: 'The description of a snippet' + optional :file_name, type: String, desc: 'The file name of the snippet' + optional :title, type: String, allow_blank: false, desc: 'The title of the snippet' optional :visibility, type: String, values: Gitlab::VisibilityLevel.string_values, desc: 'The visibility of the snippet' - at_least_one_of :title, :file_name, :content, :visibility + + use :update_file_params + + at_least_one_of :title, :file_name, :content, :files, :visibility end # rubocop: disable CodeReuse/ActiveRecord put ":id/snippets/:snippet_id" do @@ -100,8 +103,9 @@ module API authorize! :update_snippet, snippet - snippet_params = declared_params(include_missing: false) - .merge(request: request, api: true) + validate_params_for_multiple_files(snippet) + + snippet_params = process_update_params(declared_params(include_missing: false)) service_response = ::Snippets::UpdateService.new(user_project, current_user, snippet_params).execute(snippet) snippet = service_response.payload[:snippet] diff --git a/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml b/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml index 968ff0fce89..6966ce88b30 100644 --- a/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Auto-DevOps.gitlab-ci.yml @@ -150,16 +150,22 @@ workflow: - exists: - .static +# NOTE: These links point to the latest templates for development in GitLab canonical project, +# therefore the actual templates that were included for Auto DevOps pipelines +# could be different from the contents in the links. +# To view the actual templates, please replace `master` to the specific GitLab version when +# the Auto DevOps pipeline started running e.g. `v13.0.2-ee`. include: - - template: Jobs/Build.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/gitlab/ci/templates/Jobs/Build.gitlab-ci.yml - - template: Jobs/Test.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/gitlab/ci/templates/Jobs/Test.gitlab-ci.yml - - template: Jobs/Code-Quality.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/gitlab/ci/templates/Jobs/Code-Quality.gitlab-ci.yml - - template: Jobs/Deploy.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml - - template: Jobs/DAST-Default-Branch-Deploy.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/gitlab/ci/templates/Jobs/DAST-Default-Branch-Deploy.gitlab-ci.yml - - template: Jobs/Browser-Performance-Testing.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/gitlab/ci/templates/Jobs/Browser-Performance-Testing.gitlab-ci.yml - - template: Security/DAST.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/gitlab/ci/templates/Security/DAST.gitlab-ci.yml - - template: Security/Container-Scanning.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml - - template: Security/Dependency-Scanning.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/gitlab/ci/templates/Security/Dependency-Scanning.gitlab-ci.yml - - template: Security/License-Scanning.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/gitlab/ci/templates/Security/License-Scanning.gitlab-ci.yml - - template: Security/SAST.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/gitlab/ci/templates/Security/SAST.gitlab-ci.yml - - template: Security/Secret-Detection.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab-foss/blob/master/lib/gitlab/ci/templates/Security/Secret-Detection.gitlab-ci.yml + - template: Jobs/Build.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Jobs/Build.gitlab-ci.yml + - template: Jobs/Test.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Jobs/Test.gitlab-ci.yml + - template: Jobs/Code-Quality.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Jobs/Code-Quality.gitlab-ci.yml + - template: Jobs/Deploy.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml + - template: Jobs/Deploy/ECS.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Jobs/Deploy/ECS.gitlab-ci.yml + - template: Jobs/DAST-Default-Branch-Deploy.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Jobs/DAST-Default-Branch-Deploy.gitlab-ci.yml + - template: Jobs/Browser-Performance-Testing.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Jobs/Browser-Performance-Testing.gitlab-ci.yml + - template: Security/DAST.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Security/DAST.gitlab-ci.yml + - template: Security/Container-Scanning.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml + - template: Security/Dependency-Scanning.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Security/Dependency-Scanning.gitlab-ci.yml + - template: Security/License-Scanning.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Security/License-Scanning.gitlab-ci.yml + - template: Security/SAST.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Security/SAST.gitlab-ci.yml + - template: Security/Secret-Detection.gitlab-ci.yml # https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/ci/templates/Security/Secret-Detection.gitlab-ci.yml diff --git a/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml b/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml index 2922e1c6e88..829fd7a722f 100644 --- a/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Jobs/Deploy.gitlab-ci.yml @@ -2,9 +2,6 @@ image: "registry.gitlab.com/gitlab-org/cluster-integration/auto-deploy-image:v1.0.3" dependencies: [] -include: - - template: Jobs/Deploy/ECS.gitlab-ci.yml - review: extends: .auto-deploy stage: review diff --git a/lib/gitlab/template/finders/global_template_finder.rb b/lib/gitlab/template/finders/global_template_finder.rb index 3669d652fd3..9b39d386674 100644 --- a/lib/gitlab/template/finders/global_template_finder.rb +++ b/lib/gitlab/template/finders/global_template_finder.rb @@ -5,10 +5,10 @@ module Gitlab module Template module Finders class GlobalTemplateFinder < BaseTemplateFinder - def initialize(base_dir, extension, categories = {}, exclusions: []) + def initialize(base_dir, extension, categories = {}, excluded_patterns: []) @categories = categories @extension = extension - @exclusions = exclusions + @excluded_patterns = excluded_patterns super(base_dir) end @@ -43,7 +43,7 @@ module Gitlab private def excluded?(file_name) - @exclusions.include?(file_name) + @excluded_patterns.any? { |pattern| pattern.match?(file_name) } end def select_directory(file_name) diff --git a/lib/gitlab/template/gitlab_ci_yml_template.rb b/lib/gitlab/template/gitlab_ci_yml_template.rb index 26a9dc9fd38..bb1e9db55fa 100644 --- a/lib/gitlab/template/gitlab_ci_yml_template.rb +++ b/lib/gitlab/template/gitlab_ci_yml_template.rb @@ -3,12 +3,16 @@ module Gitlab module Template class GitlabCiYmlTemplate < BaseTemplate + BASE_EXCLUDED_PATTERNS = [%r{\.latest$}].freeze + def content explanation = "# This file is a template, and might need editing before it works on your project." [explanation, super].join("\n") end class << self + include Gitlab::Utils::StrongMemoize + def extension '.gitlab-ci.yml' end @@ -22,10 +26,14 @@ module Gitlab } end - def disabled_templates - %w[ - Verify/Browser-Performance - ] + def excluded_patterns + strong_memoize(:excluded_patterns) do + BASE_EXCLUDED_PATTERNS + additional_excluded_patterns + end + end + + def additional_excluded_patterns + [%r{Verify/Browser-Performance}] end def base_dir @@ -34,7 +42,7 @@ module Gitlab def finder(project = nil) Gitlab::Template::Finders::GlobalTemplateFinder.new( - self.base_dir, self.extension, self.categories, exclusions: self.disabled_templates + self.base_dir, self.extension, self.categories, excluded_patterns: self.excluded_patterns ) end end diff --git a/lib/gitlab/usage_data_counters/hll_redis_counter.rb b/lib/gitlab/usage_data_counters/hll_redis_counter.rb index 4d9d4468cfb..67fc305080d 100644 --- a/lib/gitlab/usage_data_counters/hll_redis_counter.rb +++ b/lib/gitlab/usage_data_counters/hll_redis_counter.rb @@ -34,6 +34,8 @@ module Gitlab # * Get unique counts per user: Gitlab::UsageDataCounters::HLLRedisCounter.unique_events(event_names: 'g_compliance_dashboard', start_date: 28.days.ago, end_date: Date.current) class << self def track_event(entity_id, event_name, time = Time.zone.now) + return unless Gitlab::CurrentSettings.usage_ping_enabled? + event = event_for(event_name) raise UnknownEvent.new("Unknown event #{event_name}") unless event.present? diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b7a2efd2710..d2848891a85 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2708,7 +2708,7 @@ msgstr "" msgid "An error occurred while fetching the job log." msgstr "" -msgid "An error occurred while fetching the job trace." +msgid "An error occurred while fetching the job logs." msgstr "" msgid "An error occurred while fetching the job." diff --git a/spec/frontend/design_management/components/design_notes/design_discussion_spec.js b/spec/frontend/design_management/components/design_notes/design_discussion_spec.js index b04bfa65e37..98f190bc33a 100644 --- a/spec/frontend/design_management/components/design_notes/design_discussion_spec.js +++ b/spec/frontend/design_management/components/design_notes/design_discussion_spec.js @@ -32,7 +32,6 @@ describe('Design discussions component', () => { const mutationVariables = { mutation: createNoteMutation, - update: expect.anything(), variables: { input: { noteableId: 'noteable-id', @@ -41,7 +40,7 @@ describe('Design discussions component', () => { }, }, }; - const mutate = jest.fn(() => Promise.resolve()); + const mutate = jest.fn().mockResolvedValue({ data: { createNote: { errors: [] } } }); const $apollo = { mutate, }; @@ -227,7 +226,7 @@ describe('Design discussions component', () => { }); }); - it('calls mutation on submitting form and closes the form', () => { + it('calls mutation on submitting form and closes the form', async () => { createComponent( { discussionWithOpenForm: defaultMockDiscussion.id }, { discussionComment: 'test', isFormRendered: true }, @@ -236,13 +235,10 @@ describe('Design discussions component', () => { findReplyForm().vm.$emit('submitForm'); expect(mutate).toHaveBeenCalledWith(mutationVariables); - return mutate() - .then(() => { - return wrapper.vm.$nextTick(); - }) - .then(() => { - expect(findReplyForm().exists()).toBe(false); - }); + await mutate(); + await wrapper.vm.$nextTick(); + + expect(findReplyForm().exists()).toBe(false); }); it('clears the discussion comment on closing comment form', () => { diff --git a/spec/frontend/design_management/utils/cache_update_spec.js b/spec/frontend/design_management/utils/cache_update_spec.js index e8a5cf3949d..6c859e8c3e8 100644 --- a/spec/frontend/design_management/utils/cache_update_spec.js +++ b/spec/frontend/design_management/utils/cache_update_spec.js @@ -1,14 +1,12 @@ import { InMemoryCache } from 'apollo-cache-inmemory'; import { updateStoreAfterDesignsDelete, - updateStoreAfterAddDiscussionComment, updateStoreAfterAddImageDiffNote, updateStoreAfterUploadDesign, updateStoreAfterUpdateImageDiffNote, } from '~/design_management/utils/cache_update'; import { designDeletionError, - ADD_DISCUSSION_COMMENT_ERROR, ADD_IMAGE_DIFF_NOTE_ERROR, UPDATE_IMAGE_DIFF_NOTE_ERROR, } from '~/design_management/utils/error_messages'; @@ -28,12 +26,11 @@ describe('Design Management cache update', () => { describe('error handling', () => { it.each` - fnName | subject | errorMessage | extraArgs - ${'updateStoreAfterDesignsDelete'} | ${updateStoreAfterDesignsDelete} | ${designDeletionError({ singular: true })} | ${[[design]]} - ${'updateStoreAfterAddDiscussionComment'} | ${updateStoreAfterAddDiscussionComment} | ${ADD_DISCUSSION_COMMENT_ERROR} | ${[]} - ${'updateStoreAfterAddImageDiffNote'} | ${updateStoreAfterAddImageDiffNote} | ${ADD_IMAGE_DIFF_NOTE_ERROR} | ${[]} - ${'updateStoreAfterUploadDesign'} | ${updateStoreAfterUploadDesign} | ${mockErrors[0]} | ${[]} - ${'updateStoreAfterUpdateImageDiffNote'} | ${updateStoreAfterUpdateImageDiffNote} | ${UPDATE_IMAGE_DIFF_NOTE_ERROR} | ${[]} + fnName | subject | errorMessage | extraArgs + ${'updateStoreAfterDesignsDelete'} | ${updateStoreAfterDesignsDelete} | ${designDeletionError({ singular: true })} | ${[[design]]} + ${'updateStoreAfterAddImageDiffNote'} | ${updateStoreAfterAddImageDiffNote} | ${ADD_IMAGE_DIFF_NOTE_ERROR} | ${[]} + ${'updateStoreAfterUploadDesign'} | ${updateStoreAfterUploadDesign} | ${mockErrors[0]} | ${[]} + ${'updateStoreAfterUpdateImageDiffNote'} | ${updateStoreAfterUpdateImageDiffNote} | ${UPDATE_IMAGE_DIFF_NOTE_ERROR} | ${[]} `('$fnName handles errors in response', ({ subject, extraArgs, errorMessage }) => { expect(createFlash).not.toHaveBeenCalled(); expect(() => subject(mockStore, { errors: mockErrors }, {}, ...extraArgs)).toThrow(); diff --git a/spec/frontend/ide/components/jobs/detail_spec.js b/spec/frontend/ide/components/jobs/detail_spec.js index acd30dee718..496d8284fdd 100644 --- a/spec/frontend/ide/components/jobs/detail_spec.js +++ b/spec/frontend/ide/components/jobs/detail_spec.js @@ -24,7 +24,7 @@ describe('IDE jobs detail view', () => { beforeEach(() => { vm = createComponent(); - jest.spyOn(vm, 'fetchJobTrace').mockResolvedValue(); + jest.spyOn(vm, 'fetchJobLogs').mockResolvedValue(); }); afterEach(() => { @@ -36,8 +36,8 @@ describe('IDE jobs detail view', () => { vm = vm.$mount(); }); - it('calls fetchJobTrace', () => { - expect(vm.fetchJobTrace).toHaveBeenCalled(); + it('calls fetchJobLogs', () => { + expect(vm.fetchJobLogs).toHaveBeenCalled(); }); it('scrolls to bottom', () => { @@ -96,7 +96,7 @@ describe('IDE jobs detail view', () => { describe('scroll buttons', () => { beforeEach(() => { vm = createComponent(); - jest.spyOn(vm, 'fetchJobTrace').mockResolvedValue(); + jest.spyOn(vm, 'fetchJobLogs').mockResolvedValue(); }); afterEach(() => { diff --git a/spec/frontend/ide/stores/modules/pipelines/actions_spec.js b/spec/frontend/ide/stores/modules/pipelines/actions_spec.js index 71918e7e2c2..8511843cc92 100644 --- a/spec/frontend/ide/stores/modules/pipelines/actions_spec.js +++ b/spec/frontend/ide/stores/modules/pipelines/actions_spec.js @@ -15,10 +15,10 @@ import { fetchJobs, toggleStageCollapsed, setDetailJob, - requestJobTrace, - receiveJobTraceError, - receiveJobTraceSuccess, - fetchJobTrace, + requestJobLogs, + receiveJobLogsError, + receiveJobLogsSuccess, + fetchJobLogs, resetLatestPipeline, } from '~/ide/stores/modules/pipelines/actions'; import state from '~/ide/stores/modules/pipelines/state'; @@ -324,24 +324,24 @@ describe('IDE pipelines actions', () => { }); }); - describe('requestJobTrace', () => { + describe('requestJobLogs', () => { it('commits request', done => { - testAction(requestJobTrace, null, mockedState, [{ type: types.REQUEST_JOB_TRACE }], [], done); + testAction(requestJobLogs, null, mockedState, [{ type: types.REQUEST_JOB_LOGS }], [], done); }); }); - describe('receiveJobTraceError', () => { + describe('receiveJobLogsError', () => { it('commits error', done => { testAction( - receiveJobTraceError, + receiveJobLogsError, null, mockedState, - [{ type: types.RECEIVE_JOB_TRACE_ERROR }], + [{ type: types.RECEIVE_JOB_LOGS_ERROR }], [ { type: 'setErrorMessage', payload: { - text: 'An error occurred while fetching the job trace.', + text: 'An error occurred while fetching the job logs.', action: expect.any(Function), actionText: 'Please try again', actionPayload: null, @@ -353,20 +353,20 @@ describe('IDE pipelines actions', () => { }); }); - describe('receiveJobTraceSuccess', () => { + describe('receiveJobLogsSuccess', () => { it('commits data', done => { testAction( - receiveJobTraceSuccess, + receiveJobLogsSuccess, 'data', mockedState, - [{ type: types.RECEIVE_JOB_TRACE_SUCCESS, payload: 'data' }], + [{ type: types.RECEIVE_JOB_LOGS_SUCCESS, payload: 'data' }], [], done, ); }); }); - describe('fetchJobTrace', () => { + describe('fetchJobLogs', () => { beforeEach(() => { mockedState.detailJob = { path: `${TEST_HOST}/project/builds` }; }); @@ -379,20 +379,20 @@ describe('IDE pipelines actions', () => { it('dispatches request', done => { testAction( - fetchJobTrace, + fetchJobLogs, null, mockedState, [], [ - { type: 'requestJobTrace' }, - { type: 'receiveJobTraceSuccess', payload: { html: 'html' } }, + { type: 'requestJobLogs' }, + { type: 'receiveJobLogsSuccess', payload: { html: 'html' } }, ], done, ); }); it('sends get request to correct URL', () => { - fetchJobTrace({ + fetchJobLogs({ state: mockedState, dispatch() {}, @@ -410,11 +410,11 @@ describe('IDE pipelines actions', () => { it('dispatches error', done => { testAction( - fetchJobTrace, + fetchJobLogs, null, mockedState, [], - [{ type: 'requestJobTrace' }, { type: 'receiveJobTraceError' }], + [{ type: 'requestJobLogs' }, { type: 'receiveJobLogsError' }], done, ); }); diff --git a/spec/frontend/ide/stores/modules/pipelines/mutations_spec.js b/spec/frontend/ide/stores/modules/pipelines/mutations_spec.js index 3b7f92cfa74..7d2f5d5d710 100644 --- a/spec/frontend/ide/stores/modules/pipelines/mutations_spec.js +++ b/spec/frontend/ide/stores/modules/pipelines/mutations_spec.js @@ -175,37 +175,37 @@ describe('IDE pipelines mutations', () => { }); }); - describe('REQUEST_JOB_TRACE', () => { + describe('REQUEST_JOB_LOGS', () => { beforeEach(() => { mockedState.detailJob = { ...jobs[0] }; }); it('sets loading on detail job', () => { - mutations[types.REQUEST_JOB_TRACE](mockedState); + mutations[types.REQUEST_JOB_LOGS](mockedState); expect(mockedState.detailJob.isLoading).toBe(true); }); }); - describe('RECEIVE_JOB_TRACE_ERROR', () => { + describe('RECEIVE_JOB_LOGS_ERROR', () => { beforeEach(() => { mockedState.detailJob = { ...jobs[0], isLoading: true }; }); it('sets loading to false on detail job', () => { - mutations[types.RECEIVE_JOB_TRACE_ERROR](mockedState); + mutations[types.RECEIVE_JOB_LOGS_ERROR](mockedState); expect(mockedState.detailJob.isLoading).toBe(false); }); }); - describe('RECEIVE_JOB_TRACE_SUCCESS', () => { + describe('RECEIVE_JOB_LOGS_SUCCESS', () => { beforeEach(() => { mockedState.detailJob = { ...jobs[0], isLoading: true }; }); it('sets output on detail job', () => { - mutations[types.RECEIVE_JOB_TRACE_SUCCESS](mockedState, { html: 'html' }); + mutations[types.RECEIVE_JOB_LOGS_SUCCESS](mockedState, { html: 'html' }); expect(mockedState.detailJob.output).toBe('html'); expect(mockedState.detailJob.isLoading).toBe(false); }); diff --git a/spec/frontend/vue_shared/components/confirm_modal_spec.js b/spec/frontend/vue_shared/components/confirm_modal_spec.js index 7bccd6f1a64..5d92af64de0 100644 --- a/spec/frontend/vue_shared/components/confirm_modal_spec.js +++ b/spec/frontend/vue_shared/components/confirm_modal_spec.js @@ -1,5 +1,4 @@ import { shallowMount } from '@vue/test-utils'; -import { GlModal } from '@gitlab/ui'; import { TEST_HOST } from 'helpers/test_constants'; import ConfirmModal from '~/vue_shared/components/confirm_modal.vue'; @@ -21,9 +20,14 @@ describe('vue_shared/components/confirm_modal', () => { selector: '.test-button', }; - const actionSpies = { - openModal: jest.fn(), - closeModal: jest.fn(), + const popupMethods = { + hide: jest.fn(), + show: jest.fn(), + }; + + const GlModalStub = { + template: '<div><slot></slot></div>', + methods: popupMethods, }; let wrapper; @@ -34,8 +38,8 @@ describe('vue_shared/components/confirm_modal', () => { ...defaultProps, ...props, }, - methods: { - ...actionSpies, + stubs: { + GlModal: GlModalStub, }, }); }; @@ -44,7 +48,7 @@ describe('vue_shared/components/confirm_modal', () => { wrapper.destroy(); }); - const findModal = () => wrapper.find(GlModal); + const findModal = () => wrapper.find(GlModalStub); const findForm = () => wrapper.find('form'); const findFormData = () => findForm() @@ -103,7 +107,7 @@ describe('vue_shared/components/confirm_modal', () => { }); it('does not close modal', () => { - expect(actionSpies.closeModal).not.toHaveBeenCalled(); + expect(popupMethods.hide).not.toHaveBeenCalled(); }); describe('when modal closed', () => { @@ -112,7 +116,7 @@ describe('vue_shared/components/confirm_modal', () => { }); it('closes modal', () => { - expect(actionSpies.closeModal).toHaveBeenCalled(); + expect(popupMethods.hide).toHaveBeenCalled(); }); }); }); diff --git a/spec/frontend/vue_shared/components/diff_viewer/viewers/renamed_spec.js b/spec/frontend/vue_shared/components/diff_viewer/viewers/renamed_spec.js index e0e982f4e11..e91e6577aaf 100644 --- a/spec/frontend/vue_shared/components/diff_viewer/viewers/renamed_spec.js +++ b/spec/frontend/vue_shared/components/diff_viewer/viewers/renamed_spec.js @@ -14,19 +14,13 @@ import { const localVue = createLocalVue(); localVue.use(Vuex); -function createRenamedComponent({ - props = {}, - methods = {}, - store = new Vuex.Store({}), - deep = false, -}) { +function createRenamedComponent({ props = {}, store = new Vuex.Store({}), deep = false }) { const mnt = deep ? mount : shallowMount; return mnt(Renamed, { propsData: { ...props }, localVue, store, - methods, }); } @@ -258,25 +252,17 @@ describe('Renamed Diff Viewer', () => { 'includes a link to the full file for alternate viewer type "$altType"', ({ altType, linkText }) => { const file = { ...diffFile }; - const clickMock = jest.fn().mockImplementation(() => {}); file.alternate_viewer.name = altType; wrapper = createRenamedComponent({ deep: true, props: { diffFile: file }, - methods: { - clickLink: clickMock, - }, }); const link = wrapper.find('a'); expect(link.text()).toEqual(linkText); expect(link.attributes('href')).toEqual(DIFF_FILE_VIEW_PATH); - - link.vm.$emit('click'); - - expect(clickMock).toHaveBeenCalled(); }, ); }); diff --git a/spec/lib/gitlab/ci/templates/templates_spec.rb b/spec/lib/gitlab/ci/templates/templates_spec.rb index 685243d6315..768256ee6b3 100644 --- a/spec/lib/gitlab/ci/templates/templates_spec.rb +++ b/spec/lib/gitlab/ci/templates/templates_spec.rb @@ -7,17 +7,17 @@ RSpec.describe 'CI YML Templates' do let(:all_templates) { Gitlab::Template::GitlabCiYmlTemplate.all.map(&:full_name) } - let(:disabled_templates) do - Gitlab::Template::GitlabCiYmlTemplate.disabled_templates.map do |template| - template + Gitlab::Template::GitlabCiYmlTemplate.extension + let(:excluded_templates) do + all_templates.select do |name| + Gitlab::Template::GitlabCiYmlTemplate.excluded_patterns.any? { |pattern| pattern.match?(name) } end end - context 'included in a CI YAML configuration' do + context 'when including available templates in a CI YAML configuration' do using RSpec::Parameterized::TableSyntax where(:template_name) do - all_templates - disabled_templates + all_templates - excluded_templates end with_them do @@ -41,4 +41,29 @@ RSpec.describe 'CI YML Templates' do end end end + + context 'when including unavailable templates in a CI YAML configuration' do + using RSpec::Parameterized::TableSyntax + + where(:template_name) do + excluded_templates + end + + with_them do + let(:content) do + <<~EOS + include: + - template: #{template_name} + + concrete_build_implemented_by_a_user: + stage: test + script: do something + EOS + end + + it 'is not valid' do + expect(subject).not_to be_valid + end + end + end end diff --git a/spec/lib/gitlab/template/finders/global_template_finder_spec.rb b/spec/lib/gitlab/template/finders/global_template_finder_spec.rb index e776284b3e8..e2751d194d3 100644 --- a/spec/lib/gitlab/template/finders/global_template_finder_spec.rb +++ b/spec/lib/gitlab/template/finders/global_template_finder_spec.rb @@ -15,9 +15,9 @@ RSpec.describe Gitlab::Template::Finders::GlobalTemplateFinder do FileUtils.rm_rf(base_dir) end - subject(:finder) { described_class.new(base_dir, '', { 'General' => '', 'Bar' => 'Bar' }, exclusions: exclusions) } + subject(:finder) { described_class.new(base_dir, '', { 'General' => '', 'Bar' => 'Bar' }, excluded_patterns: excluded_patterns) } - let(:exclusions) { [] } + let(:excluded_patterns) { [] } describe '.find' do context 'with a non-prefixed General template' do @@ -38,7 +38,7 @@ RSpec.describe Gitlab::Template::Finders::GlobalTemplateFinder do end context 'while listed as an exclusion' do - let(:exclusions) { %w[test-template] } + let(:excluded_patterns) { [%r{^test-template$}] } it 'does not find the template without a prefix' do expect(finder.find('test-template')).to be_nil @@ -77,7 +77,7 @@ RSpec.describe Gitlab::Template::Finders::GlobalTemplateFinder do end context 'while listed as an exclusion' do - let(:exclusions) { %w[Bar/test-template] } + let(:excluded_patterns) { [%r{^Bar/test-template$}] } it 'does not find the template with a prefix' do expect(finder.find('Bar/test-template')).to be_nil @@ -96,6 +96,17 @@ RSpec.describe Gitlab::Template::Finders::GlobalTemplateFinder do expect(finder.find('Bar/test-template')).to be_nil end end + + context 'while listed as an exclusion' do + let(:excluded_patterns) { [%r{\.latest$}] } + + it 'excludes the template matched the pattern' do + create_template!('test-template.latest') + + expect(finder.find('test-template')).to be_present + expect(finder.find('test-template.latest')).to be_nil + end + end end end end diff --git a/spec/lib/gitlab/template/gitlab_ci_yml_template_spec.rb b/spec/lib/gitlab/template/gitlab_ci_yml_template_spec.rb index 55444114d39..26c83ed6793 100644 --- a/spec/lib/gitlab/template/gitlab_ci_yml_template_spec.rb +++ b/spec/lib/gitlab/template/gitlab_ci_yml_template_spec.rb @@ -13,6 +13,12 @@ RSpec.describe Gitlab::Template::GitlabCiYmlTemplate do expect(all).to include('Docker') expect(all).to include('Ruby') end + + it 'does not include Browser-Performance template in FOSS' do + all = subject.all.map(&:name) + + expect(all).not_to include('Browser-Performance') unless Gitlab.ee? + end end describe '#content' do diff --git a/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb index 7b9650a966c..6da33a83330 100644 --- a/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb +++ b/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb @@ -62,65 +62,81 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s end describe '.track_event' do - it "raise error if metrics don't have same aggregation" do - expect { described_class.track_event(entity1, different_aggregation, Date.current) } .to raise_error(Gitlab::UsageDataCounters::HLLRedisCounter::UnknownAggregation) - end + context 'when usage_ping is disabled' do + it 'does not track the event' do + stub_application_setting(usage_ping_enabled: false) - it 'raise error if metrics of unknown aggregation' do - expect { described_class.track_event(entity1, 'unknown', Date.current) } .to raise_error(Gitlab::UsageDataCounters::HLLRedisCounter::UnknownEvent) + described_class.track_event(entity1, weekly_event, Date.current) + + expect(Gitlab::Redis::HLL).not_to receive(:add) + end end - context 'for weekly events' do - it 'sets the keys in Redis to expire automatically after the given expiry time' do - described_class.track_event(entity1, "g_analytics_contribution") + context 'when usage_ping is enabled' do + before do + stub_application_setting(usage_ping_enabled: true) + end - Gitlab::Redis::SharedState.with do |redis| - keys = redis.scan_each(match: "g_{analytics}_contribution-*").to_a - expect(keys).not_to be_empty + it "raise error if metrics don't have same aggregation" do + expect { described_class.track_event(entity1, different_aggregation, Date.current) } .to raise_error(Gitlab::UsageDataCounters::HLLRedisCounter::UnknownAggregation) + end - keys.each do |key| - expect(redis.ttl(key)).to be_within(5.seconds).of(12.weeks) + it 'raise error if metrics of unknown aggregation' do + expect { described_class.track_event(entity1, 'unknown', Date.current) } .to raise_error(Gitlab::UsageDataCounters::HLLRedisCounter::UnknownEvent) + end + + context 'for weekly events' do + it 'sets the keys in Redis to expire automatically after the given expiry time' do + described_class.track_event(entity1, "g_analytics_contribution") + + Gitlab::Redis::SharedState.with do |redis| + keys = redis.scan_each(match: "g_{analytics}_contribution-*").to_a + expect(keys).not_to be_empty + + keys.each do |key| + expect(redis.ttl(key)).to be_within(5.seconds).of(12.weeks) + end end end - end - it 'sets the keys in Redis to expire automatically after 6 weeks by default' do - described_class.track_event(entity1, "g_compliance_dashboard") + it 'sets the keys in Redis to expire automatically after 6 weeks by default' do + described_class.track_event(entity1, "g_compliance_dashboard") - Gitlab::Redis::SharedState.with do |redis| - keys = redis.scan_each(match: "g_{compliance}_dashboard-*").to_a - expect(keys).not_to be_empty + Gitlab::Redis::SharedState.with do |redis| + keys = redis.scan_each(match: "g_{compliance}_dashboard-*").to_a + expect(keys).not_to be_empty - keys.each do |key| - expect(redis.ttl(key)).to be_within(5.seconds).of(6.weeks) + keys.each do |key| + expect(redis.ttl(key)).to be_within(5.seconds).of(6.weeks) + end end end end - end - context 'for daily events' do - it 'sets the keys in Redis to expire after the given expiry time' do - described_class.track_event(entity1, "g_analytics_search") + context 'for daily events' do + it 'sets the keys in Redis to expire after the given expiry time' do + described_class.track_event(entity1, "g_analytics_search") - Gitlab::Redis::SharedState.with do |redis| - keys = redis.scan_each(match: "*-g_{analytics}_search").to_a - expect(keys).not_to be_empty + Gitlab::Redis::SharedState.with do |redis| + keys = redis.scan_each(match: "*-g_{analytics}_search").to_a + expect(keys).not_to be_empty - keys.each do |key| - expect(redis.ttl(key)).to be_within(5.seconds).of(84.days) + keys.each do |key| + expect(redis.ttl(key)).to be_within(5.seconds).of(84.days) + end end end - end - it 'sets the keys in Redis to expire after 29 days by default' do - described_class.track_event(entity1, "no_slot") + it 'sets the keys in Redis to expire after 29 days by default' do + described_class.track_event(entity1, "no_slot") - Gitlab::Redis::SharedState.with do |redis| - keys = redis.scan_each(match: "*-{no_slot}").to_a - expect(keys).not_to be_empty + Gitlab::Redis::SharedState.with do |redis| + keys = redis.scan_each(match: "*-{no_slot}").to_a + expect(keys).not_to be_empty - keys.each do |key| - expect(redis.ttl(key)).to be_within(5.seconds).of(29.days) + keys.each do |key| + expect(redis.ttl(key)).to be_within(5.seconds).of(29.days) + end end end end diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index 66dcb40d2ec..0746dee3e51 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -304,30 +304,9 @@ RSpec.describe API::ProjectSnippets do let(:visibility_level) { Snippet::PUBLIC } let(:snippet) { create(:project_snippet, :repository, author: admin, visibility_level: visibility_level, project: project) } - it 'updates snippet' do - new_content = 'New content' - new_description = 'New description' - - update_snippet(params: { content: new_content, description: new_description, visibility: 'private' }) - - expect(response).to have_gitlab_http_status(:ok) - snippet.reload - expect(snippet.content).to eq(new_content) - expect(snippet.description).to eq(new_description) - expect(snippet.visibility).to eq('private') - end - - it 'updates snippet with content parameter' do - new_content = 'New content' - new_description = 'New description' - - update_snippet(params: { content: new_content, description: new_description }) - - expect(response).to have_gitlab_http_status(:ok) - snippet.reload - expect(snippet.content).to eq(new_content) - expect(snippet.description).to eq(new_description) - end + it_behaves_like 'snippet file updates' + it_behaves_like 'snippet non-file updates' + it_behaves_like 'invalid snippet updates' it 'updates snippet with visibility parameter' do expect { update_snippet(params: { visibility: 'private' }) } @@ -336,33 +315,6 @@ RSpec.describe API::ProjectSnippets do expect(snippet.visibility).to eq('private') end - it 'returns 404 for invalid snippet id' do - update_snippet(snippet_id: non_existing_record_id, params: { title: 'foo' }) - - expect(response).to have_gitlab_http_status(:not_found) - expect(json_response['message']).to eq('404 Snippet Not Found') - end - - it 'returns 400 for missing parameters' do - update_snippet - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq 'title, file_name, content, visibility are missing, at least one parameter must be provided' - end - - it 'returns 400 if content is blank' do - update_snippet(params: { content: '' }) - - expect(response).to have_gitlab_http_status(:bad_request) - end - - it 'returns 400 if title is blank' do - update_snippet(params: { title: '' }) - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq 'title is empty' - end - it_behaves_like 'update with repository actions' do let(:snippet_without_repo) { create(:project_snippet, author: admin, project: project, visibility_level: visibility_level) } end diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index 508a39ce227..5bba308a2d3 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -368,7 +368,7 @@ RSpec.describe API::Snippets do context 'when the snippet is public' do let(:extra_params) { { visibility: 'public' } } - it 'rejects the shippet' do + it 'rejects the snippet' do expect { subject }.not_to change { Snippet.count } expect(response).to have_gitlab_http_status(:bad_request) @@ -391,98 +391,17 @@ RSpec.describe API::Snippets do create(:personal_snippet, :repository, author: user, visibility_level: visibility_level) end - let(:create_action) { { action: 'create', file_path: 'foo.txt', content: 'bar' } } - let(:update_action) { { action: 'update', file_path: 'CHANGELOG', content: 'bar' } } - let(:move_action) { { action: 'move', file_path: '.old-gitattributes', previous_path: '.gitattributes' } } - let(:delete_action) { { action: 'delete', file_path: 'CONTRIBUTING.md' } } - let(:bad_file_path) { { action: 'create', file_path: '../../etc/passwd', content: 'bar' } } - let(:bad_previous_path) { { action: 'create', previous_path: '../../etc/passwd', file_path: 'CHANGELOG', content: 'bar' } } - let(:invalid_move) { { action: 'move', file_path: 'missing_previous_path.txt' } } - - context 'with snippet file changes' do - using RSpec::Parameterized::TableSyntax - - where(:is_multi_file, :file_name, :content, :files, :status) do - true | nil | nil | [create_action] | :success - true | nil | nil | [update_action] | :success - true | nil | nil | [move_action] | :success - true | nil | nil | [delete_action] | :success - true | nil | nil | [create_action, update_action] | :success - true | 'foo.txt' | 'bar' | [create_action] | :bad_request - true | 'foo.txt' | 'bar' | nil | :bad_request - true | nil | nil | nil | :bad_request - true | 'foo.txt' | nil | [create_action] | :bad_request - true | nil | 'bar' | [create_action] | :bad_request - true | '' | nil | [create_action] | :bad_request - true | nil | '' | [create_action] | :bad_request - true | nil | nil | [bad_file_path] | :bad_request - true | nil | nil | [bad_previous_path] | :bad_request - true | nil | nil | [invalid_move] | :unprocessable_entity - - false | 'foo.txt' | 'bar' | nil | :success - false | 'foo.txt' | nil | nil | :success - false | nil | 'bar' | nil | :success - false | 'foo.txt' | 'bar' | [create_action] | :bad_request - false | nil | nil | nil | :bad_request - false | nil | '' | nil | :bad_request - false | nil | nil | [bad_file_path] | :bad_request - false | nil | nil | [bad_previous_path] | :bad_request - end - - with_them do - before do - allow_any_instance_of(Snippet).to receive(:multiple_files?).and_return(is_multi_file) - end - - it 'has the correct response' do - update_params = {}.tap do |params| - params[:files] = files if files - params[:file_name] = file_name if file_name - params[:content] = content if content - end - - update_snippet(params: update_params) - - expect(response).to have_gitlab_http_status(status) - end - end - - context 'when save fails due to a repository commit error' do - before do - allow_next_instance_of(Repository) do |instance| - allow(instance).to receive(:multi_action).and_raise(Gitlab::Git::CommitError) - end - - update_snippet(params: { files: [create_action] }) - end - - it 'returns a bad request response' do - expect(response).to have_gitlab_http_status(:bad_request) - end - end - end - - shared_examples 'snippet non-file updates' do - it 'updates a snippet non-file attributes' do - new_description = 'New description' - new_title = 'New title' - new_visibility = 'internal' - - update_snippet(params: { title: new_title, description: new_description, visibility: new_visibility }) + it_behaves_like 'snippet file updates' + it_behaves_like 'snippet non-file updates' + it_behaves_like 'invalid snippet updates' - snippet.reload + it "returns 404 for another user's snippet" do + update_snippet(requester: other_user, params: { title: 'foobar' }) - aggregate_failures do - expect(response).to have_gitlab_http_status(:ok) - expect(snippet.description).to eq(new_description) - expect(snippet.visibility).to eq(new_visibility) - expect(snippet.title).to eq(new_title) - end - end + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 Snippet Not Found') end - it_behaves_like 'snippet non-file updates' - context 'with restricted visibility settings' do before do stub_application_setting(restricted_visibility_levels: @@ -493,33 +412,6 @@ RSpec.describe API::Snippets do it_behaves_like 'snippet non-file updates' end - it 'returns 404 for invalid snippet id' do - update_snippet(snippet_id: non_existing_record_id, params: { title: 'Foo' }) - - expect(response).to have_gitlab_http_status(:not_found) - expect(json_response['message']).to eq('404 Snippet Not Found') - end - - it "returns 404 for another user's snippet" do - update_snippet(requester: other_user, params: { title: 'foobar' }) - - expect(response).to have_gitlab_http_status(:not_found) - expect(json_response['message']).to eq('404 Snippet Not Found') - end - - it 'returns 400 for missing parameters' do - update_snippet - - expect(response).to have_gitlab_http_status(:bad_request) - end - - it 'returns 400 if title is blank' do - update_snippet(params: { title: '' }) - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq 'title is empty' - end - it_behaves_like 'update with repository actions' do let(:snippet_without_repo) { create(:personal_snippet, author: user, visibility_level: visibility_level) } end @@ -543,7 +435,7 @@ RSpec.describe API::Snippets do context 'when the snippet is public' do let(:visibility_level) { Snippet::PUBLIC } - it 'rejects the shippet' do + it 'rejects the snippet' do expect { update_snippet(params: { title: 'Foo' }) } .not_to change { snippet.reload.title } diff --git a/spec/support/helpers/stubbed_feature.rb b/spec/support/helpers/stubbed_feature.rb index e78efcf6b75..d4e9af7a031 100644 --- a/spec/support/helpers/stubbed_feature.rb +++ b/spec/support/helpers/stubbed_feature.rb @@ -37,10 +37,7 @@ module StubbedFeature # We do `m.call` as we want to validate the execution of method arguments # and a feature flag state if it is not persisted unless Feature.persisted_name?(args.first) - # TODO: this is hack to support `promo_feature_available?` - # We enable all feature flags by default unless they are `promo_` - # Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/218667 - feature_flag = true unless args.first.to_s.start_with?('promo_') + feature_flag = true end feature_flag diff --git a/spec/support/shared_examples/requests/api/snippets_shared_examples.rb b/spec/support/shared_examples/requests/api/snippets_shared_examples.rb index cfbb84dd099..97f85977a20 100644 --- a/spec/support/shared_examples/requests/api/snippets_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/snippets_shared_examples.rb @@ -77,3 +77,123 @@ RSpec.shared_examples 'raw snippet files' do end end end + +RSpec.shared_examples 'snippet file updates' do + let(:create_action) { { action: 'create', file_path: 'foo.txt', content: 'bar' } } + let(:update_action) { { action: 'update', file_path: 'CHANGELOG', content: 'bar' } } + let(:move_action) { { action: 'move', file_path: '.old-gitattributes', previous_path: '.gitattributes' } } + let(:delete_action) { { action: 'delete', file_path: 'CONTRIBUTING.md' } } + let(:bad_file_path) { { action: 'create', file_path: '../../etc/passwd', content: 'bar' } } + let(:bad_previous_path) { { action: 'create', previous_path: '../../etc/passwd', file_path: 'CHANGELOG', content: 'bar' } } + let(:invalid_move) { { action: 'move', file_path: 'missing_previous_path.txt' } } + + context 'with various snippet file changes' do + using RSpec::Parameterized::TableSyntax + + where(:is_multi_file, :file_name, :content, :files, :status) do + true | nil | nil | [create_action] | :success + true | nil | nil | [update_action] | :success + true | nil | nil | [move_action] | :success + true | nil | nil | [delete_action] | :success + true | nil | nil | [create_action, update_action] | :success + true | 'foo.txt' | 'bar' | [create_action] | :bad_request + true | 'foo.txt' | 'bar' | nil | :bad_request + true | nil | nil | nil | :bad_request + true | 'foo.txt' | nil | [create_action] | :bad_request + true | nil | 'bar' | [create_action] | :bad_request + true | '' | nil | [create_action] | :bad_request + true | nil | '' | [create_action] | :bad_request + true | nil | nil | [bad_file_path] | :bad_request + true | nil | nil | [bad_previous_path] | :bad_request + true | nil | nil | [invalid_move] | :unprocessable_entity + + false | 'foo.txt' | 'bar' | nil | :success + false | 'foo.txt' | nil | nil | :success + false | nil | 'bar' | nil | :success + false | 'foo.txt' | 'bar' | [create_action] | :bad_request + false | nil | nil | nil | :bad_request + false | nil | '' | nil | :bad_request + false | nil | nil | [bad_file_path] | :bad_request + false | nil | nil | [bad_previous_path] | :bad_request + end + + with_them do + before do + allow_any_instance_of(Snippet).to receive(:multiple_files?).and_return(is_multi_file) + end + + it 'has the correct response' do + update_params = {}.tap do |params| + params[:files] = files if files + params[:file_name] = file_name if file_name + params[:content] = content if content + end + + update_snippet(params: update_params) + + expect(response).to have_gitlab_http_status(status) + end + end + + context 'when save fails due to a repository commit error' do + before do + allow_next_instance_of(Repository) do |instance| + allow(instance).to receive(:multi_action).and_raise(Gitlab::Git::CommitError) + end + + update_snippet(params: { files: [create_action] }) + end + + it 'returns a bad request response' do + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end +end + +RSpec.shared_examples 'snippet non-file updates' do + it 'updates a snippet non-file attributes' do + new_description = 'New description' + new_title = 'New title' + new_visibility = 'internal' + + update_snippet(params: { title: new_title, description: new_description, visibility: new_visibility }) + + snippet.reload + + aggregate_failures do + expect(response).to have_gitlab_http_status(:ok) + expect(snippet.description).to eq(new_description) + expect(snippet.visibility).to eq(new_visibility) + expect(snippet.title).to eq(new_title) + end + end +end + +RSpec.shared_examples 'invalid snippet updates' do + it 'returns 404 for invalid snippet id' do + update_snippet(snippet_id: non_existing_record_id, params: { title: 'foo' }) + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 Snippet Not Found') + end + + it 'returns 400 for missing parameters' do + update_snippet + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'returns 400 if content is blank' do + update_snippet(params: { content: '' }) + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'returns 400 if title is blank' do + update_snippet(params: { title: '' }) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq 'title is empty' + end +end |
