diff options
42 files changed, 951 insertions, 84 deletions
diff --git a/app/assets/javascripts/graphql_shared/issuable_client.js b/app/assets/javascripts/graphql_shared/issuable_client.js index 15e7ef7d62c..adc6df09b30 100644 --- a/app/assets/javascripts/graphql_shared/issuable_client.js +++ b/app/assets/javascripts/graphql_shared/issuable_client.js @@ -5,6 +5,7 @@ import { concatPagination } from '@apollo/client/utilities'; import getIssueStateQuery from '~/issues/show/queries/get_issue_state.query.graphql'; import createDefaultClient from '~/lib/graphql'; import typeDefs from '~/work_items/graphql/typedefs.graphql'; +import { WIDGET_TYPE_NOTES } from '~/work_items/constants'; export const config = { typeDefs, @@ -22,10 +23,20 @@ export const config = { }, }, }, + WorkItemWidgetNotes: { + fields: { + // If we add any key args, the discussions field becomes discussions({"filter":"ONLY_ACTIVITY","first":10}) and + // kills any possibility to handle it on the widget level without hardcoding a string. + discussions: { + keyArgs: false, + }, + }, + }, WorkItem: { fields: { + // widgets policy because otherwise the subscriptions invalidate the cache widgets: { - merge(existing = [], incoming) { + merge(existing = [], incoming, context) { if (existing.length === 0) { return incoming; } @@ -33,6 +44,24 @@ export const config = { const incomingWidget = incoming.find( (w) => w.type && w.type === existingWidget.type, ); + // We don't want to override existing notes with empty widget on work item updates + if (incomingWidget?.type === WIDGET_TYPE_NOTES && !context.variables.pageSize) { + return existingWidget; + } + // we want to concat next page of discussions to the existing ones + if (incomingWidget?.type === WIDGET_TYPE_NOTES && context.variables.after) { + // concatPagination won't work because we were placing new widget here so we have to do this manually + return { + ...incomingWidget, + discussions: { + ...incomingWidget.discussions, + nodes: [ + ...existingWidget.discussions.nodes, + ...incomingWidget.discussions.nodes, + ], + }, + }; + } return incomingWidget || existingWidget; }); }, diff --git a/app/assets/javascripts/notes/components/note_header.vue b/app/assets/javascripts/notes/components/note_header.vue index 36f7d720e48..79b6139d4b1 100644 --- a/app/assets/javascripts/notes/components/note_header.vue +++ b/app/assets/javascripts/notes/components/note_header.vue @@ -1,6 +1,7 @@ <script> import { GlIcon, GlBadge, GlLoadingIcon, GlTooltipDirective } from '@gitlab/ui'; import { mapActions } from 'vuex'; +import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import { __, s__ } from '~/locale'; import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; @@ -74,6 +75,12 @@ export default { }; }, computed: { + authorId() { + return getIdFromGraphQLId(this.author.id); + }, + authorHref() { + return this.author.path || this.author.webUrl; + }, toggleChevronIconName() { return this.expanded ? 'chevron-up' : 'chevron-down'; }, @@ -145,9 +152,9 @@ export default { <template v-if="hasAuthor"> <a ref="authorNameLink" - :href="author.path" + :href="authorHref" :class="authorLinkClasses" - :data-user-id="author.id" + :data-user-id="authorId" :data-username="author.username" > <span class="note-header-author-name gl-font-weight-bold"> diff --git a/app/assets/javascripts/work_items/components/notes/activity_filter.vue b/app/assets/javascripts/work_items/components/notes/activity_filter.vue new file mode 100644 index 00000000000..71784d3a807 --- /dev/null +++ b/app/assets/javascripts/work_items/components/notes/activity_filter.vue @@ -0,0 +1,113 @@ +<script> +import { GlDropdown, GlDropdownItem } from '@gitlab/ui'; +import { __ } from '~/locale'; +import Tracking from '~/tracking'; +import LocalStorageSync from '~/vue_shared/components/local_storage_sync.vue'; +import { ASC, DESC } from '~/notes/constants'; +import { TRACKING_CATEGORY_SHOW, WORK_ITEM_NOTES_SORT_ORDER_KEY } from '~/work_items/constants'; + +const SORT_OPTIONS = [ + { key: DESC, text: __('Newest first'), dataid: 'js-newest-first' }, + { key: ASC, text: __('Oldest first'), dataid: 'js-oldest-first' }, +]; + +export default { + SORT_OPTIONS, + components: { + GlDropdown, + GlDropdownItem, + LocalStorageSync, + }, + mixins: [Tracking.mixin()], + props: { + sortOrder: { + type: String, + default: ASC, + required: false, + }, + loading: { + type: Boolean, + default: false, + required: false, + }, + workItemType: { + type: String, + required: true, + }, + }, + data() { + return { + persistSortOrder: true, + }; + }, + computed: { + tracking() { + return { + category: TRACKING_CATEGORY_SHOW, + label: 'item_track_notes_sorting', + property: `type_${this.workItemType}`, + }; + }, + selectedSortOption() { + const isSortOptionValid = this.sortOrder === ASC || this.sortOrder === DESC; + return isSortOptionValid ? SORT_OPTIONS.find(({ key }) => this.sortOrder === key) : ASC; + }, + getDropdownSelectedText() { + return this.selectedSortOption.text; + }, + }, + methods: { + setDiscussionSortDirection(direction) { + this.$emit('updateSavedSortOrder', direction); + }, + fetchSortedDiscussions(direction) { + if (this.isSortDropdownItemActive(direction)) { + return; + } + this.track('notes_sort_order_changed'); + this.$emit('changeSortOrder', direction); + }, + isSortDropdownItemActive(sortDir) { + return sortDir === this.sortOrder; + }, + }, + WORK_ITEM_NOTES_SORT_ORDER_KEY, +}; +</script> + +<template> + <div + id="discussion-preferences" + data-testid="discussion-preferences" + class="gl-display-inline-block gl-vertical-align-bottom gl-w-full gl-sm-w-auto" + > + <local-storage-sync + :value="sortOrder" + :storage-key="$options.WORK_ITEM_NOTES_SORT_ORDER_KEY" + :persist="persistSortOrder" + as-string + @input="setDiscussionSortDirection" + /> + <gl-dropdown + :id="`discussion-preferences-dropdown-${workItemType}`" + class="gl-xs-w-full" + size="small" + :text="getDropdownSelectedText" + :disabled="loading" + right + > + <div id="discussion-sort"> + <gl-dropdown-item + v-for="{ text, key, dataid } in $options.SORT_OPTIONS" + :key="text" + :data-testid="dataid" + is-check-item + :is-checked="isSortDropdownItemActive(key)" + @click="fetchSortedDiscussions(key)" + > + {{ text }} + </gl-dropdown-item> + </div> + </gl-dropdown> + </div> +</template> diff --git a/app/assets/javascripts/work_items/components/work_item_detail.vue b/app/assets/javascripts/work_items/components/work_item_detail.vue index 23dfa04ba5f..1855a0a37f2 100644 --- a/app/assets/javascripts/work_items/components/work_item_detail.vue +++ b/app/assets/javascripts/work_items/components/work_item_detail.vue @@ -640,6 +640,7 @@ export default { :query-variables="queryVariables" :full-path="fullPath" :fetch-by-iid="fetchByIid" + :work-item-type="workItemType" class="gl-pt-5" @error="updateError = $event" /> diff --git a/app/assets/javascripts/work_items/components/work_item_notes.vue b/app/assets/javascripts/work_items/components/work_item_notes.vue index 91e90589a93..b8019dcad1a 100644 --- a/app/assets/javascripts/work_items/components/work_item_notes.vue +++ b/app/assets/javascripts/work_items/components/work_item_notes.vue @@ -1,8 +1,10 @@ <script> -import { GlSkeletonLoader } from '@gitlab/ui'; +import { GlSkeletonLoader, GlIntersectionObserver } from '@gitlab/ui'; import { s__ } from '~/locale'; import SystemNote from '~/work_items/components/notes/system_note.vue'; +import ActivityFilter from '~/work_items/components/notes/activity_filter.vue'; import { i18n, DEFAULT_PAGE_SIZE_NOTES } from '~/work_items/constants'; +import { ASC, DESC } from '~/notes/constants'; import { getWorkItemNotesQuery } from '~/work_items/utils'; export default { @@ -17,36 +19,62 @@ export default { components: { SystemNote, GlSkeletonLoader, + ActivityFilter, + GlIntersectionObserver, }, props: { workItemId: { type: String, required: true, }, + fetchByIid: { + type: Boolean, + required: false, + default: false, + }, queryVariables: { type: Object, required: true, }, - fullPath: { + workItemType: { type: String, required: true, }, - fetchByIid: { - type: Boolean, - required: false, - default: false, - }, + }, + data() { + return { + notesArray: [], + isLoadingMore: false, + perPage: DEFAULT_PAGE_SIZE_NOTES, + sortOrder: ASC, + changeNotesSortOrderAfterLoading: false, + }; }, computed: { - areNotesLoading() { - return this.$apollo.queries.workItemNotes.loading; - }, - notes() { - return this.workItemNotes?.nodes; + initialLoading() { + return this.$apollo.queries.workItemNotes.loading && !this.isLoadingMore; }, pageInfo() { return this.workItemNotes?.pageInfo; }, + hasNextPage() { + return this.pageInfo?.hasNextPage; + }, + showInitialLoader() { + return this.initialLoading || this.changeNotesSortOrderAfterLoading; + }, + showTimeline() { + return this.notesArray?.length && !this.changeNotesSortOrderAfterLoading; + }, + showLoadingMoreSkeleton() { + return this.isLoadingMore && !this.changeNotesSortOrderAfterLoading; + }, + disableActivityFilter() { + return this.initialLoading || this.isLoadingMore; + }, + showIntersectionObserver() { + return this.hasNextPage && !this.isLoadingMore; + }, }, apollo: { workItemNotes: { @@ -59,6 +87,7 @@ export default { variables() { return { ...this.queryVariables, + after: this.after, pageSize: DEFAULT_PAGE_SIZE_NOTES, }; }, @@ -66,7 +95,11 @@ export default { const workItemWidgets = this.fetchByIid ? data.workspace?.workItems?.nodes[0]?.widgets : data.workItem?.widgets; - return workItemWidgets.find((widget) => widget.type === 'NOTES').discussions || []; + const discussionNodes = + workItemWidgets.find((widget) => widget.type === 'NOTES')?.discussions || []; + this.notesArray = discussionNodes?.nodes || []; + this.updateSortingOrderIfApplicable(); + return discussionNodes; }, skip() { return !this.queryVariables.id && !this.queryVariables.iid; @@ -76,13 +109,73 @@ export default { }, }, }, + methods: { + updateSortingOrderIfApplicable() { + // when the sort order is DESC in local storage and there is only a single page, call + // changeSortOrder manually + if ( + this.changeNotesSortOrderAfterLoading && + this.perPage === DEFAULT_PAGE_SIZE_NOTES && + !this.hasNextPage + ) { + this.changeNotesSortOrder(DESC); + } + }, + updateInitialSortedOrder(direction) { + this.sortOrder = direction; + // when the direction is reverse , we need to load all since the sorting is on the frontend + if (direction === DESC) { + this.changeNotesSortOrderAfterLoading = true; + } + }, + changeNotesSortOrder(direction) { + this.sortOrder = direction; + this.notesArray = [...this.notesArray].reverse(); + this.changeNotesSortOrderAfterLoading = false; + }, + async fetchMoreNotes() { + this.isLoadingMore = true; + // copied from discussions batch logic - every fetchMore call has a higher + // amount of page size than the previous one with the limit being 100 + this.perPage = Math.min(Math.round(this.perPage * 1.5), 100); + await this.$apollo.queries.workItemNotes + .fetchMore({ + variables: { + ...this.queryVariables, + pageSize: this.perPage, + after: this.pageInfo?.endCursor, + }, + }) + .catch((error) => this.$emit('error', error.message)); + this.isLoadingMore = false; + if (this.changeNotesSortOrderAfterLoading && !this.hasNextPage) { + this.changeNotesSortOrder(this.sortOrder); + } + }, + }, }; </script> <template> <div class="gl-border-t gl-mt-5"> - <label class="gl-mb-0">{{ $options.i18n.ACTIVITY_LABEL }}</label> - <div v-if="areNotesLoading" class="gl-mt-5"> + <gl-intersection-observer + v-if="showIntersectionObserver" + class="gl-absolute gl-top-0" + height="100" + @appear="fetchMoreNotes" + /> + <div class="gl-display-flex gl-justify-content-space-between gl-flex-wrap"> + <label class="gl-mb-0">{{ $options.i18n.ACTIVITY_LABEL }}</label> + <activity-filter + class="gl-min-h-5 gl-pb-3" + :loading="disableActivityFilter" + :sort-order="sortOrder" + :work-item-type="workItemType" + @changeSortOrder="changeNotesSortOrder" + @updateSavedSortOrder="updateInitialSortedOrder" + /> + </div> + <div v-if="showInitialLoader" class="gl-mt-5"> <gl-skeleton-loader v-for="index in $options.loader.repeat" :key="index" @@ -94,16 +187,30 @@ export default { <rect width="500" x="45" y="15" height="10" rx="4" /> </gl-skeleton-loader> </div> - <div v-else class="issuable-discussion gl-mb-5 work-item-notes"> - <template v-if="notes && notes.length"> - <ul class="notes main-notes-list timeline"> + <div v-else class="issuable-discussion gl-mb-5 gl-clearfix!"> + <template v-if="showTimeline"> + <ul class="notes main-notes-list timeline gl-clearfix!"> <system-note - v-for="note in notes" + v-for="note in notesArray" :key="note.notes.nodes[0].id" :note="note.notes.nodes[0]" + :data-testid="note.notes.nodes[0].id" /> </ul> </template> + + <template v-if="showLoadingMoreSkeleton"> + <gl-skeleton-loader + v-for="index in $options.loader.repeat" + :key="index" + :width="$options.loader.width" + :height="$options.loader.height" + preserve-aspect-ratio="xMinYMax meet" + > + <circle cx="20" cy="20" r="16" /> + <rect width="500" x="45" y="15" height="10" rx="4" /> + </gl-skeleton-loader> + </template> </div> </div> </template> diff --git a/app/assets/javascripts/work_items/constants.js b/app/assets/javascripts/work_items/constants.js index a2249d187be..81f9bf04bc8 100644 --- a/app/assets/javascripts/work_items/constants.js +++ b/app/assets/javascripts/work_items/constants.js @@ -173,4 +173,6 @@ export const FORM_TYPES = { }; export const DEFAULT_PAGE_SIZE_ASSIGNEES = 10; -export const DEFAULT_PAGE_SIZE_NOTES = 100; +export const DEFAULT_PAGE_SIZE_NOTES = 30; + +export const WORK_ITEM_NOTES_SORT_ORDER_KEY = 'sort_direction_work_item'; diff --git a/app/models/chat_name.rb b/app/models/chat_name.rb index 5996e15df4e..9bd618c1008 100644 --- a/app/models/chat_name.rb +++ b/app/models/chat_name.rb @@ -7,7 +7,6 @@ class ChatName < ApplicationRecord belongs_to :user validates :user, presence: true - validates :integration, presence: true validates :team_id, presence: true validates :chat_id, presence: true diff --git a/app/models/integrations/base_slash_commands.rb b/app/models/integrations/base_slash_commands.rb index 11ff7547325..619579a543a 100644 --- a/app/models/integrations/base_slash_commands.rb +++ b/app/models/integrations/base_slash_commands.rb @@ -66,7 +66,7 @@ module Integrations # rubocop: disable CodeReuse/ServiceClass def authorize_chat_name_url(params) - ChatNames::AuthorizeUserService.new(self, params).execute + ChatNames::AuthorizeUserService.new(params).execute end # rubocop: enable CodeReuse/ServiceClass end diff --git a/app/services/chat_names/authorize_user_service.rb b/app/services/chat_names/authorize_user_service.rb index 6c28a1cea7e..32714831fb8 100644 --- a/app/services/chat_names/authorize_user_service.rb +++ b/app/services/chat_names/authorize_user_service.rb @@ -4,8 +4,7 @@ module ChatNames class AuthorizeUserService include Gitlab::Routing - def initialize(integration, params) - @integration = integration + def initialize(params) @params = params end @@ -29,7 +28,6 @@ module ChatNames def chat_name_params { - integration_id: @integration.id, team_id: @params[:team_id], team_domain: @params[:team_domain], chat_id: @params[:user_id], diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 468cadb03c7..5f00b4d85c6 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -184,16 +184,18 @@ module MergeRequests merge_request, merge_request.project, current_user, old_reviewers) end - def create_pipeline_for(merge_request, user, async: false) + def create_pipeline_for(merge_request, user, async: false, allow_duplicate: false) + create_pipeline_params = params.slice(:push_options).merge(allow_duplicate: allow_duplicate) + if async MergeRequests::CreatePipelineWorker.perform_async( project.id, user.id, merge_request.id, - params.slice(:push_options).deep_stringify_keys) + create_pipeline_params.deep_stringify_keys) else MergeRequests::CreatePipelineService - .new(project: project, current_user: user, params: params.slice(:push_options)) + .new(project: project, current_user: user, params: create_pipeline_params) .execute(merge_request) end end diff --git a/app/views/profiles/chat_names/_chat_name.html.haml b/app/views/profiles/chat_names/_chat_name.html.haml index 0b45869bdf9..ce2fc2098c5 100644 --- a/app/views/profiles/chat_names/_chat_name.html.haml +++ b/app/views/profiles/chat_names/_chat_name.html.haml @@ -1,18 +1,20 @@ - integration = chat_name.integration -- project = integration.project +- project = integration&.project %tr %td %strong - - if can?(current_user, :read_project, project) + - if project.present? && can?(current_user, :read_project, project) = link_to project.full_name, project_path(project) - else .light= _('Not applicable.') %td %strong - - if can?(current_user, :admin_project, project) + - if integration.present? && can?(current_user, :admin_project, project) = link_to integration.title, edit_project_settings_integration_path(project, integration) - - else + - elsif integration.present? = integration.title + - else + .light= _('Not applicable.') %td = chat_name.team_domain %td diff --git a/app/workers/merge_requests/create_pipeline_worker.rb b/app/workers/merge_requests/create_pipeline_worker.rb index b40408cf647..096f2500019 100644 --- a/app/workers/merge_requests/create_pipeline_worker.rb +++ b/app/workers/merge_requests/create_pipeline_worker.rb @@ -25,11 +25,15 @@ module MergeRequests merge_request = MergeRequest.find_by_id(merge_request_id) return unless merge_request + allow_duplicate = params.with_indifferent_access[:allow_duplicate] push_options = params.with_indifferent_access[:push_options] MergeRequests::CreatePipelineService - .new(project: project, current_user: user, params: { push_options: push_options }) - .execute(merge_request) + .new( + project: project, + current_user: user, + params: { allow_duplicate: allow_duplicate, push_options: push_options } + ).execute(merge_request) merge_request.update_head_pipeline end diff --git a/db/migrate/20221228083452_remove_check_constraint_on_chat_names_on_integration.rb b/db/migrate/20221228083452_remove_check_constraint_on_chat_names_on_integration.rb new file mode 100644 index 00000000000..6a75bd1badd --- /dev/null +++ b/db/migrate/20221228083452_remove_check_constraint_on_chat_names_on_integration.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class RemoveCheckConstraintOnChatNamesOnIntegration < Gitlab::Database::Migration[2.1] + CONSTRAINT_NAME = 'check_2b0a0d0f0f' + + disable_ddl_transaction! + + def up + remove_check_constraint(:chat_names, CONSTRAINT_NAME) + end + + def down + # noop: rollback would not work as we can have records where `integration_id` IS NULL + end +end diff --git a/db/schema_migrations/20221228083452 b/db/schema_migrations/20221228083452 new file mode 100644 index 00000000000..6be5bb4eb6c --- /dev/null +++ b/db/schema_migrations/20221228083452 @@ -0,0 +1 @@ +44e854a2afa7bebeb1e220ec1dee2f204173be59e403ba8a70ba3d22675be7a9
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index baa9c6c75e8..62a5bb0ce7c 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -12594,8 +12594,7 @@ CREATE TABLE chat_names ( last_used_at timestamp without time zone, created_at timestamp without time zone NOT NULL, updated_at timestamp without time zone NOT NULL, - integration_id integer, - CONSTRAINT check_2b0a0d0f0f CHECK ((integration_id IS NOT NULL)) + integration_id integer ); CREATE SEQUENCE chat_names_id_seq diff --git a/doc/development/documentation/testing.md b/doc/development/documentation/testing.md index 8b8f281d7c1..019cfe78bb2 100644 --- a/doc/development/documentation/testing.md +++ b/doc/development/documentation/testing.md @@ -282,16 +282,16 @@ Vale returns three types of results: - **Error** - For branding guidelines, trademark guidelines, and anything that causes content on the docs site to render incorrectly. -- **Warning** - For Technical Writing team style preferences. -- **Suggestion** - For basic technical writing tenets and best practices. +- **Warning** - For general style guide rules, tenets, and best practices. +- **Suggestion** - For technical writing style preferences that may require refactoring of documentation or updates to an exceptions list. The result types have these attributes: -| Result type | Displayed in CI/CD job output | Causes CI/CD jobs to fail | Vale rule link | -|--------------|-------------------------------|---------------------------|----------------| -| `error` | **{check-circle}** Yes | **{check-circle}** Yes | [Error-level Vale rules](https://gitlab.com/search?utf8=✓&snippets=false&scope=&repository_ref=master&search=path%3Adoc%2F.vale%2Fgitlab+Error%3A&group_id=9970&project_id=278964) | -| `warning` | **{dotted-circle}** No | **{dotted-circle}** No | [Warning-level Vale rules](https://gitlab.com/search?utf8=✓&snippets=false&scope=&repository_ref=master&search=path%3Adoc%2F.vale%2Fgitlab+Warning%3A&group_id=9970&project_id=278964) | -| `suggestion` | **{dotted-circle}** No | **{dotted-circle}** No | [Suggestion-level Vale rules](https://gitlab.com/search?utf8=✓&snippets=false&scope=&repository_ref=master&search=path%3Adoc%2F.vale%2Fgitlab+Suggestion%3A&group_id=9970&project_id=278964) | +| Result type | Displays in CI/CD job output | Displays in MR diff | Causes CI/CD jobs to fail | Vale rule link | +|--------------|------------------------------|---------------------|---------------------------|----------------| +| `error` | **{check-circle}** Yes | **{check-circle}** Yes | **{check-circle}** Yes | [Error-level Vale rules](https://gitlab.com/search?utf8=✓&snippets=false&scope=&repository_ref=master&search=path%3Adoc%2F.vale%2Fgitlab+Error%3A&group_id=9970&project_id=278964) | +| `warning` | **{dotted-circle}** No | **{check-circle}** Yes | **{dotted-circle}** No | [Warning-level Vale rules](https://gitlab.com/search?utf8=✓&snippets=false&scope=&repository_ref=master&search=path%3Adoc%2F.vale%2Fgitlab+Warning%3A&group_id=9970&project_id=278964) | +| `suggestion` | **{dotted-circle}** No | **{dotted-circle}** No | **{dotted-circle}** No | [Suggestion-level Vale rules](https://gitlab.com/search?utf8=✓&snippets=false&scope=&repository_ref=master&search=path%3Adoc%2F.vale%2Fgitlab+Suggestion%3A&group_id=9970&project_id=278964) | #### Vale spelling test @@ -360,6 +360,10 @@ In general, follow these guidelines: If the rule is too subjective, it cannot be adequately enforced and creates unnecessary additional warnings. + - Whether it's appropriate to display in the merge request diff in the GitLab UI. + If the rule is difficult to implement directly in the merge request (for example, + it requires page refactoring), set it to suggestion-level so it displays in local editors only. + ### Install linters At a minimum, install [markdownlint](#markdownlint) and [Vale](#vale) to match the checks run in diff --git a/doc/operations/incident_management/alerts.md b/doc/operations/incident_management/alerts.md index 0dffa15351c..d42ee237749 100644 --- a/doc/operations/incident_management/alerts.md +++ b/doc/operations/incident_management/alerts.md @@ -37,11 +37,6 @@ The alert list displays the following information: - **Resolved**: No further work is required. - **Ignored**: No action will be taken on the alert. -NOTE: -Check out a live example available from the -[`tanuki-inc` project page](https://gitlab.com/gitlab-examples/ops/incident-setup/everyone/tanuki-inc) -in GitLab to examine alerts in action. - ## Alert severity Each level of alert contains a uniquely shaped and color-coded icon to help @@ -70,10 +65,6 @@ Alerts contain one of the following icons: Navigate to the Alert details view by visiting the [Alert list](alerts.md) and selecting an alert from the list. You need at least the Developer role to access alerts. - -NOTE: -To review live examples of GitLab alerts, visit the -[alert list](https://gitlab.com/gitlab-examples/ops/incident-setup/everyone/tanuki-inc/-/alert_management) for this demo project. Select any alert in the list to examine its alert details page. diff --git a/lib/gitlab/chat_name_token.rb b/lib/gitlab/chat_name_token.rb index 76f2a4ae38c..9b4cb9d0134 100644 --- a/lib/gitlab/chat_name_token.rb +++ b/lib/gitlab/chat_name_token.rb @@ -16,9 +16,7 @@ module Gitlab def get Gitlab::Redis::SharedState.with do |redis| data = redis.get(redis_shared_state_key) - params = Gitlab::Json.parse(data, symbolize_names: true) if data - params[:integration_id] ||= params.delete(:service_id) if params && params[:service_id] - params + Gitlab::Json.parse(data, symbolize_names: true) if data end end diff --git a/lib/gitlab/database.rb b/lib/gitlab/database.rb index 51d5bfcee38..40e2e637114 100644 --- a/lib/gitlab/database.rb +++ b/lib/gitlab/database.rb @@ -84,7 +84,7 @@ module Gitlab # # TODO: https://gitlab.com/gitlab-org/geo-team/discussions/-/issues/5032 def self.database_base_models_using_load_balancing - @database_base_models_with_gitlab_shared ||= { + @database_base_models_using_load_balancing ||= { # Note that we use ActiveRecord::Base here and not ApplicationRecord. # This is deliberate, as we also use these classes to apply load # balancing to, and the load balancer must be enabled for _all_ models diff --git a/lib/sidebars/your_work/menus/issues_menu.rb b/lib/sidebars/your_work/menus/issues_menu.rb new file mode 100644 index 00000000000..6046b78e54e --- /dev/null +++ b/lib/sidebars/your_work/menus/issues_menu.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +module Sidebars + module YourWork + module Menus + class IssuesMenu < ::Sidebars::Menu + include Gitlab::Utils::StrongMemoize + + override :link + def link + issues_dashboard_path(assignee_username: @context.current_user.username) + end + + override :title + def title + _('Issues') + end + + override :sprite_icon + def sprite_icon + 'issues' + end + + override :render? + def render? + !!context.current_user + end + + override :active_routes + def active_routes + { path: 'dashboard#issues' } + end + + override :has_pill? + def has_pill? + pill_count > 0 + end + + override :pill_count + def pill_count + context.current_user.assigned_open_issues_count + end + strong_memoize_attr :pill_count + end + end + end +end diff --git a/lib/sidebars/your_work/menus/merge_requests_menu.rb b/lib/sidebars/your_work/menus/merge_requests_menu.rb new file mode 100644 index 00000000000..695c2ffdf46 --- /dev/null +++ b/lib/sidebars/your_work/menus/merge_requests_menu.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +module Sidebars + module YourWork + module Menus + class MergeRequestsMenu < ::Sidebars::Menu + include Gitlab::Utils::StrongMemoize + + override :link + def link + merge_requests_dashboard_path(assignee_username: @context.current_user.username) + end + + override :title + def title + _('Merge requests') + end + + override :sprite_icon + def sprite_icon + 'merge-request' + end + + override :render? + def render? + !!context.current_user + end + + override :active_routes + def active_routes + { path: 'dashboard#merge_requests' } + end + + override :has_pill? + def has_pill? + pill_count > 0 + end + + override :pill_count + def pill_count + context.current_user.assigned_open_merge_requests_count + end + strong_memoize_attr :pill_count + end + end + end +end diff --git a/lib/sidebars/your_work/menus/todos_menu.rb b/lib/sidebars/your_work/menus/todos_menu.rb new file mode 100644 index 00000000000..d37ffadb579 --- /dev/null +++ b/lib/sidebars/your_work/menus/todos_menu.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +module Sidebars + module YourWork + module Menus + class TodosMenu < ::Sidebars::Menu + include Gitlab::Utils::StrongMemoize + + override :link + def link + dashboard_todos_path + end + + override :title + def title + _('To-Do List') + end + + override :sprite_icon + def sprite_icon + 'todo-done' + end + + override :render? + def render? + !!context.current_user + end + + override :active_routes + def active_routes + { path: 'dashboard/todos#index' } + end + + override :has_pill? + def has_pill? + pill_count > 0 + end + + override :pill_count + def pill_count + context.current_user.todos_pending_count + end + strong_memoize_attr :pill_count + end + end + end +end diff --git a/lib/sidebars/your_work/panel.rb b/lib/sidebars/your_work/panel.rb index bd529318408..ed43f1dc4bd 100644 --- a/lib/sidebars/your_work/panel.rb +++ b/lib/sidebars/your_work/panel.rb @@ -22,6 +22,9 @@ module Sidebars def add_menus add_menu(Sidebars::YourWork::Menus::ProjectsMenu.new(context)) + add_menu(Sidebars::YourWork::Menus::IssuesMenu.new(context)) + add_menu(Sidebars::YourWork::Menus::MergeRequestsMenu.new(context)) + add_menu(Sidebars::YourWork::Menus::TodosMenu.new(context)) add_menu(Sidebars::YourWork::Menus::MilestonesMenu.new(context)) add_menu(Sidebars::YourWork::Menus::SnippetsMenu.new(context)) add_menu(Sidebars::YourWork::Menus::ActivityMenu.new(context)) diff --git a/package.json b/package.json index 097816ffa9e..900f7ec276e 100644 --- a/package.json +++ b/package.json @@ -122,7 +122,7 @@ "dateformat": "^5.0.1", "deckar01-task_list": "^2.3.1", "diff": "^3.4.0", - "dompurify": "^2.4.1", + "dompurify": "^2.4.2", "dropzone": "^4.2.0", "editorconfig": "^0.15.3", "emoji-regex": "^10.0.0", diff --git a/spec/features/dashboard/issues_spec.rb b/spec/features/dashboard/issues_spec.rb index d74965f58fa..ae375bd3e13 100644 --- a/spec/features/dashboard/issues_spec.rb +++ b/spec/features/dashboard/issues_spec.rb @@ -21,6 +21,8 @@ RSpec.describe 'Dashboard Issues', feature_category: :team_planning do visit issues_dashboard_path(assignee_username: current_user.username) end + it_behaves_like 'a dashboard page with sidebar', :issues_dashboard_path, :issues + describe 'issues' do it 'shows issues assigned to current user' do expect(page).to have_content(assigned_issue.title) diff --git a/spec/features/dashboard/merge_requests_spec.rb b/spec/features/dashboard/merge_requests_spec.rb index 56d7c45de5d..78b690c200a 100644 --- a/spec/features/dashboard/merge_requests_spec.rb +++ b/spec/features/dashboard/merge_requests_spec.rb @@ -19,6 +19,8 @@ RSpec.describe 'Dashboard Merge Requests', feature_category: :code_review do sign_in(current_user) end + it_behaves_like 'a dashboard page with sidebar', :merge_requests_dashboard_path, :merge_requests + it 'disables target branch filter' do visit merge_requests_dashboard_path diff --git a/spec/features/dashboard/todos/todos_spec.rb b/spec/features/dashboard/todos/todos_spec.rb index dd1c5e6bd1f..3df879a84b4 100644 --- a/spec/features/dashboard/todos/todos_spec.rb +++ b/spec/features/dashboard/todos/todos_spec.rb @@ -15,6 +15,8 @@ RSpec.describe 'Dashboard Todos', feature_category: :team_planning do project.add_developer(user) end + it_behaves_like 'a dashboard page with sidebar', :dashboard_todos_path, :todos + context 'User does not have todos' do before do sign_in(user) diff --git a/spec/features/profiles/chat_names_spec.rb b/spec/features/profiles/chat_names_spec.rb index b3d65ab3a3c..14fdb8ba56f 100644 --- a/spec/features/profiles/chat_names_spec.rb +++ b/spec/features/profiles/chat_names_spec.rb @@ -15,7 +15,7 @@ RSpec.describe 'Profile > Chat', feature_category: :users do { team_id: 'T00', team_domain: 'my_chat_team', user_id: 'U01', user_name: 'my_chat_user' } end - let!(:authorize_url) { ChatNames::AuthorizeUserService.new(integration, params).execute } + let!(:authorize_url) { ChatNames::AuthorizeUserService.new(params).execute } let(:authorize_path) { URI.parse(authorize_url).request_uri } before do diff --git a/spec/frontend/work_items/components/notes/activity_filter_spec.js b/spec/frontend/work_items/components/notes/activity_filter_spec.js new file mode 100644 index 00000000000..eb4bcbf942b --- /dev/null +++ b/spec/frontend/work_items/components/notes/activity_filter_spec.js @@ -0,0 +1,74 @@ +import { GlDropdown, GlDropdownItem } from '@gitlab/ui'; +import { nextTick } from 'vue'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import ActivityFilter from '~/work_items/components/notes/activity_filter.vue'; +import LocalStorageSync from '~/vue_shared/components/local_storage_sync.vue'; +import { ASC, DESC } from '~/notes/constants'; + +import { mockTracking } from 'helpers/tracking_helper'; +import { TRACKING_CATEGORY_SHOW } from '~/work_items/constants'; + +describe('Activity Filter', () => { + let wrapper; + + const findLocalStorageSync = () => wrapper.findComponent(LocalStorageSync); + const findDropdown = () => wrapper.findComponent(GlDropdown); + const findAllDropdownItems = () => wrapper.findAllComponents(GlDropdownItem); + const findNewestFirstItem = () => wrapper.findByTestId('js-newest-first'); + + const createComponent = ({ sortOrder = ASC, loading = false, workItemType = 'Task' } = {}) => { + wrapper = shallowMountExtended(ActivityFilter, { + propsData: { + sortOrder, + loading, + workItemType, + }, + }); + }; + + beforeEach(() => { + createComponent(); + }); + + describe('default', () => { + it('has a dropdown with 2 options', () => { + expect(findDropdown().exists()).toBe(true); + expect(findAllDropdownItems()).toHaveLength(ActivityFilter.SORT_OPTIONS.length); + }); + + it('has local storage sync with the correct props', () => { + expect(findLocalStorageSync().props('asString')).toBe(true); + }); + + it('emits `updateSavedSortOrder` event when update is emitted', async () => { + findLocalStorageSync().vm.$emit('input', ASC); + + await nextTick(); + expect(wrapper.emitted('updateSavedSortOrder')).toHaveLength(1); + expect(wrapper.emitted('updateSavedSortOrder')).toEqual([[ASC]]); + }); + }); + + describe('when asc', () => { + describe('when the dropdown is clicked', () => { + it('calls the right actions', async () => { + const trackingSpy = mockTracking(undefined, wrapper.element, jest.spyOn); + findNewestFirstItem().vm.$emit('click'); + await nextTick(); + + expect(wrapper.emitted('changeSortOrder')).toHaveLength(1); + expect(wrapper.emitted('changeSortOrder')).toEqual([[DESC]]); + + expect(trackingSpy).toHaveBeenCalledWith( + TRACKING_CATEGORY_SHOW, + 'notes_sort_order_changed', + { + category: TRACKING_CATEGORY_SHOW, + label: 'item_track_notes_sorting', + property: 'type_Task', + }, + ); + }); + }); + }); +}); diff --git a/spec/frontend/work_items/components/work_item_notes_spec.js b/spec/frontend/work_items/components/work_item_notes_spec.js index ed68d214fc9..98b2e34b860 100644 --- a/spec/frontend/work_items/components/work_item_notes_spec.js +++ b/spec/frontend/work_items/components/work_item_notes_spec.js @@ -1,18 +1,21 @@ -import { GlSkeletonLoader } from '@gitlab/ui'; +import { GlSkeletonLoader, GlIntersectionObserver } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; -import Vue from 'vue'; +import Vue, { nextTick } from 'vue'; import VueApollo from 'vue-apollo'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; import SystemNote from '~/work_items/components/notes/system_note.vue'; import WorkItemNotes from '~/work_items/components/work_item_notes.vue'; +import ActivityFilter from '~/work_items/components/notes/activity_filter.vue'; import workItemNotesQuery from '~/work_items/graphql/work_item_notes.query.graphql'; import workItemNotesByIidQuery from '~/work_items/graphql/work_item_notes_by_iid.query.graphql'; -import { WIDGET_TYPE_NOTES } from '~/work_items/constants'; +import { DEFAULT_PAGE_SIZE_NOTES, WIDGET_TYPE_NOTES } from '~/work_items/constants'; +import { DESC } from '~/notes/constants'; import { mockWorkItemNotesResponse, workItemQueryResponse, mockWorkItemNotesByIidResponse, + mockMoreWorkItemNotesResponse, } from '../mock_data'; const mockWorkItemId = workItemQueryResponse.data.workItem.id; @@ -24,6 +27,12 @@ const mockNotesByIidWidgetResponse = mockWorkItemNotesByIidResponse.data.workspa (widget) => widget.type === WIDGET_TYPE_NOTES, ); +const mockMoreNotesWidgetResponse = mockMoreWorkItemNotesResponse.data.workItem.widgets.find( + (widget) => widget.type === WIDGET_TYPE_NOTES, +); + +const firstSystemNodeId = mockNotesWidgetResponse.discussions.nodes[0].notes.nodes[0].id; + describe('WorkItemNotes component', () => { let wrapper; @@ -32,15 +41,23 @@ describe('WorkItemNotes component', () => { const findAllSystemNotes = () => wrapper.findAllComponents(SystemNote); const findActivityLabel = () => wrapper.find('label'); const findSkeletonLoader = () => wrapper.findComponent(GlSkeletonLoader); + const findIntersectionObserver = () => wrapper.findComponent(GlIntersectionObserver); + const findSortingFilter = () => wrapper.findComponent(ActivityFilter); + const findSystemNoteAtIndex = (index) => findAllSystemNotes().at(index); const workItemNotesQueryHandler = jest.fn().mockResolvedValue(mockWorkItemNotesResponse); const workItemNotesByIidQueryHandler = jest .fn() .mockResolvedValue(mockWorkItemNotesByIidResponse); + const workItemMoreNotesQueryHandler = jest.fn().mockResolvedValue(mockMoreWorkItemNotesResponse); - const createComponent = ({ workItemId = mockWorkItemId, fetchByIid = false } = {}) => { + const createComponent = ({ + workItemId = mockWorkItemId, + fetchByIid = false, + defaultWorkItemNotesQueryHandler = workItemNotesQueryHandler, + } = {}) => { wrapper = shallowMount(WorkItemNotes, { apolloProvider: createMockApollo([ - [workItemNotesQuery, workItemNotesQueryHandler], + [workItemNotesQuery, defaultWorkItemNotesQueryHandler], [workItemNotesByIidQuery, workItemNotesByIidQueryHandler], ]), propsData: { @@ -50,6 +67,7 @@ describe('WorkItemNotes component', () => { }, fullPath: 'test-path', fetchByIid, + workItemType: 'task', }, provide: { glFeatures: { @@ -63,10 +81,6 @@ describe('WorkItemNotes component', () => { createComponent(); }); - afterEach(() => { - wrapper.destroy(); - }); - it('renders activity label', () => { expect(findActivityLabel().exists()).toBe(true); }); @@ -98,10 +112,63 @@ describe('WorkItemNotes component', () => { await waitForPromises(); }); - it('shows the notes list', () => { + it('renders the notes list to the length of the response', () => { expect(findAllSystemNotes()).toHaveLength( mockNotesByIidWidgetResponse.discussions.nodes.length, ); }); }); + + describe('Pagination', () => { + describe('When there is no next page', () => { + it('fetch more notes is not called', () => { + createComponent(); + expect(findIntersectionObserver().exists()).toBe(false); + expect(workItemMoreNotesQueryHandler).not.toHaveBeenCalled(); + }); + }); + + describe('when there is next page', () => { + beforeEach(async () => { + createComponent({ defaultWorkItemNotesQueryHandler: workItemMoreNotesQueryHandler }); + await waitForPromises(); + }); + + it('fetch more notes should be called', async () => { + expect(workItemMoreNotesQueryHandler).toHaveBeenCalledWith({ + pageSize: DEFAULT_PAGE_SIZE_NOTES, + id: 'gid://gitlab/WorkItem/1', + }); + + findIntersectionObserver().vm.$emit('appear'); + + await nextTick(); + + expect(workItemMoreNotesQueryHandler).toHaveBeenCalledWith({ + pageSize: 45, + id: 'gid://gitlab/WorkItem/1', + after: mockMoreNotesWidgetResponse.discussions.pageInfo.endCursor, + }); + }); + }); + }); + + describe('Sorting', () => { + beforeEach(async () => { + createComponent(); + await waitForPromises(); + }); + + it('filter exists', () => { + expect(findSortingFilter().exists()).toBe(true); + }); + + it('sorts the list when the `changeSortOrder` event is emitted', async () => { + expect(findSystemNoteAtIndex(0).props('note').id).toEqual(firstSystemNodeId); + + await findSortingFilter().vm.$emit('changeSortOrder', DESC); + + expect(findSystemNoteAtIndex(0).props('note').id).not.toEqual(firstSystemNodeId); + }); + }); }); diff --git a/spec/frontend/work_items/mock_data.js b/spec/frontend/work_items/mock_data.js index 3f88bc536d8..7fc897fd21c 100644 --- a/spec/frontend/work_items/mock_data.js +++ b/spec/frontend/work_items/mock_data.js @@ -1579,7 +1579,7 @@ export const mockWorkItemNotesResponse = { notes: { nodes: [ { - id: 'gid://gitlab/MilestoneNote/not-persisted', + id: 'gid://gitlab/MilestoneNote/0f2f195ec0d1ef95ee9d5b10446b8e96a7d83864', body: 'changed milestone to %5', bodyHtml: '<p data-sourcepos="1:1-1:23" dir="auto">changed milestone to <a href="/flightjs/Flight/-/milestones/5" data-reference-type="milestone" data-original="%5" data-link="false" data-link-reference="false" data-project="6" data-milestone="30" data-container=body data-placement="top" title="" class="gfm gfm-milestone has-tooltip">%v4.0</a></p>', @@ -1607,7 +1607,7 @@ export const mockWorkItemNotesResponse = { notes: { nodes: [ { - id: 'gid://gitlab/WeightNote/not-persisted', + id: 'gid://gitlab/WeightNote/0f2f195ec0d1ef95ee9d5b10446b8e96a9883864', body: 'changed weight to 89', bodyHtml: '<p dir="auto">changed weight to <strong>89</strong></p>', systemNoteIconName: 'weight', @@ -1788,3 +1788,137 @@ export const mockWorkItemNotesByIidResponse = { }, }, }; +export const mockMoreWorkItemNotesResponse = { + data: { + workItem: { + id: 'gid://gitlab/WorkItem/600', + iid: '60', + widgets: [ + { + __typename: 'WorkItemWidgetIteration', + }, + { + __typename: 'WorkItemWidgetWeight', + }, + { + __typename: 'WorkItemWidgetAssignees', + }, + { + __typename: 'WorkItemWidgetLabels', + }, + { + __typename: 'WorkItemWidgetDescription', + }, + { + __typename: 'WorkItemWidgetHierarchy', + }, + { + __typename: 'WorkItemWidgetStartAndDueDate', + }, + { + __typename: 'WorkItemWidgetMilestone', + }, + { + type: 'NOTES', + discussions: { + pageInfo: { + hasNextPage: true, + hasPreviousPage: false, + startCursor: null, + endCursor: 'endCursor', + __typename: 'PageInfo', + }, + nodes: [ + { + id: + 'gid://gitlab/IndividualNoteDiscussion/8bbc4890b6ff0f2cde93a5a0947cd2b8a13d3b6e', + notes: { + nodes: [ + { + id: 'gid://gitlab/Note/2428', + body: 'added #31 as parent issue', + bodyHtml: + '<p data-sourcepos="1:1-1:25" dir="auto">added <a href="/flightjs/Flight/-/issues/31" data-reference-type="issue" data-original="#31" data-link="false" data-link-reference="false" data-project="6" data-issue="224" data-project-path="flightjs/Flight" data-iid="31" data-issue-type="issue" data-container=body data-placement="top" title="Perferendis est quae totam quia laborum tempore ut voluptatem." class="gfm gfm-issue">#31</a> as parent issue</p>', + systemNoteIconName: 'link', + createdAt: '2022-11-14T04:18:59Z', + author: { + avatarUrl: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + id: 'gid://gitlab/User/1', + name: 'Administrator', + username: 'root', + webUrl: 'http://127.0.0.1:3000/root', + __typename: 'UserCore', + }, + __typename: 'Note', + }, + ], + __typename: 'NoteConnection', + }, + __typename: 'Discussion', + }, + { + id: + 'gid://gitlab/IndividualNoteDiscussion/7b08b89a728a5ceb7de8334246837ba1d07270dc', + notes: { + nodes: [ + { + id: 'gid://gitlab/MilestoneNote/0f2f195ec0d1ef95ee9d5b10446b8e96a7d83823', + body: 'changed milestone to %5', + bodyHtml: + '<p data-sourcepos="1:1-1:23" dir="auto">changed milestone to <a href="/flightjs/Flight/-/milestones/5" data-reference-type="milestone" data-original="%5" data-link="false" data-link-reference="false" data-project="6" data-milestone="30" data-container=body data-placement="top" title="" class="gfm gfm-milestone has-tooltip">%v4.0</a></p>', + systemNoteIconName: 'clock', + createdAt: '2022-11-14T04:18:59Z', + author: { + avatarUrl: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + id: 'gid://gitlab/User/1', + name: 'Administrator', + username: 'root', + webUrl: 'http://127.0.0.1:3000/root', + __typename: 'UserCore', + }, + __typename: 'Note', + }, + ], + __typename: 'NoteConnection', + }, + __typename: 'Discussion', + }, + { + id: + 'gid://gitlab/IndividualNoteDiscussion/0f2f195ec0d1ef95ee9d5b10446b8e96a7d83864', + notes: { + nodes: [ + { + id: 'gid://gitlab/WeightNote/0f2f195ec0d1ef95ee9d5b10446b8e96a7d83864', + body: 'changed weight to 89', + bodyHtml: '<p dir="auto">changed weight to <strong>89</strong></p>', + systemNoteIconName: 'weight', + createdAt: '2022-11-25T07:16:20Z', + author: { + avatarUrl: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + id: 'gid://gitlab/User/1', + name: 'Administrator', + username: 'root', + webUrl: 'http://127.0.0.1:3000/root', + __typename: 'UserCore', + }, + __typename: 'Note', + }, + ], + __typename: 'NoteConnection', + }, + __typename: 'Discussion', + }, + ], + __typename: 'DiscussionConnection', + }, + __typename: 'WorkItemWidgetNotes', + }, + ], + __typename: 'WorkItem', + }, + }, +}; diff --git a/spec/lib/gitlab/database_spec.rb b/spec/lib/gitlab/database_spec.rb index 1a482b33a92..86bc8e71fd7 100644 --- a/spec/lib/gitlab/database_spec.rb +++ b/spec/lib/gitlab/database_spec.rb @@ -302,6 +302,26 @@ RSpec.describe Gitlab::Database do end end + describe '.database_base_models_with_gitlab_shared' do + before do + Gitlab::Database.instance_variable_set(:@database_base_models_with_gitlab_shared, nil) + end + + it 'memoizes the models' do + expect { Gitlab::Database.database_base_models_with_gitlab_shared }.to change { Gitlab::Database.instance_variable_get(:@database_base_models_with_gitlab_shared) }.from(nil) + end + end + + describe '.database_base_models_using_load_balancing' do + before do + Gitlab::Database.instance_variable_set(:@database_base_models_using_load_balancing, nil) + end + + it 'memoizes the models' do + expect { Gitlab::Database.database_base_models_using_load_balancing }.to change { Gitlab::Database.instance_variable_get(:@database_base_models_using_load_balancing) }.from(nil) + end + end + describe '#true_value' do it 'returns correct value' do expect(described_class.true_value).to eq "'t'" diff --git a/spec/lib/sidebars/your_work/menus/issues_menu_spec.rb b/spec/lib/sidebars/your_work/menus/issues_menu_spec.rb new file mode 100644 index 00000000000..a1206c0bc1c --- /dev/null +++ b/spec/lib/sidebars/your_work/menus/issues_menu_spec.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Sidebars::YourWork::Menus::IssuesMenu, feature_category: :navigation do + let(:user) { create(:user) } + let(:context) { Sidebars::Context.new(current_user: user, container: nil) } + + subject { described_class.new(context) } + + include_examples 'menu item shows pill based on count', :assigned_open_issues_count +end diff --git a/spec/lib/sidebars/your_work/menus/merge_requests_menu_spec.rb b/spec/lib/sidebars/your_work/menus/merge_requests_menu_spec.rb new file mode 100644 index 00000000000..b3251a54178 --- /dev/null +++ b/spec/lib/sidebars/your_work/menus/merge_requests_menu_spec.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Sidebars::YourWork::Menus::MergeRequestsMenu, feature_category: :navigation do + let(:user) { create(:user) } + let(:context) { Sidebars::Context.new(current_user: user, container: nil) } + + subject { described_class.new(context) } + + include_examples 'menu item shows pill based on count', :assigned_open_merge_requests_count +end diff --git a/spec/lib/sidebars/your_work/menus/todos_menu_spec.rb b/spec/lib/sidebars/your_work/menus/todos_menu_spec.rb new file mode 100644 index 00000000000..a8177a6a01b --- /dev/null +++ b/spec/lib/sidebars/your_work/menus/todos_menu_spec.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Sidebars::YourWork::Menus::TodosMenu, feature_category: :navigation do + let(:user) { create(:user) } + let(:context) { Sidebars::Context.new(current_user: user, container: nil) } + + subject { described_class.new(context) } + + include_examples 'menu item shows pill based on count', :todos_pending_count +end diff --git a/spec/models/chat_name_spec.rb b/spec/models/chat_name_spec.rb index 3bb5ee5870b..0838c232872 100644 --- a/spec/models/chat_name_spec.rb +++ b/spec/models/chat_name_spec.rb @@ -11,7 +11,6 @@ RSpec.describe ChatName, feature_category: :integrations do it { is_expected.to belong_to(:user) } it { is_expected.to validate_presence_of(:user) } - it { is_expected.to validate_presence_of(:integration) } it { is_expected.to validate_presence_of(:team_id) } it { is_expected.to validate_presence_of(:chat_id) } diff --git a/spec/services/chat_names/authorize_user_service_spec.rb b/spec/services/chat_names/authorize_user_service_spec.rb index 53d90c7f100..4c261ece504 100644 --- a/spec/services/chat_names/authorize_user_service_spec.rb +++ b/spec/services/chat_names/authorize_user_service_spec.rb @@ -2,12 +2,11 @@ require 'spec_helper' -RSpec.describe ChatNames::AuthorizeUserService do +RSpec.describe ChatNames::AuthorizeUserService, feature_category: :users do describe '#execute' do - let(:integration) { create(:integration) } let(:result) { subject.execute } - subject { described_class.new(integration, params) } + subject { described_class.new(params) } context 'when all parameters are valid' do let(:params) { { team_id: 'T0001', team_domain: 'myteam', user_id: 'U0001', user_name: 'user' } } diff --git a/spec/services/merge_requests/base_service_spec.rb b/spec/services/merge_requests/base_service_spec.rb index da9c8651b0c..79a447718c5 100644 --- a/spec/services/merge_requests/base_service_spec.rb +++ b/spec/services/merge_requests/base_service_spec.rb @@ -2,6 +2,14 @@ require 'spec_helper' +module MergeRequests + class ExampleService < MergeRequests::BaseService + def execute(merge_request, async: false, allow_duplicate: false) + create_pipeline_for(merge_request, current_user, async: async, allow_duplicate: allow_duplicate) + end + end +end + RSpec.describe MergeRequests::BaseService, feature_category: :code_review do include ProjectForksHelper @@ -57,4 +65,62 @@ RSpec.describe MergeRequests::BaseService, feature_category: :code_review do it_behaves_like 'does not enqueue Jira sync worker' end end + + describe `#create_pipeline_for` do + let_it_be(:merge_request) { create(:merge_request) } + + subject { MergeRequests::ExampleService.new(project: project, current_user: project.first_owner, params: params) } + + context 'async: false' do + it 'creates a pipeline directly' do + expect(MergeRequests::CreatePipelineService) + .to receive(:new) + .with(hash_including(project: project, current_user: project.first_owner, params: { allow_duplicate: false })) + .and_call_original + expect(MergeRequests::CreatePipelineWorker).not_to receive(:perform_async) + + subject.execute(merge_request, async: false) + end + + context 'allow_duplicate: true' do + it 'passes :allow_duplicate as true' do + expect(MergeRequests::CreatePipelineService) + .to receive(:new) + .with(hash_including(project: project, current_user: project.first_owner, params: { allow_duplicate: true })) + .and_call_original + expect(MergeRequests::CreatePipelineWorker).not_to receive(:perform_async) + + subject.execute(merge_request, async: false, allow_duplicate: true) + end + end + end + + context 'async: true' do + it 'enques a CreatePipelineWorker' do + expect(MergeRequests::CreatePipelineService).not_to receive(:new) + expect(MergeRequests::CreatePipelineWorker) + .to receive(:perform_async) + .with(project.id, project.first_owner.id, merge_request.id, { "allow_duplicate" => false }) + .and_call_original + + Sidekiq::Testing.fake! do + expect { subject.execute(merge_request, async: true) }.to change(MergeRequests::CreatePipelineWorker.jobs, :size).by(1) + end + end + + context 'allow_duplicate: true' do + it 'passes :allow_duplicate as true' do + expect(MergeRequests::CreatePipelineService).not_to receive(:new) + expect(MergeRequests::CreatePipelineWorker) + .to receive(:perform_async) + .with(project.id, project.first_owner.id, merge_request.id, { "allow_duplicate" => true }) + .and_call_original + + Sidekiq::Testing.fake! do + expect { subject.execute(merge_request, async: true, allow_duplicate: true) }.to change(MergeRequests::CreatePipelineWorker.jobs, :size).by(1) + end + end + end + end + end end diff --git a/spec/support/shared_contexts/navbar_structure_context.rb b/spec/support/shared_contexts/navbar_structure_context.rb index 10b795f1d7e..c1ad062ea75 100644 --- a/spec/support/shared_contexts/navbar_structure_context.rb +++ b/spec/support/shared_contexts/navbar_structure_context.rb @@ -234,6 +234,18 @@ RSpec.shared_context 'dashboard navbar structure' do nav_sub_items: [] }, { + nav_item: _("Issues"), + nav_sub_items: [] + }, + { + nav_item: _("Merge requests"), + nav_sub_items: [] + }, + { + nav_item: _("To-Do List"), + nav_sub_items: [] + }, + { nav_item: _("Milestones"), nav_sub_items: [] }, diff --git a/spec/support/shared_examples/lib/sidebars/your_work/menus/menu_item_examples.rb b/spec/support/shared_examples/lib/sidebars/your_work/menus/menu_item_examples.rb new file mode 100644 index 00000000000..19c94a3ba5b --- /dev/null +++ b/spec/support/shared_examples/lib/sidebars/your_work/menus/menu_item_examples.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.shared_examples 'menu item shows pill based on count' do |count| + describe '#has_pill?' do + context 'when count is zero' do + it 'returns false' do + allow(user).to receive(count).and_return(0) + expect(subject.has_pill?).to eq false + end + end + + context 'when count is larger than zero' do + it 'returns true' do + allow(user).to receive(count).and_return(3) + expect(subject.has_pill?).to eq true + end + end + end + + describe '#pill_count' do + it "returns the #{count} of the user" do + allow(user).to receive(count).and_return(123) + expect(subject.pill_count).to eq 123 + end + + it 'memoizes the query' do + subject.pill_count + + control = ActiveRecord::QueryRecorder.new do + subject.pill_count + end + + expect(control.count).to eq 0 + end + end +end diff --git a/spec/workers/merge_requests/create_pipeline_worker_spec.rb b/spec/workers/merge_requests/create_pipeline_worker_spec.rb index bacf4084e1f..afb9fa1a549 100644 --- a/spec/workers/merge_requests/create_pipeline_worker_spec.rb +++ b/spec/workers/merge_requests/create_pipeline_worker_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe MergeRequests::CreatePipelineWorker, feature_category: :code_review_workflow do +RSpec.describe MergeRequests::CreatePipelineWorker, feature_category: :continuous_integration do describe '#perform' do let(:user) { create(:user) } let(:project) { create(:project) } @@ -17,7 +17,7 @@ RSpec.describe MergeRequests::CreatePipelineWorker, feature_category: :code_revi expect_next_instance_of(MergeRequests::CreatePipelineService, project: project, current_user: user, - params: { push_options: nil }) do |service| + params: { allow_duplicate: nil, push_options: nil }) do |service| expect(service).to receive(:execute).with(merge_request) end @@ -38,7 +38,7 @@ RSpec.describe MergeRequests::CreatePipelineWorker, feature_category: :code_revi expect_next_instance_of(MergeRequests::CreatePipelineService, project: project, current_user: user, - params: { push_options: { ci: { skip: true } } }) do |service| + params: { allow_duplicate: nil, push_options: { ci: { skip: true } } }) do |service| expect(service).to receive(:execute).with(merge_request) end diff --git a/yarn.lock b/yarn.lock index fcb6e577baa..3c28081dd14 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5097,10 +5097,10 @@ dompurify@2.3.8: resolved "https://registry.yarnpkg.com/dompurify/-/dompurify-2.3.8.tgz#224fe9ae57d7ebd9a1ae1ac18c1c1ca3f532226f" integrity sha512-eVhaWoVibIzqdGYjwsBWodIQIaXFSB+cKDf4cfxLMsK0xiud6SE+/WCVx/Xw/UwQsa4cS3T2eITcdtmTg2UKcw== -dompurify@^2.4.1: - version "2.4.1" - resolved "https://registry.yarnpkg.com/dompurify/-/dompurify-2.4.1.tgz#f9cb1a275fde9af6f2d0a2644ef648dd6847b631" - integrity sha512-ewwFzHzrrneRjxzmK6oVz/rZn9VWspGFRDb4/rRtIsM1n36t9AKma/ye8syCpcw+XJ25kOK/hOG7t1j2I2yBqA== +dompurify@^2.4.1, dompurify@^2.4.2: + version "2.4.2" + resolved "https://registry.yarnpkg.com/dompurify/-/dompurify-2.4.2.tgz#c3409b49357804c9b00e1fbebea81f26514c5bc3" + integrity sha512-ckbbxcGpfTJ7SNHC2yT2pHSCYxo2oQgSfdoDHQANzMzQyGzVmalF9W/B+X97Cdik5xFwWtwJP232gIP2+1kNEA== domutils@^2.5.2, domutils@^2.6.0: version "2.6.0" |