diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-10-23 12:08:41 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-10-23 12:08:41 +0300 |
commit | ca7bdd871e35d2a0b079cf7622df10f72f8a5d0f (patch) | |
tree | 666704be546e69df022fbfc2cc00531f2f7bb770 | |
parent | bde2d0d618efdb2a6c1f14a9f660f2dc84c0c643 (diff) |
Add latest changes from gitlab-org/gitlab@master
79 files changed, 1542 insertions, 468 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 9520be55502..e56b9398241 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -6c37027d46cbbf03ed637187451fec56039a789b +10b19a9fe0fe008247056f4a90fde9006b8a7fbb @@ -307,6 +307,9 @@ gem 'rack-attack', '~> 6.3.0' # Sentry integration gem 'sentry-raven', '~> 3.0' +# PostgreSQL query parsing +gem 'gitlab-pg_query', '~> 1.3', require: 'pg_query' + gem 'premailer-rails', '~> 1.10.3' # LabKit: Tracing and Correlation diff --git a/Gemfile.lock b/Gemfile.lock index 863ab0b5112..4d6496fdcf0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -439,6 +439,7 @@ GEM gitlab-mail_room (0.0.7) gitlab-markup (1.7.1) gitlab-net-dns (0.9.1) + gitlab-pg_query (1.3.0) gitlab-puma (4.3.5.gitlab.3) nio4r (~> 2.0) gitlab-puma_worker_killer (0.1.1.gitlab.1) @@ -1336,6 +1337,7 @@ DEPENDENCIES gitlab-mail_room (~> 0.0.7) gitlab-markup (~> 1.7.1) gitlab-net-dns (~> 0.9.1) + gitlab-pg_query (~> 1.3) gitlab-puma (~> 4.3.3.gitlab.2) gitlab-puma_worker_killer (~> 0.1.1.gitlab.1) gitlab-sidekiq-fetcher (= 0.5.2) diff --git a/app/assets/javascripts/dropzone_input.js b/app/assets/javascripts/dropzone_input.js index f65e22a31c5..f56c402091f 100644 --- a/app/assets/javascripts/dropzone_input.js +++ b/app/assets/javascripts/dropzone_input.js @@ -7,6 +7,7 @@ import csrf from './lib/utils/csrf'; import axios from './lib/utils/axios_utils'; import { n__, __ } from '~/locale'; import { getFilename } from '~/lib/utils/file_upload'; +import { spriteIcon } from '~/lib/utils/common_utils'; Dropzone.autoDiscover = false; @@ -25,7 +26,7 @@ function getErrorMessage(res) { export default function dropzoneInput(form, config = { parallelUploads: 2 }) { const divHover = '<div class="div-dropzone-hover"></div>'; - const iconPaperclip = '<i class="fa fa-paperclip div-dropzone-icon"></i>'; + const iconPaperclip = spriteIcon('paperclip', 'div-dropzone-icon'); const $attachButton = form.find('.button-attach-file'); const $attachingFileMessage = form.find('.attaching-file-message'); const $cancelButton = form.find('.button-cancel-uploading-files'); diff --git a/app/assets/javascripts/issuable_list/components/issuable_item.vue b/app/assets/javascripts/issuable_list/components/issuable_item.vue index d8cb1ab07cd..283e49c287a 100644 --- a/app/assets/javascripts/issuable_list/components/issuable_item.vue +++ b/app/assets/javascripts/issuable_list/components/issuable_item.vue @@ -1,15 +1,20 @@ <script> -import { GlLink, GlLabel, GlTooltipDirective } from '@gitlab/ui'; +import { GlLink, GlIcon, GlLabel, GlTooltipDirective } from '@gitlab/ui'; import { __, sprintf } from '~/locale'; +import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import { getTimeago } from '~/lib/utils/datetime_utility'; import { isScopedLabel } from '~/lib/utils/common_utils'; import timeagoMixin from '~/vue_shared/mixins/timeago'; +import IssuableAssignees from '~/vue_shared/components/issue/issue_assignees.vue'; + export default { components: { GlLink, + GlIcon, GlLabel, + IssuableAssignees, }, directives: { GlTooltip: GlTooltipDirective, @@ -24,25 +29,33 @@ export default { type: Object, required: true, }, + enableLabelPermalinks: { + type: Boolean, + required: true, + }, }, computed: { author() { return this.issuable.author; }, authorId() { - const id = parseInt(this.author.id, 10); - - if (Number.isNaN(id)) { - return this.author.id.includes('gid') - ? this.author.id.split('gid://gitlab/User/').pop() - : ''; + return getIdFromGraphQLId(`${this.author.id}`); + }, + isIssuableUrlExternal() { + // Check if URL is relative, which means it is internal. + if (!/^https?:\/\//g.test(this.issuable.webUrl)) { + return false; } - - return id; + // In case URL is absolute, it may or may not be internal, + // hence use `gon.gitlab_url` which is current instance domain. + return !this.issuable.webUrl.includes(gon.gitlab_url); }, labels() { return this.issuable.labels?.nodes || this.issuable.labels || []; }, + assignees() { + return this.issuable.assignees || []; + }, createdAt() { return sprintf(__('created %{timeAgo}'), { timeAgo: getTimeago().format(this.issuable.createdAt), @@ -53,11 +66,33 @@ export default { timeAgo: getTimeago().format(this.issuable.updatedAt), }); }, + issuableTitleProps() { + if (this.isIssuableUrlExternal) { + return { + target: '_blank', + }; + } + return {}; + }, }, methods: { + hasSlotContents(slotName) { + return Boolean(this.$slots[slotName]); + }, scopedLabel(label) { return isScopedLabel(label); }, + labelTitle(label) { + return label.title || label.name; + }, + labelTarget(label) { + if (this.enableLabelPermalinks) { + const key = encodeURIComponent('label_name[]'); + const value = encodeURIComponent(this.labelTitle(label)); + return `?${key}=${value}`; + } + return '#'; + }, /** * This is needed as an independent method since * when user changes current page, `$refs.authorLink` @@ -74,17 +109,20 @@ export default { </script> <template> - <li class="issue"> + <li class="issue px-3"> <div class="issue-box"> <div class="issuable-info-container"> <div class="issuable-main-info"> <div data-testid="issuable-title" class="issue-title title"> <span class="issue-title-text" dir="auto"> - <gl-link :href="issuable.webUrl">{{ issuable.title }}</gl-link> + <gl-link :href="issuable.webUrl" v-bind="issuableTitleProps" + >{{ issuable.title }}<gl-icon v-if="isIssuableUrlExternal" name="external-link" + /></gl-link> </span> </div> <div class="issuable-info"> - <span data-testid="issuable-reference" class="issuable-reference" + <slot v-if="hasSlotContents('reference')" name="reference"></slot> + <span v-else data-testid="issuable-reference" class="issuable-reference" >{{ issuableSymbol }}{{ issuable.iid }}</span > <span class="issuable-authored d-none d-sm-inline-block"> @@ -113,15 +151,30 @@ export default { v-for="(label, index) in labels" :key="index" :background-color="label.color" - :title="label.title" + :title="labelTitle(label)" :description="label.description" :scoped="scopedLabel(label)" + :target="labelTarget(label)" :class="{ 'gl-ml-2': index }" size="sm" /> </div> </div> <div class="issuable-meta"> + <ul v-if="hasSlotContents('status') || issuable.assignees" class="controls"> + <li v-if="hasSlotContents('status')" class="issuable-status"> + <slot name="status"></slot> + </li> + <li v-if="assignees.length" class="gl-display-flex"> + <issuable-assignees + :assignees="issuable.assignees" + :icon-size="16" + :max-visible="4" + img-css-classes="gl-mr-2!" + class="gl-align-items-center gl-display-flex gl-ml-3" + /> + </li> + </ul> <div data-testid="issuable-updated-at" class="float-right issuable-updated-at d-none d-sm-inline-block" diff --git a/app/assets/javascripts/issuable_list/components/issuable_list_root.vue b/app/assets/javascripts/issuable_list/components/issuable_list_root.vue index 7535203dea1..52ebb9acdc3 100644 --- a/app/assets/javascripts/issuable_list/components/issuable_list_root.vue +++ b/app/assets/javascripts/issuable_list/components/issuable_list_root.vue @@ -1,6 +1,7 @@ <script> import { GlLoadingIcon, GlPagination } from '@gitlab/ui'; +import { updateHistory, setUrlParams } from '~/lib/utils/url_utility'; import FilteredSearchBar from '~/vue_shared/components/filtered_search_bar/filtered_search_bar_root.vue'; import IssuableTabs from './issuable_tabs.vue'; @@ -35,6 +36,11 @@ export default { type: Array, required: true, }, + urlParams: { + type: Object, + required: false, + default: () => ({}), + }, initialFilterValue: { type: Array, required: false, @@ -55,7 +61,8 @@ export default { }, tabCounts: { type: Object, - required: true, + required: false, + default: null, }, currentTab: { type: String, @@ -81,6 +88,11 @@ export default { required: false, default: 20, }, + totalPages: { + type: Number, + required: false, + default: 0, + }, currentPage: { type: Number, required: false, @@ -96,6 +108,26 @@ export default { required: false, default: 2, }, + enableLabelPermalinks: { + type: Boolean, + required: false, + default: true, + }, + }, + watch: { + urlParams: { + deep: true, + immediate: true, + handler(params) { + if (Object.keys(params).length) { + updateHistory({ + url: setUrlParams(params, window.location.href, true), + title: document.title, + replace: true, + }); + } + }, + }, }, }; </script> @@ -135,12 +167,21 @@ export default { :key="issuable.id" :issuable-symbol="issuableSymbol" :issuable="issuable" - /> + :enable-label-permalinks="enableLabelPermalinks" + > + <template #reference> + <slot name="reference" :issuable="issuable"></slot> + </template> + <template #status> + <slot name="status" :issuable="issuable"></slot> + </template> + </issuable-item> </ul> <slot v-if="!issuablesLoading && !issuables.length" name="empty-state"></slot> <gl-pagination v-if="showPaginationControls" :per-page="defaultPageSize" + :total-items="totalPages" :value="currentPage" :prev-page="previousPage" :next-page="nextPage" diff --git a/app/assets/javascripts/issuable_list/components/issuable_tabs.vue b/app/assets/javascripts/issuable_list/components/issuable_tabs.vue index df544ce69e7..d9aab004077 100644 --- a/app/assets/javascripts/issuable_list/components/issuable_tabs.vue +++ b/app/assets/javascripts/issuable_list/components/issuable_tabs.vue @@ -14,7 +14,8 @@ export default { }, tabCounts: { type: Object, - required: true, + required: false, + default: null, }, currentTab: { type: String, @@ -40,7 +41,7 @@ export default { > <template #title> <span :title="tab.titleTooltip">{{ tab.title }}</span> - <gl-badge variant="neutral" size="sm" class="gl-px-2 gl-py-1!">{{ + <gl-badge v-if="tabCounts" variant="neutral" size="sm" class="gl-px-2 gl-py-1!">{{ tabCounts[tab.name] }}</gl-badge> </template> diff --git a/app/assets/javascripts/issuable_list/constants.js b/app/assets/javascripts/issuable_list/constants.js new file mode 100644 index 00000000000..4d3ce61366a --- /dev/null +++ b/app/assets/javascripts/issuable_list/constants.js @@ -0,0 +1,49 @@ +import { __ } from '~/locale'; + +export const IssuableStates = { + Opened: 'opened', + Closed: 'closed', + All: 'all', +}; + +export const IssuableListTabs = [ + { + id: 'state-opened', + name: IssuableStates.Opened, + title: __('Open'), + titleTooltip: __('Filter by issues that are currently opened.'), + }, + { + id: 'state-closed', + name: IssuableStates.Closed, + title: __('Closed'), + titleTooltip: __('Filter by issues that are currently closed.'), + }, + { + id: 'state-all', + name: IssuableStates.All, + title: __('All'), + titleTooltip: __('Show all issues.'), + }, +]; + +export const AvailableSortOptions = [ + { + id: 1, + title: __('Created date'), + sortDirection: { + descending: 'created_desc', + ascending: 'created_asc', + }, + }, + { + id: 2, + title: __('Last updated'), + sortDirection: { + descending: 'updated_desc', + ascending: 'updated_asc', + }, + }, +]; + +export const DEFAULT_PAGE_SIZE = 20; diff --git a/app/assets/javascripts/notes/components/note_attachment.vue b/app/assets/javascripts/notes/components/note_attachment.vue index 72f9a4c7e74..b20facc4032 100644 --- a/app/assets/javascripts/notes/components/note_attachment.vue +++ b/app/assets/javascripts/notes/components/note_attachment.vue @@ -1,6 +1,11 @@ <script> +import { GlIcon } from '@gitlab/ui'; + export default { name: 'NoteAttachment', + components: { + GlIcon, + }, props: { attachment: { type: Object, @@ -29,7 +34,7 @@ export default { target="_blank" rel="noopener noreferrer" > - <i class="fa fa-paperclip" aria-hidden="true"> </i> {{ attachment.filename }} + <gl-icon name="paperclip" /> {{ attachment.filename }} </a> </div> </div> diff --git a/app/assets/javascripts/pipelines/components/pipelines_list/pipelines_actions.vue b/app/assets/javascripts/pipelines/components/pipelines_list/pipelines_actions.vue index 97595e5d2ce..e52afe08336 100644 --- a/app/assets/javascripts/pipelines/components/pipelines_list/pipelines_actions.vue +++ b/app/assets/javascripts/pipelines/components/pipelines_list/pipelines_actions.vue @@ -87,7 +87,7 @@ export default { :aria-label="__('Run manual or delayed jobs')" > <gl-icon name="play" class="icon-play" /> - <i class="fa fa-caret-down" aria-hidden="true"></i> + <gl-icon name="chevron-down" /> <gl-loading-icon v-if="isLoading" /> </button> diff --git a/app/assets/javascripts/pipelines/components/pipelines_list/pipelines_artifacts.vue b/app/assets/javascripts/pipelines/components/pipelines_list/pipelines_artifacts.vue index 4a3d134685e..55c71e299be 100644 --- a/app/assets/javascripts/pipelines/components/pipelines_list/pipelines_artifacts.vue +++ b/app/assets/javascripts/pipelines/components/pipelines_list/pipelines_artifacts.vue @@ -29,7 +29,7 @@ export default { :aria-label="__('Artifacts')" > <gl-icon name="download" /> - <i class="fa fa-caret-down" aria-hidden="true"></i> + <gl-icon name="chevron-down" /> </button> <ul class="dropdown-menu dropdown-menu-right"> <li v-for="(artifact, i) in artifacts" :key="i"> diff --git a/app/assets/javascripts/sidebar/components/time_tracking/collapsed_state.vue b/app/assets/javascripts/sidebar/components/time_tracking/collapsed_state.vue index 9d72bf4394e..7b67c34ded6 100644 --- a/app/assets/javascripts/sidebar/components/time_tracking/collapsed_state.vue +++ b/app/assets/javascripts/sidebar/components/time_tracking/collapsed_state.vue @@ -96,7 +96,12 @@ export default { </script> <template> - <div v-gl-tooltip:body.viewport.left :title="tooltipText" class="sidebar-collapsed-icon"> + <div + v-gl-tooltip:body.viewport.left + :title="tooltipText" + data-testid="collapsedState" + class="sidebar-collapsed-icon" + > <gl-icon name="timer" /> <div class="time-tracking-collapsed-summary"> <div :class="divClass"> diff --git a/app/assets/javascripts/sidebar/components/time_tracking/comparison_pane.vue b/app/assets/javascripts/sidebar/components/time_tracking/comparison_pane.vue index d4cc98e3743..99302993b9a 100644 --- a/app/assets/javascripts/sidebar/components/time_tracking/comparison_pane.vue +++ b/app/assets/javascripts/sidebar/components/time_tracking/comparison_pane.vue @@ -70,14 +70,19 @@ export default { </script> <template> - <div class="time-tracking-comparison-pane"> + <div data-testid="timeTrackingComparisonPane"> <div v-gl-tooltip + data-testid="compareMeter" :title="timeRemainingTooltip" :class="timeRemainingStatusClass" class="compare-meter" > - <gl-progress-bar :value="timeRemainingPercent" :variant="progressBarVariant" /> + <gl-progress-bar + data-testid="timeRemainingProgress" + :value="timeRemainingPercent" + :variant="progressBarVariant" + /> <div class="compare-display-container"> <div class="compare-display float-left"> <span class="compare-label">{{ s__('TimeTracking|Spent') }}</span> diff --git a/app/assets/javascripts/sidebar/components/time_tracking/estimate_only_pane.vue b/app/assets/javascripts/sidebar/components/time_tracking/estimate_only_pane.vue index 305726d9725..8a80b1bf13f 100644 --- a/app/assets/javascripts/sidebar/components/time_tracking/estimate_only_pane.vue +++ b/app/assets/javascripts/sidebar/components/time_tracking/estimate_only_pane.vue @@ -11,7 +11,8 @@ export default { </script> <template> - <div class="time-tracking-estimate-only-pane"> - <span class="bold"> {{ s__('TimeTracking|Estimated:') }} </span> {{ timeEstimateHumanReadable }} + <div data-testid="estimateOnlyPane"> + <span class="gl-font-weight-bold">{{ s__('TimeTracking|Estimated:') }} </span + >{{ timeEstimateHumanReadable }} </div> </template> diff --git a/app/assets/javascripts/sidebar/components/time_tracking/help_state.vue b/app/assets/javascripts/sidebar/components/time_tracking/help_state.vue index b45746e789d..8bc828091c0 100644 --- a/app/assets/javascripts/sidebar/components/time_tracking/help_state.vue +++ b/app/assets/javascripts/sidebar/components/time_tracking/help_state.vue @@ -34,7 +34,7 @@ export default { </script> <template> - <div class="time-tracking-help-state"> + <div data-testid="helpPane" class="time-tracking-help-state"> <div class="time-tracking-info"> <h4>{{ __('Track time with quick actions') }}</h4> <p>{{ __('Quick actions can be used in the issues description and comment boxes.') }}</p> diff --git a/app/assets/javascripts/sidebar/components/time_tracking/no_tracking_pane.vue b/app/assets/javascripts/sidebar/components/time_tracking/no_tracking_pane.vue index 45552589e50..2d3d0ce8dc5 100644 --- a/app/assets/javascripts/sidebar/components/time_tracking/no_tracking_pane.vue +++ b/app/assets/javascripts/sidebar/components/time_tracking/no_tracking_pane.vue @@ -5,7 +5,7 @@ export default { </script> <template> - <div class="time-tracking-no-tracking-pane"> - <span class="no-value"> {{ __('No estimate or time spent') }} </span> + <div data-testid="noTrackingPane"> + <span class="no-value">{{ __('No estimate or time spent') }}</span> </div> </template> diff --git a/app/assets/javascripts/sidebar/components/time_tracking/spent_only_pane.vue b/app/assets/javascripts/sidebar/components/time_tracking/spent_only_pane.vue index b2b3b289c5c..33c6ac6e2ba 100644 --- a/app/assets/javascripts/sidebar/components/time_tracking/spent_only_pane.vue +++ b/app/assets/javascripts/sidebar/components/time_tracking/spent_only_pane.vue @@ -15,7 +15,7 @@ export default { return sprintf( s__('TimeTracking|%{startTag}Spent: %{endTag}%{timeSpentHumanReadable}'), { - startTag: '<span class="bold">', + startTag: '<span class="gl-font-weight-bold">', endTag: '</span>', timeSpentHumanReadable: this.timeSpentHumanReadable, }, @@ -27,5 +27,5 @@ export default { </script> <template> - <div class="time-tracking-spend-only-pane" v-html="timeSpent"></div> + <div data-testid="spentOnlyPane" v-html="timeSpent"></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 a2fb0ebcbc6..95864462a68 100644 --- a/app/assets/javascripts/sidebar/components/time_tracking/time_tracker.vue +++ b/app/assets/javascripts/sidebar/components/time_tracking/time_tracker.vue @@ -105,11 +105,17 @@ export default { /> <div class="title hide-collapsed"> {{ __('Time tracking') }} - <div v-if="!showHelpState" class="help-button float-right" @click="toggleHelpState(true)"> + <div + v-if="!showHelpState" + data-testid="helpButton" + class="help-button float-right" + @click="toggleHelpState(true)" + > <gl-icon name="question-o" /> </div> <div - v-if="showHelpState" + v-else + data-testid="closeHelpButton" class="close-help-button float-right" @click="toggleHelpState(false)" > diff --git a/app/assets/javascripts/tracking.js b/app/assets/javascripts/tracking.js index c1521882682..0a1211d0a76 100644 --- a/app/assets/javascripts/tracking.js +++ b/app/assets/javascripts/tracking.js @@ -12,6 +12,7 @@ const DEFAULT_SNOWPLOW_OPTIONS = { contexts: { webPage: true, performanceTiming: true }, formTracking: false, linkClickTracking: false, + pageUnloadTimer: 10, }; const createEventPayload = (el, { suffix = '' } = {}) => { diff --git a/app/assets/stylesheets/_page_specific_files.scss b/app/assets/stylesheets/_page_specific_files.scss index a31cb0b0485..ad4d42abae6 100644 --- a/app/assets/stylesheets/_page_specific_files.scss +++ b/app/assets/stylesheets/_page_specific_files.scss @@ -39,7 +39,6 @@ @import './pages/settings'; @import './pages/settings_ci_cd'; @import './pages/sherlock'; -@import './pages/status'; @import './pages/storage_quota'; @import './pages/tree'; @import './pages/trials'; diff --git a/app/assets/stylesheets/fontawesome_custom.scss b/app/assets/stylesheets/fontawesome_custom.scss index 7b1953be69d..937ed5119cb 100644 --- a/app/assets/stylesheets/fontawesome_custom.scss +++ b/app/assets/stylesheets/fontawesome_custom.scss @@ -117,10 +117,6 @@ content: '\f077'; } -.fa-paperclip::before { - content: '\f0c6'; -} - .fa-bug::before { content: '\f188'; } diff --git a/app/assets/stylesheets/pages/status.scss b/app/assets/stylesheets/page_bundles/ci_status.scss index b37c5172ad2..8522a0a8fe4 100644 --- a/app/assets/stylesheets/pages/status.scss +++ b/app/assets/stylesheets/page_bundles/ci_status.scss @@ -1,3 +1,5 @@ +@import 'mixins_and_variables_and_functions'; + .ci-status { padding: 2px 7px 4px; border: 1px solid $gray-darker; diff --git a/app/assets/stylesheets/page_bundles/todos.scss b/app/assets/stylesheets/page_bundles/todos.scss index 3eec5b53a30..3e20ca9c62f 100644 --- a/app/assets/stylesheets/page_bundles/todos.scss +++ b/app/assets/stylesheets/page_bundles/todos.scss @@ -219,7 +219,6 @@ .todos-empty-content { align-self: center; max-width: 480px; - margin-right: 20px; } .todos-empty-hero { diff --git a/app/graphql/resolvers/releases_resolver.rb b/app/graphql/resolvers/releases_resolver.rb index 85892c2abeb..8e8127cf279 100644 --- a/app/graphql/resolvers/releases_resolver.rb +++ b/app/graphql/resolvers/releases_resolver.rb @@ -4,6 +4,10 @@ module Resolvers class ReleasesResolver < BaseResolver type Types::ReleaseType.connection_type, null: true + argument :sort, Types::ReleaseSortEnum, + required: false, default_value: :released_at_desc, + description: 'Sort releases by this criteria' + alias_method :project, :object # This resolver has a custom singular resolver @@ -11,12 +15,20 @@ module Resolvers Resolvers::ReleaseResolver end - def resolve(**args) + SORT_TO_PARAMS_MAP = { + released_at_desc: { order_by: 'released_at', sort: 'desc' }, + released_at_asc: { order_by: 'released_at', sort: 'asc' }, + created_desc: { order_by: 'created_at', sort: 'desc' }, + created_asc: { order_by: 'created_at', sort: 'asc' } + }.freeze + + def resolve(sort:) return unless Feature.enabled?(:graphql_release_data, project, default_enabled: true) ReleasesFinder.new( project, - current_user + current_user, + SORT_TO_PARAMS_MAP[sort] ).execute end end diff --git a/app/graphql/types/release_sort_enum.rb b/app/graphql/types/release_sort_enum.rb new file mode 100644 index 00000000000..2f9af1bced9 --- /dev/null +++ b/app/graphql/types/release_sort_enum.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Types + # Not inheriting from Types::SortEnum since we only want + # to implement a subset of the sort values it defines. + class ReleaseSortEnum < BaseEnum + graphql_name 'ReleaseSort' + description 'Values for sorting releases' + + # Borrowed from Types::SortEnum + # These values/descriptions should stay in-sync as much as possible. + value 'CREATED_DESC', 'Created at descending order', value: :created_desc + value 'CREATED_ASC', 'Created at ascending order', value: :created_asc + + value 'RELEASED_AT_DESC', 'Released at by descending order', value: :released_at_desc + value 'RELEASED_AT_ASC', 'Released at by ascending order', value: :released_at_asc + end +end diff --git a/app/models/service.rb b/app/models/service.rb index 764f417362f..0998a9a102a 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -8,6 +8,7 @@ class Service < ApplicationRecord include ProjectServicesLoggable include DataFields include FromUnion + include EachBatch SERVICE_NAMES = %w[ alerts asana assembla bamboo bugzilla buildkite campfire confluence custom_issue_tracker discord @@ -294,7 +295,7 @@ class Service < ApplicationRecord end def initialize_properties - self.properties = {} if properties.nil? + self.properties = {} if has_attribute?(:properties) && properties.nil? end def title diff --git a/app/services/admin/propagate_integration_service.rb b/app/services/admin/propagate_integration_service.rb index 96a6d861e47..b5f034186df 100644 --- a/app/services/admin/propagate_integration_service.rb +++ b/app/services/admin/propagate_integration_service.rb @@ -20,7 +20,7 @@ module Admin # rubocop: disable Cop/InBatches def update_inherited_integrations - Service.by_type(integration.type).inherit_from_id(integration.id).in_batches(of: BATCH_SIZE) do |services| + Service.by_type(integration.type).inherit_from_id(integration.id).each_batch(of: BATCH_SIZE) do |services| min_id, max_id = services.pick("MIN(services.id), MAX(services.id)") PropagateIntegrationInheritWorker.perform_async(integration.id, min_id, max_id) end diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 48f44affb23..b2826b5c905 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -73,6 +73,8 @@ module Notes if note.for_merge_request? && note.diff_note? && note.start_of_discussion? Discussions::CaptureDiffNotePositionService.new(note.noteable, note.diff_file&.paths).execute(note.discussion) end + + track_note_creation_usage_for_issues(note) if note.for_issue? end def do_commands(note, update_params, message, only_commands) @@ -113,5 +115,9 @@ module Notes track_usage_event(:incident_management_incident_comment, user.id) end + + def track_note_creation_usage_for_issues(note) + Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_comment_added_action(author: note.author) + end end end diff --git a/app/services/notes/destroy_service.rb b/app/services/notes/destroy_service.rb index ee8a680fcb4..2b6ec47eaef 100644 --- a/app/services/notes/destroy_service.rb +++ b/app/services/notes/destroy_service.rb @@ -8,6 +8,13 @@ module Notes end clear_noteable_diffs_cache(note) + track_note_removal_usage_for_issues(note) if note.for_issue? + end + + private + + def track_note_removal_usage_for_issues(note) + Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_comment_removed_action(author: note.author) end end end diff --git a/app/services/notes/update_service.rb b/app/services/notes/update_service.rb index 193d3080078..37872f7fbdb 100644 --- a/app/services/notes/update_service.rb +++ b/app/services/notes/update_service.rb @@ -14,6 +14,8 @@ module Notes note.save end + track_note_edit_usage_for_issues(note) if note.for_issue? + only_commands = false quick_actions_service = QuickActionsService.new(project, current_user) @@ -89,6 +91,10 @@ module Notes Note.id_in(note.discussion.notes.map(&:id)).update_all(confidential: params[:confidential]) end + + def track_note_edit_usage_for_issues(note) + Gitlab::UsageDataCounters::IssueActivityUniqueCounter.track_issue_comment_edited_action(author: note.author) + end end end diff --git a/app/views/dashboard/todos/index.html.haml b/app/views/dashboard/todos/index.html.haml index ebff5a182c4..9a9fbfc1ee8 100644 --- a/app/views/dashboard/todos/index.html.haml +++ b/app/views/dashboard/todos/index.html.haml @@ -114,7 +114,7 @@ .todos-empty .todos-empty-hero.svg-content = image_tag 'illustrations/todos_empty.svg' - .todos-empty-content + .todos-empty-content.gl-mx-5 %h4 Your To-Do List shows what to work on next %p diff --git a/app/views/events/event/_note.html.haml b/app/views/events/event/_note.html.haml index 4e936025c74..2fa595503e5 100644 --- a/app/views/events/event/_note.html.haml +++ b/app/views/events/event/_note.html.haml @@ -25,5 +25,5 @@ = image_tag note.attachment.url, class: 'note-image-attach' - else = link_to note.attachment.url, target: '_blank', class: 'note-file-attach' do - %i.fa.fa-paperclip + = sprite_icon("paperclip") = note.attachment_identifier diff --git a/app/views/projects/jobs/index.html.haml b/app/views/projects/jobs/index.html.haml index 0b4b4aafeee..a1960fc99cf 100644 --- a/app/views/projects/jobs/index.html.haml +++ b/app/views/projects/jobs/index.html.haml @@ -1,4 +1,5 @@ - page_title _("Jobs") +- add_page_specific_style 'page_bundles/ci_status' .top-area - build_path_proc = ->(scope) { project_jobs_path(@project, scope: scope) } diff --git a/app/views/projects/jobs/show.html.haml b/app/views/projects/jobs/show.html.haml index d7a778088ee..3ac81d4fad6 100644 --- a/app/views/projects/jobs/show.html.haml +++ b/app/views/projects/jobs/show.html.haml @@ -2,6 +2,7 @@ - breadcrumb_title "##{@build.id}" - page_title "#{@build.name} (##{@build.id})", _("Jobs") - add_page_specific_style 'page_bundles/xterm' +- add_page_specific_style 'page_bundles/ci_status' = render_if_exists "shared/shared_runners_minutes_limit_flash_message" diff --git a/app/views/projects/merge_requests/creations/new.html.haml b/app/views/projects/merge_requests/creations/new.html.haml index 4c968c8e8eb..0741b24a5a1 100644 --- a/app/views/projects/merge_requests/creations/new.html.haml +++ b/app/views/projects/merge_requests/creations/new.html.haml @@ -2,6 +2,7 @@ - breadcrumb_title _("New") - page_title _("New Merge Request") - add_page_specific_style 'page_bundles/pipelines' +- add_page_specific_style 'page_bundles/ci_status' - if @merge_request.can_be_created && !params[:change_branches] = render 'new_submit' diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml index 1dbcd613ceb..6b506c38795 100644 --- a/app/views/projects/merge_requests/show.html.haml +++ b/app/views/projects/merge_requests/show.html.haml @@ -11,6 +11,7 @@ - add_page_specific_style 'page_bundles/merge_requests' - add_page_specific_style 'page_bundles/pipelines' - add_page_specific_style 'page_bundles/reports' +- add_page_specific_style 'page_bundles/ci_status' .merge-request{ data: { mr_action: mr_action, url: merge_request_path(@merge_request, format: :json), project_path: project_path(@merge_request.project), lock_version: @merge_request.lock_version } } = render "projects/merge_requests/mr_title" diff --git a/app/views/projects/pipelines/index.html.haml b/app/views/projects/pipelines/index.html.haml index ca07f33136b..6aa1a564499 100644 --- a/app/views/projects/pipelines/index.html.haml +++ b/app/views/projects/pipelines/index.html.haml @@ -1,5 +1,6 @@ - page_title _('Pipelines') - add_page_specific_style 'page_bundles/pipelines' +- add_page_specific_style 'page_bundles/ci_status' = render_if_exists "shared/shared_runners_minutes_limit_flash_message" diff --git a/app/views/projects/pipelines/show.html.haml b/app/views/projects/pipelines/show.html.haml index 34f7744f825..a25c30d01d9 100644 --- a/app/views/projects/pipelines/show.html.haml +++ b/app/views/projects/pipelines/show.html.haml @@ -4,6 +4,7 @@ - pipeline_has_errors = @pipeline.builds.empty? && @pipeline.yaml_errors.present? - add_page_specific_style 'page_bundles/pipeline' - add_page_specific_style 'page_bundles/reports' +- add_page_specific_style 'page_bundles/ci_status' .js-pipeline-container{ data: { controller_action: "#{controller.action_name}" } } #js-pipeline-header-vue.pipeline-header-container{ data: {full_path: @project.full_path, retry_path: retry_project_pipeline_path(@pipeline.project, @pipeline), cancel_path: cancel_project_pipeline_path(@pipeline.project, @pipeline), delete_path: project_pipeline_path(@pipeline.project, @pipeline), pipeline_iid: @pipeline.iid, pipeline_id: @pipeline.id} } diff --git a/app/views/shared/notes/_note.html.haml b/app/views/shared/notes/_note.html.haml index 97ed2852871..f1352be28e3 100644 --- a/app/views/shared/notes/_note.html.haml +++ b/app/views/shared/notes/_note.html.haml @@ -72,7 +72,7 @@ = image_tag note.attachment.url, class: 'note-image-attach' .attachment = link_to note.attachment.url, target: '_blank' do - = icon('paperclip') + = sprite_icon('paperclip') = note.attachment_identifier = link_to delete_attachment_project_note_path(note.project, note), title: _('Delete this attachment'), method: :delete, remote: true, data: { confirm: _('Are you sure you want to remove the attachment?') }, class: 'danger js-note-attachment-delete' do diff --git a/changelogs/unreleased/229918-issuedata-comments.yml b/changelogs/unreleased/229918-issuedata-comments.yml new file mode 100644 index 00000000000..90250d977ca --- /dev/null +++ b/changelogs/unreleased/229918-issuedata-comments.yml @@ -0,0 +1,5 @@ +--- +title: UsageData for issues added/removed/edited +merge_request: 45609 +author: +type: added diff --git a/changelogs/unreleased/271528-todo-css-on-mobile.yml b/changelogs/unreleased/271528-todo-css-on-mobile.yml new file mode 100644 index 00000000000..d863180dc7c --- /dev/null +++ b/changelogs/unreleased/271528-todo-css-on-mobile.yml @@ -0,0 +1,5 @@ +--- +title: Fix CSS for To-Do List on mobile +merge_request: 45969 +author: Takuya Noguchi +type: fixed diff --git a/changelogs/unreleased/mw-replace-fa-icons-in-pipeline-action-button.yml b/changelogs/unreleased/mw-replace-fa-icons-in-pipeline-action-button.yml new file mode 100644 index 00000000000..c91e0e20632 --- /dev/null +++ b/changelogs/unreleased/mw-replace-fa-icons-in-pipeline-action-button.yml @@ -0,0 +1,5 @@ +--- +title: Replace fa-caret-down with chevron-down SVG in pipeline action buttons +merge_request: 45881 +author: +type: changed diff --git a/changelogs/unreleased/nfriend-add-graphql-release-sorting.yml b/changelogs/unreleased/nfriend-add-graphql-release-sorting.yml new file mode 100644 index 00000000000..9da63f36e05 --- /dev/null +++ b/changelogs/unreleased/nfriend-add-graphql-release-sorting.yml @@ -0,0 +1,5 @@ +--- +title: Allow sorting of releases from GraphQL +merge_request: 45577 +author: +type: added diff --git a/config/application.rb b/config/application.rb index 75befc8a248..6c28bc94bed 100644 --- a/config/application.rb +++ b/config/application.rb @@ -175,6 +175,7 @@ module Gitlab config.assets.precompile << "mailers/*.css" config.assets.precompile << "page_bundles/_mixins_and_variables_and_functions.css" config.assets.precompile << "page_bundles/boards.css" + config.assets.precompile << "page_bundles/ci_status.css" config.assets.precompile << "page_bundles/cycle_analytics.css" config.assets.precompile << "page_bundles/dev_ops_report.css" config.assets.precompile << "page_bundles/environments.css" diff --git a/config/feature_flags/development/api_json_content_type.yml b/config/feature_flags/development/api_json_content_type.yml new file mode 100644 index 00000000000..bd153550635 --- /dev/null +++ b/config/feature_flags/development/api_json_content_type.yml @@ -0,0 +1,7 @@ +--- +name: api_json_content_type +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/42229 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/270067 +group: group::ecosystem +type: development +default_enabled: false diff --git a/doc/administration/troubleshooting/log_parsing.md b/doc/administration/troubleshooting/log_parsing.md index 7914628a756..3c16e5a3ae0 100644 --- a/doc/administration/troubleshooting/log_parsing.md +++ b/doc/administration/troubleshooting/log_parsing.md @@ -2,7 +2,7 @@ We recommend using log aggregation and search tools like Kibana and Splunk whenever possible, but if they are not available you can still quickly parse -[GitLab logs](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/26311) in JSON format +[GitLab logs](../logs.md) in JSON format (the default in GitLab 12.0 and later) using [`jq`](https://stedolan.github.io/jq/). ## What is JQ? diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index 729a6ed4893..ff2bdd8b7b3 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -14745,6 +14745,11 @@ type Project { Returns the last _n_ elements from the list. """ last: Int + + """ + Sort releases by this criteria + """ + sort: ReleaseSort = RELEASED_AT_DESC ): ReleaseConnection """ @@ -16520,6 +16525,31 @@ type ReleaseLinks { } """ +Values for sorting releases +""" +enum ReleaseSort { + """ + Created at ascending order + """ + CREATED_ASC + + """ + Created at descending order + """ + CREATED_DESC + + """ + Released at by ascending order + """ + RELEASED_AT_ASC + + """ + Released at by descending order + """ + RELEASED_AT_DESC +} + +""" Represents the source code attached to a release in a particular format """ type ReleaseSource { diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index a9939fc2113..eb421067668 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -42603,6 +42603,16 @@ "description": "Releases of the project", "args": [ { + "name": "sort", + "description": "Sort releases by this criteria", + "type": { + "kind": "ENUM", + "name": "ReleaseSort", + "ofType": null + }, + "defaultValue": "RELEASED_AT_DESC" + }, + { "name": "after", "description": "Returns the elements in the list that come after the specified cursor.", "type": { @@ -47518,6 +47528,41 @@ "possibleTypes": null }, { + "kind": "ENUM", + "name": "ReleaseSort", + "description": "Values for sorting releases", + "fields": null, + "inputFields": null, + "interfaces": null, + "enumValues": [ + { + "name": "CREATED_DESC", + "description": "Created at descending order", + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "CREATED_ASC", + "description": "Created at ascending order", + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "RELEASED_AT_DESC", + "description": "Released at by descending order", + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "RELEASED_AT_ASC", + "description": "Released at by ascending order", + "isDeprecated": false, + "deprecationReason": null + } + ], + "possibleTypes": null + }, + { "kind": "OBJECT", "name": "ReleaseSource", "description": "Represents the source code attached to a release in a particular format", diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index a44e493fc43..93bbcb18bf3 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -3643,6 +3643,17 @@ Type of the link: `other`, `runbook`, `image`, `package`; defaults to `other`. | `PACKAGE` | Package link type | | `RUNBOOK` | Runbook link type | +### ReleaseSort + +Values for sorting releases. + +| Value | Description | +| ----- | ----------- | +| `CREATED_ASC` | Created at ascending order | +| `CREATED_DESC` | Created at descending order | +| `RELEASED_AT_ASC` | Released at by ascending order | +| `RELEASED_AT_DESC` | Released at by descending order | + ### RequirementState State of a requirement. diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 26ac8159a09..63f476a2266 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -2774,10 +2774,6 @@ Otherwise cache content can be overwritten. > Introduced in GitLab Runner v1.0.0. -The cache is shared between jobs, so if you're using different -paths for different jobs, you should also set a different `cache:key`. -Otherwise cache content can be overwritten. - The `key` keyword defines the affinity of caching between jobs. You can have a single cache for all jobs, cache per-job, cache per-branch, or any other way that fits your workflow. This way, you can fine tune caching, diff --git a/doc/development/cicd/templates.md b/doc/development/cicd/templates.md index 7d522a55559..e4c3e622ede 100644 --- a/doc/development/cicd/templates.md +++ b/doc/development/cicd/templates.md @@ -174,3 +174,7 @@ is updated in a major version GitLab release. A template could contain malicious code. For example, a template that contains the `export` shell command in a job might accidentally expose project secret variables in a job log. If you're unsure if it's secure or not, you need to ask security experts for cross-validation. + +## Contribute CI/CD Template Merge Requests + +After your CI/CD Template MR is created and labeled with `ci::templates`, DangerBot suggests one reviewer and one maintainer that can review your code. When your merge request is ready for review, please `@mention` the reviewer and ask them to review your CI/CD Template changes. See details in the merge request that added [a DangerBot task for CI/CD Template MRs](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/44688). diff --git a/lib/api/api.rb b/lib/api/api.rb index 84b4d5a5835..ec252faa6cb 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -115,7 +115,14 @@ module API format :json formatter :json, Gitlab::Json::GrapeFormatter - content_type :txt, "text/plain" + + # There is a small chance some users depend on the old behavior. + # We this change under a feature flag to see if affects GitLab.com users. + if Feature.enabled?(:api_json_content_type) + content_type :json, 'application/json' + else + content_type :txt, 'text/plain' + end # Ensure the namespace is right, otherwise we might load Grape::API::Helpers helpers ::API::Helpers diff --git a/lib/api/ci/runner.rb b/lib/api/ci/runner.rb index ef679147c9f..5b68208061b 100644 --- a/lib/api/ci/runner.rb +++ b/lib/api/ci/runner.rb @@ -5,6 +5,8 @@ module API class Runner < ::API::Base helpers ::API::Helpers::Runner + content_type :txt, 'text/plain' + resource :runners do desc 'Registers a new Runner' do success Entities::RunnerRegistrationDetails diff --git a/lib/api/go_proxy.rb b/lib/api/go_proxy.rb index 30f0cfb4dfd..85724287c56 100755 --- a/lib/api/go_proxy.rb +++ b/lib/api/go_proxy.rb @@ -9,6 +9,8 @@ module API MODULE_VERSION_REQUIREMENTS = { module_version: MODULE_VERSION_REGEX }.freeze + content_type :txt, 'text/plain' + before { require_packages_enabled! } helpers do diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb index 38ac1f22a48..8704c37cbd7 100644 --- a/lib/api/repositories.rb +++ b/lib/api/repositories.rb @@ -6,6 +6,8 @@ module API class Repositories < ::API::Base include PaginationParams + content_type :txt, 'text/plain' + helpers ::API::Helpers::HeadersHelpers before { authorize! :download_code, user_project } diff --git a/lib/gitlab/database/partitioning/replace_table.rb b/lib/gitlab/database/partitioning/replace_table.rb new file mode 100644 index 00000000000..13178431703 --- /dev/null +++ b/lib/gitlab/database/partitioning/replace_table.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +module Gitlab + module Database + module Partitioning + class ReplaceTable + DELIMITER = ";\n\n" + + attr_reader :original_table, :replacement_table, :replaced_table, :primary_key_column, + :sequence, :original_primary_key, :replacement_primary_key, :replaced_primary_key + + def initialize(original_table, replacement_table, replaced_table, primary_key_column) + @original_table = original_table + @replacement_table = replacement_table + @replaced_table = replaced_table + @primary_key_column = primary_key_column + + @sequence = default_sequence(original_table, primary_key_column) + @original_primary_key = default_primary_key(original_table) + @replacement_primary_key = default_primary_key(replacement_table) + @replaced_primary_key = default_primary_key(replaced_table) + end + + def perform + yield sql_to_replace_table if block_given? + + execute(sql_to_replace_table) + end + + private + + delegate :execute, :quote_table_name, :quote_column_name, to: :connection + def connection + @connection ||= ActiveRecord::Base.connection + end + + def default_sequence(table, column) + "#{table}_#{column}_seq" + end + + def default_primary_key(table) + "#{table}_pkey" + end + + def sql_to_replace_table + @sql_to_replace_table ||= [ + drop_default_sql(original_table, primary_key_column), + set_default_sql(replacement_table, primary_key_column, "nextval('#{quote_table_name(sequence)}'::regclass)"), + + change_sequence_owner_sql(sequence, replacement_table, primary_key_column), + + rename_table_sql(original_table, replaced_table), + rename_constraint_sql(replaced_table, original_primary_key, replaced_primary_key), + + rename_table_sql(replacement_table, original_table), + rename_constraint_sql(original_table, replacement_primary_key, original_primary_key) + ].join(DELIMITER) + end + + def drop_default_sql(table, column) + "ALTER TABLE #{quote_table_name(table)} ALTER COLUMN #{quote_column_name(column)} DROP DEFAULT" + end + + def set_default_sql(table, column, expression) + "ALTER TABLE #{quote_table_name(table)} ALTER COLUMN #{quote_column_name(column)} SET DEFAULT #{expression}" + end + + def change_sequence_owner_sql(sequence, table, column) + "ALTER SEQUENCE #{quote_table_name(sequence)} OWNED BY #{quote_table_name(table)}.#{quote_column_name(column)}" + end + + def rename_table_sql(old_name, new_name) + "ALTER TABLE #{quote_table_name(old_name)} RENAME TO #{quote_table_name(new_name)}" + end + + def rename_constraint_sql(table, old_name, new_name) + "ALTER TABLE #{quote_table_name(table)} RENAME CONSTRAINT #{quote_column_name(old_name)} TO #{quote_column_name(new_name)}" + end + end + end + end +end diff --git a/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb b/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb index f7b0306b769..6adbe046cb0 100644 --- a/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb +++ b/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb @@ -66,7 +66,10 @@ module Gitlab create_range_partitioned_copy(table_name, partitioned_table_name, partition_column, primary_key) create_daterange_partitions(partitioned_table_name, partition_column.name, min_date, max_date) end - create_trigger_to_sync_tables(table_name, partitioned_table_name, primary_key) + + with_lock_retries do + create_trigger_to_sync_tables(table_name, partitioned_table_name, primary_key) + end end # Clean up a partitioned copy of an existing table. First, deletes the database function and trigger that were @@ -81,13 +84,9 @@ module Gitlab assert_not_in_transaction_block(scope: ERROR_SCOPE) with_lock_retries do - trigger_name = make_sync_trigger_name(table_name) - drop_trigger(table_name, trigger_name) + drop_sync_trigger(table_name) end - function_name = make_sync_function_name(table_name) - drop_function(function_name) - partitioned_table_name = make_partitioned_table_name(table_name) drop_table(partitioned_table_name) end @@ -177,6 +176,52 @@ module Gitlab end end + # Replaces a non-partitioned table with its partitioned copy. This is the final step in a partitioning + # migration, which makes the partitioned table ready for use by the application. The partitioned copy should be + # replaced with the original table in such a way that it appears seamless to any database clients. The replaced + # table will be renamed to "#{replaced_table}_archived" + # + # **NOTE** This method should only be used after all other migration steps have completed successfully. + # There are several limitations to this method that MUST be handled before, or during, the swap migration: + # + # - Secondary indexes and foreign keys are not automatically recreated on the partitioned table. + # - Some types of constraints (UNIQUE and EXCLUDE) which rely on indexes, will not automatically be recreated + # on the partitioned table, since the underlying index will not be present. + # - Foreign keys referencing the original non-partitioned table, would also need to be updated to reference the + # partitioned table, but unfortunately this is not supported in PG11. + # - Views referencing the original table will not be automatically updated to reference the partitioned table. + # + # Example: + # + # replace_with_partitioned_table :audit_events + # + def replace_with_partitioned_table(table_name) + assert_table_is_allowed(table_name) + + partitioned_table_name = make_partitioned_table_name(table_name) + archived_table_name = make_archived_table_name(table_name) + primary_key_name = connection.primary_key(table_name) + + replace_table(table_name, partitioned_table_name, archived_table_name, primary_key_name) + end + + # Rolls back a migration that replaced a non-partitioned table with its partitioned copy. This can be used to + # restore the original non-partitioned table in the event of an unexpected issue. + # + # Example: + # + # rollback_replace_with_partitioned_table :audit_events + # + def rollback_replace_with_partitioned_table(table_name) + assert_table_is_allowed(table_name) + + partitioned_table_name = make_partitioned_table_name(table_name) + archived_table_name = make_archived_table_name(table_name) + primary_key_name = connection.primary_key(archived_table_name) + + replace_table(table_name, archived_table_name, partitioned_table_name, primary_key_name) + end + private def assert_table_is_allowed(table_name) @@ -190,6 +235,10 @@ module Gitlab tmp_table_name("#{table}_part") end + def make_archived_table_name(table) + "#{table}_archived" + end + def make_sync_function_name(table) object_name(table, 'table_sync_function') end @@ -270,12 +319,18 @@ module Gitlab function_name = make_sync_function_name(source_table_name) trigger_name = make_sync_trigger_name(source_table_name) - with_lock_retries do - create_sync_function(function_name, partitioned_table_name, unique_key) - create_comment('FUNCTION', function_name, "Partitioning migration: table sync for #{source_table_name} table") + create_sync_function(function_name, partitioned_table_name, unique_key) + create_comment('FUNCTION', function_name, "Partitioning migration: table sync for #{source_table_name} table") - create_sync_trigger(source_table_name, trigger_name, function_name) - end + create_sync_trigger(source_table_name, trigger_name, function_name) + end + + def drop_sync_trigger(source_table_name) + trigger_name = make_sync_trigger_name(source_table_name) + drop_trigger(source_table_name, trigger_name) + + function_name = make_sync_function_name(source_table_name) + drop_function(function_name) end def create_sync_function(name, partitioned_table_name, unique_key) @@ -358,6 +413,21 @@ module Gitlab end end end + + def replace_table(original_table_name, replacement_table_name, replaced_table_name, primary_key_name) + replace_table = Gitlab::Database::Partitioning::ReplaceTable.new(original_table_name, + replacement_table_name, replaced_table_name, primary_key_name) + + with_lock_retries do + drop_sync_trigger(original_table_name) + + replace_table.perform do |sql| + say("replace_table(\"#{sql}\")") + end + + create_trigger_to_sync_tables(original_table_name, replaced_table_name, primary_key_name) + end + end end end end diff --git a/lib/gitlab/error_tracking.rb b/lib/gitlab/error_tracking.rb index 803acef9a40..a5ace2be773 100644 --- a/lib/gitlab/error_tracking.rb +++ b/lib/gitlab/error_tracking.rb @@ -123,6 +123,7 @@ module Gitlab end extra = sanitize_request_parameters(extra) + inject_sql_query_into_extra(exception, extra) if sentry && Raven.configuration.server Raven.capture_exception(exception, tags: default_tags, extra: extra) @@ -149,6 +150,12 @@ module Gitlab filter.filter(parameters) end + def inject_sql_query_into_extra(exception, extra) + return unless exception.is_a?(ActiveRecord::StatementInvalid) + + extra[:sql] = PgQuery.normalize(exception.sql.to_s) + end + def sentry_dsn return unless Rails.env.production? || Rails.env.development? return unless Gitlab.config.sentry.enabled diff --git a/lib/gitlab/usage_data_counters/issue_activity_unique_counter.rb b/lib/gitlab/usage_data_counters/issue_activity_unique_counter.rb index e8839875109..b6d570a3d08 100644 --- a/lib/gitlab/usage_data_counters/issue_activity_unique_counter.rb +++ b/lib/gitlab/usage_data_counters/issue_activity_unique_counter.rb @@ -33,6 +33,9 @@ module Gitlab ISSUE_DUE_DATE_CHANGED = 'g_project_management_issue_due_date_changed' ISSUE_TIME_ESTIMATE_CHANGED = 'g_project_management_issue_time_estimate_changed' ISSUE_TIME_SPENT_CHANGED = 'g_project_management_issue_time_spent_changed' + ISSUE_COMMENT_ADDED = 'g_project_management_issue_comment_added' + ISSUE_COMMENT_EDITED = 'g_project_management_issue_comment_edited' + ISSUE_COMMENT_REMOVED = 'g_project_management_issue_comment_removed' class << self def track_issue_created_action(author:, time: Time.zone.now) @@ -147,6 +150,18 @@ module Gitlab track_unique_action(ISSUE_TIME_SPENT_CHANGED, author, time) end + def track_issue_comment_added_action(author:, time: Time.zone.now) + track_unique_action(ISSUE_COMMENT_ADDED, author, time) + end + + def track_issue_comment_edited_action(author:, time: Time.zone.now) + track_unique_action(ISSUE_COMMENT_EDITED, author, time) + end + + def track_issue_comment_removed_action(author:, time: Time.zone.now) + track_unique_action(ISSUE_COMMENT_REMOVED, author, time) + end + private def track_unique_action(action, author, time) diff --git a/lib/gitlab/usage_data_counters/known_events.yml b/lib/gitlab/usage_data_counters/known_events.yml index bc56c5d6d9b..023427946aa 100644 --- a/lib/gitlab/usage_data_counters/known_events.yml +++ b/lib/gitlab/usage_data_counters/known_events.yml @@ -303,3 +303,15 @@ category: issues_edit redis_slot: project_management aggregation: daily +- name: g_project_management_issue_comment_added + category: issues_edit + redis_slot: project_management + aggregation: daily +- name: g_project_management_issue_comment_edited + category: issues_edit + redis_slot: project_management + aggregation: daily +- name: g_project_management_issue_comment_removed + category: issues_edit + redis_slot: project_management + aggregation: daily diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 40799480e81..1fabf661d13 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -11517,6 +11517,9 @@ msgstr "" msgid "Filter by issues that are currently closed." msgstr "" +msgid "Filter by issues that are currently opened." +msgstr "" + msgid "Filter by label" msgstr "" @@ -24258,6 +24261,9 @@ msgstr "" msgid "Show all activity" msgstr "" +msgid "Show all issues." +msgstr "" + msgid "Show all members" msgstr "" diff --git a/spec/features/milestone_spec.rb b/spec/features/milestone_spec.rb index fefa2916c30..dce76e4df6d 100644 --- a/spec/features/milestone_spec.rb +++ b/spec/features/milestone_spec.rb @@ -76,7 +76,7 @@ RSpec.describe 'Milestone' do wait_for_requests - page.within('.time-tracking-no-tracking-pane') do + page.within('[data-testid="noTrackingPane"]') do expect(page).to have_content 'No estimate or time spent' end end @@ -94,7 +94,7 @@ RSpec.describe 'Milestone' do wait_for_requests - page.within('.time-tracking-spend-only-pane') do + page.within('[data-testid="spentOnlyPane"]') do expect(page).to have_content 'Spent: 3h' end end diff --git a/spec/frontend/issuable_list/components/issuable_item_spec.js b/spec/frontend/issuable_list/components/issuable_item_spec.js index a96a4e15e6c..5c8b679e4e1 100644 --- a/spec/frontend/issuable_list/components/issuable_item_spec.js +++ b/spec/frontend/issuable_list/components/issuable_item_spec.js @@ -2,28 +2,34 @@ import { shallowMount } from '@vue/test-utils'; import { GlLink, GlLabel } from '@gitlab/ui'; import IssuableItem from '~/issuable_list/components/issuable_item.vue'; +import IssuableAssignees from '~/vue_shared/components/issue/issue_assignees.vue'; import { mockIssuable, mockRegularLabel, mockScopedLabel } from '../mock_data'; -const createComponent = ({ issuableSymbol = '#', issuable = mockIssuable } = {}) => +const createComponent = ({ issuableSymbol = '#', issuable = mockIssuable, slots = {} } = {}) => shallowMount(IssuableItem, { propsData: { issuableSymbol, issuable, + enableLabelPermalinks: true, }, + slots, }); describe('IssuableItem', () => { const mockLabels = mockIssuable.labels.nodes; const mockAuthor = mockIssuable.author; + const originalUrl = gon.gitlab_url; let wrapper; beforeEach(() => { + gon.gitlab_url = 'http://0.0.0.0:3000'; wrapper = createComponent(); }); afterEach(() => { wrapper.destroy(); + gon.gitlab_url = originalUrl; }); describe('computed', () => { @@ -38,8 +44,8 @@ describe('IssuableItem', () => { authorId | returnValue ${1} | ${1} ${'1'} | ${1} - ${'gid://gitlab/User/1'} | ${'1'} - ${'foo'} | ${''} + ${'gid://gitlab/User/1'} | ${1} + ${'foo'} | ${null} `( 'returns $returnValue when value of `issuable.author.id` is $authorId', async ({ authorId, returnValue }) => { @@ -60,6 +66,30 @@ describe('IssuableItem', () => { ); }); + describe('isIssuableUrlExternal', () => { + it.each` + issuableWebUrl | urlType | returnValue + ${'/gitlab-org/gitlab-test/-/issues/2'} | ${'relative'} | ${false} + ${'http://0.0.0.0:3000/gitlab-org/gitlab-test/-/issues/1'} | ${'absolute and internal'} | ${false} + ${'http://jira.atlassian.net/browse/IG-1'} | ${'external'} | ${true} + ${'https://github.com/gitlabhq/gitlabhq/issues/1'} | ${'external'} | ${true} + `( + 'returns $returnValue when `issuable.webUrl` is $urlType', + async ({ issuableWebUrl, returnValue }) => { + wrapper.setProps({ + issuable: { + ...mockIssuable, + webUrl: issuableWebUrl, + }, + }); + + await wrapper.vm.$nextTick(); + + expect(wrapper.vm.isIssuableUrlExternal).toBe(returnValue); + }, + ); + }); + describe('labels', () => { it('returns `issuable.labels.nodes` reference when it is available', () => { expect(wrapper.vm.labels).toEqual(mockLabels); @@ -92,6 +122,12 @@ describe('IssuableItem', () => { }); }); + describe('assignees', () => { + it('returns `issuable.assignees` reference when it is available', () => { + expect(wrapper.vm.assignees).toBe(mockIssuable.assignees); + }); + }); + describe('createdAt', () => { it('returns string containing timeago string based on `issuable.createdAt`', () => { expect(wrapper.vm.createdAt).toContain('created'); @@ -120,6 +156,34 @@ describe('IssuableItem', () => { }, ); }); + + describe('labelTitle', () => { + it.each` + label | propWithTitle | returnValue + ${{ title: 'foo' }} | ${'title'} | ${'foo'} + ${{ name: 'foo' }} | ${'name'} | ${'foo'} + `('returns string value of `label.$propWithTitle`', ({ label, returnValue }) => { + expect(wrapper.vm.labelTitle(label)).toBe(returnValue); + }); + }); + + describe('labelTarget', () => { + it('returns target string for a provided label param when `enableLabelPermalinks` is true', () => { + expect(wrapper.vm.labelTarget(mockRegularLabel)).toBe( + '?label_name%5B%5D=Documentation%20Update', + ); + }); + + it('returns string "#" for a provided label param when `enableLabelPermalinks` is false', async () => { + wrapper.setProps({ + enableLabelPermalinks: false, + }); + + await wrapper.vm.$nextTick(); + + expect(wrapper.vm.labelTarget(mockRegularLabel)).toBe('#'); + }); + }); }); describe('template', () => { @@ -128,9 +192,28 @@ describe('IssuableItem', () => { expect(titleEl.exists()).toBe(true); expect(titleEl.find(GlLink).attributes('href')).toBe(mockIssuable.webUrl); + expect(titleEl.find(GlLink).attributes('target')).not.toBeDefined(); expect(titleEl.find(GlLink).text()).toBe(mockIssuable.title); }); + it('renders issuable title with `target` set as "_blank" when issuable.webUrl is external', async () => { + wrapper.setProps({ + issuable: { + ...mockIssuable, + webUrl: 'http://jira.atlassian.net/browse/IG-1', + }, + }); + + await wrapper.vm.$nextTick(); + + expect( + wrapper + .find('[data-testid="issuable-title"]') + .find(GlLink) + .attributes('target'), + ).toBe('_blank'); + }); + it('renders issuable reference', () => { const referenceEl = wrapper.find('[data-testid="issuable-reference"]'); @@ -138,6 +221,24 @@ describe('IssuableItem', () => { expect(referenceEl.text()).toBe(`#${mockIssuable.iid}`); }); + it('renders issuable reference via slot', () => { + const wrapperWithRefSlot = createComponent({ + issuableSymbol: '#', + issuable: mockIssuable, + slots: { + reference: ` + <b class="js-reference">${mockIssuable.iid}</b> + `, + }, + }); + const referenceEl = wrapperWithRefSlot.find('.js-reference'); + + expect(referenceEl.exists()).toBe(true); + expect(referenceEl.text()).toBe(`${mockIssuable.iid}`); + + wrapperWithRefSlot.destroy(); + }); + it('renders issuable createdAt info', () => { const createdAtEl = wrapper.find('[data-testid="issuable-created-at"]'); @@ -151,7 +252,7 @@ describe('IssuableItem', () => { expect(authorEl.exists()).toBe(true); expect(authorEl.attributes()).toMatchObject({ - 'data-user-id': wrapper.vm.authorId, + 'data-user-id': `${wrapper.vm.authorId}`, 'data-username': mockAuthor.username, 'data-name': mockAuthor.name, 'data-avatar-url': mockAuthor.avatarUrl, @@ -170,10 +271,40 @@ describe('IssuableItem', () => { title: mockLabels[0].title, description: mockLabels[0].description, scoped: false, + target: wrapper.vm.labelTarget(mockLabels[0]), size: 'sm', }); }); + it('renders issuable status via slot', () => { + const wrapperWithStatusSlot = createComponent({ + issuableSymbol: '#', + issuable: mockIssuable, + slots: { + status: ` + <b class="js-status">${mockIssuable.state}</b> + `, + }, + }); + const statusEl = wrapperWithStatusSlot.find('.js-status'); + + expect(statusEl.exists()).toBe(true); + expect(statusEl.text()).toBe(`${mockIssuable.state}`); + + wrapperWithStatusSlot.destroy(); + }); + + it('renders issuable-assignees component', () => { + const assigneesEl = wrapper.find(IssuableAssignees); + + expect(assigneesEl.exists()).toBe(true); + expect(assigneesEl.props()).toMatchObject({ + assignees: mockIssuable.assignees, + iconSize: 16, + maxVisible: 4, + }); + }); + it('renders issuable updatedAt info', () => { const updatedAtEl = wrapper.find('[data-testid="issuable-updated-at"]'); diff --git a/spec/frontend/issuable_list/components/issuable_list_root_spec.js b/spec/frontend/issuable_list/components/issuable_list_root_spec.js index 34184522b55..e560b0915bc 100644 --- a/spec/frontend/issuable_list/components/issuable_list_root_spec.js +++ b/spec/frontend/issuable_list/components/issuable_list_root_spec.js @@ -1,6 +1,8 @@ import { mount } from '@vue/test-utils'; import { GlLoadingIcon, GlPagination } from '@gitlab/ui'; +import { TEST_HOST } from 'helpers/test_constants'; + import IssuableListRoot from '~/issuable_list/components/issuable_list_root.vue'; import IssuableTabs from '~/issuable_list/components/issuable_tabs.vue'; import IssuableItem from '~/issuable_list/components/issuable_item.vue'; @@ -32,6 +34,29 @@ describe('IssuableListRoot', () => { wrapper.destroy(); }); + describe('watch', () => { + describe('urlParams', () => { + it('updates window URL reflecting props within `urlParams`', async () => { + const urlParams = { + state: 'closed', + sort: 'updated_asc', + page: 1, + search: 'foo', + }; + + wrapper.setProps({ + urlParams, + }); + + await wrapper.vm.$nextTick(); + + expect(global.window.location.href).toBe( + `${TEST_HOST}/?state=${urlParams.state}&sort=${urlParams.sort}&page=${urlParams.page}&search=${urlParams.search}`, + ); + }); + }); + }); + describe('template', () => { it('renders component container element with class "issuable-list-container"', () => { expect(wrapper.classes()).toContain('issuable-list-container'); @@ -114,6 +139,7 @@ describe('IssuableListRoot', () => { it('renders gl-pagination when `showPaginationControls` prop is true', async () => { wrapper.setProps({ showPaginationControls: true, + totalPages: 10, }); await wrapper.vm.$nextTick(); @@ -125,6 +151,7 @@ describe('IssuableListRoot', () => { value: 1, prevPage: 0, nextPage: 2, + totalItems: 10, align: 'center', }); }); diff --git a/spec/frontend/issuable_list/mock_data.js b/spec/frontend/issuable_list/mock_data.js index 8eab2ca6f94..b18e49e2102 100644 --- a/spec/frontend/issuable_list/mock_data.js +++ b/spec/frontend/issuable_list/mock_data.js @@ -51,6 +51,7 @@ export const mockIssuable = { labels: { nodes: mockLabels, }, + assignees: [mockAuthor], }; export const mockIssuables = [ diff --git a/spec/frontend/sidebar/components/time_tracking/time_tracker_spec.js b/spec/frontend/sidebar/components/time_tracking/time_tracker_spec.js index 5307be0bf58..abaa2948dc2 100644 --- a/spec/frontend/sidebar/components/time_tracking/time_tracker_spec.js +++ b/spec/frontend/sidebar/components/time_tracking/time_tracker_spec.js @@ -1,277 +1,209 @@ -import Vue from 'vue'; - -import mountComponent from 'helpers/vue_mount_component_helper'; +import { createMockDirective } from 'helpers/vue_mock_directive'; +import { mount } from '@vue/test-utils'; import TimeTracker from '~/sidebar/components/time_tracking/time_tracker.vue'; describe('Issuable Time Tracker', () => { - let initialData; - let vm; - - const initTimeTrackingComponent = ({ - timeEstimate, - timeSpent, - timeEstimateHumanReadable, - timeSpentHumanReadable, - limitToHours, - }) => { - setFixtures(` - <div> - <div id="mock-container"></div> - </div> - `); - - initialData = { - timeEstimate, - timeSpent, - humanTimeEstimate: timeEstimateHumanReadable, - humanTimeSpent: timeSpentHumanReadable, - limitToHours: Boolean(limitToHours), - rootPath: '/', - }; - - const TimeTrackingComponent = Vue.extend({ - ...TimeTracker, - components: { - ...TimeTracker.components, - transition: { - // disable animations - render(h) { - return h('div', this.$slots.default); - }, - }, - }, - }); - vm = mountComponent(TimeTrackingComponent, initialData, '#mock-container'); + let wrapper; + + const findByTestId = testId => wrapper.find(`[data-testid=${testId}]`); + const findComparisonMeter = () => findByTestId('compareMeter').attributes('title'); + const findCollapsedState = () => findByTestId('collapsedState'); + const findTimeRemainingProgress = () => findByTestId('timeRemainingProgress'); + + const defaultProps = { + timeEstimate: 10_000, // 2h 46m + timeSpent: 5_000, // 1h 23m + humanTimeEstimate: '2h 46m', + humanTimeSpent: '1h 23m', + limitToHours: false, + rootPath: '/', }; + const mountComponent = ({ props = {} } = {}) => + mount(TimeTracker, { + propsData: { ...defaultProps, ...props }, + directives: { GlTooltip: createMockDirective() }, + }); + afterEach(() => { - vm.$destroy(); + wrapper.destroy(); }); describe('Initialization', () => { beforeEach(() => { - initTimeTrackingComponent({ - timeEstimate: 10000, // 2h 46m - timeSpent: 5000, // 1h 23m - timeEstimateHumanReadable: '2h 46m', - timeSpentHumanReadable: '1h 23m', - }); + wrapper = mountComponent(); }); it('should return something defined', () => { - expect(vm).toBeDefined(); + expect(wrapper).toBeDefined(); }); - it('should correctly set timeEstimate', done => { - Vue.nextTick(() => { - expect(vm.timeEstimate).toBe(initialData.timeEstimate); - done(); - }); + it('should correctly render timeEstimate', () => { + expect(findByTestId('timeTrackingComparisonPane').html()).toContain( + defaultProps.humanTimeEstimate, + ); }); - it('should correctly set time_spent', done => { - Vue.nextTick(() => { - expect(vm.timeSpent).toBe(initialData.timeSpent); - done(); - }); + it('should correctly render time_spent', () => { + expect(findByTestId('timeTrackingComparisonPane').html()).toContain( + defaultProps.humanTimeSpent, + ); }); }); - describe('Content Display', () => { - describe('Panes', () => { - describe('Comparison pane', () => { - beforeEach(() => { - initTimeTrackingComponent({ - timeEstimate: 100000, // 1d 3h - timeSpent: 5000, // 1h 23m - timeEstimateHumanReadable: '1d 3h', - timeSpentHumanReadable: '1h 23m', - }); + describe('Content panes', () => { + describe('Comparison pane', () => { + beforeEach(() => { + wrapper = mountComponent({ + props: { + timeEstimate: 100_000, // 1d 3h + timeSpent: 5_000, // 1h 23m + humanTimeEstimate: '1d 3h', + humanTimeSpent: '1h 23m', + }, }); + }); - it('should show the "Comparison" pane when timeEstimate and time_spent are truthy', done => { - Vue.nextTick(() => { - expect(vm.showComparisonState).toBe(true); - const $comparisonPane = vm.$el.querySelector('.time-tracking-comparison-pane'); - - expect($comparisonPane).toBeVisible(); - done(); - }); - }); + it('should show the "Comparison" pane when timeEstimate and time_spent are truthy', () => { + const pane = findByTestId('timeTrackingComparisonPane'); + expect(pane.exists()).toBe(true); + expect(pane.isVisible()).toBe(true); + }); - it('should show full times when the sidebar is collapsed', done => { - Vue.nextTick(() => { - const timeTrackingText = vm.$el.querySelector('.time-tracking-collapsed-summary span') - .textContent; + it('should show full times when the sidebar is collapsed', () => { + expect(findCollapsedState().text()).toBe('1h 23m / 1d 3h'); + }); - expect(timeTrackingText.trim()).toBe('1h 23m / 1d 3h'); - done(); - }); + describe('Remaining meter', () => { + it('should display the remaining meter with the correct width', () => { + expect(findTimeRemainingProgress().attributes('value')).toBe('5'); }); - describe('Remaining meter', () => { - it('should display the remaining meter with the correct width', done => { - Vue.nextTick(() => { - expect( - vm.$el.querySelector('.time-tracking-comparison-pane .progress[value="5"]'), - ).not.toBeNull(); - done(); - }); - }); + it('should display the remaining meter with the correct background color when within estimate', () => { + expect(findTimeRemainingProgress().attributes('variant')).toBe('primary'); + }); - it('should display the remaining meter with the correct background color when within estimate', done => { - Vue.nextTick(() => { - expect( - vm.$el.querySelector('.time-tracking-comparison-pane .progress[variant="primary"]'), - ).not.toBeNull(); - done(); - }); + it('should display the remaining meter with the correct background color when over estimate', () => { + wrapper = mountComponent({ + props: { + timeEstimate: 10_000, // 2h 46m + timeSpent: 20_000_000, // 231 days + }, }); - it('should display the remaining meter with the correct background color when over estimate', done => { - vm.timeEstimate = 10000; // 2h 46m - vm.timeSpent = 20000000; // 231 days - Vue.nextTick(() => { - expect( - vm.$el.querySelector('.time-tracking-comparison-pane .progress[variant="danger"]'), - ).not.toBeNull(); - done(); - }); - }); + expect(findTimeRemainingProgress().attributes('variant')).toBe('danger'); }); }); + }); - describe('Comparison pane when limitToHours is true', () => { - beforeEach(() => { - initTimeTrackingComponent({ - timeEstimate: 100000, // 1d 3h - timeSpent: 5000, // 1h 23m - timeEstimateHumanReadable: '', - timeSpentHumanReadable: '', + describe('Comparison pane when limitToHours is true', () => { + beforeEach(async () => { + wrapper = mountComponent({ + props: { + timeEstimate: 100_000, // 1d 3h limitToHours: true, - }); + }, }); + }); - it('should show the correct tooltip text', done => { - Vue.nextTick(() => { - expect(vm.showComparisonState).toBe(true); - const $title = vm.$el.querySelector('.time-tracking-content .compare-meter').title; + it('should show the correct tooltip text', async () => { + expect(findByTestId('timeTrackingComparisonPane').exists()).toBe(true); + await wrapper.vm.$nextTick(); - expect($title).toBe('Time remaining: 26h 23m'); - done(); - }); - }); + expect(findComparisonMeter()).toBe('Time remaining: 26h 23m'); }); + }); - describe('Estimate only pane', () => { - beforeEach(() => { - initTimeTrackingComponent({ - timeEstimate: 10000, // 2h 46m + describe('Estimate only pane', () => { + beforeEach(async () => { + wrapper = mountComponent({ + props: { + timeEstimate: 10_000, // 2h 46m timeSpent: 0, timeEstimateHumanReadable: '2h 46m', timeSpentHumanReadable: '', - }); + }, }); + await wrapper.vm.$nextTick(); + }); - it('should display the human readable version of time estimated', done => { - Vue.nextTick(() => { - const estimateText = vm.$el.querySelector('.time-tracking-estimate-only-pane') - .textContent; - const correctText = 'Estimated: 2h 46m'; - - expect(estimateText.trim()).toBe(correctText); - done(); - }); - }); + it('should display the human readable version of time estimated', () => { + const estimateText = findByTestId('estimateOnlyPane').text(); + expect(estimateText.trim()).toBe('Estimated: 2h 46m'); }); + }); - describe('Spent only pane', () => { - beforeEach(() => { - initTimeTrackingComponent({ + describe('Spent only pane', () => { + beforeEach(() => { + wrapper = mountComponent({ + props: { timeEstimate: 0, - timeSpent: 5000, // 1h 23m + timeSpent: 5_000, // 1h 23m timeEstimateHumanReadable: '2h 46m', timeSpentHumanReadable: '1h 23m', - }); + }, }); + }); - it('should display the human readable version of time spent', done => { - Vue.nextTick(() => { - const spentText = vm.$el.querySelector('.time-tracking-spend-only-pane').textContent; - const correctText = 'Spent: 1h 23m'; - - expect(spentText).toBe(correctText); - done(); - }); - }); + it('should display the human readable version of time spent', () => { + const spentText = findByTestId('spentOnlyPane').text(); + expect(spentText.trim()).toBe('Spent: 1h 23m'); }); + }); - describe('No time tracking pane', () => { - beforeEach(() => { - initTimeTrackingComponent({ + describe('No time tracking pane', () => { + beforeEach(() => { + wrapper = mountComponent({ + props: { timeEstimate: 0, timeSpent: 0, timeEstimateHumanReadable: '', timeSpentHumanReadable: '', - }); + }, }); + }); - it('should only show the "No time tracking" pane when both timeEstimate and time_spent are falsey', done => { - Vue.nextTick(() => { - const $noTrackingPane = vm.$el.querySelector('.time-tracking-no-tracking-pane'); - const noTrackingText = $noTrackingPane.textContent; - const correctText = 'No estimate or time spent'; - - expect(vm.showNoTimeTrackingState).toBe(true); - expect($noTrackingPane).toBeVisible(); - expect(noTrackingText.trim()).toBe(correctText); - done(); - }); - }); + it('should only show the "No time tracking" pane when both timeEstimate and time_spent are falsey', () => { + const pane = findByTestId('noTrackingPane'); + const correctText = 'No estimate or time spent'; + expect(pane.exists()).toBe(true); + expect(pane.text().trim()).toBe(correctText); }); + }); - describe('Help pane', () => { - const helpButton = () => vm.$el.querySelector('.help-button'); - const closeHelpButton = () => vm.$el.querySelector('.close-help-button'); - const helpPane = () => vm.$el.querySelector('.time-tracking-help-state'); + describe('Help pane', () => { + const findHelpButton = () => findByTestId('helpButton'); + const findCloseHelpButton = () => findByTestId('closeHelpButton'); - beforeEach(() => { - initTimeTrackingComponent({ timeEstimate: 0, timeSpent: 0 }); + beforeEach(async () => { + wrapper = mountComponent({ props: { timeEstimate: 0, timeSpent: 0 } }); + await wrapper.vm.$nextTick(); + }); - return vm.$nextTick(); - }); + it('should not show the "Help" pane by default', () => { + expect(findByTestId('helpPane').exists()).toBe(false); + }); - it('should not show the "Help" pane by default', () => { - expect(vm.showHelpState).toBe(false); - expect(helpPane()).toBeNull(); - }); + it('should show the "Help" pane when help button is clicked', async () => { + findHelpButton().trigger('click'); - it('should show the "Help" pane when help button is clicked', () => { - helpButton().click(); + await wrapper.vm.$nextTick(); - return vm.$nextTick().then(() => { - expect(vm.showHelpState).toBe(true); + expect(findByTestId('helpPane').exists()).toBe(true); + }); - // let animations run - jest.advanceTimersByTime(500); + it('should not show the "Help" pane when help button is clicked and then closed', async () => { + findHelpButton().trigger('click'); + await wrapper.vm.$nextTick(); - expect(helpPane()).toBeVisible(); - }); - }); + expect(findByTestId('helpPane').classes('help-state-toggle-enter')).toBe(true); + expect(findByTestId('helpPane').classes('help-state-toggle-leave')).toBe(false); - it('should not show the "Help" pane when help button is clicked and then closed', done => { - helpButton().click(); - - Vue.nextTick() - .then(() => closeHelpButton().click()) - .then(() => Vue.nextTick()) - .then(() => { - expect(vm.showHelpState).toBe(false); - expect(helpPane()).toBeNull(); - }) - .then(done) - .catch(done.fail); - }); + findCloseHelpButton().trigger('click'); + await wrapper.vm.$nextTick(); + + expect(findByTestId('helpPane').classes('help-state-toggle-leave')).toBe(true); + expect(findByTestId('helpPane').classes('help-state-toggle-enter')).toBe(false); }); }); }); diff --git a/spec/frontend/snippets/components/__snapshots__/snippet_description_edit_spec.js.snap b/spec/frontend/snippets/components/__snapshots__/snippet_description_edit_spec.js.snap index 93684ed48ee..7c11d073566 100644 --- a/spec/frontend/snippets/components/__snapshots__/snippet_description_edit_spec.js.snap +++ b/spec/frontend/snippets/components/__snapshots__/snippet_description_edit_spec.js.snap @@ -52,9 +52,13 @@ exports[`Snippet Description Edit component rendering matches the snapshot 1`] = <div class="div-dropzone-hover" > - <i - class="fa fa-paperclip div-dropzone-icon" - /> + <svg + class="div-dropzone-icon" + > + <use + xlink:href="undefined#paperclip" + /> + </svg> </div> </div> diff --git a/spec/frontend/tracking_spec.js b/spec/frontend/tracking_spec.js index 8c2bef60e74..d4b97532cdd 100644 --- a/spec/frontend/tracking_spec.js +++ b/spec/frontend/tracking_spec.js @@ -31,6 +31,7 @@ describe('Tracking', () => { contexts: { webPage: true, performanceTiming: true }, formTracking: false, linkClickTracking: false, + pageUnloadTimer: 10, }); }); }); diff --git a/spec/graphql/resolvers/releases_resolver_spec.rb b/spec/graphql/resolvers/releases_resolver_spec.rb index ee8b33fc748..b9b90686aa7 100644 --- a/spec/graphql/resolvers/releases_resolver_spec.rb +++ b/spec/graphql/resolvers/releases_resolver_spec.rb @@ -5,12 +5,19 @@ require 'spec_helper' RSpec.describe Resolvers::ReleasesResolver do include GraphqlHelpers + let_it_be(:today) { Time.now } + let_it_be(:yesterday) { today - 1.day } + let_it_be(:tomorrow) { today + 1.day } + let_it_be(:project) { create(:project, :private) } - let_it_be(:release_v1) { create(:release, project: project, tag: 'v1.0.0') } - let_it_be(:release_v2) { create(:release, project: project, tag: 'v2.0.0') } + let_it_be(:release_v1) { create(:release, project: project, tag: 'v1.0.0', released_at: yesterday, created_at: tomorrow) } + let_it_be(:release_v2) { create(:release, project: project, tag: 'v2.0.0', released_at: today, created_at: yesterday) } + let_it_be(:release_v3) { create(:release, project: project, tag: 'v3.0.0', released_at: tomorrow, created_at: today) } let_it_be(:developer) { create(:user) } let_it_be(:public_user) { create(:user) } + let(:args) { { sort: :released_at_desc } } + before do project.add_developer(developer) end @@ -28,7 +35,41 @@ RSpec.describe Resolvers::ReleasesResolver do let(:current_user) { developer } it 'returns all releases associated to the project' do - expect(resolve_releases).to eq([release_v1, release_v2]) + expect(resolve_releases).to eq([release_v3, release_v2, release_v1]) + end + + describe 'sorting behavior' do + context 'with sort: :released_at_desc' do + let(:args) { { sort: :released_at_desc } } + + it 'returns the releases ordered by released_at in descending order' do + expect(resolve_releases).to eq([release_v3, release_v2, release_v1]) + end + end + + context 'with sort: :released_at_asc' do + let(:args) { { sort: :released_at_asc } } + + it 'returns the releases ordered by released_at in ascending order' do + expect(resolve_releases).to eq([release_v1, release_v2, release_v3]) + end + end + + context 'with sort: :created_desc' do + let(:args) { { sort: :created_desc } } + + it 'returns the releases ordered by created_at in descending order' do + expect(resolve_releases).to eq([release_v1, release_v3, release_v2]) + end + end + + context 'with sort: :created_asc' do + let(:args) { { sort: :created_asc } } + + it 'returns the releases ordered by created_at in ascending order' do + expect(resolve_releases).to eq([release_v2, release_v3, release_v1]) + end + end end end end @@ -37,6 +78,6 @@ RSpec.describe Resolvers::ReleasesResolver do def resolve_releases context = { current_user: current_user } - resolve(described_class, obj: project, args: {}, ctx: context) + resolve(described_class, obj: project, args: args, ctx: context) end end diff --git a/spec/lib/gitlab/database/partitioning/replace_table_spec.rb b/spec/lib/gitlab/database/partitioning/replace_table_spec.rb new file mode 100644 index 00000000000..8800ff845ae --- /dev/null +++ b/spec/lib/gitlab/database/partitioning/replace_table_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::Partitioning::ReplaceTable, '#perform' do + include TableSchemaHelpers + + subject(:replace_table) { described_class.new(original_table, replacement_table, archived_table, 'id').perform } + + let(:original_table) { '_test_original_table' } + let(:replacement_table) { '_test_replacement_table' } + let(:archived_table) { '_test_archived_table' } + + let(:original_sequence) { "#{original_table}_id_seq" } + + let(:original_primary_key) { "#{original_table}_pkey" } + let(:replacement_primary_key) { "#{replacement_table}_pkey" } + let(:archived_primary_key) { "#{archived_table}_pkey" } + + before do + connection.execute(<<~SQL) + CREATE TABLE #{original_table} ( + id serial NOT NULL PRIMARY KEY, + original_column text NOT NULL, + created_at timestamptz NOT NULL); + + CREATE TABLE #{replacement_table} ( + id int NOT NULL, + replacement_column text NOT NULL, + created_at timestamptz NOT NULL, + PRIMARY KEY (id, created_at)) + PARTITION BY RANGE (created_at); + + CREATE TABLE #{replacement_table}_202001 PARTITION OF #{replacement_table} + FOR VALUES FROM ('2020-01-01') TO ('2020-02-01'); + SQL + end + + it 'replaces the current table, archiving the old' do + expect_table_to_be_replaced { replace_table } + end + + it 'transfers the primary key sequence to the replacement table' do + expect(sequence_owned_by(original_table, 'id')).to eq(original_sequence) + expect(default_expression_for(original_table, 'id')).to eq("nextval('#{original_sequence}'::regclass)") + + expect(sequence_owned_by(replacement_table, 'id')).to be_nil + expect(default_expression_for(replacement_table, 'id')).to be_nil + + expect_table_to_be_replaced { replace_table } + + expect(sequence_owned_by(original_table, 'id')).to eq(original_sequence) + expect(default_expression_for(original_table, 'id')).to eq("nextval('#{original_sequence}'::regclass)") + expect(sequence_owned_by(archived_table, 'id')).to be_nil + expect(default_expression_for(archived_table, 'id')).to be_nil + end + + it 'renames the primary key constraints to match the new table names' do + expect(primary_key_constraint_name(original_table)).to eq(original_primary_key) + expect(primary_key_constraint_name(replacement_table)).to eq(replacement_primary_key) + + expect_table_to_be_replaced { replace_table } + + expect(primary_key_constraint_name(original_table)).to eq(original_primary_key) + expect(primary_key_constraint_name(archived_table)).to eq(archived_primary_key) + end + + def expect_table_to_be_replaced(&block) + super(original_table: original_table, replacement_table: replacement_table, archived_table: archived_table, &block) + end +end diff --git a/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb b/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb index 147637cf471..f10ff704c17 100644 --- a/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb +++ b/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers do include PartitioningHelpers include TriggerHelpers + include TableSchemaHelpers let(:migration) do ActiveRecord::Migration.new.extend(described_class) @@ -629,6 +630,76 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe end end + describe '#replace_with_partitioned_table' do + let(:archived_table) { "#{source_table}_archived" } + + before do + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date + end + + it 'replaces the original table with the partitioned table' do + expect(table_type(source_table)).to eq('normal') + expect(table_type(partitioned_table)).to eq('partitioned') + expect(table_type(archived_table)).to be_nil + + expect_table_to_be_replaced { migration.replace_with_partitioned_table(source_table) } + + expect(table_type(source_table)).to eq('partitioned') + expect(table_type(archived_table)).to eq('normal') + expect(table_type(partitioned_table)).to be_nil + end + + it 'moves the trigger from the original table to the new table' do + expect_function_to_exist(function_name) + expect_valid_function_trigger(source_table, trigger_name, function_name, after: %w[delete insert update]) + + expect_table_to_be_replaced { migration.replace_with_partitioned_table(source_table) } + + expect_function_to_exist(function_name) + expect_valid_function_trigger(source_table, trigger_name, function_name, after: %w[delete insert update]) + end + + def expect_table_to_be_replaced(&block) + super(original_table: source_table, replacement_table: partitioned_table, archived_table: archived_table, &block) + end + end + + describe '#rollback_replace_with_partitioned_table' do + let(:archived_table) { "#{source_table}_archived" } + + before do + migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date + + migration.replace_with_partitioned_table source_table + end + + it 'replaces the partitioned table with the non-partitioned table' do + expect(table_type(source_table)).to eq('partitioned') + expect(table_type(archived_table)).to eq('normal') + expect(table_type(partitioned_table)).to be_nil + + expect_table_to_be_replaced { migration.rollback_replace_with_partitioned_table(source_table) } + + expect(table_type(source_table)).to eq('normal') + expect(table_type(partitioned_table)).to eq('partitioned') + expect(table_type(archived_table)).to be_nil + end + + it 'moves the trigger from the partitioned table to the non-partitioned table' do + expect_function_to_exist(function_name) + expect_valid_function_trigger(source_table, trigger_name, function_name, after: %w[delete insert update]) + + expect_table_to_be_replaced { migration.rollback_replace_with_partitioned_table(source_table) } + + expect_function_to_exist(function_name) + expect_valid_function_trigger(source_table, trigger_name, function_name, after: %w[delete insert update]) + end + + def expect_table_to_be_replaced(&block) + super(original_table: source_table, replacement_table: archived_table, archived_table: partitioned_table, &block) + end + end + def filter_columns_by_name(columns, names) columns.reject { |c| names.include?(c.name) } end diff --git a/spec/lib/gitlab/error_tracking_spec.rb b/spec/lib/gitlab/error_tracking_spec.rb index 2cc9ff36c99..68a46b11487 100644 --- a/spec/lib/gitlab/error_tracking_spec.rb +++ b/spec/lib/gitlab/error_tracking_spec.rb @@ -198,47 +198,39 @@ RSpec.describe Gitlab::ErrorTracking do end describe '.track_exception' do - it 'calls Raven.capture_exception' do - expected_extras = { - some_other_info: 'info', - issue_url: issue_url - } + let(:extra) { { issue_url: issue_url, some_other_info: 'info' } } - expected_tags = { - correlation_id: 'cid' - } + subject(:track_exception) { described_class.track_exception(exception, extra) } - expect(Raven).to receive(:capture_exception) - .with(exception, - tags: a_hash_including(expected_tags), - extra: a_hash_including(expected_extras)) - - described_class.track_exception( - exception, - issue_url: issue_url, - some_other_info: 'info' - ) + before do + allow(Raven).to receive(:capture_exception).and_call_original + allow(Gitlab::ErrorTracking::Logger).to receive(:error) + end + + it 'calls Raven.capture_exception' do + track_exception + + expect(Raven).to have_received(:capture_exception) + .with(exception, + tags: a_hash_including(correlation_id: 'cid'), + extra: a_hash_including(some_other_info: 'info', issue_url: issue_url)) end it 'calls Gitlab::ErrorTracking::Logger.error with formatted payload' do - expect(Gitlab::ErrorTracking::Logger).to receive(:error) - .with(a_hash_including(*expected_payload_includes)) + track_exception - described_class.track_exception( - exception, - issue_url: issue_url, - some_other_info: 'info' - ) + expect(Gitlab::ErrorTracking::Logger).to have_received(:error) + .with(a_hash_including(*expected_payload_includes)) end context 'with filterable parameters' do let(:extra) { { test: 1, my_token: 'test' } } it 'filters parameters' do - expect(Gitlab::ErrorTracking::Logger).to receive(:error).with( - hash_including({ 'extra.test' => 1, 'extra.my_token' => '[FILTERED]' })) + track_exception - described_class.track_exception(exception, extra) + expect(Gitlab::ErrorTracking::Logger).to have_received(:error) + .with(hash_including({ 'extra.test' => 1, 'extra.my_token' => '[FILTERED]' })) end end @@ -247,44 +239,58 @@ RSpec.describe Gitlab::ErrorTracking do let(:exception) { double(message: 'bang!', sentry_extra_data: extra_info, backtrace: caller) } it 'includes the extra data from the exception in the tracking information' do - expect(Raven).to receive(:capture_exception) - .with(exception, a_hash_including(extra: a_hash_including(extra_info))) + track_exception - described_class.track_exception(exception) + expect(Raven).to have_received(:capture_exception) + .with(exception, a_hash_including(extra: a_hash_including(extra_info))) end end context 'the exception implements :sentry_extra_data, which returns nil' do let(:exception) { double(message: 'bang!', sentry_extra_data: nil, backtrace: caller) } + let(:extra) { { issue_url: issue_url } } it 'just includes the other extra info' do - extra_info = { issue_url: issue_url } - expect(Raven).to receive(:capture_exception) - .with(exception, a_hash_including(extra: a_hash_including(extra_info))) + track_exception - described_class.track_exception(exception, extra_info) + expect(Raven).to have_received(:capture_exception) + .with(exception, a_hash_including(extra: a_hash_including(extra))) end end context 'with sidekiq args' do - it 'ensures extra.sidekiq.args is a string' do - extra = { sidekiq: { 'class' => 'PostReceive', 'args' => [1, { 'id' => 2, 'name' => 'hello' }, 'some-value', 'another-value'] } } + context 'when the args does not have anything sensitive' do + let(:extra) { { sidekiq: { 'class' => 'PostReceive', 'args' => [1, { 'id' => 2, 'name' => 'hello' }, 'some-value', 'another-value'] } } } - expect(Gitlab::ErrorTracking::Logger).to receive(:error).with( - hash_including({ 'extra.sidekiq' => { 'class' => 'PostReceive', 'args' => ['1', '{"id"=>2, "name"=>"hello"}', 'some-value', 'another-value'] } })) + it 'ensures extra.sidekiq.args is a string' do + track_exception - described_class.track_exception(exception, extra) + expect(Gitlab::ErrorTracking::Logger).to have_received(:error).with( + hash_including({ 'extra.sidekiq' => { 'class' => 'PostReceive', 'args' => ['1', '{"id"=>2, "name"=>"hello"}', 'some-value', 'another-value'] } })) + end end - it 'filters sensitive arguments before sending' do - extra = { sidekiq: { 'class' => 'UnknownWorker', 'args' => ['sensitive string', 1, 2] } } + context 'when the args has sensitive information' do + let(:extra) { { sidekiq: { 'class' => 'UnknownWorker', 'args' => ['sensitive string', 1, 2] } } } + + it 'filters sensitive arguments before sending' do + track_exception + + expect(sentry_event.dig('extra', 'sidekiq', 'args')).to eq(['[FILTERED]', 1, 2]) + expect(Gitlab::ErrorTracking::Logger).to have_received(:error).with( + hash_including('extra.sidekiq' => { 'class' => 'UnknownWorker', 'args' => ['[FILTERED]', '1', '2'] })) + end + end + end - expect(Gitlab::ErrorTracking::Logger).to receive(:error).with( - hash_including('extra.sidekiq' => { 'class' => 'UnknownWorker', 'args' => ['[FILTERED]', '1', '2'] })) + context 'when the error is kind of an `ActiveRecord::StatementInvalid`' do + let(:exception) { ActiveRecord::StatementInvalid.new(sql: 'SELECT "users".* FROM "users" WHERE "users"."id" = 1 AND "users"."foo" = $1') } - described_class.track_exception(exception, extra) + it 'injects the normalized sql query into extra' do + track_exception - expect(sentry_event.dig('extra', 'sidekiq', 'args')).to eq(['[FILTERED]', 1, 2]) + expect(Raven).to have_received(:capture_exception) + .with(exception, a_hash_including(extra: a_hash_including(sql: 'SELECT "users".* FROM "users" WHERE "users"."id" = $2 AND "users"."foo" = $1'))) end end end diff --git a/spec/lib/gitlab/usage_data_counters/issue_activity_unique_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/issue_activity_unique_counter_spec.rb index e08dc41d0cc..a53ab6ff97b 100644 --- a/spec/lib/gitlab/usage_data_counters/issue_activity_unique_counter_spec.rb +++ b/spec/lib/gitlab/usage_data_counters/issue_activity_unique_counter_spec.rb @@ -292,6 +292,36 @@ RSpec.describe Gitlab::UsageDataCounters::IssueActivityUniqueCounter, :clean_git end end + context 'for Issue comment added actions' do + it_behaves_like 'tracks and counts action' do + let(:action) { described_class::ISSUE_COMMENT_ADDED } + + def track_action(params) + described_class.track_issue_comment_added_action(**params) + end + end + end + + context 'for Issue comment edited actions' do + it_behaves_like 'tracks and counts action' do + let(:action) { described_class::ISSUE_COMMENT_EDITED } + + def track_action(params) + described_class.track_issue_comment_edited_action(**params) + end + end + end + + context 'for Issue comment removed actions' do + it_behaves_like 'tracks and counts action' do + let(:action) { described_class::ISSUE_COMMENT_REMOVED } + + def track_action(params) + described_class.track_issue_comment_removed_action(**params) + end + end + end + it 'can return the count of actions per user deduplicated', :aggregate_failures do described_class.track_issue_title_changed_action(author: user1) described_class.track_issue_description_changed_action(author: user1) diff --git a/spec/requests/api/graphql/project/releases_spec.rb b/spec/requests/api/graphql/project/releases_spec.rb index 7c57c0e9177..26700da262d 100644 --- a/spec/requests/api/graphql/project/releases_spec.rb +++ b/spec/requests/api/graphql/project/releases_spec.rb @@ -300,4 +300,77 @@ RSpec.describe 'Query.project(fullPath).releases()' do it_behaves_like 'no access to any release data' end end + + describe 'sorting behavior' do + let_it_be(:today) { Time.now } + let_it_be(:yesterday) { today - 1.day } + let_it_be(:tomorrow) { today + 1.day } + + let_it_be(:project) { create(:project, :repository, :public) } + + let_it_be(:release_v1) { create(:release, project: project, tag: 'v1', released_at: yesterday, created_at: tomorrow) } + let_it_be(:release_v2) { create(:release, project: project, tag: 'v2', released_at: today, created_at: yesterday) } + let_it_be(:release_v3) { create(:release, project: project, tag: 'v3', released_at: tomorrow, created_at: today) } + + let(:current_user) { developer } + + let(:params) { nil } + + let(:sorted_tags) do + graphql_data.dig('project', 'releases', 'nodes').map { |release| release['tagName'] } + end + + let(:query) do + graphql_query_for(:project, { fullPath: project.full_path }, + %{ + releases#{params ? "(#{params})" : ""} { + nodes { + tagName + } + } + }) + end + + before do + post_query + end + + context 'when no sort: parameter is provided' do + it 'returns the results with the default sort applied (sort: RELEASED_AT_DESC)' do + expect(sorted_tags).to eq(%w(v3 v2 v1)) + end + end + + context 'with sort: RELEASED_AT_DESC' do + let(:params) { 'sort: RELEASED_AT_DESC' } + + it 'returns the releases ordered by released_at in descending order' do + expect(sorted_tags).to eq(%w(v3 v2 v1)) + end + end + + context 'with sort: RELEASED_AT_ASC' do + let(:params) { 'sort: RELEASED_AT_ASC' } + + it 'returns the releases ordered by released_at in ascending order' do + expect(sorted_tags).to eq(%w(v1 v2 v3)) + end + end + + context 'with sort: CREATED_DESC' do + let(:params) { 'sort: CREATED_DESC' } + + it 'returns the releases ordered by created_at in descending order' do + expect(sorted_tags).to eq(%w(v1 v3 v2)) + end + end + + context 'with sort: CREATED_ASC' do + let(:params) { 'sort: CREATED_ASC' } + + it 'returns the releases ordered by created_at in ascending order' do + expect(sorted_tags).to eq(%w(v2 v3 v1)) + end + end + end end diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index 70b5e70cab6..3118956951e 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -67,147 +67,164 @@ RSpec.describe Notes::CreateService do let(:current_user) { user } end end - end - context 'noteable highlight cache clearing' do - let(:project_with_repo) { create(:project, :repository) } - let(:merge_request) do - create(:merge_request, source_project: project_with_repo, - target_project: project_with_repo) - end + it 'tracks issue comment usage data', :clean_gitlab_redis_shared_state do + event = Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_COMMENT_ADDED + counter = Gitlab::UsageDataCounters::HLLRedisCounter - let(:position) do - Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb", - new_path: "files/ruby/popen.rb", - old_line: nil, - new_line: 14, - diff_refs: merge_request.diff_refs) + expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_comment_added_action).with(author: user).and_call_original + expect do + described_class.new(project, user, opts).execute + end.to change { counter.unique_events(event_names: event, start_date: 1.day.ago, end_date: 1.day.from_now) }.by(1) end - let(:new_opts) do - opts.merge(in_reply_to_discussion_id: nil, - type: 'DiffNote', - noteable_type: 'MergeRequest', - noteable_id: merge_request.id, - position: position.to_h) - end + context 'in a merge request' do + let_it_be(:project_with_repo) { create(:project, :repository) } + let_it_be(:merge_request) do + create(:merge_request, source_project: project_with_repo, + target_project: project_with_repo) + end - before do - allow_any_instance_of(Gitlab::Diff::Position) - .to receive(:unfolded_diff?) { true } - end + context 'issue comment usage data' do + let(:opts) do + { note: 'Awesome comment', noteable_type: 'MergeRequest', noteable_id: merge_request.id } + end - it 'clears noteable diff cache when it was unfolded for the note position' do - expect_any_instance_of(Gitlab::Diff::HighlightCache).to receive(:clear) + it 'does not track' do + expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_comment_added_action) - described_class.new(project_with_repo, user, new_opts).execute - end + described_class.new(project, user, opts).execute + end + end - it 'does not clear cache when note is not the first of the discussion' do - prev_note = - create(:diff_note_on_merge_request, noteable: merge_request, - project: project_with_repo) - reply_opts = - opts.merge(in_reply_to_discussion_id: prev_note.discussion_id, - type: 'DiffNote', - noteable_type: 'MergeRequest', - noteable_id: merge_request.id, - position: position.to_h) + context 'noteable highlight cache clearing' do + let(:position) do + Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 14, + diff_refs: merge_request.diff_refs) + end - expect(merge_request).not_to receive(:diffs) + let(:new_opts) do + opts.merge(in_reply_to_discussion_id: nil, + type: 'DiffNote', + noteable_type: 'MergeRequest', + noteable_id: merge_request.id, + position: position.to_h) + end - described_class.new(project_with_repo, user, reply_opts).execute - end - end + before do + allow_any_instance_of(Gitlab::Diff::Position) + .to receive(:unfolded_diff?) { true } + end - context 'note diff file' do - let(:project_with_repo) { create(:project, :repository) } - let(:merge_request) do - create(:merge_request, - source_project: project_with_repo, - target_project: project_with_repo) - end + it 'clears noteable diff cache when it was unfolded for the note position' do + expect_any_instance_of(Gitlab::Diff::HighlightCache).to receive(:clear) - let(:line_number) { 14 } - let(:position) do - Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb", - new_path: "files/ruby/popen.rb", - old_line: nil, - new_line: line_number, - diff_refs: merge_request.diff_refs) - end + described_class.new(project_with_repo, user, new_opts).execute + end - let(:previous_note) do - create(:diff_note_on_merge_request, noteable: merge_request, project: project_with_repo) - end + it 'does not clear cache when note is not the first of the discussion' do + prev_note = + create(:diff_note_on_merge_request, noteable: merge_request, + project: project_with_repo) + reply_opts = + opts.merge(in_reply_to_discussion_id: prev_note.discussion_id, + type: 'DiffNote', + noteable_type: 'MergeRequest', + noteable_id: merge_request.id, + position: position.to_h) - before do - project_with_repo.add_maintainer(user) - end + expect(merge_request).not_to receive(:diffs) - context 'when eligible to have a note diff file' do - let(:new_opts) do - opts.merge(in_reply_to_discussion_id: nil, - type: 'DiffNote', - noteable_type: 'MergeRequest', - noteable_id: merge_request.id, - position: position.to_h) + described_class.new(project_with_repo, user, reply_opts).execute + end end - it 'note is associated with a note diff file' do - MergeRequests::MergeToRefService.new(merge_request.project, merge_request.author).execute(merge_request) - - note = described_class.new(project_with_repo, user, new_opts).execute - - expect(note).to be_persisted - expect(note.note_diff_file).to be_present - expect(note.diff_note_positions).to be_present - end - end + context 'note diff file' do + let(:line_number) { 14 } + let(:position) do + Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: line_number, + diff_refs: merge_request.diff_refs) + end - context 'when DiffNote is a reply' do - let(:new_opts) do - opts.merge(in_reply_to_discussion_id: previous_note.discussion_id, - type: 'DiffNote', - noteable_type: 'MergeRequest', - noteable_id: merge_request.id, - position: position.to_h) - end + let(:previous_note) do + create(:diff_note_on_merge_request, noteable: merge_request, project: project_with_repo) + end - it 'note is not associated with a note diff file' do - expect(Discussions::CaptureDiffNotePositionService).not_to receive(:new) + before do + project_with_repo.add_maintainer(user) + end - note = described_class.new(project_with_repo, user, new_opts).execute + context 'when eligible to have a note diff file' do + let(:new_opts) do + opts.merge(in_reply_to_discussion_id: nil, + type: 'DiffNote', + noteable_type: 'MergeRequest', + noteable_id: merge_request.id, + position: position.to_h) + end - expect(note).to be_persisted - expect(note.note_diff_file).to be_nil - end + it 'note is associated with a note diff file' do + MergeRequests::MergeToRefService.new(merge_request.project, merge_request.author).execute(merge_request) - context 'when DiffNote from an image' do - let(:image_position) do - Gitlab::Diff::Position.new(old_path: "files/images/6049019_460s.jpg", - new_path: "files/images/6049019_460s.jpg", - width: 100, - height: 100, - x: 1, - y: 100, - diff_refs: merge_request.diff_refs, - position_type: 'image') - end + note = described_class.new(project_with_repo, user, new_opts).execute - let(:new_opts) do - opts.merge(in_reply_to_discussion_id: nil, - type: 'DiffNote', - noteable_type: 'MergeRequest', - noteable_id: merge_request.id, - position: image_position.to_h) + expect(note).to be_persisted + expect(note.note_diff_file).to be_present + expect(note.diff_note_positions).to be_present + end end - it 'note is not associated with a note diff file' do - note = described_class.new(project_with_repo, user, new_opts).execute - - expect(note).to be_persisted - expect(note.note_diff_file).to be_nil + context 'when DiffNote is a reply' do + let(:new_opts) do + opts.merge(in_reply_to_discussion_id: previous_note.discussion_id, + type: 'DiffNote', + noteable_type: 'MergeRequest', + noteable_id: merge_request.id, + position: position.to_h) + end + + it 'note is not associated with a note diff file' do + expect(Discussions::CaptureDiffNotePositionService).not_to receive(:new) + + note = described_class.new(project_with_repo, user, new_opts).execute + + expect(note).to be_persisted + expect(note.note_diff_file).to be_nil + end + + context 'when DiffNote from an image' do + let(:image_position) do + Gitlab::Diff::Position.new(old_path: "files/images/6049019_460s.jpg", + new_path: "files/images/6049019_460s.jpg", + width: 100, + height: 100, + x: 1, + y: 100, + diff_refs: merge_request.diff_refs, + position_type: 'image') + end + + let(:new_opts) do + opts.merge(in_reply_to_discussion_id: nil, + type: 'DiffNote', + noteable_type: 'MergeRequest', + noteable_id: merge_request.id, + position: image_position.to_h) + end + + it 'note is not associated with a note diff file' do + note = described_class.new(project_with_repo, user, new_opts).execute + + expect(note).to be_persisted + expect(note.note_diff_file).to be_nil + end + end end end end diff --git a/spec/services/notes/destroy_service_spec.rb b/spec/services/notes/destroy_service_spec.rb index d1076f77cec..f0e5b29ac9b 100644 --- a/spec/services/notes/destroy_service_spec.rb +++ b/spec/services/notes/destroy_service_spec.rb @@ -24,36 +24,54 @@ RSpec.describe Notes::DestroyService do .to change { user.todos_pending_count }.from(1).to(0) end - context 'noteable highlight cache clearing' do - let(:repo_project) { create(:project, :repository) } - let(:merge_request) do + it 'tracks issue comment removal usage data', :clean_gitlab_redis_shared_state do + note = create(:note, project: project, noteable: issue) + event = Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_COMMENT_REMOVED + counter = Gitlab::UsageDataCounters::HLLRedisCounter + + expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_comment_removed_action).with(author: user).and_call_original + expect do + described_class.new(project, user).execute(note) + end.to change { counter.unique_events(event_names: event, start_date: 1.day.ago, end_date: 1.day.from_now) }.by(1) + end + + context 'in a merge request' do + let_it_be(:repo_project) { create(:project, :repository) } + let_it_be(:merge_request) do create(:merge_request, source_project: repo_project, - target_project: repo_project) + target_project: repo_project) end - - let(:note) do + let_it_be(:note) do create(:diff_note_on_merge_request, project: repo_project, - noteable: merge_request) + noteable: merge_request) end - before do - allow(note.position).to receive(:unfolded_diff?) { true } - end - - it 'clears noteable diff cache when it was unfolded for the note position' do - expect(merge_request).to receive_message_chain(:diffs, :clear_cache) + it 'does not track issue comment removal usage data' do + expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_comment_removed_action) described_class.new(repo_project, user).execute(note) end - it 'does not clear cache when note is not the first of the discussion' do - reply_note = create(:diff_note_on_merge_request, in_reply_to: note, - project: repo_project, - noteable: merge_request) + context 'noteable highlight cache clearing' do + before do + allow(note.position).to receive(:unfolded_diff?) { true } + end + + it 'clears noteable diff cache when it was unfolded for the note position' do + expect(merge_request).to receive_message_chain(:diffs, :clear_cache) + + described_class.new(repo_project, user).execute(note) + end + + it 'does not clear cache when note is not the first of the discussion' do + reply_note = create(:diff_note_on_merge_request, in_reply_to: note, + project: repo_project, + noteable: merge_request) - expect(merge_request).not_to receive(:diffs) + expect(merge_request).not_to receive(:diffs) - described_class.new(repo_project, user).execute(reply_note) + described_class.new(repo_project, user).execute(reply_note) + end end end end diff --git a/spec/services/notes/update_service_spec.rb b/spec/services/notes/update_service_spec.rb index 66efdf8abe7..e2f51c9af67 100644 --- a/spec/services/notes/update_service_spec.rb +++ b/spec/services/notes/update_service_spec.rb @@ -47,6 +47,22 @@ RSpec.describe Notes::UpdateService do end end + it 'does not track usage data when params is blank' do + expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_comment_edited_action) + + update_note({}) + end + + it 'tracks usage data', :clean_gitlab_redis_shared_state do + event = Gitlab::UsageDataCounters::IssueActivityUniqueCounter::ISSUE_COMMENT_EDITED + counter = Gitlab::UsageDataCounters::HLLRedisCounter + + expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).to receive(:track_issue_comment_edited_action).with(author: user).and_call_original + expect do + update_note(note: 'new text') + end.to change { counter.unique_events(event_names: event, start_date: 1.day.ago, end_date: 1.day.from_now) }.by(1) + end + context 'with system note' do before do note.update_column(:system, true) @@ -55,6 +71,12 @@ RSpec.describe Notes::UpdateService do it 'does not update the note' do expect { update_note(note: 'new text') }.not_to change { note.reload.note } end + + it 'does not track usage data' do + expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_comment_edited_action) + + update_note(note: 'new text') + end end context 'suggestions' do @@ -220,6 +242,12 @@ RSpec.describe Notes::UpdateService do expect(note).not_to receive(:create_new_cross_references!) update_note({ note: "Updated with new reference: #{issue.to_reference}" }) end + + it 'does not track usage data' do + expect(Gitlab::UsageDataCounters::IssueActivityUniqueCounter).not_to receive(:track_issue_comment_edited_action) + + update_note(note: 'new text') + end end end end diff --git a/spec/support/helpers/table_schema_helpers.rb b/spec/support/helpers/table_schema_helpers.rb new file mode 100644 index 00000000000..39f783caa0a --- /dev/null +++ b/spec/support/helpers/table_schema_helpers.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +module TableSchemaHelpers + def connection + ActiveRecord::Base.connection + end + + def expect_table_to_be_replaced(original_table:, replacement_table:, archived_table:) + original_oid = table_oid(original_table) + replacement_oid = table_oid(replacement_table) + + yield + + expect(table_oid(original_table)).to eq(replacement_oid) + expect(table_oid(archived_table)).to eq(original_oid) + expect(table_oid(replacement_table)).to be_nil + end + + def table_oid(name) + connection.select_value(<<~SQL) + SELECT oid + FROM pg_catalog.pg_class + WHERE relname = '#{name}' + SQL + end + + def table_type(name) + connection.select_value(<<~SQL) + SELECT + CASE class.relkind + WHEN 'r' THEN 'normal' + WHEN 'p' THEN 'partitioned' + ELSE 'other' + END as table_type + FROM pg_catalog.pg_class class + WHERE class.relname = '#{name}' + SQL + end + + def sequence_owned_by(table_name, column_name) + connection.select_value(<<~SQL) + SELECT + sequence.relname as name + FROM pg_catalog.pg_class as sequence + INNER JOIN pg_catalog.pg_depend depend + ON depend.objid = sequence.relfilenode + INNER JOIN pg_catalog.pg_class class + ON class.relfilenode = depend.refobjid + INNER JOIN pg_catalog.pg_attribute attribute + ON attribute.attnum = depend.refobjsubid + AND attribute.attrelid = depend.refobjid + WHERE class.relname = '#{table_name}' + AND attribute.attname = '#{column_name}' + SQL + end + + def default_expression_for(table_name, column_name) + connection.select_value(<<~SQL) + SELECT + pg_get_expr(attrdef.adbin, attrdef.adrelid) AS default_value + FROM pg_catalog.pg_attribute attribute + INNER JOIN pg_catalog.pg_attrdef attrdef + ON attribute.attrelid = attrdef.adrelid + AND attribute.attnum = attrdef.adnum + WHERE attribute.attrelid = '#{table_name}'::regclass + AND attribute.attname = '#{column_name}' + SQL + end + + def primary_key_constraint_name(table_name) + connection.select_value(<<~SQL) + SELECT + conname AS constraint_name + FROM pg_catalog.pg_constraint + WHERE conrelid = '#{table_name}'::regclass + AND contype = 'p' + SQL + end +end diff --git a/spec/support/shared_examples/quick_actions/issuable/time_tracking_quick_action_shared_examples.rb b/spec/support/shared_examples/quick_actions/issuable/time_tracking_quick_action_shared_examples.rb index 37ee2548dfe..17fd2b836d3 100644 --- a/spec/support/shared_examples/quick_actions/issuable/time_tracking_quick_action_shared_examples.rb +++ b/spec/support/shared_examples/quick_actions/issuable/time_tracking_quick_action_shared_examples.rb @@ -13,7 +13,7 @@ RSpec.shared_examples 'issuable time tracker' do |issuable_type| end it 'renders the sidebar component empty state' do - page.within '.time-tracking-no-tracking-pane' do + page.within '[data-testid="noTrackingPane"]' do expect(page).to have_content 'No estimate or time spent' end end @@ -22,7 +22,7 @@ RSpec.shared_examples 'issuable time tracker' do |issuable_type| submit_time('/estimate 3w 1d 1h') wait_for_requests - page.within '.time-tracking-estimate-only-pane' do + page.within '[data-testid="estimateOnlyPane"]' do expect(page).to have_content '3w 1d 1h' end end @@ -31,7 +31,7 @@ RSpec.shared_examples 'issuable time tracker' do |issuable_type| submit_time('/spend 3w 1d 1h') wait_for_requests - page.within '.time-tracking-spend-only-pane' do + page.within '[data-testid="spentOnlyPane"]' do expect(page).to have_content '3w 1d 1h' end end @@ -41,7 +41,7 @@ RSpec.shared_examples 'issuable time tracker' do |issuable_type| submit_time('/spend 3w 1d 1h') wait_for_requests - page.within '.time-tracking-comparison-pane' do + page.within '[data-testid="timeTrackingComparisonPane"]' do expect(page).to have_content '3w 1d 1h' end end |