diff options
31 files changed, 985 insertions, 151 deletions
diff --git a/app/assets/javascripts/content_editor/services/hast_to_prosemirror_converter.js b/app/assets/javascripts/content_editor/services/hast_to_prosemirror_converter.js index f38e4514393..2c462cdde91 100644 --- a/app/assets/javascripts/content_editor/services/hast_to_prosemirror_converter.js +++ b/app/assets/javascripts/content_editor/services/hast_to_prosemirror_converter.js @@ -63,7 +63,7 @@ function maybeMerge(a, b) { function createSourceMapAttributes(hastNode, source) { const { position } = hastNode; - return position.end + return position && position.end ? { sourceMapKey: `${position.start.offset}:${position.end.offset}`, sourceMarkdown: source.substring(position.start.offset, position.end.offset), diff --git a/app/assets/javascripts/content_editor/services/serialization_helpers.js b/app/assets/javascripts/content_editor/services/serialization_helpers.js index 055a32420b2..88f5192af77 100644 --- a/app/assets/javascripts/content_editor/services/serialization_helpers.js +++ b/app/assets/javascripts/content_editor/services/serialization_helpers.js @@ -12,22 +12,6 @@ const ignoreAttrs = { const tableMap = new WeakMap(); -// Source taken from -// prosemirror-markdown/src/to_markdown.js -export function isPlainURL(link, parent, index, side) { - if (link.attrs.title || !/^\w+:/.test(link.attrs.href)) return false; - const content = parent.child(index + (side < 0 ? -1 : 0)); - if ( - !content.isText || - content.text !== link.attrs.href || - content.marks[content.marks.length - 1] !== link - ) - return false; - if (index === (side < 0 ? 1 : parent.childCount - 1)) return true; - const next = parent.child(index + (side < 0 ? -2 : 1)); - return !link.isInSet(next.marks); -} - function containsOnlyText(node) { if (node.childCount === 1) { const child = node.child(0); @@ -498,10 +482,79 @@ const linkType = (sourceMarkdown) => { return LINK_HTML; }; +const removeUrlProtocol = (url) => url.replace(/^\w+:\/?\/?/, ''); + +const normalizeUrl = (url) => decodeURIComponent(removeUrlProtocol(url)); + +/** + * Validates that the provided URL is well-formed + * + * @param {String} url + * @returns Returns true when the browser’s URL constructor + * can successfully parse the URL string + */ +const isValidUrl = (url) => { + try { + return new URL(url) && true; + } catch { + return false; + } +}; + +const findChildWithMark = (mark, parent) => { + let child; + let offset; + let index; + + parent.forEach((_child, _offset, _index) => { + if (mark.isInSet(_child.marks)) { + child = _child; + offset = _offset; + index = _index; + } + }); + + return child ? { child, offset, index } : null; +}; + +/** + * This function detects whether a link should be serialized + * as an autolink. + * + * See https://github.github.com/gfm/#autolinks-extension- + * to understand the parsing rules of autolinks. + * */ +const isAutoLink = (linkMark, parent) => { + const { title, href } = linkMark.attrs; + + if (title || !/^\w+:/.test(href)) { + return false; + } + + const { child } = findChildWithMark(linkMark, parent); + + if ( + !child || + !child.isText || + !isValidUrl(href) || + normalizeUrl(child.text) !== normalizeUrl(href) + ) { + return false; + } + + return true; +}; + +/** + * Returns true if the user used brackets to the define + * the autolink in the original markdown source + */ +const isBracketAutoLink = (sourceMarkdown) => /^<.+?>$/.test(sourceMarkdown); + export const link = { - open(state, mark, parent, index) { - if (isPlainURL(mark, parent, index, 1)) { - return '<'; + open(state, mark, parent) { + if (isAutoLink(mark, parent)) { + return isBracketAutoLink(mark.attrs.sourceMarkdown) ? '<' : ''; } const { canonicalSrc, href, title, sourceMarkdown } = mark.attrs; @@ -518,9 +571,9 @@ export const link = { return openTag('a', attrs); }, - close(state, mark, parent, index) { - if (isPlainURL(mark, parent, index, -1)) { - return '>'; + close(state, mark, parent) { + if (isAutoLink(mark, parent)) { + return isBracketAutoLink(mark.attrs.sourceMarkdown) ? '>' : ''; } const { canonicalSrc, href, title, sourceMarkdown } = mark.attrs; diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index a75262ee303..07316f9433a 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -19,8 +19,6 @@ import { scrollToElement } from '~/lib/utils/common_utils'; import { truncateSha } from '~/lib/utils/text_utility'; import { __, s__, sprintf } from '~/locale'; import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; -import FileIcon from '~/vue_shared/components/file_icon.vue'; -import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { DIFF_FILE_AUTOMATIC_COLLAPSE } from '../constants'; import { DIFF_FILE_HEADER } from '../i18n'; @@ -33,7 +31,6 @@ export default { components: { ClipboardButton, GlIcon, - FileIcon, DiffStats, GlBadge, GlButton, @@ -48,7 +45,7 @@ export default { GlTooltip: GlTooltipDirective, SafeHtml: GlSafeHtmlDirective, }, - mixins: [glFeatureFlagsMixin(), IdState({ idProp: (vm) => vm.diffFile.file_hash })], + mixins: [IdState({ idProp: (vm) => vm.diffFile.file_hash })], i18n: { ...DIFF_FILE_HEADER, compareButtonLabel: __('Compare submodule commit revisions'), @@ -301,14 +298,6 @@ export default { :href="titleLink" @click="handleFileNameClick" > - <file-icon - v-if="!glFeatures.removeDiffHeaderIcons" - :file-name="filePath" - :size="16" - aria-hidden="true" - css-classes="gl-mr-2" - :submodule="diffFile.submodule" - /> <span v-if="isFileRenamed"> <strong v-gl-tooltip diff --git a/app/assets/javascripts/graphql_shared/fragments/issuable_timelogs.fragment.graphql b/app/assets/javascripts/graphql_shared/fragments/issuable_timelogs.fragment.graphql index 824997f8e33..fb771d7ec8a 100644 --- a/app/assets/javascripts/graphql_shared/fragments/issuable_timelogs.fragment.graphql +++ b/app/assets/javascripts/graphql_shared/fragments/issuable_timelogs.fragment.graphql @@ -11,4 +11,7 @@ fragment TimelogFragment on Timelog { body } summary + userPermissions { + adminTimelog + } } diff --git a/app/assets/javascripts/sidebar/components/time_tracking/graphql/cache_update.js b/app/assets/javascripts/sidebar/components/time_tracking/graphql/cache_update.js new file mode 100644 index 00000000000..70177d84b1b --- /dev/null +++ b/app/assets/javascripts/sidebar/components/time_tracking/graphql/cache_update.js @@ -0,0 +1,20 @@ +import produce from 'immer'; + +export function removeTimelogFromStore(store, deletedTimelogId, query, variables) { + const sourceData = store.readQuery({ + query, + variables, + }); + + const data = produce(sourceData, (draftData) => { + draftData.issuable.timelogs.nodes = draftData.issuable.timelogs.nodes.filter( + ({ id }) => id !== deletedTimelogId, + ); + }); + + store.writeQuery({ + query, + variables, + data, + }); +} diff --git a/app/assets/javascripts/sidebar/components/time_tracking/graphql/mutations/delete_timelog.mutation.graphql b/app/assets/javascripts/sidebar/components/time_tracking/graphql/mutations/delete_timelog.mutation.graphql new file mode 100644 index 00000000000..17bbad1acb1 --- /dev/null +++ b/app/assets/javascripts/sidebar/components/time_tracking/graphql/mutations/delete_timelog.mutation.graphql @@ -0,0 +1,5 @@ +mutation deleteTimelog($input: TimelogDeleteInput!) { + timelogDelete(input: $input) { + errors + } +} diff --git a/app/assets/javascripts/sidebar/components/time_tracking/report.vue b/app/assets/javascripts/sidebar/components/time_tracking/report.vue index b4c4c31bd7a..79ef5a32474 100644 --- a/app/assets/javascripts/sidebar/components/time_tracking/report.vue +++ b/app/assets/javascripts/sidebar/components/time_tracking/report.vue @@ -1,11 +1,13 @@ <script> -import { GlLoadingIcon, GlTableLite } from '@gitlab/ui'; +import { GlLoadingIcon, GlTableLite, GlButton, GlTooltipDirective } from '@gitlab/ui'; import createFlash from '~/flash'; import { TYPE_ISSUE, TYPE_MERGE_REQUEST } from '~/graphql_shared/constants'; import { convertToGraphQLId } from '~/graphql_shared/utils'; import { formatDate, parseSeconds, stringifyTime } from '~/lib/utils/datetime_utility'; -import { __ } from '~/locale'; +import { __, s__ } from '~/locale'; import { timelogQueries } from '~/sidebar/constants'; +import deleteTimelogMutation from './graphql/mutations/delete_timelog.mutation.graphql'; +import { removeTimelogFromStore } from './graphql/cache_update'; const TIME_DATE_FORMAT = 'mmmm d, yyyy, HH:MM ("UTC:" o)'; @@ -13,6 +15,10 @@ export default { components: { GlLoadingIcon, GlTableLite, + GlButton, + }, + directives: { + GlTooltip: GlTooltipDirective, }, inject: ['issuableType'], props: { @@ -27,7 +33,7 @@ export default { }, }, data() { - return { report: [], isLoading: true }; + return { report: [], isLoading: true, removingIds: [] }; }, apollo: { report: { @@ -35,9 +41,7 @@ export default { return timelogQueries[this.issuableType].query; }, variables() { - return { - id: convertToGraphQLId(this.getGraphQLEntityType(), this.issuableId), - }; + return this.getQueryVariables(); }, update(data) { this.isLoading = false; @@ -48,10 +52,23 @@ export default { }, }, }, + computed: { + deleteButtonTooltip() { + return s__('TimeTracking|Delete time spent'); + }, + }, methods: { + isDeletingTimelog(timelogId) { + return this.removingIds.includes(timelogId); + }, isIssue() { return this.issuableType === 'issue'; }, + getQueryVariables() { + return { + id: convertToGraphQLId(this.getGraphQLEntityType(), this.issuableId), + }; + }, getGraphQLEntityType() { return this.isIssue() ? TYPE_ISSUE : TYPE_MERGE_REQUEST; }, @@ -76,12 +93,44 @@ export default { stringifyTime(parseSeconds(seconds, { limitToHours: this.limitToHours })) ); }, + deleteTimelog(timelogId) { + this.removingIds.push(timelogId); + this.$apollo + .mutate({ + mutation: deleteTimelogMutation, + variables: { input: { id: timelogId } }, + update: (store) => { + removeTimelogFromStore( + store, + timelogId, + timelogQueries[this.issuableType].query, + this.getQueryVariables(), + ); + }, + }) + .then(({ data }) => { + if (data.timelogDelete?.errors?.length) { + throw new Error(data.timelogDelete.errors[0]); + } + }) + .catch((error) => { + createFlash({ + message: s__('TimeTracking|An error occurred while removing the timelog.'), + captureError: true, + error, + }); + }) + .finally(() => { + this.removingIds.splice(this.removingIds.indexOf(timelogId), 1); + }); + }, }, fields: [ - { key: 'spentAt', label: __('Spent At'), sortable: true, tdClass: 'gl-w-quarter' }, + { key: 'spentAt', label: __('Spent at'), sortable: true, tdClass: 'gl-w-quarter' }, { key: 'user', label: __('User'), sortable: true }, - { key: 'timeSpent', label: __('Time Spent'), sortable: true, tdClass: 'gl-w-15' }, - { key: 'summary', label: __('Summary / Note'), sortable: true }, + { key: 'timeSpent', label: __('Time spent'), sortable: true, tdClass: 'gl-w-15' }, + { key: 'summary', label: __('Summary / note'), sortable: true }, + { key: 'actions', label: '', tdClass: 'gl-w-10' }, ], }; </script> @@ -110,7 +159,28 @@ export default { <template #cell(summary)="{ item: { summary, note } }"> <div>{{ getSummary(summary, note) }}</div> </template> - <template #foot(note)> </template> + <template #foot(summary)> </template> + + <template + #cell(actions)="{ + item: { + id, + userPermissions: { adminTimelog }, + }, + }" + > + <div v-if="adminTimelog"> + <gl-button + v-gl-tooltip="{ title: deleteButtonTooltip }" + category="secondary" + icon="remove" + data-testid="deleteButton" + :loading="isDeletingTimelog(id)" + @click="deleteTimelog(id)" + /> + </div> + </template> + <template #foot(actions)> </template> </gl-table-lite> </div> </template> diff --git a/app/assets/javascripts/sidebar/components/time_tracking/time_tracker.vue b/app/assets/javascripts/sidebar/components/time_tracking/time_tracker.vue index 057bb9f0100..e39d9f9fb49 100644 --- a/app/assets/javascripts/sidebar/components/time_tracking/time_tracker.vue +++ b/app/assets/javascripts/sidebar/components/time_tracking/time_tracker.vue @@ -252,6 +252,7 @@ export default { size="lg" :title="__('Time tracking report')" :hide-footer="true" + @hide="refresh" > <time-tracking-report :limit-to-hours="limitToHours" :issuable-id="issuableId" /> </gl-modal> diff --git a/app/assets/javascripts/vue_shared/components/web_ide_link.vue b/app/assets/javascripts/vue_shared/components/web_ide_link.vue index 15f84e48179..cac0d5a45c9 100644 --- a/app/assets/javascripts/vue_shared/components/web_ide_link.vue +++ b/app/assets/javascripts/vue_shared/components/web_ide_link.vue @@ -307,7 +307,7 @@ export default { <actions-button :actions="actions" :selected-key="selection" - :variant="isBlob ? 'info' : 'default'" + :variant="isBlob ? 'confirm' : 'default'" :category="isBlob ? 'primary' : 'secondary'" @select="select" /> diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index c1416e20aea..b63fd941a9b 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -200,11 +200,6 @@ $tabs-holder-z-index: 250; } } -.assign-to-me-link { - padding-left: 12px; - white-space: nowrap; -} - .table-holder { .ci-table { th { @@ -252,13 +247,6 @@ $tabs-holder-z-index: 250; } } -.merge-request-tabs { - display: flex; - flex-wrap: nowrap; - margin-bottom: 0; - padding: 0; -} - .limit-container-width { .merge-request-tabs-container { max-width: $limited-layout-width; @@ -274,9 +262,6 @@ $tabs-holder-z-index: 250; } .merge-request-tabs-container { - display: flex; - justify-content: space-between; - @include media-breakpoint-down(xs) { .discussion-filter-container { margin-bottom: $gl-padding-4; diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 5b4d79fab88..6dea73298a5 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -45,7 +45,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo push_frontend_feature_flag(:realtime_labels, project) push_frontend_feature_flag(:refactor_security_extension, @project) push_frontend_feature_flag(:mr_attention_requests, current_user) - push_frontend_feature_flag(:remove_diff_header_icons, project) push_frontend_feature_flag(:moved_mr_sidebar, project) push_frontend_feature_flag(:paginated_mr_discussions, project) end diff --git a/app/views/projects/merge_requests/creations/_new_submit.html.haml b/app/views/projects/merge_requests/creations/_new_submit.html.haml index 3c39c04d3a3..ef3174efcc7 100644 --- a/app/views/projects/merge_requests/creations/_new_submit.html.haml +++ b/app/views/projects/merge_requests/creations/_new_submit.html.haml @@ -18,11 +18,11 @@ = custom_icon ('illustration_no_commits') - else .merge-request-tabs-holder{ class: ("js-tabs-affix" unless ENV['RAILS_ENV'] == 'test') } - .merge-request-tabs-container + .merge-request-tabs-container.gl-display-flex.gl-justify-content-space-between .scrolling-tabs-container.inner-page-scroll-tabs.is-smaller .fade-left= sprite_icon('chevron-lg-left', size: 12) .fade-right= sprite_icon('chevron-lg-right', size: 12) - %ul.merge-request-tabs.nav.nav-tabs.nav-links.no-top.no-bottom.js-tabs-affix + %ul.merge-request-tabs.nav.nav-tabs.nav-links.no-top.no-bottom.gl-display-flex.gl-flex-nowrap.gl-m-0.gl-p-0.js-tabs-affix %li.commits-tab.new-tab = link_to url_for(safe_params), data: {target: 'div#commits', action: 'new', toggle: 'tabvue'} do Commits diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml index 0ac5410eec9..e0adf34c6a8 100644 --- a/app/views/projects/merge_requests/show.html.haml +++ b/app/views/projects/merge_requests/show.html.haml @@ -21,8 +21,8 @@ .merge-request-details.issuable-details{ data: { id: @merge_request.project.id } } = render "projects/merge_requests/mr_box" .merge-request-tabs-holder{ class: ("js-tabs-affix" unless ENV['RAILS_ENV'] == 'test') } - .merge-request-tabs-container{ class: "#{'is-merge-request' if Feature.enabled?(:moved_mr_sidebar, @project) && !fluid_layout}" } - %ul.merge-request-tabs.nav-tabs.nav.nav-links{ class: "#{'gl-w-full gl-lg-w-auto!' if Feature.enabled?(:moved_mr_sidebar, @project)}" } + .merge-request-tabs-container.gl-display-flex.gl-justify-content-space-between{ class: "#{'is-merge-request' if Feature.enabled?(:moved_mr_sidebar, @project) && !fluid_layout}" } + %ul.merge-request-tabs.nav-tabs.nav.nav-links.gl-display-flex.gl-flex-nowrap.gl-m-0.gl-p-0{ class: "#{'gl-w-full gl-lg-w-auto!' if Feature.enabled?(:moved_mr_sidebar, @project)}" } = render "projects/merge_requests/tabs/tab", class: "notes-tab", qa_selector: "notes_tab" do = tab_link_for @merge_request, :show, force_link: @commit.present? do = _("Overview") diff --git a/app/views/shared/issuable/form/_metadata_issuable_assignee.html.haml b/app/views/shared/issuable/form/_metadata_issuable_assignee.html.haml index 507c5a89649..f9c3c11eed8 100644 --- a/app/views/shared/issuable/form/_metadata_issuable_assignee.html.haml +++ b/app/views/shared/issuable/form/_metadata_issuable_assignee.html.haml @@ -8,4 +8,4 @@ = hidden_field_tag "#{issuable.to_ability_name}[assignee_ids][]", 0, id: nil, data: { meta: '' } = dropdown_tag(users_dropdown_label(issuable.assignees), options: assignees_dropdown_options(issuable.to_ability_name)) - = link_to _('Assign to me'), '#', class: "assign-to-me-link qa-assign-to-me-link #{'hide' if issuable.assignees.include?(current_user)}" + = link_to _('Assign to me'), '#', class: "assign-to-me-link gl-white-space-nowrap gl-pl-4 qa-assign-to-me-link #{'hide' if issuable.assignees.include?(current_user)}" diff --git a/config/feature_flags/development/remove_diff_header_icons.yml b/config/feature_flags/development/remove_diff_header_icons.yml deleted file mode 100644 index 213e911c414..00000000000 --- a/config/feature_flags/development/remove_diff_header_icons.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: remove_diff_header_icons -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/87289 -rollout_issue_url: -milestone: '15.0' -type: development -group: group::code review -default_enabled: true diff --git a/doc/ci/environments/protected_environments.md b/doc/ci/environments/protected_environments.md index 701c5355191..bd22d203c0b 100644 --- a/doc/ci/environments/protected_environments.md +++ b/doc/ci/environments/protected_environments.md @@ -127,20 +127,16 @@ they have the following privileges: ## Deployment-only access to protected environments Users granted access to a protected environment, but not push or merge access -to the branch deployed to it, are only granted access to deploy the environment. An individual in a -group with the Reporter role, or in groups added to the project with the Reporter -role, appears in the dropdown menu for deployment-only access. +to the branch deployed to it, are only granted access to deploy the environment. +[Invited groups](../../user/project/members/share_project_with_groups.md#share-a-project-with-a-group-of-users) added +to the project with [Reporter role](../../user/permissions.md#project-members-permissions), appear in the dropdown menu for deployment-only access. To add deployment-only access: -1. Add a group with the Reporter role. -1. Add users to the group. -1. Invite the group to be a project member. +1. Create a group with members who are granted to access to the protected environment, if it doesn't exist yet. +1. [Invite the group](../../user/project/members/share_project_with_groups.md#share-a-project-with-a-group-of-users) to the project with the Reporter role. 1. Follow the steps in [Protecting Environments](#protecting-environments). -Note that deployment-only access is the only possible access level for groups with the Reporter -role. - ## Modifying and unprotecting environments Maintainers can: diff --git a/doc/development/testing_guide/contract/consumer_tests.md b/doc/development/testing_guide/contract/consumer_tests.md new file mode 100644 index 00000000000..b4d6882a655 --- /dev/null +++ b/doc/development/testing_guide/contract/consumer_tests.md @@ -0,0 +1,308 @@ +--- +stage: none +group: Development +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + +# Writing consumer tests + +This tutorial guides you through writing a consumer test from scratch. To start, the consumer tests are written using [`jest-pact`](https://github.com/pact-foundation/jest-pact) that builds on top of [`pact-js`](https://github.com/pact-foundation/pact-js). This tutorial shows you how to write a consumer test for the `/discussions.json` endpoint, which is actually `/:namespace_name/:project_name/-/merge_requests/:id/discussions.json`. + +## Create the skeleton + +Start by creating the skeleton of a consumer test. Create a file under `spec/contracts/consumer/specs` called `discussions.spec.js`. +Then, populate it with the following function and parameters: + +- [`pactWith`](#the-pactwith-function) +- [`PactOptions`](#the-pactoptions-parameter) +- [`PactFn`](#the-pactfn-parameter) + +### The `pactWith` function + +The Pact consumer test is defined through the `pactWith` function that takes `PactOptions` and the `PactFn`. + +```javascript +const { pactWith } = require('jest-pact'); + +pactWith(PactOptions, PactFn); +``` + +### The `PactOptions` parameter + +`PactOptions` with `jest-pact` introduces [additional options](https://github.com/pact-foundation/jest-pact/blob/dce370c1ab4b7cb5dff12c4b62246dc229c53d0e/README.md#defaults) that build on top of the ones [provided in `pact-js`](https://github.com/pact-foundation/pact-js#constructor). In most cases, you define the `consumer`, `provider`, `log`, and `dir` options for these tests. + +```javascript +const { pactWith } = require('jest-pact'); + +pactWith( + { + consumer: 'Merge Request Page', + provider: 'Merge Request Discussions Endpoint', + log: '../logs/consumer.log', + dir: '../contracts', + }, + PactFn +); +``` + +### The `PactFn` parameter + +The `PactFn` is where your tests are defined. This is where you set up the mock provider and where you can use the standard Jest methods like [`Jest.describe`](https://jestjs.io/docs/api#describename-fn), [`Jest.beforeEach`](https://jestjs.io/docs/api#beforeeachfn-timeout), and [`Jest.it`](https://jestjs.io/docs/api#testname-fn-timeout). For more information, see [https://jestjs.io/docs/api](https://jestjs.io/docs/api). + +```javascript +const { pactWith } = require('jest-pact'); + +pactWith( + { + consumer: 'Merge Request Page', + provider: 'Merge Request Discussions Endpoint', + log: '../logs/consumer.log', + dir: '../contracts', + }, + + (provider) => { + describe('Discussions Endpoint', () => { + beforeEach(() => { + + }); + + it('return a successful body', () => { + + }); + }); + }, +); +``` + +## Set up the mock provider + +Before you run your test, set up the mock provider that handles the specified requests and returns a specified response. To do that, define the state and the expected request and response in an [`Interaction`](https://github.com/pact-foundation/pact-js/blob/master/src/dsl/interaction.ts). + +For this tutorial, define four attributes for the `Interaction`: + +1. `state`: A description of what the prerequisite state is before the request is made. +1. `uponReceiving`: A description of what kind of request this `Interaction` is handling. +1. `withRequest`: Where you define the request specifications. It contains the request `method`, `path`, and any `headers`, `body`, or `query`. +1. `willRespondWith`: Where you define the expected response. It contains the response `status`, `headers`, and `body`. + +After you define the `Interaction`, add that interaction to the mock provider by calling `addInteraction`. + +```javascript +const { pactWith } = require('jest-pact'); +const { Matchers } = require('@pact-foundation/pact'); + +pactWith( + { + consumer: 'Merge Request Page', + provider: 'Merge Request Discussions Endpoint', + log: '../logs/consumer.log', + dir: '../contracts', + }, + + (provider) => { + describe('Discussions Endpoint', () => { + beforeEach(() => { + const interaction = { + state: 'a merge request with discussions exists', + uponReceiving: 'a request for discussions', + withRequest: { + method: 'GET', + path: '/gitlab-org/gitlab-qa/-/merge_requests/1/discussions.json', + headers: { + Accept: '*/*', + }, + }, + willRespondWith: { + status: 200, + headers: { + 'Content-Type': 'application/json; charset=utf-8', + }, + body: Matchers.eachLike({ + id: Matchers.string('fd73763cbcbf7b29eb8765d969a38f7d735e222a'), + project_id: Matchers.integer(6954442), + ... + resolved: Matchers.boolean(true) + }), + }, + }; + provider.addInteraction(interaction); + }); + + it('return a successful body', () => { + + }); + }); + }, +); +``` + +### Response body `Matchers` + +Notice how we use `Matchers` in the `body` of the expected response. This allows us to be flexible enough to accept different values but still be strict enough to distinguish between valid and invalid values. We must ensure that we have a tight definition that is neither too strict nor too lax. Read more about the [different types of `Matchers`](https://github.com/pact-foundation/pact-js#using-the-v3-matching-rules). + +## Write the test + +After the mock provider is set up, you can write the test. For this test, you make a request and expect a particular response. + +First, set up the client that makes the API request. To do that, either create or find an existing file under `spec/contracts/consumer/endpoints` and add the following API request. + +```javascript +const axios = require('axios'); + +exports.getDiscussions = (endpoint) => { + const url = endpoint.url; + + return axios + .request({ + method: 'GET', + baseURL: url, + url: '/gitlab-org/gitlab-qa/-/merge_requests/1/discussions.json', + headers: { Accept: '*/*' }, + }) + .then((response) => response.data); +}; +``` + +After that's set up, import it to the test file and call it to make the request. Then, you can make the request and define your expectations. + +```javascript +const { pactWith } = require('jest-pact'); +const { Matchers } = require('@pact-foundation/pact'); + +const { getDiscussions } = require('../endpoints/merge_requests'); + +pactWith( + { + consumer: 'Merge Request Page', + provider: 'Merge Request Discussions Endpoint', + log: '../logs/consumer.log', + dir: '../contracts', + }, + + (provider) => { + describe('Discussions Endpoint', () => { + beforeEach(() => { + const interaction = { + state: 'a merge request with discussions exists', + uponReceiving: 'a request for discussions', + withRequest: { + method: 'GET', + path: '/gitlab-org/gitlab-qa/-/merge_requests/1/discussions.json', + headers: { + Accept: '*/*', + }, + }, + willRespondWith: { + status: 200, + headers: { + 'Content-Type': 'application/json; charset=utf-8', + }, + body: Matchers.eachLike({ + id: Matchers.string('fd73763cbcbf7b29eb8765d969a38f7d735e222a'), + project_id: Matchers.integer(6954442), + ... + resolved: Matchers.boolean(true) + }), + }, + }; + }); + + it('return a successful body', () => { + return getDiscussions({ + url: provider.mockService.baseUrl, + }).then((discussions) => { + expect(discussions).toEqual(Matchers.eachLike({ + id: 'fd73763cbcbf7b29eb8765d969a38f7d735e222a', + project_id: 6954442, + ... + resolved: true + })); + }); + }); + }); + }, +); +``` + +There we have it! The consumer test is now set up. You can now try [running this test](index.md#run-the-consumer-tests). + +## Improve test readability + +As you may have noticed, the request and response definitions can get large. This results in the test being difficult to read, with a lot of scrolling to find what you want. You can make the test easier to read by extracting these out to a `fixture`. + +Create a file under `spec/contracts/consumer/fixtures` called `discussions.fixture.js`. You place the `request` and `response` definitions here. + +```javascript +const { Matchers } = require('@pact-foundation/pact'); + +const body = Matchers.eachLike({ + id: Matchers.string('fd73763cbcbf7b29eb8765d969a38f7d735e222a'), + project_id: Matchers.integer(6954442), + ... + resolved: Matchers.boolean(true) +}); + +const Discussions = { + body: Matchers.extractPayload(body), + + success: { + status: 200, + headers: { + 'Content-Type': 'application/json; charset=utf-8', + }, + body: body, + }, + + request: { + uponReceiving: 'a request for discussions', + withRequest: { + method: 'GET', + path: '/gitlab-org/gitlab-qa/-/merge_requests/1/discussions.json', + headers: { + Accept: '*/*', + }, + }, + }, +}; + +exports.Discussions = Discussions; +``` + +With all of that moved to the `fixture`, you can simplify the test to the following: + +```javascript +const { pactWith } = require('jest-pact'); + +const { Discussions } = require('../fixtures/discussions.fixture'); +const { getDiscussions } = require('../endpoints/merge_requests'); + +pactWith( + { + consumer: 'Merge Request Page', + provider: 'Merge Request Discussions Endpoint', + log: '../logs/consumer.log', + dir: '../contracts', + }, + + (provider) => { + describe('Discussions Endpoint', () => { + beforeEach(() => { + const interaction = { + state: 'a merge request with discussions exists', + ...Discussions.request, + willRespondWith: Discussions.success, + }; + return provider.addInteraction(interaction); + }); + + it('return a successful body', () => { + return getDiscussions({ + url: provider.mockService.baseUrl, + }).then((discussions) => { + expect(discussions).toEqual(Discussions.body); + }); + }); + }); + }, +); +``` diff --git a/doc/development/testing_guide/contract/index.md b/doc/development/testing_guide/contract/index.md new file mode 100644 index 00000000000..6556bd85624 --- /dev/null +++ b/doc/development/testing_guide/contract/index.md @@ -0,0 +1,39 @@ +--- +stage: none +group: Development +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + +# Contract testing + +Contract tests consist of two parts — consumer tests and provider tests. A simple example of a consumer and provider relationship is between the frontend and backend. The frontend would be the consumer and the backend is the provider. The frontend consumes the API that is provided by the backend. The test helps ensure that these two sides follow an agreed upon contract and any divergence from the contract triggers a meaningful conversation to prevent breaking changes from slipping through. + +Consumer tests are similar to unit tests with each spec defining a requests and an expected mock responses and creating a contract based on those definitions. On the other hand, provider tests are similar to integration tests as each spec takes the request defined in the contract and runs that request against the actual service which is then matched against the contract to validate the contract. + +You can check out the existing contract tests at: + +- [`spec/contracts/consumer/specs`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/spec/contracts/consumer/specs) for the consumer tests. +- [`spec/contracts/provider/specs`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/spec/contracts/provider/specs) for the provider tests. + +The contracts themselves are stored in [`/spec/contracts/contracts`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/spec/contracts/contracts) at the moment. The plan is to use [PactBroker](https://docs.pact.io/pact_broker/docker_images) hosted in AWS or another similar service. + +## Write the tests + +- [Writing consumer tests](consumer_tests.md) +- [Writing provider tests](provider_tests.md) + +### Run the consumer tests + +Before running the consumer tests, go to `spec/contracts/consumer` and run `npm install`. To run all the consumer tests, you just need to run `npm test -- /specs`. Otherwise, to run a specific spec file, replace `/specs` with the specific spec filename. + +### Run the provider tests + +Before running the provider tests, make sure your GDK (GitLab Development Kit) is fully set up and running. You can follow the setup instructions detailed in the [GDK repository](https://gitlab.com/gitlab-org/gitlab-development-kit/-/tree/main). To run the provider tests, you use Rake tasks that are defined in [`./lib/tasks/contracts.rake`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/lib/tasks/contracts.rake). To get a list of all the Rake tasks related to the provider tests, run `bundle exec rake -T contracts`. For example: + +```shell +$ bundle exec rake -T contracts +rake contracts:mr:pact:verify:diffs # Verify provider against the consumer pacts for diffs +rake contracts:mr:pact:verify:discussions # Verify provider against the consumer pacts for discussions +rake contracts:mr:pact:verify:metadata # Verify provider against the consumer pacts for metadata +rake contracts:mr:test:merge_request[contract_mr] # Run all merge request contract tests +``` diff --git a/doc/development/testing_guide/contract/provider_tests.md b/doc/development/testing_guide/contract/provider_tests.md new file mode 100644 index 00000000000..0da5bcb4aef --- /dev/null +++ b/doc/development/testing_guide/contract/provider_tests.md @@ -0,0 +1,177 @@ +--- +stage: none +group: Development +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + +# Writing provider tests + +This tutorial guides you through writing a provider test from scratch. It is a continuation of the [consumer test tutorial](consumer_tests.md). To start, the provider tests are written using [`pact-ruby`](https://github.com/pact-foundation/pact-ruby). In this tutorial, you write a provider test that addresses the contract generated by `discussions.spec.js`. + +## Create the skeleton + +Provider tests are quite simple. The goal is to set up the test data and then link that with the corresponding contract. Start by creating a file called `discussions_helper.rb` under `spec/contracts/provider/specs`. Note that the files are called `helpers` to match how they are called by Pact in the Rake tasks, which are set up at the end of this tutorial. + +### The `service_provider` block + +The `service_provider` block is where the provider test is defined. For this block, put in a description of the service provider. Name it exactly as it is called in the contracts that are derived from the consumer tests. + +```ruby +require_relative '../spec_helper' + +module Provider + module DiscussionsHelper + Pact.service_provider 'Merge Request Discussions Endpoint' do + + end + end +end +``` + +### The `honours_pact_with` block + +The `honours_pact_with` block describes which consumer this provider test is addressing. Similar to the `service_provider` block, name this exactly the same as it's called in the contracts that are derived from the consumer tests. + +```ruby +require_relative '../spec_helper' + +module Provider + module DiscussionsHelper + Pact.service_provider 'Merge Request Discussions Endpoint' do + honours_pact_with 'Merge Request Page' do + + end + end + end +end +``` + +## Configure the test app + +For the provider tests to verify the contracts, you must hook it up to a test app that makes the actual request and return a response to verify against the contract. To do this, configure the `app` the test uses as `Environment::Test.app`, which is defined in [`spec/contracts/provider/environments/test.rb`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/spec/contracts/provider/environments/test.rb). + +```ruby +require_relative '../spec_helper' + +module Provider + module DiscussionsHelper + Pact.service_provider 'Merge Request Discussions Endpoint' do + app { Environment::Test.app } + + honours_pact_with 'Merge Request Page' do + + end + end + end +end +``` + +## Define the contract to verify + +Now that the test app is configured, all that is left is to define which contract this provider test is verifying. To do this, set the `pact_uri`. + +```ruby +require_relative '../spec_helper' + +module Provider + module DiscussionsHelper + Pact.service_provider 'Merge Request Discussions Endpoint' do + app { Environment::Test.app } + + honours_pact_with 'Merge Request Page' do + pact_uri '../contracts/merge_request_page-merge_request_discussions_endpoint.json' + end + end + end +end +``` + +## Add / update the Rake tasks + +Now that you have a test created, you must create Rake tasks that run this test. The Rake tasks are defined in [`lib/tasks/contracts.rake`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/lib/tasks/contracts.rake) where we have individual Rake tasks to run individual specs, but also Rake tasks that run a group of tests. + +Under the `contracts:mr` namespace, introduce the Rake task to run this new test specifically. In it, call `pact.uri` to define the location of the contract and the provider test that tests that contract. Notice here that `pact_uri` has a parameter called `pact_helper`. This is why the provider tests are called `_helper.rb`. + +```ruby +Pact::VerificationTask.new(:discussions) do |pact| + pact.uri( + "#{contracts}/contracts/merge_request_page-merge_request_discussions_endpoint.json", + pact_helper: "#{provider}/specs/discussions_helper.rb" + ) +end +``` + +At the same time, add your new `:discussions` Rake task to be included in the `test:merge_request` Rake task. In that Rake task, there is an array defined (`%w[metadata diffs]`). You must add `discussions` in that list. + +## Create test data + +As the last step, create the test data that allows the provider test to return the contract's expected response. You might wonder why you create the test data last. It's really a matter of preference. With the test already configured, you can easily run the test to verify and make sure all the necessary test data are created to produce the expected response. + +You can read more about [provider states](https://docs.pact.io/implementation_guides/ruby/provider_states). We can do global provider states but for this tutorial, the provider state is for one specific `state`. + +To create the test data, create `discussions_state.rb` under `spec/contracts/provider/states`. As a quick aside, make sure to also import this state file in the `discussions_helper.rb` file. + +### Default user in `spec/contracts/provider/spec_helper.rb` + +Before you create the test data, note that a default user is created in the [`spec_helper`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/spec/contracts/provider/spec_helper.rb), which is the user being used for the test runs. This user is configured using `RSpec.configure`, as Pact actually is built on top of RSpec. This step allows us to configure the user before any of the test runs. + +```ruby +RSpec.configure do |config| + config.include Devise::Test::IntegrationHelpers + config.before do + user = FactoryBot.create(:user, name: "Contract Test").tap do |user| + user.current_sign_in_at = Time.current + end + sign_in user + end +end +``` + +Any further modifications to the user that's needed can be done through the individual provider state files. + +### The `provider_states_for` block + +In the state file, you must define which consumer this provider state is for. You can do that with `provider_states_for`. Make sure that the `name` provided matches the name defined for the consumer. + +```ruby +Pact.provider_states_for 'Merge Request Page' do +end +``` + +### The `provider_state` block + +In the `provider_states_for` block, you then define the state the test data is for. These states are also defined in the consumer test. In this case, there is a `'a merge request with discussions exists'` state. + +```ruby +Pact.provider_states_for "Merge Request Page" do + provider_state "a merge request with discussions exists" do + + end +end +``` + +### The `set_up` block + +This is where you define the test data creation steps. Use `FactoryBot` to create the data. As you create the test data, you can keep [running the provider test](index.md#run-the-provider-tests) to check on the status of the test and figure out what else is missing in your data setup. + +```ruby +Pact.provider_states_for "Merge Request Page" do + provider_state "a merge request with discussions exists" do + set_up do + user = User.find_by(name: Provider::UsersHelper::CONTRACT_USER_NAME) + namespace = create(:namespace, name: 'gitlab-org') + project = create(:project, name: 'gitlab-qa', namespace: namespace) + + project.add_maintainer(user) + + merge_request = create(:merge_request_with_diffs, id: 1, source_project: project, author: user) + + create(:discussion_note_on_merge_request, noteable: merge_request, project: project, author: user) + end + end +end +``` + +Note the `Provider::UsersHelper::CONTRACT_USER_NAME` here to fetch a user is a user that is from the [`spec_helper`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/spec/contracts/provider/spec_helper.rb) that sets up a user before any of these tests run. + +And with that, the provider tests for `discussion_helper.rb` should now pass with this. diff --git a/doc/development/testing_guide/index.md b/doc/development/testing_guide/index.md index 2e00a00c454..fa9f1f1ac3e 100644 --- a/doc/development/testing_guide/index.md +++ b/doc/development/testing_guide/index.md @@ -70,4 +70,8 @@ Everything you should know about how to run end-to-end tests using Everything you should know about how to test migrations. +## [Contract tests](contract/index.md) + +Introduction to contract testing, how to run the tests, and how to write them. + [Return to Development documentation](../index.md) diff --git a/doc/user/project/img/time_tracking_report_v13_12.png b/doc/user/project/img/time_tracking_report_v13_12.png Binary files differdeleted file mode 100644 index 2132ca01cf4..00000000000 --- a/doc/user/project/img/time_tracking_report_v13_12.png +++ /dev/null diff --git a/doc/user/project/img/time_tracking_report_v15_1.png b/doc/user/project/img/time_tracking_report_v15_1.png Binary files differnew file mode 100644 index 00000000000..a9ddefebb2f --- /dev/null +++ b/doc/user/project/img/time_tracking_report_v15_1.png diff --git a/doc/user/project/time_tracking.md b/doc/user/project/time_tracking.md index 0891e02f3f7..971ecf66a3c 100644 --- a/doc/user/project/time_tracking.md +++ b/doc/user/project/time_tracking.md @@ -102,7 +102,7 @@ type `/spend 1h 2021-01-31`. If you type a future date, no time is logged. -### Remove time spent +### Subtract time spent Prerequisites: @@ -112,11 +112,10 @@ To subtract time, enter a negative value. For example, `/spend -3d` removes thre days from the total time spent. You can't go below 0 minutes of time spent, so if you remove more time than already entered, GitLab ignores the subtraction. -To remove all the time spent at once, use the `/remove_time_spent` [quick action](quick_actions.md). - ### Delete time spent -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/356796) in GitLab 15.0. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/356796) in GitLab 14.10. +> - Delete button [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/356796) in GitLab 15.1. A timelog is a single entry of time spent, either positive or negative. @@ -124,7 +123,18 @@ Prerequisites: - You must be the author of the timelog or have at least the Maintainer role for the project. -You can [delete timelogs](../../api/graphql/reference/index.md#mutationtimelogdelete) using the GraphQL API. +To delete a timelog, either: + +- In the time tracking report, on the right of a timelog entry, select **Delete time spent** (**{remove}**). +- Use the [GraphQL API](../../api/graphql/reference/index.md#mutationtimelogdelete). + +### Delete all the time spent + +Prerequisites: + +- You must have at least the Reporter role for the project. + +To delete all the time spent at once, use the `/remove_time_spent` [quick action](quick_actions.md). ## View a time tracking report @@ -137,7 +147,7 @@ To view a time tracking report: 1. Go to an issue or a merge request. 1. In the right sidebar, select **Time tracking report**. - + The breakdown of spent time is limited to a maximum of 100 entries. diff --git a/locale/gitlab.pot b/locale/gitlab.pot index c91b8f943d0..e04f347bb2f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -36241,7 +36241,7 @@ msgstr "" msgid "Speed up your pipelines with Needs relationships" msgstr "" -msgid "Spent At" +msgid "Spent at" msgstr "" msgid "Squash commit message" @@ -36994,7 +36994,7 @@ msgstr "" msgid "Summary" msgstr "" -msgid "Summary / Note" +msgid "Summary / note" msgstr "" msgid "Sunday" @@ -39378,9 +39378,6 @@ msgstr "" msgid "Time (in hours) that users are allowed to skip forced configuration of two-factor authentication." msgstr "" -msgid "Time Spent" -msgstr "" - msgid "Time based: Yes" msgstr "" @@ -39444,6 +39441,12 @@ msgstr "" msgid "TimeTracking|%{spentStart}Spent: %{spentEnd}" msgstr "" +msgid "TimeTracking|An error occurred while removing the timelog." +msgstr "" + +msgid "TimeTracking|Delete time spent" +msgstr "" + msgid "TimeTracking|Estimated:" msgstr "" diff --git a/qa/qa/specs/features/browser_ui/5_package/package_registry/generic_repository_spec.rb b/qa/qa/specs/features/browser_ui/5_package/package_registry/generic_repository_spec.rb index e37102c17f7..677b8970a75 100644 --- a/qa/qa/specs/features/browser_ui/5_package/package_registry/generic_repository_spec.rb +++ b/qa/qa/specs/features/browser_ui/5_package/package_registry/generic_repository_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module QA - RSpec.describe 'Package', :orchestrated, :packages, :object_storage do + RSpec.describe 'Package', :orchestrated, :packages, :object_storage, :reliable do describe 'Generic Repository' do include Runtime::Fixtures diff --git a/spec/contracts/README.md b/spec/contracts/README.md new file mode 100644 index 00000000000..d1f902064f1 --- /dev/null +++ b/spec/contracts/README.md @@ -0,0 +1,15 @@ +# Contract testing for GitLab + +This directory contains the contract test suites for GitLab, which use the [Pact](https://pact.io/) framework. + +The consumer tests are written using [`jest-pact`](https://github.com/pact-foundation/jest-pact) and the provider tests are written using [`pact-ruby`](https://github.com/pact-foundation/pact-ruby). + +## Write the tests + +- [Writing consumer tests](../../doc/development/testing_guide/contract/consumer_tests.md) +- [Writing provider tests](../../doc/development/testing_guide/contract/provider_tests.md) + +### Run the tests + +- [Running consumer tests](../../doc/development/testing_guide/contract/index.md#run-the-consumer-tests) +- [Running provider tests](../../doc/development/testing_guide/contract/index.md#run-the-provider-tests) diff --git a/spec/frontend/content_editor/remark_markdown_processing_spec.js b/spec/frontend/content_editor/remark_markdown_processing_spec.js index fce08097f58..cbe809a0788 100644 --- a/spec/frontend/content_editor/remark_markdown_processing_spec.js +++ b/spec/frontend/content_editor/remark_markdown_processing_spec.js @@ -313,6 +313,73 @@ describe('Client side Markdown processing', () => { ), }, { + markdown: 'www.commonmark.org', + expectedDoc: doc( + paragraph( + sourceAttrs('0:18', 'www.commonmark.org'), + link( + { + ...sourceAttrs('0:18', 'www.commonmark.org'), + href: 'http://www.commonmark.org', + }, + 'www.commonmark.org', + ), + ), + ), + }, + { + markdown: 'Visit www.commonmark.org/help for more information.', + expectedDoc: doc( + paragraph( + sourceAttrs('0:51', 'Visit www.commonmark.org/help for more information.'), + 'Visit ', + link( + { + ...sourceAttrs('6:29', 'www.commonmark.org/help'), + href: 'http://www.commonmark.org/help', + }, + 'www.commonmark.org/help', + ), + ' for more information.', + ), + ), + }, + { + markdown: 'hello@mail+xyz.example isn’t valid, but hello+xyz@mail.example is.', + expectedDoc: doc( + paragraph( + sourceAttrs('0:66', 'hello@mail+xyz.example isn’t valid, but hello+xyz@mail.example is.'), + 'hello@mail+xyz.example isn’t valid, but ', + link( + { + ...sourceAttrs('40:62', 'hello+xyz@mail.example'), + href: 'mailto:hello+xyz@mail.example', + }, + 'hello+xyz@mail.example', + ), + ' is.', + ), + ), + }, + { + only: true, + markdown: '[https://gitlab.com>', + expectedDoc: doc( + paragraph( + sourceAttrs('0:20', '[https://gitlab.com>'), + '[', + link( + { + ...sourceAttrs(), + href: 'https://gitlab.com', + }, + 'https://gitlab.com', + ), + '>', + ), + ), + }, + { markdown: ` This is a paragraph with a\\ hard line break`, diff --git a/spec/frontend/content_editor/services/markdown_serializer_spec.js b/spec/frontend/content_editor/services/markdown_serializer_spec.js index ba869cdd86a..13e9efaea59 100644 --- a/spec/frontend/content_editor/services/markdown_serializer_spec.js +++ b/spec/frontend/content_editor/services/markdown_serializer_spec.js @@ -196,7 +196,7 @@ describe('markdownSerializer', () => { it('correctly serializes a plain URL link', () => { expect(serialize(paragraph(link({ href: 'https://example.com' }, 'https://example.com')))).toBe( - '<https://example.com>', + 'https://example.com', ); }); @@ -1155,46 +1155,63 @@ Oranges are orange [^1] ); }); + const defaultEditAction = (initialContent) => { + tiptapEditor.chain().setContent(initialContent.toJSON()).insertContent(' modified').run(); + }; + + const prependContentEditAction = (initialContent) => { + tiptapEditor + .chain() + .setContent(initialContent.toJSON()) + .setTextSelection(0) + .insertContent('modified ') + .run(); + }; + it.each` - mark | content | modifiedContent - ${'bold'} | ${'**bold**'} | ${'**bold modified**'} - ${'bold'} | ${'__bold__'} | ${'__bold modified__'} - ${'bold'} | ${'<strong>bold</strong>'} | ${'<strong>bold modified</strong>'} - ${'bold'} | ${'<b>bold</b>'} | ${'<b>bold modified</b>'} - ${'italic'} | ${'_italic_'} | ${'_italic modified_'} - ${'italic'} | ${'*italic*'} | ${'*italic modified*'} - ${'italic'} | ${'<em>italic</em>'} | ${'<em>italic modified</em>'} - ${'italic'} | ${'<i>italic</i>'} | ${'<i>italic modified</i>'} - ${'link'} | ${'[gitlab](https://gitlab.com)'} | ${'[gitlab modified](https://gitlab.com)'} - ${'link'} | ${'<a href="https://gitlab.com">link</a>'} | ${'<a href="https://gitlab.com">link modified</a>'} - ${'code'} | ${'`code`'} | ${'`code modified`'} - ${'code'} | ${'<code>code</code>'} | ${'<code>code modified</code>'} - ${'strike'} | ${'~~striked~~'} | ${'~~striked modified~~'} - ${'strike'} | ${'<del>striked</del>'} | ${'<del>striked modified</del>'} - ${'strike'} | ${'<strike>striked</strike>'} | ${'<strike>striked modified</strike>'} - ${'strike'} | ${'<s>striked</s>'} | ${'<s>striked modified</s>'} - ${'list'} | ${'- list item'} | ${'- list item modified'} - ${'list'} | ${'* list item'} | ${'* list item modified'} - ${'list'} | ${'+ list item'} | ${'+ list item modified'} - ${'list'} | ${'- list item 1\n- list item 2'} | ${'- list item 1\n- list item 2 modified'} - ${'list'} | ${'2) list item'} | ${'2) list item modified'} - ${'list'} | ${'1. list item'} | ${'1. list item modified'} - ${'taskList'} | ${'2) [ ] task list item'} | ${'2) [ ] task list item modified'} - ${'taskList'} | ${'2) [x] task list item'} | ${'2) [x] task list item modified'} + mark | content | modifiedContent | editAction + ${'bold'} | ${'**bold**'} | ${'**bold modified**'} | ${defaultEditAction} + ${'bold'} | ${'__bold__'} | ${'__bold modified__'} | ${defaultEditAction} + ${'bold'} | ${'<strong>bold</strong>'} | ${'<strong>bold modified</strong>'} | ${defaultEditAction} + ${'bold'} | ${'<b>bold</b>'} | ${'<b>bold modified</b>'} | ${defaultEditAction} + ${'italic'} | ${'_italic_'} | ${'_italic modified_'} | ${defaultEditAction} + ${'italic'} | ${'*italic*'} | ${'*italic modified*'} | ${defaultEditAction} + ${'italic'} | ${'<em>italic</em>'} | ${'<em>italic modified</em>'} | ${defaultEditAction} + ${'italic'} | ${'<i>italic</i>'} | ${'<i>italic modified</i>'} | ${defaultEditAction} + ${'link'} | ${'[gitlab](https://gitlab.com)'} | ${'[gitlab modified](https://gitlab.com)'} | ${defaultEditAction} + ${'link'} | ${'<a href="https://gitlab.com">link</a>'} | ${'<a href="https://gitlab.com">link modified</a>'} | ${defaultEditAction} + ${'link'} | ${'link www.gitlab.com'} | ${'modified link www.gitlab.com'} | ${prependContentEditAction} + ${'link'} | ${'link https://www.gitlab.com'} | ${'modified link https://www.gitlab.com'} | ${prependContentEditAction} + ${'link'} | ${'link(https://www.gitlab.com)'} | ${'modified link(https://www.gitlab.com)'} | ${prependContentEditAction} + ${'link'} | ${'link(engineering@gitlab.com)'} | ${'modified link(engineering@gitlab.com)'} | ${prependContentEditAction} + ${'link'} | ${'link <https://www.gitlab.com>'} | ${'modified link <https://www.gitlab.com>'} | ${prependContentEditAction} + ${'link'} | ${'link [https://www.gitlab.com>'} | ${'modified link \\[https://www.gitlab.com>'} | ${prependContentEditAction} + ${'link'} | ${'link <https://www.gitlab.com'} | ${'modified link <https://www.gitlab.com'} | ${prependContentEditAction} + ${'link'} | ${'link https://www.gitlab.com>'} | ${'modified link https://www.gitlab.com>'} | ${prependContentEditAction} + ${'link'} | ${'link **https://www.gitlab.com]**'} | ${'modified link [**https://www.gitlab.com\\]**](https://www.gitlab.com%5D)'} | ${prependContentEditAction} + ${'code'} | ${'`code`'} | ${'`code modified`'} | ${defaultEditAction} + ${'code'} | ${'<code>code</code>'} | ${'<code>code modified</code>'} | ${defaultEditAction} + ${'strike'} | ${'~~striked~~'} | ${'~~striked modified~~'} | ${defaultEditAction} + ${'strike'} | ${'<del>striked</del>'} | ${'<del>striked modified</del>'} | ${defaultEditAction} + ${'strike'} | ${'<strike>striked</strike>'} | ${'<strike>striked modified</strike>'} | ${defaultEditAction} + ${'strike'} | ${'<s>striked</s>'} | ${'<s>striked modified</s>'} | ${defaultEditAction} + ${'list'} | ${'- list item'} | ${'- list item modified'} | ${defaultEditAction} + ${'list'} | ${'* list item'} | ${'* list item modified'} | ${defaultEditAction} + ${'list'} | ${'+ list item'} | ${'+ list item modified'} | ${defaultEditAction} + ${'list'} | ${'- list item 1\n- list item 2'} | ${'- list item 1\n- list item 2 modified'} | ${defaultEditAction} + ${'list'} | ${'2) list item'} | ${'2) list item modified'} | ${defaultEditAction} + ${'list'} | ${'1. list item'} | ${'1. list item modified'} | ${defaultEditAction} + ${'taskList'} | ${'2) [ ] task list item'} | ${'2) [ ] task list item modified'} | ${defaultEditAction} + ${'taskList'} | ${'2) [x] task list item'} | ${'2) [x] task list item modified'} | ${defaultEditAction} `( 'preserves original $mark syntax when sourceMarkdown is available for $content', - async ({ content, modifiedContent }) => { + async ({ content, modifiedContent, editAction }) => { const { document } = await remarkMarkdownDeserializer().deserialize({ schema: tiptapEditor.schema, content, }); - tiptapEditor - .chain() - .setContent(document.toJSON()) - // changing the document ensures that block preservation doesn’t yield false positives - .insertContent(' modified') - .run(); + editAction(document); const serialized = markdownSerializer({}).serialize({ pristineDoc: document, diff --git a/spec/frontend/diffs/components/diff_file_header_spec.js b/spec/frontend/diffs/components/diff_file_header_spec.js index f22bd312a6d..d90afeb6b82 100644 --- a/spec/frontend/diffs/components/diff_file_header_spec.js +++ b/spec/frontend/diffs/components/diff_file_header_spec.js @@ -14,7 +14,6 @@ import { scrollToElement } from '~/lib/utils/common_utils'; import { truncateSha } from '~/lib/utils/text_utility'; import { __, sprintf } from '~/locale'; import ClipboardButton from '~/vue_shared/components/clipboard_button.vue'; -import FileIcon from '~/vue_shared/components/file_icon.vue'; import testAction from '../../__helpers__/vuex_action_helper'; import diffDiscussionsMockData from '../mock_data/diff_discussions'; @@ -224,16 +223,6 @@ describe('DiffFileHeader component', () => { }); expect(findFileActions().exists()).toBe(false); }); - - it('renders submodule icon', () => { - createComponent({ - props: { - diffFile: submoduleDiffFile, - }, - }); - - expect(wrapper.find(FileIcon).props('submodule')).toBe(true); - }); }); describe('for any file', () => { diff --git a/spec/frontend/sidebar/components/time_tracking/mock_data.js b/spec/frontend/sidebar/components/time_tracking/mock_data.js index ba2781118d9..f161ae677d0 100644 --- a/spec/frontend/sidebar/components/time_tracking/mock_data.js +++ b/spec/frontend/sidebar/components/time_tracking/mock_data.js @@ -1,3 +1,5 @@ +export const timelogToRemoveId = 'gid://gitlab/Timelog/18'; + export const getIssueTimelogsQueryResponse = { data: { issuable: { @@ -9,7 +11,7 @@ export const getIssueTimelogsQueryResponse = { nodes: [ { __typename: 'Timelog', - id: 'gid://gitlab/Timelog/18', + id: timelogToRemoveId, timeSpent: 14400, user: { id: 'user-1', @@ -23,6 +25,10 @@ export const getIssueTimelogsQueryResponse = { __typename: 'Note', }, summary: 'A summary', + userPermissions: { + adminTimelog: true, + __typename: 'TimelogPermissions', + }, }, { __typename: 'Timelog', @@ -36,6 +42,10 @@ export const getIssueTimelogsQueryResponse = { spentAt: '2021-05-07T13:19:01Z', note: null, summary: 'A summary', + userPermissions: { + adminTimelog: false, + __typename: 'TimelogPermissions', + }, }, { __typename: 'Timelog', @@ -53,6 +63,10 @@ export const getIssueTimelogsQueryResponse = { __typename: 'Note', }, summary: null, + userPermissions: { + adminTimelog: false, + __typename: 'TimelogPermissions', + }, }, ], __typename: 'TimelogConnection', @@ -85,6 +99,10 @@ export const getMrTimelogsQueryResponse = { __typename: 'Note', }, summary: null, + userPermissions: { + adminTimelog: true, + __typename: 'TimelogPermissions', + }, }, { __typename: 'Timelog', @@ -98,6 +116,10 @@ export const getMrTimelogsQueryResponse = { spentAt: '2021-05-07T14:44:39Z', note: null, summary: null, + userPermissions: { + adminTimelog: true, + __typename: 'TimelogPermissions', + }, }, { __typename: 'Timelog', @@ -115,6 +137,10 @@ export const getMrTimelogsQueryResponse = { __typename: 'Note', }, summary: null, + userPermissions: { + adminTimelog: true, + __typename: 'TimelogPermissions', + }, }, ], __typename: 'TimelogConnection', diff --git a/spec/frontend/sidebar/components/time_tracking/report_spec.js b/spec/frontend/sidebar/components/time_tracking/report_spec.js index 2b17e6dd6c3..5ed8810e95e 100644 --- a/spec/frontend/sidebar/components/time_tracking/report_spec.js +++ b/spec/frontend/sidebar/components/time_tracking/report_spec.js @@ -1,15 +1,21 @@ import { GlLoadingIcon } from '@gitlab/ui'; -import { getAllByRole, getByRole } from '@testing-library/dom'; +import { getAllByRole, getByRole, getAllByTestId } from '@testing-library/dom'; import { shallowMount, mount } from '@vue/test-utils'; import Vue from 'vue'; import VueApollo from 'vue-apollo'; +import { extendedWrapper } from 'helpers/vue_test_utils_helper'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; import createFlash from '~/flash'; import Report from '~/sidebar/components/time_tracking/report.vue'; import getIssueTimelogsQuery from '~/vue_shared/components/sidebar/queries/get_issue_timelogs.query.graphql'; import getMrTimelogsQuery from '~/vue_shared/components/sidebar/queries/get_mr_timelogs.query.graphql'; -import { getIssueTimelogsQueryResponse, getMrTimelogsQueryResponse } from './mock_data'; +import deleteTimelogMutation from '~/sidebar/components/time_tracking/graphql/mutations/delete_timelog.mutation.graphql'; +import { + getIssueTimelogsQueryResponse, + getMrTimelogsQueryResponse, + timelogToRemoveId, +} from './mock_data'; jest.mock('~/flash'); @@ -18,6 +24,7 @@ describe('Issuable Time Tracking Report', () => { let wrapper; let fakeApollo; const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); + const findDeleteButton = () => wrapper.findByTestId('deleteButton'); const successIssueQueryHandler = jest.fn().mockResolvedValue(getIssueTimelogsQueryResponse); const successMrQueryHandler = jest.fn().mockResolvedValue(getMrTimelogsQueryResponse); @@ -31,14 +38,16 @@ describe('Issuable Time Tracking Report', () => { [getIssueTimelogsQuery, queryHandler], [getMrTimelogsQuery, queryHandler], ]); - wrapper = mountFunction(Report, { - provide: { - issuableId: 1, - issuableType, - }, - propsData: { limitToHours, issuableId: '1' }, - apolloProvider: fakeApollo, - }); + wrapper = extendedWrapper( + mountFunction(Report, { + provide: { + issuableId: 1, + issuableType, + }, + propsData: { limitToHours, issuableId: '1' }, + apolloProvider: fakeApollo, + }), + ); }; afterEach(() => { @@ -75,6 +84,7 @@ describe('Issuable Time Tracking Report', () => { expect(getAllByRole(wrapper.element, 'row', { name: /Administrator/i })).toHaveLength(2); expect(getAllByRole(wrapper.element, 'row', { name: /A note/i })).toHaveLength(1); expect(getAllByRole(wrapper.element, 'row', { name: /A summary/i })).toHaveLength(2); + expect(getAllByTestId(wrapper.element, 'deleteButton')).toHaveLength(1); }); }); @@ -95,6 +105,7 @@ describe('Issuable Time Tracking Report', () => { await waitForPromises(); expect(getAllByRole(wrapper.element, 'row', { name: /Administrator/i })).toHaveLength(3); + expect(getAllByTestId(wrapper.element, 'deleteButton')).toHaveLength(3); }); }); @@ -123,4 +134,59 @@ describe('Issuable Time Tracking Report', () => { }); }); }); + + describe('when clicking on the delete timelog button', () => { + beforeEach(() => { + mountComponent({ mountFunction: mount }); + }); + + it('calls `$apollo.mutate` with deleteTimelogMutation mutation and removes the row', async () => { + const mutateSpy = jest.spyOn(wrapper.vm.$apollo, 'mutate').mockResolvedValue({ + data: { + timelogDelete: { + errors: [], + }, + }, + }); + + await waitForPromises(); + await findDeleteButton().trigger('click'); + await waitForPromises(); + + expect(createFlash).not.toHaveBeenCalled(); + expect(mutateSpy).toHaveBeenCalledWith({ + mutation: deleteTimelogMutation, + variables: { + input: { + id: timelogToRemoveId, + }, + }, + update: expect.anything(), + }); + }); + + it('calls `createFlash` with errorMessage and does not remove the row on promise reject', async () => { + const mutateSpy = jest.spyOn(wrapper.vm.$apollo, 'mutate').mockRejectedValue({}); + + await waitForPromises(); + await findDeleteButton().trigger('click'); + await waitForPromises(); + + expect(mutateSpy).toHaveBeenCalledWith({ + mutation: deleteTimelogMutation, + variables: { + input: { + id: timelogToRemoveId, + }, + }, + update: expect.anything(), + }); + + expect(createFlash).toHaveBeenCalledWith({ + message: 'An error occurred while removing the timelog.', + captureError: true, + error: expect.any(Object), + }); + }); + }); }); |
