diff options
40 files changed, 810 insertions, 538 deletions
diff --git a/GITLAB_PAGES_VERSION b/GITLAB_PAGES_VERSION index bb120e876c6..4d5fde5bd16 100644 --- a/GITLAB_PAGES_VERSION +++ b/GITLAB_PAGES_VERSION @@ -1 +1 @@ -1.59.0 +1.60.0 diff --git a/app/assets/javascripts/design_management/constants.js b/app/assets/javascripts/design_management/constants.js index 92928ca429f..afe621ac3c5 100644 --- a/app/assets/javascripts/design_management/constants.js +++ b/app/assets/javascripts/design_management/constants.js @@ -12,3 +12,5 @@ export const ACTIVE_DISCUSSION_SOURCE_TYPES = { }; export const DESIGN_DETAIL_LAYOUT_CLASSLIST = ['design-detail-layout', 'overflow-hidden', 'm-0']; + +export const MAXIMUM_FILE_UPLOAD_LIMIT = 10; diff --git a/app/assets/javascripts/design_management/pages/index.vue b/app/assets/javascripts/design_management/pages/index.vue index f81d4f6662f..51983b19677 100644 --- a/app/assets/javascripts/design_management/pages/index.vue +++ b/app/assets/javascripts/design_management/pages/index.vue @@ -4,16 +4,15 @@ import { GlBreakpointInstance } from '@gitlab/ui/dist/utils'; import VueDraggable from 'vuedraggable'; import permissionsQuery from 'shared_queries/design_management/design_permissions.query.graphql'; import getDesignListQuery from 'shared_queries/design_management/get_design_list.query.graphql'; -import createFlash, { FLASH_TYPES } from '~/flash'; import { getFilename, validateImageName } from '~/lib/utils/file_upload'; -import { __, s__, sprintf } from '~/locale'; +import { __, s__ } from '~/locale'; import DesignDropzone from '~/vue_shared/components/upload_dropzone/upload_dropzone.vue'; import DeleteButton from '../components/delete_button.vue'; import DesignDestroyer from '../components/design_destroyer.vue'; import Design from '../components/list/item.vue'; import UploadButton from '../components/upload/button.vue'; import DesignVersionDropdown from '../components/upload/design_version_dropdown.vue'; -import { VALID_DESIGN_FILE_MIMETYPE } from '../constants'; +import { MAXIMUM_FILE_UPLOAD_LIMIT, VALID_DESIGN_FILE_MIMETYPE } from '../constants'; import moveDesignMutation from '../graphql/mutations/move_design.mutation.graphql'; import uploadDesignMutation from '../graphql/mutations/upload_design.mutation.graphql'; import allDesignsMixin from '../mixins/all_designs'; @@ -35,11 +34,10 @@ import { UPLOAD_DESIGN_INVALID_FILETYPE_ERROR, designUploadSkippedWarning, designDeletionError, + MAXIMUM_FILE_UPLOAD_LIMIT_REACHED, } from '../utils/error_messages'; import { trackDesignCreate, trackDesignUpdate } from '../utils/tracking'; -const MAXIMUM_FILE_UPLOAD_LIMIT = 10; - export default { components: { GlLoadingIcon, @@ -87,6 +85,7 @@ export default { isDraggingDesign: false, reorderedDesigns: null, isReorderingInProgress: false, + uploadError: null, }; }, computed: { @@ -159,16 +158,7 @@ export default { if (!this.canCreateDesign) return false; if (files.length > MAXIMUM_FILE_UPLOAD_LIMIT) { - createFlash({ - message: sprintf( - s__( - 'DesignManagement|The maximum number of designs allowed to be uploaded is %{upload_limit}. Please try again.', - ), - { - upload_limit: MAXIMUM_FILE_UPLOAD_LIMIT, - }, - ), - }); + this.uploadError = MAXIMUM_FILE_UPLOAD_LIMIT_REACHED; return false; } @@ -206,7 +196,7 @@ export default { const skippedFiles = res?.data?.designManagementUpload?.skippedDesigns || []; const skippedWarningMessage = designUploadSkippedWarning(this.filesToBeSaved, skippedFiles); if (skippedWarningMessage) { - createFlash({ message: skippedWarningMessage, types: FLASH_TYPES.WARNING }); + this.uploadError = skippedWarningMessage; } // if this upload resulted in a new version being created, redirect user to the latest version @@ -229,7 +219,7 @@ export default { }, onUploadDesignError() { this.resetFilesToBeSaved(); - createFlash({ message: UPLOAD_DESIGN_ERROR }); + this.uploadError = UPLOAD_DESIGN_ERROR; }, changeSelectedDesigns(filename) { if (this.isDesignSelected(filename)) { @@ -260,21 +250,21 @@ export default { }, onDesignDeleteError() { const errorMessage = designDeletionError(this.selectedDesigns.length); - createFlash({ message: errorMessage }); + this.uploadError = errorMessage; }, onDesignDropzoneError() { - createFlash({ message: UPLOAD_DESIGN_INVALID_FILETYPE_ERROR }); + this.uploadError = UPLOAD_DESIGN_INVALID_FILETYPE_ERROR; }, onExistingDesignDropzoneChange(files, existingDesignFilename) { const filesArr = Array.from(files); if (filesArr.length > 1) { - createFlash({ message: EXISTING_DESIGN_DROP_MANY_FILES_MESSAGE }); + this.uploadError = EXISTING_DESIGN_DROP_MANY_FILES_MESSAGE; return; } if (!filesArr.some(({ name }) => existingDesignFilename === name)) { - createFlash({ message: EXISTING_DESIGN_DROP_INVALID_FILENAME_MESSAGE }); + this.uploadError = EXISTING_DESIGN_DROP_INVALID_FILENAME_MESSAGE; return; } @@ -329,7 +319,7 @@ export default { optimisticResponse: moveDesignOptimisticResponse(this.reorderedDesigns), }) .catch(() => { - createFlash({ message: MOVE_DESIGN_ERROR }); + this.uploadError = MOVE_DESIGN_ERROR; }) .finally(() => { this.isReorderingInProgress = false; @@ -338,6 +328,9 @@ export default { onDesignMove(designs) { this.reorderedDesigns = designs; }, + unsetUpdateError() { + this.uploadError = null; + }, }, dragOptions: { animation: 200, @@ -356,6 +349,15 @@ export default { @mouseenter="toggleOnPasteListener" @mouseleave="toggleOffPasteListener" > + <gl-alert + v-if="uploadError" + variant="danger" + class="gl-mb-3" + data-testid="design-update-alert" + @dismiss="unsetUpdateError" + > + {{ uploadError }} + </gl-alert> <header v-if="showToolbar" class="gl-display-flex gl-my-0 gl-text-gray-900" @@ -371,6 +373,7 @@ export default { <div v-show="hasDesigns" class="qa-selector-toolbar gl-display-flex gl-align-items-center gl-my-2" + data-testid="design-selector-toolbar" > <gl-button v-if="isLatestVersion" diff --git a/app/assets/javascripts/design_management/utils/error_messages.js b/app/assets/javascripts/design_management/utils/error_messages.js index 981b50329b2..42f752efc9e 100644 --- a/app/assets/javascripts/design_management/utils/error_messages.js +++ b/app/assets/javascripts/design_management/utils/error_messages.js @@ -1,4 +1,5 @@ import { __, s__, n__, sprintf } from '~/locale'; +import { MAXIMUM_FILE_UPLOAD_LIMIT } from '../constants'; export const ADD_DISCUSSION_COMMENT_ERROR = s__( 'DesignManagement|Could not add a new comment. Please try again.', @@ -27,11 +28,11 @@ export const DESIGN_NOT_FOUND_ERROR = __('Could not find design.'); export const DESIGN_VERSION_NOT_EXIST_ERROR = __('Requested design version does not exist.'); export const EXISTING_DESIGN_DROP_MANY_FILES_MESSAGE = __( - 'You can only upload one design when dropping onto an existing design.', + 'Your update failed. You can only upload one design when dropping onto an existing design.', ); export const EXISTING_DESIGN_DROP_INVALID_FILENAME_MESSAGE = __( - 'You must upload a file with the same file name when dropping onto an existing design.', + 'Your update failed. You must upload a file with the same file name when dropping onto an existing design.', ); export const MOVE_DESIGN_ERROR = __( @@ -122,3 +123,12 @@ export const designUploadSkippedWarning = (uploadedDesigns, skippedFiles) => { return someDesignsSkippedMessage(skippedFiles); }; + +export const MAXIMUM_FILE_UPLOAD_LIMIT_REACHED = sprintf( + s__( + 'DesignManagement|The maximum number of designs allowed to be uploaded is %{upload_limit}. Please try again.', + ), + { + upload_limit: MAXIMUM_FILE_UPLOAD_LIMIT, + }, +); diff --git a/app/assets/javascripts/invite_members/components/invite_members_modal.vue b/app/assets/javascripts/invite_members/components/invite_members_modal.vue index d597c7e53bb..b71cfbb6112 100644 --- a/app/assets/javascripts/invite_members/components/invite_members_modal.vue +++ b/app/assets/javascripts/invite_members/components/invite_members_modal.vue @@ -7,12 +7,13 @@ import { GlSprintf, GlFormCheckboxGroup, } from '@gitlab/ui'; -import { partition, isString, uniqueId } from 'lodash'; +import { partition, isString, uniqueId, isEmpty } from 'lodash'; import InviteModalBase from 'ee_else_ce/invite_members/components/invite_modal_base.vue'; import Api from '~/api'; import ExperimentTracking from '~/experimentation/experiment_tracking'; import { BV_SHOW_MODAL, BV_HIDE_MODAL } from '~/lib/utils/constants'; import { getParameterValues } from '~/lib/utils/url_utility'; +import { n__ } from '~/locale'; import { CLOSE_TO_LIMIT_COUNT, USERS_FILTER_ALL, @@ -21,7 +22,8 @@ import { LEARN_GITLAB, } from '../constants'; import eventHub from '../event_hub'; -import { responseMessageFromSuccess } from '../utils/response_message_parser'; +import { responseFromSuccess } from '../utils/response_message_parser'; +import { memberName } from '../utils/member_utils'; import { getInvalidFeedbackMessage } from '../utils/get_invalid_feedback_message'; import ModalConfetti from './confetti.vue'; import MembersTokenSelect from './members_token_select.vue'; @@ -101,6 +103,7 @@ export default { isLoading: false, modalId: uniqueId('invite-members-modal-'), newUsersToInvite: [], + invalidMembers: {}, selectedTasksToBeDone: [], selectedTaskProject: this.projects[0], source: 'unknown', @@ -125,6 +128,16 @@ export default { inviteDisabled() { return this.newUsersToInvite.length === 0; }, + hasInvalidMembers() { + return !isEmpty(this.invalidMembers); + }, + memberErrorTitle() { + return n__( + "InviteMembersModal|The following member couldn't be invited", + "InviteMembersModal|The following %d members couldn't be invited", + Object.keys(this.invalidMembers).length, + ); + }, tasksToBeDoneEnabled() { return ( (getParameterValues('open_modal')[0] === 'invite_members_for_task' || @@ -218,7 +231,7 @@ export default { }, sendInvite({ accessLevel, expiresAt }) { this.isLoading = true; - this.invalidFeedbackMessage = ''; + this.clearValidation(); const [usersToInviteByEmail, usersToAddById] = this.partitionNewUsersToInvite(); @@ -242,12 +255,10 @@ export default { ...userId, }) .then((response) => { - const message = responseMessageFromSuccess(response); + const { error, message } = responseFromSuccess(response); - if (message) { - this.showInvalidFeedbackMessage({ - response: { data: { message } }, - }); + if (error) { + this.showMemberErrors(message); } else { this.showSuccessMessage(); } @@ -257,6 +268,13 @@ export default { this.isLoading = false; }); }, + showMemberErrors(message) { + this.invalidMembers = message; + }, + tokenName(username) { + // initial token creation hits this and nothing is found... so safe navigation + return this.newUsersToInvite.find((member) => memberName(member) === username)?.name; + }, trackinviteMembersForTask() { const label = 'selected_tasks_to_be_done'; const property = this.selectedTasksToBeDone.join(','); @@ -264,8 +282,8 @@ export default { tracking.event(INVITE_MEMBERS_FOR_TASK.submit); }, resetFields() { + this.clearValidation(); this.isLoading = false; - this.invalidFeedbackMessage = ''; this.newUsersToInvite = []; this.selectedTasksToBeDone = []; [this.selectedTaskProject] = this.projects; @@ -287,6 +305,11 @@ export default { }, clearValidation() { this.invalidFeedbackMessage = ''; + this.invalidMembers = {}; + }, + removeToken(token) { + delete this.invalidMembers[memberName(token)]; + this.invalidMembers = { ...this.invalidMembers }; }, }, labels: MEMBER_MODAL_LABELS, @@ -324,23 +347,40 @@ export default { <modal-confetti v-if="isCelebration" /> </template> - <template #user-limit-notification> + <template #alert> + <gl-alert + v-if="hasInvalidMembers" + variant="danger" + :dismissible="false" + :title="memberErrorTitle" + data-testid="alert-member-error" + > + {{ $options.labels.memberErrorListText }} + <ul class="gl-pl-5"> + <li v-for="(error, member) in invalidMembers" :key="member"> + <strong>{{ tokenName(member) }}:</strong> {{ error }} + </li> + </ul> + </gl-alert> <user-limit-notification + v-else :close-to-limit="closeToLimit" :reached-limit="reachedLimit" :users-limit-dataset="usersLimitDataset" /> </template> - <template #select="{ validationState, labelId }"> + <template #select="{ exceptionState, labelId }"> <members-token-select v-model="newUsersToInvite" class="gl-mb-2" - :validation-state="validationState" + :exception-state="exceptionState" :aria-labelledby="labelId" :users-filter="usersFilter" :filter-id="filterId" + :invalid-members="invalidMembers" @clear="clearValidation" + @token-remove="removeToken" /> </template> <template #form-after> diff --git a/app/assets/javascripts/invite_members/components/invite_modal_base.vue b/app/assets/javascripts/invite_members/components/invite_modal_base.vue index 90d266c3155..f917ebc35c2 100644 --- a/app/assets/javascripts/invite_members/components/invite_modal_base.vue +++ b/app/assets/javascripts/invite_members/components/invite_modal_base.vue @@ -159,7 +159,7 @@ export default { introText() { return sprintf(this.labelIntroText, { name: this.name }); }, - validationState() { + exceptionState() { return this.invalidFeedbackMessage ? false : null; }, selectLabelId() { @@ -306,11 +306,11 @@ export default { <slot name="intro-text-after"></slot> </div> - <slot name="user-limit-notification"></slot> + <slot name="alert"></slot> <gl-form-group :invalid-feedback="invalidFeedbackMessage" - :state="validationState" + :state="exceptionState" data-testid="members-form-group" > <template #description> @@ -320,7 +320,7 @@ export default { <label :id="selectLabelId" :class="selectLabelClass">{{ labelSearchField }}</label> <gl-form-input v-if="reachedLimit" data-testid="disabled-input" disabled /> - <slot v-else name="select" v-bind="{ validationState, labelId: selectLabelId }"></slot> + <slot v-else name="select" v-bind="{ exceptionState, labelId: selectLabelId }"></slot> </gl-form-group> <template v-if="!reachedLimit"> diff --git a/app/assets/javascripts/invite_members/components/members_token_select.vue b/app/assets/javascripts/invite_members/components/members_token_select.vue index 30c9294344e..b2bcb9a5906 100644 --- a/app/assets/javascripts/invite_members/components/members_token_select.vue +++ b/app/assets/javascripts/invite_members/components/members_token_select.vue @@ -3,6 +3,7 @@ import { GlTokenSelector, GlAvatar, GlAvatarLabeled, GlIcon, GlSprintf } from '@ import { debounce } from 'lodash'; import { __ } from '~/locale'; import { getUsers } from '~/rest_api'; +import { memberName } from '../utils/member_utils'; import { SEARCH_DELAY, USERS_FILTER_ALL, USERS_FILTER_SAML_PROVIDER_ID } from '../constants'; export default { @@ -23,7 +24,7 @@ export default { type: String, required: true, }, - validationState: { + exceptionState: { type: Boolean, required: false, default: false, @@ -38,6 +39,10 @@ export default { required: false, default: null, }, + invalidMembers: { + type: Object, + required: true, + }, }, data() { return { @@ -109,13 +114,18 @@ export default { this.hasBeenFocused = true; }, - handleTokenRemove() { + handleTokenRemove(value) { if (this.selectedTokens.length) { + this.$emit('token-remove', value); + return; } this.$emit('clear'); }, + hasError(token) { + return Object.keys(this.invalidMembers).includes(memberName(token)); + }, }, defaultQueryOptions: { without_project_bots: true, active: true }, i18n: { @@ -127,7 +137,7 @@ export default { <template> <gl-token-selector v-model="selectedTokens" - :state="validationState" + :state="exceptionState" :dropdown-items="users" :loading="loading" :allow-user-defined-tokens="emailIsValid" @@ -145,8 +155,19 @@ export default { @token-remove="handleTokenRemove" > <template #token-content="{ token }"> - <gl-icon v-if="validationState === false" name="error" :size="16" class="gl-mr-2" /> - <gl-avatar v-else-if="token.avatar_url" :src="token.avatar_url" :size="16" /> + <gl-icon + v-if="hasError(token)" + name="error" + :size="16" + class="gl-mr-2" + :data-testid="`error-icon-${token.id}`" + /> + <gl-avatar + v-else-if="token.avatar_url" + :src="token.avatar_url" + :size="16" + data-testid="token-avatar" + /> {{ token.name }} </template> diff --git a/app/assets/javascripts/invite_members/constants.js b/app/assets/javascripts/invite_members/constants.js index beb8f5b5aab..6141e5e9e0b 100644 --- a/app/assets/javascripts/invite_members/constants.js +++ b/app/assets/javascripts/invite_members/constants.js @@ -74,6 +74,9 @@ export const INVITE_BUTTON_TEXT_DISABLED = s__('InviteMembersModal|Manage member export const CANCEL_BUTTON_TEXT = s__('InviteMembersModal|Cancel'); export const CANCEL_BUTTON_TEXT_DISABLED = s__('InviteMembersModal|Explore paid plans'); export const HEADER_CLOSE_LABEL = s__('InviteMembersModal|Close invite team members'); +export const MEMBER_ERROR_LIST_TEXT = s__( + 'InviteMembersModal|Review the invite errors and try again:', +); export const MEMBER_MODAL_LABELS = { modal: { @@ -109,6 +112,7 @@ export const MEMBER_MODAL_LABELS = { title: MEMBERS_TASKS_PROJECTS_TITLE, }, toastMessageSuccessful: TOAST_MESSAGE_SUCCESSFUL, + memberErrorListText: MEMBER_ERROR_LIST_TEXT, }; export const GROUP_MODAL_LABELS = { diff --git a/app/assets/javascripts/invite_members/utils/member_utils.js b/app/assets/javascripts/invite_members/utils/member_utils.js new file mode 100644 index 00000000000..d85162626f1 --- /dev/null +++ b/app/assets/javascripts/invite_members/utils/member_utils.js @@ -0,0 +1,4 @@ +export function memberName(member) { + // user defined tokens(invites by email) will have email in `name` and will not contain `username` + return member.username || member.name; +} diff --git a/app/assets/javascripts/invite_members/utils/response_message_parser.js b/app/assets/javascripts/invite_members/utils/response_message_parser.js index db8ac303dc4..6e6431b89d9 100644 --- a/app/assets/javascripts/invite_members/utils/response_message_parser.js +++ b/app/assets/javascripts/invite_members/utils/response_message_parser.js @@ -1,15 +1,4 @@ -import { isString } from 'lodash'; - -function responseKeyedMessageParsed(keyedMessage) { - try { - const keys = Object.keys(keyedMessage); - const msg = keyedMessage[keys[0]]; - - return msg; - } catch { - return ''; - } -} +import { isString, isArray } from 'lodash'; export function responseMessageFromError(response) { if (!response?.response?.data) { @@ -23,9 +12,9 @@ export function responseMessageFromError(response) { return data.error || data.message?.error || data.message || ''; } -export function responseMessageFromSuccess(response) { +export function responseFromSuccess(response) { if (!response?.data) { - return ''; + return { error: false }; } const { data } = response; @@ -34,11 +23,19 @@ export function responseMessageFromSuccess(response) { const { message } = data; if (isString(message)) { - return message; + return { message, error: true }; + } + + if (isArray(message)) { + return { message: message[0], error: true }; } + // we assume object now with our keyed format + return { message: { ...message }, error: true }; + } - return responseKeyedMessageParsed(message); + if (data.error) { + return { message: data.error, error: true }; } - return data.error || ''; + return { error: false }; } diff --git a/app/assets/javascripts/milestones/components/delete_milestone_modal.vue b/app/assets/javascripts/milestones/components/delete_milestone_modal.vue index 34f9fe778ea..3a13c123d77 100644 --- a/app/assets/javascripts/milestones/components/delete_milestone_modal.vue +++ b/app/assets/javascripts/milestones/components/delete_milestone_modal.vue @@ -1,6 +1,6 @@ <script> -import { GlSafeHtmlDirective as SafeHtml, GlModal } from '@gitlab/ui'; -import createFlash from '~/flash'; +import { GlSprintf, GlModal } from '@gitlab/ui'; +import { createAlert } from '~/flash'; import axios from '~/lib/utils/axios_utils'; import { redirectTo } from '~/lib/utils/url_utility'; @@ -10,9 +10,7 @@ import eventHub from '../event_hub'; export default { components: { GlModal, - }, - directives: { - SafeHtml, + GlSprintf, }, props: { issueCount: { @@ -38,20 +36,10 @@ export default { }, computed: { text() { - const milestoneTitle = sprintf('<strong>%{milestoneTitle}</strong>', { - milestoneTitle: this.milestoneTitle, - }); - if (this.issueCount === 0 && this.mergeRequestCount === 0) { - return sprintf( - s__(`Milestones| + return s__(`Milestones| You’re about to permanently delete the milestone %{milestoneTitle}. -This milestone is not currently used in any issues or merge requests.`), - { - milestoneTitle, - }, - false, - ); +This milestone is not currently used in any issues or merge requests.`); } return sprintf( @@ -59,7 +47,6 @@ This milestone is not currently used in any issues or merge requests.`), You’re about to permanently delete the milestone %{milestoneTitle} and remove it from %{issuesWithCount} and %{mergeRequestsWithCount}. Once deleted, it cannot be undone or recovered.`), { - milestoneTitle, issuesWithCount: n__('%d issue', '%d issues', this.issueCount), mergeRequestsWithCount: n__( '%d merge request', @@ -98,13 +85,13 @@ Once deleted, it cannot be undone or recovered.`), }); if (error.response && error.response.status === 404) { - createFlash({ + createAlert({ message: sprintf(s__('Milestones|Milestone %{milestoneTitle} was not found'), { milestoneTitle: this.milestoneTitle, }), }); } else { - createFlash({ + createAlert({ message: sprintf(s__('Milestones|Failed to delete milestone %{milestoneTitle}'), { milestoneTitle: this.milestoneTitle, }), @@ -132,6 +119,10 @@ Once deleted, it cannot be undone or recovered.`), :action-cancel="$options.cancelProps" @primary="onSubmit" > - <p v-safe-html="text"></p> + <gl-sprintf :message="text"> + <template #milestoneTitle> + <strong>{{ milestoneTitle }}</strong> + </template> + </gl-sprintf> </gl-modal> </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 aa0085437ec..4fb4a18c460 100644 --- a/app/assets/javascripts/work_items/components/work_item_detail.vue +++ b/app/assets/javascripts/work_items/components/work_item_detail.vue @@ -1,5 +1,5 @@ <script> -import { GlAlert, GlSkeletonLoader } from '@gitlab/ui'; +import { GlAlert, GlButton, GlSkeletonLoader } from '@gitlab/ui'; import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import { i18n, @@ -20,6 +20,7 @@ export default { i18n, components: { GlAlert, + GlButton, GlSkeletonLoader, WorkItemAssignees, WorkItemActions, @@ -30,6 +31,11 @@ export default { }, mixins: [glFeatureFlagMixin()], props: { + isModal: { + type: Boolean, + required: false, + default: false, + }, workItemId: { type: String, required: false, @@ -113,24 +119,29 @@ export default { </gl-skeleton-loader> </div> <template v-else> - <div class="gl-font-weight-bold gl-text-secondary gl-mb-2">{{ workItemType }}</div> - <div class="gl-display-flex gl-align-items-start"> - <work-item-title - :work-item-id="workItem.id" - :work-item-title="workItem.title" - :work-item-type="workItemType" - :work-item-parent-id="workItemParentId" - class="gl-mr-5" - @error="error = $event" - /> + <div class="gl-display-flex gl-align-items-center"> + <span class="gl-font-weight-bold gl-text-secondary gl-mr-auto">{{ workItemType }}</span> <work-item-actions :work-item-id="workItem.id" :can-delete="canDelete" - class="gl-mt-4" @deleteWorkItem="$emit('deleteWorkItem')" @error="error = $event" /> + <gl-button + v-if="isModal" + category="tertiary" + icon="close" + :aria-label="__('Close')" + @click="$emit('close')" + /> </div> + <work-item-title + :work-item-id="workItem.id" + :work-item-title="workItem.title" + :work-item-type="workItemType" + :work-item-parent-id="workItemParentId" + @error="error = $event" + /> <work-item-state :work-item="workItem" :work-item-parent-id="workItemParentId" diff --git a/app/assets/javascripts/work_items/components/work_item_detail_modal.vue b/app/assets/javascripts/work_items/components/work_item_detail_modal.vue index 50753c79741..b4f9e38a64a 100644 --- a/app/assets/javascripts/work_items/components/work_item_detail_modal.vue +++ b/app/assets/javascripts/work_items/components/work_item_detail_modal.vue @@ -87,6 +87,9 @@ export default { this.error = ''; this.$emit('close'); }, + hide() { + this.$refs.modal.hide(); + }, setErrorMessage(message) { this.error = message; }, @@ -111,9 +114,11 @@ export default { </gl-alert> <work-item-detail + is-modal :work-item-parent-id="issueGid" :work-item-id="workItemId" class="gl-p-5 gl-mt-n3" + @close="hide" @deleteWorkItem="deleteWorkItem" /> </gl-modal> diff --git a/doc/administration/audit_event_streaming.md b/doc/administration/audit_event_streaming.md index 0b8c0592419..8a0bf0e1695 100644 --- a/doc/administration/audit_event_streaming.md +++ b/doc/administration/audit_event_streaming.md @@ -179,8 +179,7 @@ The header is created if the returned `errors` object is empty. FLAG: On self-managed GitLab, by default the UI for this feature is not available. To make it available per group, ask an administrator to [enable the feature flag](../administration/feature_flags.md) named `custom_headers_streaming_audit_events_ui`. On GitLab.com, the UI for this feature is -not available. The UI for this feature is not ready for production use. Custom header values are not saved by the GitLab UI. To track progress on saving -custom header values in the GitLab UI, [see the relevant issue](https://gitlab.com/gitlab-org/gitlab/-/issues/361631). +not available. The UI for this feature is not ready for production use. Users with at least the Owner role for a group can add event streaming destinations and custom HTTP headers for it: diff --git a/doc/administration/operations/puma.md b/doc/administration/operations/puma.md index cd258104496..bc95ee626ce 100644 --- a/doc/administration/operations/puma.md +++ b/doc/administration/operations/puma.md @@ -373,6 +373,34 @@ separate Rails process to debug the issue: 1. In a new window, run `top`. It should show this Ruby process using 100% CPU. Write down the PID. 1. Follow step 2 from the previous section on using GDB. +### GitLab: API is not accessible + +This often occurs when GitLab Shell attempts to request authorization via the +[internal API](../../development/internal_api/index.md) (for example, `http://localhost:8080/api/v4/internal/allowed`), and +something in the check fails. There are many reasons why this may happen: + +1. Timeout connecting to a database (for example, PostgreSQL or Redis) +1. Error in Git hooks or push rules +1. Error accessing the repository (for example, stale NFS handles) + +To diagnose this problem, try to reproduce the problem and then see if there +is a Puma worker that is spinning via `top`. Try to use the `gdb` +techniques above. In addition, using `strace` may help isolate issues: + +```shell +strace -ttTfyyy -s 1024 -p <PID of puma worker> -o /tmp/puma.txt +``` + +If you cannot isolate which Puma worker is the issue, try to run `strace` +on all the Puma workers to see where the +[`/internal/allowed`](../../development/internal_api/index.md) endpoint gets stuck: + +```shell +ps auwx | grep puma | awk '{ print " -p " $2}' | xargs strace -ttTfyyy -s 1024 -o /tmp/puma.txt +``` + +The output in `/tmp/puma.txt` may help diagnose the root cause. + ## Related topics - [Use a dedicated metrics server to export web metrics](../monitoring/prometheus/puma_exporter.md) diff --git a/doc/administration/troubleshooting/debug.md b/doc/administration/troubleshooting/debug.md index 76bb21df699..dbd37f83a6e 100644 --- a/doc/administration/troubleshooting/debug.md +++ b/doc/administration/troubleshooting/debug.md @@ -110,39 +110,6 @@ in Omnibus, run as root: /opt/gitlab/embedded/bin/ruby /opt/gitlab/embedded/bin/rbtrace ``` -## Common Problems - -Many of the tips to diagnose issues below apply to many different situations. We use one -concrete example to illustrate what you can do to learn what is going wrong. - -### GitLab: API is not accessible - -This often occurs when GitLab Shell attempts to request authorization via the -[internal API](../../development/internal_api/index.md) (for example, `http://localhost:8080/api/v4/internal/allowed`), and -something in the check fails. There are many reasons why this may happen: - -1. Timeout connecting to a database (for example, PostgreSQL or Redis) -1. Error in Git hooks or push rules -1. Error accessing the repository (for example, stale NFS handles) - -To diagnose this problem, try to reproduce the problem and then see if there -is a Unicorn worker that is spinning via `top`. Try to use the `gdb` -techniques above. In addition, using `strace` may help isolate issues: - -```shell -strace -ttTfyyy -s 1024 -p <PID of puma worker> -o /tmp/puma.txt -``` - -If you cannot isolate which Unicorn worker is the issue, try to run `strace` -on all the Unicorn workers to see where the -[`/internal/allowed`](../../development/internal_api/index.md) endpoint gets stuck: - -```shell -ps auwx | grep puma | awk '{ print " -p " $2}' | xargs strace -ttTfyyy -s 1024 -o /tmp/puma.txt -``` - -The output in `/tmp/puma.txt` may help diagnose the root cause. - ## More information - [Debugging Stuck Ruby Processes](https://newrelic.com/blog/best-practices/debugging-stuck-ruby-processes-what-to-do-before-you-kill-9) diff --git a/doc/development/go_guide/index.md b/doc/development/go_guide/index.md index f5b0da2f162..1a11321b70f 100644 --- a/doc/development/go_guide/index.md +++ b/doc/development/go_guide/index.md @@ -443,6 +443,43 @@ of the Code Review Comments page on the Go wiki for more details. Most editors/IDEs allow you to run commands before/after saving a file, you can set it up to run `goimports -local gitlab.com/gitlab-org` so that it's applied to every file when saving. +### Initializing slices + +If initializing a slice, provide a capacity where possible to avoid extra +allocations. + +<table> +<tr><th>:white_check_mark: Do</th><th>:x: Don't</th></tr> +<tr> + <td> + + ```golang + s2 := make([]string, 0, size) + for _, val := range s1 { + s2 = append(s2, val) + } + ``` + + </td> + <td> + + ```golang + var s2 []string + for _, val := range s1 { + s2 = append(s2, val) + } + ``` + + </td> +</tr> +</table> + +If no capacity is passed to `make` when creating a new slice, `append` +will continuously resize the slice's backing array if it cannot hold +the values. Providing the capacity ensures that allocations are kept +to a minimum. It is recommended that the [`prealloc`](https://github.com/alexkohler/prealloc) +golanci-lint rule automatically check for this. + ### Analyzer Tests The conventional Secure [analyzer](https://gitlab.com/gitlab-org/security-products/analyzers/) has a [`convert` function](https://gitlab.com/gitlab-org/security-products/analyzers/command/-/blob/main/convert.go#L15-17) that converts SAST/DAST scanner reports into [GitLab Security Reports](https://gitlab.com/gitlab-org/security-products/security-report-schemas). When writing tests for the `convert` function, we should make use of [test fixtures](https://dave.cheney.net/2016/05/10/test-fixtures-in-go) using a `testdata` directory at the root of the analyzer's repository. The `testdata` directory should contain two subdirectories: `expect` and `reports`. The `reports` directory should contain sample SAST/DAST scanner reports which are passed into the `convert` function during the test setup. The `expect` directory should contain the expected GitLab Security Report that the `convert` returns. See Secret Detection for an [example](https://gitlab.com/gitlab-org/security-products/analyzers/secrets/-/blob/160424589ef1eed7b91b59484e019095bc7233bd/convert_test.go#L13-66). diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 202fb141d69..d5746106142 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -21398,6 +21398,9 @@ msgstr "" msgid "InviteMembersModal|Members were successfully added" msgstr "" +msgid "InviteMembersModal|Review the invite errors and try again:" +msgstr "" + msgid "InviteMembersModal|Search for a group to invite" msgstr "" @@ -21413,6 +21416,11 @@ msgstr "" msgid "InviteMembersModal|Something went wrong" msgstr "" +msgid "InviteMembersModal|The following member couldn't be invited" +msgid_plural "InviteMembersModal|The following %d members couldn't be invited" +msgstr[0] "" +msgstr[1] "" + msgid "InviteMembersModal|This feature is disabled until this group has space for more members." msgstr "" @@ -44202,9 +44210,6 @@ msgstr "" msgid "You can only transfer the project to namespaces you manage." msgstr "" -msgid "You can only upload one design when dropping onto an existing design." -msgstr "" - msgid "You can resolve the merge conflict using either the Interactive mode, by choosing %{use_ours} or %{use_theirs} buttons, or by editing the files directly. Commit these changes into %{branch_name}" msgstr "" @@ -44480,9 +44485,6 @@ msgstr "" msgid "You must solve the CAPTCHA in order to submit" msgstr "" -msgid "You must upload a file with the same file name when dropping onto an existing design." -msgstr "" - msgid "You need a different license to enable FileLocks feature" msgstr "" @@ -44940,6 +44942,12 @@ msgid_plural "Your subscription will expire in %{remaining_days} days." msgstr[0] "" msgstr[1] "" +msgid "Your update failed. You can only upload one design when dropping onto an existing design." +msgstr "" + +msgid "Your update failed. You must upload a file with the same file name when dropping onto an existing design." +msgstr "" + msgid "Your username is %{username}." msgstr "" diff --git a/spec/features/projects/files/dockerfile_dropdown_spec.rb b/spec/features/projects/files/dockerfile_dropdown_spec.rb index 3a0cc61d9c6..dd1635c900e 100644 --- a/spec/features/projects/files/dockerfile_dropdown_spec.rb +++ b/spec/features/projects/files/dockerfile_dropdown_spec.rb @@ -26,6 +26,6 @@ RSpec.describe 'Projects > Files > User wants to add a Dockerfile file', :js do wait_for_requests expect(page).to have_css('.dockerfile-selector .dropdown-toggle-text', text: 'Apply a template') - expect(editor_get_value).to have_content('COPY ./ /usr/local/apache2/htdocs/') + expect(find('.monaco-editor')).to have_content('COPY ./ /usr/local/apache2/htdocs/') end end diff --git a/spec/features/projects/files/gitignore_dropdown_spec.rb b/spec/features/projects/files/gitignore_dropdown_spec.rb index 4a92216f46c..a86adf951d8 100644 --- a/spec/features/projects/files/gitignore_dropdown_spec.rb +++ b/spec/features/projects/files/gitignore_dropdown_spec.rb @@ -26,7 +26,7 @@ RSpec.describe 'Projects > Files > User wants to add a .gitignore file', :js do wait_for_requests expect(page).to have_css('.gitignore-selector .dropdown-toggle-text', text: 'Apply a template') - expect(editor_get_value).to have_content('/.bundle') - expect(editor_get_value).to have_content('config/initializers/secret_token.rb') + expect(find('.monaco-editor')).to have_content('/.bundle') + expect(find('.monaco-editor')).to have_content('config/initializers/secret_token.rb') end end diff --git a/spec/features/projects/files/gitlab_ci_yml_dropdown_spec.rb b/spec/features/projects/files/gitlab_ci_yml_dropdown_spec.rb index cdf6c219ea5..46ac0dee7eb 100644 --- a/spec/features/projects/files/gitlab_ci_yml_dropdown_spec.rb +++ b/spec/features/projects/files/gitlab_ci_yml_dropdown_spec.rb @@ -30,8 +30,8 @@ RSpec.describe 'Projects > Files > User wants to add a .gitlab-ci.yml file', :js wait_for_requests expect(page).to have_css('.gitlab-ci-yml-selector .dropdown-toggle-text', text: 'Apply a template') - expect(editor_get_value).to have_content('This file is a template, and might need editing before it works on your project') - expect(editor_get_value).to have_content('jekyll build -d test') + expect(find('.monaco-editor')).to have_content('This file is a template, and might need editing before it works on your project') + expect(find('.monaco-editor')).to have_content('jekyll build -d test') end context 'when template param is provided' do @@ -41,8 +41,8 @@ RSpec.describe 'Projects > Files > User wants to add a .gitlab-ci.yml file', :js wait_for_requests expect(page).to have_css('.gitlab-ci-yml-selector .dropdown-toggle-text', text: 'Apply a template') - expect(editor_get_value).to have_content('This file is a template, and might need editing before it works on your project') - expect(editor_get_value).to have_content('jekyll build -d test') + expect(find('.monaco-editor')).to have_content('This file is a template, and might need editing before it works on your project') + expect(find('.monaco-editor')).to have_content('jekyll build -d test') end end @@ -53,7 +53,7 @@ RSpec.describe 'Projects > Files > User wants to add a .gitlab-ci.yml file', :js wait_for_requests expect(page).to have_css('.gitlab-ci-yml-selector .dropdown-toggle-text', text: 'Apply a template') - expect(editor_get_value).to have_content('') + expect(find('.monaco-editor')).to have_content('') end end @@ -64,7 +64,7 @@ RSpec.describe 'Projects > Files > User wants to add a .gitlab-ci.yml file', :js it 'leaves the editor empty' do wait_for_requests - expect(editor_get_value).to have_content('') + expect(find('.monaco-editor')).to have_content('') end end end diff --git a/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb b/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb index 0e87622d3c2..6b1e60db5b1 100644 --- a/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb +++ b/spec/features/projects/files/project_owner_sees_link_to_create_license_file_in_empty_project_spec.rb @@ -22,8 +22,10 @@ RSpec.describe 'Projects > Files > Project owner sees a link to create a license select_template('MIT License') - expect(ide_editor_value).to have_content('MIT License') - expect(ide_editor_value).to have_content("Copyright (c) #{Time.zone.now.year} #{project.namespace.human_name}") + file_content = "Copyright (c) #{Time.zone.now.year} #{project.namespace.human_name}" + + expect(find('.monaco-editor')).to have_content('MIT License') + expect(find('.monaco-editor')).to have_content(file_content) ide_commit @@ -33,7 +35,7 @@ RSpec.describe 'Projects > Files > Project owner sees a link to create a license license_file = project.repository.blob_at('master', 'LICENSE').data expect(license_file).to have_content('MIT License') - expect(license_file).to have_content("Copyright (c) #{Time.zone.now.year} #{project.namespace.human_name}") + expect(license_file).to have_content(file_content) end def select_template(template) diff --git a/spec/frontend/design_management/pages/__snapshots__/index_spec.js.snap b/spec/frontend/design_management/pages/__snapshots__/index_spec.js.snap index be736184e60..9997f02cd01 100644 --- a/spec/frontend/design_management/pages/__snapshots__/index_spec.js.snap +++ b/spec/frontend/design_management/pages/__snapshots__/index_spec.js.snap @@ -7,6 +7,8 @@ exports[`Design management index page designs renders error 1`] = ` > <!----> + <!----> + <div class="gl-mt-6" > @@ -39,6 +41,8 @@ exports[`Design management index page designs renders loading icon 1`] = ` > <!----> + <!----> + <div class="gl-mt-6" > diff --git a/spec/frontend/design_management/pages/index_spec.js b/spec/frontend/design_management/pages/index_spec.js index 087655d10f7..21be7bd148b 100644 --- a/spec/frontend/design_management/pages/index_spec.js +++ b/spec/frontend/design_management/pages/index_spec.js @@ -1,5 +1,4 @@ import { GlEmptyState } from '@gitlab/ui'; -import { shallowMount } from '@vue/test-utils'; import Vue, { nextTick } from 'vue'; import VueApollo, { ApolloMutation } from 'vue-apollo'; @@ -9,6 +8,7 @@ import VueDraggable from 'vuedraggable'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; import { mockTracking, unmockTracking } from 'helpers/tracking_helper'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import permissionsQuery from 'shared_queries/design_management/design_permissions.query.graphql'; import getDesignListQuery from 'shared_queries/design_management/get_design_list.query.graphql'; import DeleteButton from '~/design_management/components/delete_button.vue'; @@ -23,6 +23,7 @@ import * as utils from '~/design_management/utils/design_management_utils'; import { EXISTING_DESIGN_DROP_MANY_FILES_MESSAGE, EXISTING_DESIGN_DROP_INVALID_FILENAME_MESSAGE, + UPLOAD_DESIGN_ERROR, } from '~/design_management/utils/error_messages'; import { DESIGN_TRACKING_PAGE_NAME, @@ -101,20 +102,20 @@ describe('Design management index page', () => { let moveDesignHandler; const findDesignCheckboxes = () => wrapper.findAll('.design-checkbox'); - const findSelectAllButton = () => wrapper.find('[data-testid="select-all-designs-button"'); - const findToolbar = () => wrapper.find('.qa-selector-toolbar'); - const findDesignCollectionIsCopying = () => - wrapper.find('[data-testid="design-collection-is-copying"'); - const findDeleteButton = () => wrapper.find(DeleteButton); - const findDropzone = () => wrapper.findAll(DesignDropzone).at(0); + const findSelectAllButton = () => wrapper.findByTestId('select-all-designs-button'); + const findToolbar = () => wrapper.findByTestId('design-selector-toolbar'); + const findDesignCollectionIsCopying = () => wrapper.findByTestId('design-collection-is-copying'); + const findDeleteButton = () => wrapper.findComponent(DeleteButton); + const findDropzone = () => wrapper.findAllComponents(DesignDropzone).at(0); const dropzoneClasses = () => findDropzone().classes(); - const findDropzoneWrapper = () => wrapper.find('[data-testid="design-dropzone-wrapper"]'); - const findFirstDropzoneWithDesign = () => wrapper.findAll(DesignDropzone).at(1); - const findDesignsWrapper = () => wrapper.find('[data-testid="designs-root"]'); + const findDropzoneWrapper = () => wrapper.findByTestId('design-dropzone-wrapper'); + const findFirstDropzoneWithDesign = () => wrapper.findAllComponents(DesignDropzone).at(1); + const findDesignsWrapper = () => wrapper.findByTestId('designs-root'); const findDesigns = () => wrapper.findAll(Design); const draggableAttributes = () => wrapper.find(VueDraggable).vm.$attrs; - const findDesignUploadButton = () => wrapper.find('[data-testid="design-upload-button"]'); - const findDesignToolbarWrapper = () => wrapper.find('[data-testid="design-toolbar-wrapper"]'); + const findDesignUploadButton = () => wrapper.findByTestId('design-upload-button'); + const findDesignToolbarWrapper = () => wrapper.findByTestId('design-toolbar-wrapper'); + const findDesignUpdateAlert = () => wrapper.findByTestId('design-update-alert'); async function moveDesigns(localWrapper) { await waitForPromises(); @@ -149,7 +150,7 @@ describe('Design management index page', () => { mutate, }; - wrapper = shallowMount(Index, { + wrapper = shallowMountExtended(Index, { data() { return { allVersions, @@ -185,7 +186,7 @@ describe('Design management index page', () => { ]; fakeApollo = createMockApollo(requestHandlers, {}, { addTypename: true }); - wrapper = shallowMount(Index, { + wrapper = shallowMountExtended(Index, { apolloProvider: fakeApollo, router, stubs: { VueDraggable }, @@ -412,7 +413,8 @@ describe('Design management index page', () => { await nextTick(); expect(wrapper.vm.filesToBeSaved).toEqual([]); expect(wrapper.vm.isSaving).toBeFalsy(); - expect(createFlash).toHaveBeenCalled(); + expect(findDesignUpdateAlert().exists()).toBe(true); + expect(findDesignUpdateAlert().text()).toBe(UPLOAD_DESIGN_ERROR); }); it('does not call mutation if createDesign is false', () => { @@ -431,19 +433,23 @@ describe('Design management index page', () => { wrapper.vm.onUploadDesign(new Array(MAXIMUM_FILE_UPLOAD_LIMIT).fill(mockDesigns[0])); - expect(createFlash).not.toHaveBeenCalled(); + expect(findDesignUpdateAlert().exists()).toBe(false); }); - it('warns when too many files are uploaded', () => { + it('warns when too many files are uploaded', async () => { createComponent(); wrapper.vm.onUploadDesign(new Array(MAXIMUM_FILE_UPLOAD_LIMIT + 1).fill(mockDesigns[0])); + await nextTick(); - expect(createFlash).toHaveBeenCalled(); + expect(findDesignUpdateAlert().exists()).toBe(true); + expect(findDesignUpdateAlert().text()).toBe( + 'The maximum number of designs allowed to be uploaded is 10. Please try again.', + ); }); }); - it('flashes warning if designs are skipped', async () => { + it('displays warning if designs are skipped', async () => { createComponent({ mockMutate: () => Promise.resolve({ @@ -458,11 +464,8 @@ describe('Design management index page', () => { ]); await uploadDesign; - expect(createFlash).toHaveBeenCalledTimes(1); - expect(createFlash).toHaveBeenCalledWith({ - message: 'Upload skipped. test.jpg did not change.', - types: 'warning', - }); + expect(findDesignUpdateAlert().exists()).toBe(true); + expect(findDesignUpdateAlert().text()).toBe('Upload skipped. test.jpg did not change.'); }); describe('dragging onto an existing design', () => { @@ -495,13 +498,17 @@ describe('Design management index page', () => { description | eventPayload | message ${'> 1 file'} | ${[{ name: 'test' }, { name: 'test-2' }]} | ${EXISTING_DESIGN_DROP_MANY_FILES_MESSAGE} ${'different filename'} | ${[{ name: 'wrong-name' }]} | ${EXISTING_DESIGN_DROP_INVALID_FILENAME_MESSAGE} - `('calls createFlash when upload has $description', ({ eventPayload, message }) => { - const designDropzone = findFirstDropzoneWithDesign(); - designDropzone.vm.$emit('change', eventPayload); - - expect(createFlash).toHaveBeenCalledTimes(1); - expect(createFlash).toHaveBeenCalledWith({ message }); - }); + `( + 'displays GlAlert component when upload has $description', + async ({ eventPayload, message }) => { + expect(findDesignUpdateAlert().exists()).toBe(false); + const designDropzone = findFirstDropzoneWithDesign(); + await designDropzone.vm.$emit('change', eventPayload); + + expect(findDesignUpdateAlert().exists()).toBe(true); + expect(findDesignUpdateAlert().text()).toBe(message); + }, + ); }); describe('tracking', () => { @@ -804,7 +811,7 @@ describe('Design management index page', () => { expect(createFlash).toHaveBeenCalledWith({ message: 'Houston, we have a problem' }); }); - it('displays flash if mutation had a non-recoverable error', async () => { + it('displays alert if mutation had a non-recoverable error', async () => { createComponentWithApollo({ moveHandler: jest.fn().mockRejectedValue('Error'), }); @@ -812,9 +819,10 @@ describe('Design management index page', () => { await moveDesigns(wrapper); await waitForPromises(); - expect(createFlash).toHaveBeenCalledWith({ - message: 'Something went wrong when reordering designs. Please try again', - }); + expect(findDesignUpdateAlert().exists()).toBe(true); + expect(findDesignUpdateAlert().text()).toBe( + 'Something went wrong when reordering designs. Please try again', + ); }); }); }); diff --git a/spec/frontend/invite_members/components/invite_members_modal_spec.js b/spec/frontend/invite_members/components/invite_members_modal_spec.js index 13985ce7d74..045a454e63a 100644 --- a/spec/frontend/invite_members/components/invite_members_modal_spec.js +++ b/spec/frontend/invite_members/components/invite_members_modal_spec.js @@ -35,6 +35,7 @@ import { user2, user3, user4, + user5, GlEmoji, } from '../mock_data/member_modal'; @@ -93,6 +94,11 @@ describe('InviteMembersModal', () => { const findModal = () => wrapper.findComponent(GlModal); const findBase = () => wrapper.findComponent(InviteModalBase); const findIntroText = () => wrapper.findByTestId('modal-base-intro-text').text(); + const findMemberErrorAlert = () => wrapper.findByTestId('alert-member-error'); + const findMemberErrorMessage = (element) => + `${Object.keys(invitationsApiResponse.MULTIPLE_RESTRICTED.message)[element]}: ${ + Object.values(invitationsApiResponse.MULTIPLE_RESTRICTED.message)[element] + }`; const emitEventFromModal = (eventName) => () => findModal().vm.$emit(eventName, { preventDefault: jest.fn() }); const clickInviteButton = emitEventFromModal('primary'); @@ -123,6 +129,10 @@ describe('InviteMembersModal', () => { findBase().vm.$emit('access-level', val); await nextTick(); }; + const removeMembersToken = async (val) => { + findMembersSelect().vm.$emit('token-remove', val); + await nextTick(); + }; describe('rendering the tasks to be done', () => { const setupComponent = async (props = {}, urlParameter = ['invite_members_for_task']) => { @@ -431,17 +441,20 @@ describe('InviteMembersModal', () => { }); it('clears the error when the list of members to invite is cleared', async () => { - expect(membersFormGroupInvalidFeedback()).toBe( + expect(findMemberErrorAlert().exists()).toBe(true); + expect(findMemberErrorAlert().text()).toContain( Object.values(invitationsApiResponse.EMAIL_TAKEN.message)[0], ); - expect(findMembersSelect().props('validationState')).toBe(false); + expect(membersFormGroupInvalidFeedback()).toBe(''); + expect(findMembersSelect().props('exceptionState')).not.toBe(false); findMembersSelect().vm.$emit('clear'); await nextTick(); + expect(findMemberErrorAlert().exists()).toBe(false); expect(membersFormGroupInvalidFeedback()).toBe(''); - expect(findMembersSelect().props('validationState')).not.toBe(false); + expect(findMembersSelect().props('exceptionState')).not.toBe(false); }); it('clears the error when the cancel button is clicked', async () => { @@ -450,7 +463,7 @@ describe('InviteMembersModal', () => { await nextTick(); expect(membersFormGroupInvalidFeedback()).toBe(''); - expect(findMembersSelect().props('validationState')).not.toBe(false); + expect(findMembersSelect().props('exceptionState')).not.toBe(false); }); it('clears the error when the modal is hidden', async () => { @@ -458,33 +471,12 @@ describe('InviteMembersModal', () => { await nextTick(); + expect(findMemberErrorAlert().exists()).toBe(false); expect(membersFormGroupInvalidFeedback()).toBe(''); - expect(findMembersSelect().props('validationState')).not.toBe(false); + expect(findMembersSelect().props('exceptionState')).not.toBe(false); }); }); - it('clears the invalid state and message once the list of members to invite is cleared', async () => { - mockInvitationsApi(httpStatus.CREATED, invitationsApiResponse.EMAIL_TAKEN); - - clickInviteButton(); - - await waitForPromises(); - - expect(membersFormGroupInvalidFeedback()).toBe( - Object.values(invitationsApiResponse.EMAIL_TAKEN.message)[0], - ); - expect(findMembersSelect().props('validationState')).toBe(false); - expect(findModal().props('actionPrimary').attributes.loading).toBe(false); - - findMembersSelect().vm.$emit('clear'); - - await waitForPromises(); - - expect(membersFormGroupInvalidFeedback()).toBe(''); - expect(findMembersSelect().props('validationState')).toBe(null); - expect(findModal().props('actionPrimary').attributes.loading).toBe(false); - }); - it('displays the generic error for http server error', async () => { mockInvitationsApi( httpStatus.INTERNAL_SERVER_ERROR, @@ -496,6 +488,7 @@ describe('InviteMembersModal', () => { await waitForPromises(); expect(membersFormGroupInvalidFeedback()).toBe('Something went wrong'); + expect(findMembersSelect().props('exceptionState')).toBe(false); }); it('displays the restricted user api message for response with bad request', async () => { @@ -505,20 +498,31 @@ describe('InviteMembersModal', () => { await waitForPromises(); - expect(membersFormGroupInvalidFeedback()).toBe(expectedEmailRestrictedError); + expect(findMemberErrorAlert().exists()).toBe(true); + expect(findMemberErrorAlert().text()).toContain(expectedEmailRestrictedError); + expect(membersFormGroupInvalidFeedback()).toBe(''); + expect(findMembersSelect().props('exceptionState')).not.toBe(false); }); - it('displays the first part of the error when multiple existing users are restricted by email', async () => { + it('displays all errors when there are multiple existing users that are restricted by email', async () => { mockInvitationsApi(httpStatus.CREATED, invitationsApiResponse.MULTIPLE_RESTRICTED); clickInviteButton(); await waitForPromises(); - expect(membersFormGroupInvalidFeedback()).toBe( - "The member's email address is not allowed for this project. Go to the Admin area > Sign-up restrictions, and check Allowed domains for sign-ups.", + expect(findMemberErrorAlert().exists()).toBe(true); + expect(findMemberErrorAlert().text()).toContain( + Object.values(invitationsApiResponse.MULTIPLE_RESTRICTED.message)[0], + ); + expect(findMemberErrorAlert().text()).toContain( + Object.values(invitationsApiResponse.MULTIPLE_RESTRICTED.message)[1], ); - expect(findMembersSelect().props('validationState')).toBe(false); + expect(findMemberErrorAlert().text()).toContain( + Object.values(invitationsApiResponse.MULTIPLE_RESTRICTED.message)[2], + ); + expect(membersFormGroupInvalidFeedback()).toBe(''); + expect(findMembersSelect().props('exceptionState')).not.toBe(false); }); }); }); @@ -573,10 +577,30 @@ describe('InviteMembersModal', () => { await waitForPromises(); expect(membersFormGroupInvalidFeedback()).toBe(expectedSyntaxError); - expect(findMembersSelect().props('validationState')).toBe(false); + expect(findMembersSelect().props('exceptionState')).toBe(false); expect(findModal().props('actionPrimary').attributes.loading).toBe(false); }); + it('clears the error when the modal is hidden', async () => { + mockInvitationsApi(httpStatus.BAD_REQUEST, invitationsApiResponse.EMAIL_INVALID); + + clickInviteButton(); + + await waitForPromises(); + + expect(membersFormGroupInvalidFeedback()).toBe(expectedSyntaxError); + expect(findMembersSelect().props('exceptionState')).toBe(false); + expect(findModal().props('actionPrimary').attributes.loading).toBe(false); + + findModal().vm.$emit('hidden'); + + await nextTick(); + + expect(findMemberErrorAlert().exists()).toBe(false); + expect(membersFormGroupInvalidFeedback()).toBe(''); + expect(findMembersSelect().props('exceptionState')).not.toBe(false); + }); + it('displays the restricted email error when restricted email is invited', async () => { mockInvitationsApi(httpStatus.CREATED, invitationsApiResponse.EMAIL_RESTRICTED); @@ -584,20 +608,32 @@ describe('InviteMembersModal', () => { await waitForPromises(); - expect(membersFormGroupInvalidFeedback()).toContain(expectedEmailRestrictedError); - expect(findMembersSelect().props('validationState')).toBe(false); + expect(findMemberErrorAlert().exists()).toBe(true); + expect(findMemberErrorAlert().text()).toContain(expectedEmailRestrictedError); + expect(membersFormGroupInvalidFeedback()).toBe(''); + expect(findMembersSelect().props('exceptionState')).not.toBe(false); expect(findModal().props('actionPrimary').attributes.loading).toBe(false); }); - it('displays the first error message when multiple emails return a restricted error message', async () => { + it('displays all errors when there are multiple emails that return a restricted error message', async () => { mockInvitationsApi(httpStatus.CREATED, invitationsApiResponse.MULTIPLE_RESTRICTED); clickInviteButton(); await waitForPromises(); - expect(membersFormGroupInvalidFeedback()).toContain(expectedEmailRestrictedError); - expect(findMembersSelect().props('validationState')).toBe(false); + expect(findMemberErrorAlert().exists()).toBe(true); + expect(findMemberErrorAlert().text()).toContain( + Object.values(invitationsApiResponse.MULTIPLE_RESTRICTED.message)[0], + ); + expect(findMemberErrorAlert().text()).toContain( + Object.values(invitationsApiResponse.MULTIPLE_RESTRICTED.message)[1], + ); + expect(findMemberErrorAlert().text()).toContain( + Object.values(invitationsApiResponse.MULTIPLE_RESTRICTED.message)[2], + ); + expect(membersFormGroupInvalidFeedback()).toBe(''); + expect(findMembersSelect().props('exceptionState')).not.toBe(false); }); it('displays the invalid syntax error for bad request', async () => { @@ -608,7 +644,7 @@ describe('InviteMembersModal', () => { await waitForPromises(); expect(membersFormGroupInvalidFeedback()).toBe(expectedSyntaxError); - expect(findMembersSelect().props('validationState')).toBe(false); + expect(findMembersSelect().props('exceptionState')).toBe(false); }); }); @@ -617,14 +653,51 @@ describe('InviteMembersModal', () => { createInviteMembersToGroupWrapper(); await triggerMembersTokenSelect([user3, user4]); - mockInvitationsApi(httpStatus.CREATED, invitationsApiResponse.ERROR_EMAIL_INVALID); + mockInvitationsApi(httpStatus.BAD_REQUEST, invitationsApiResponse.ERROR_EMAIL_INVALID); clickInviteButton(); await waitForPromises(); expect(membersFormGroupInvalidFeedback()).toBe(expectedSyntaxError); - expect(findMembersSelect().props('validationState')).toBe(false); + expect(findMembersSelect().props('exceptionState')).toBe(false); + }); + + it('displays errors for multiple and allows clearing', async () => { + createInviteMembersToGroupWrapper(); + + await triggerMembersTokenSelect([user3, user4, user5]); + mockInvitationsApi(httpStatus.CREATED, invitationsApiResponse.MULTIPLE_RESTRICTED); + + clickInviteButton(); + + await waitForPromises(); + + expect(findMemberErrorAlert().exists()).toBe(true); + expect(findMemberErrorAlert().props('title')).toContain( + "The following 3 members couldn't be invited", + ); + expect(findMemberErrorAlert().text()).toContain(findMemberErrorMessage(0)); + expect(findMemberErrorAlert().text()).toContain(findMemberErrorMessage(1)); + expect(findMemberErrorAlert().text()).toContain(findMemberErrorMessage(2)); + + await removeMembersToken(user3); + + expect(findMemberErrorAlert().props('title')).toContain( + "The following 2 members couldn't be invited", + ); + expect(findMemberErrorAlert().text()).not.toContain(findMemberErrorMessage(0)); + + await removeMembersToken(user4); + + expect(findMemberErrorAlert().props('title')).toContain( + "The following member couldn't be invited", + ); + expect(findMemberErrorAlert().text()).not.toContain(findMemberErrorMessage(1)); + + await removeMembersToken(user5); + + expect(findMemberErrorAlert().exists()).toBe(false); }); }); }); @@ -675,24 +748,6 @@ describe('InviteMembersModal', () => { }); }); }); - - describe('when any invite failed for any reason', () => { - beforeEach(async () => { - createInviteMembersToGroupWrapper(); - - await triggerMembersTokenSelect([user1, user3]); - - mockInvitationsApi(httpStatus.BAD_REQUEST, invitationsApiResponse.EMAIL_INVALID); - - clickInviteButton(); - }); - - it('displays the first error message', async () => { - await waitForPromises(); - - expect(membersFormGroupInvalidFeedback()).toBe(expectedSyntaxError); - }); - }); }); describe('tracking', () => { diff --git a/spec/frontend/invite_members/components/invite_modal_base_spec.js b/spec/frontend/invite_members/components/invite_modal_base_spec.js index cc19e90a5fa..b55eeb72471 100644 --- a/spec/frontend/invite_members/components/invite_modal_base_spec.js +++ b/spec/frontend/invite_members/components/invite_modal_base_spec.js @@ -254,7 +254,7 @@ describe('InviteModalBase', () => { expect(wrapper.findComponent(GlModal).props('actionPrimary').attributes.loading).toBe(true); }); - it('with invalidFeedbackMessage, set members form group validation state', () => { + it('with invalidFeedbackMessage, set members form group exception state', () => { createComponent({ invalidFeedbackMessage: 'invalid message!', }); diff --git a/spec/frontend/invite_members/components/members_token_select_spec.js b/spec/frontend/invite_members/components/members_token_select_spec.js index bf5564e4d63..6375d0f7e2e 100644 --- a/spec/frontend/invite_members/components/members_token_select_spec.js +++ b/spec/frontend/invite_members/components/members_token_select_spec.js @@ -16,6 +16,7 @@ const createComponent = (props) => { return shallowMount(MembersTokenSelect, { propsData: { ariaLabelledby: label, + invalidMembers: {}, placeholder, ...props, }, @@ -124,12 +125,14 @@ describe('MembersTokenSelect', () => { findTokenSelector().vm.$emit('token-remove', [user1]); expect(wrapper.emitted('clear')).toEqual([[]]); + expect(wrapper.emitted('token-remove')).toBeUndefined(); }); - it('does not emit `clear` event when there are still tokens selected', () => { + it('emits `token-remove` event with the token when there are still tokens selected', () => { findTokenSelector().vm.$emit('input', [user1, user2]); findTokenSelector().vm.$emit('token-remove', [user1]); + expect(wrapper.emitted('token-remove')).toEqual([[[user1]]]); expect(wrapper.emitted('clear')).toBeUndefined(); }); }); diff --git a/spec/frontend/invite_members/mock_data/member_modal.js b/spec/frontend/invite_members/mock_data/member_modal.js index 474234cfacb..7d675b6206c 100644 --- a/spec/frontend/invite_members/mock_data/member_modal.js +++ b/spec/frontend/invite_members/mock_data/member_modal.js @@ -26,13 +26,17 @@ export const user2 = { id: 2, name: 'Name Two', username: 'one_2', avatar_url: ' export const user3 = { id: 'user-defined-token', name: 'email@example.com', - username: 'one_2', avatar_url: '', }; export const user4 = { - id: 'user-defined-token', + id: 'user-defined-token2', name: 'email4@example.com', - username: 'one_4', + avatar_url: '', +}; +export const user5 = { + id: '3', + username: 'root', + name: 'root', avatar_url: '', }; diff --git a/spec/frontend/invite_members/utils/member_utils_spec.js b/spec/frontend/invite_members/utils/member_utils_spec.js new file mode 100644 index 00000000000..eb76c9845d4 --- /dev/null +++ b/spec/frontend/invite_members/utils/member_utils_spec.js @@ -0,0 +1,12 @@ +import { memberName } from '~/invite_members/utils/member_utils'; + +describe('Member Name', () => { + it.each([ + [{ username: '_username_', name: '_name_' }, '_username_'], + [{ username: '_username_' }, '_username_'], + [{ name: '_name_' }, '_name_'], + [{}, undefined], + ])(`returns name from supplied member token: %j`, (member, result) => { + expect(memberName(member)).toBe(result); + }); +}); diff --git a/spec/frontend/invite_members/utils/response_message_parser_spec.js b/spec/frontend/invite_members/utils/response_message_parser_spec.js index 8b2064df374..92f38c54c99 100644 --- a/spec/frontend/invite_members/utils/response_message_parser_spec.js +++ b/spec/frontend/invite_members/utils/response_message_parser_spec.js @@ -1,5 +1,5 @@ import { - responseMessageFromSuccess, + responseFromSuccess, responseMessageFromError, } from '~/invite_members/utils/response_message_parser'; import { invitationsApiResponse } from '../mock_data/api_responses'; @@ -11,12 +11,12 @@ describe('Response message parser', () => { const exampleKeyedMsg = { 'email@example.com': expectedMessage }; it.each([ - [{ data: { message: expectedMessage } }], - [{ data: { error: expectedMessage } }], - [{ data: { message: [expectedMessage] } }], - [{ data: { message: exampleKeyedMsg } }], - ])(`returns "${expectedMessage}" from success response: %j`, (successResponse) => { - expect(responseMessageFromSuccess(successResponse)).toBe(expectedMessage); + [{ data: { message: expectedMessage } }, { error: true, message: expectedMessage }], + [{ data: { error: expectedMessage } }, { error: true, message: expectedMessage }], + [{ data: { message: [expectedMessage] } }, { error: true, message: expectedMessage }], + [{ data: { message: exampleKeyedMsg } }, { error: true, message: { ...exampleKeyedMsg } }], + ])(`returns "${expectedMessage}" from success response: %j`, (successResponse, result) => { + expect(responseFromSuccess(successResponse)).toStrictEqual(result); }); }); @@ -30,15 +30,18 @@ describe('Response message parser', () => { }); }); - describe('displaying only the first error when a response has messages for multiple users', () => { - const expected = - "The member's email address is not allowed for this project. Go to the Admin area > Sign-up restrictions, and check Allowed domains for sign-ups."; - + describe('displaying all errors when a response has messages for multiple users', () => { it.each([ - [{ data: invitationsApiResponse.MULTIPLE_RESTRICTED }], - [{ data: invitationsApiResponse.EMAIL_RESTRICTED }], - ])(`returns "${expectedMessage}" from success response: %j`, (restrictedResponse) => { - expect(responseMessageFromSuccess(restrictedResponse)).toBe(expected); + [ + { data: invitationsApiResponse.MULTIPLE_RESTRICTED }, + { error: true, message: { ...invitationsApiResponse.MULTIPLE_RESTRICTED.message } }, + ], + [ + { data: invitationsApiResponse.EMAIL_RESTRICTED }, + { error: true, message: { ...invitationsApiResponse.EMAIL_RESTRICTED.message } }, + ], + ])(`returns "${expectedMessage}" from success response: %j`, (restrictedResponse, result) => { + expect(responseFromSuccess(restrictedResponse)).toStrictEqual(result); }); }); }); diff --git a/spec/frontend/milestones/components/delete_milestone_modal_spec.js b/spec/frontend/milestones/components/delete_milestone_modal_spec.js index b9ba0833c4f..6692a3b9347 100644 --- a/spec/frontend/milestones/components/delete_milestone_modal_spec.js +++ b/spec/frontend/milestones/components/delete_milestone_modal_spec.js @@ -1,44 +1,59 @@ -import Vue from 'vue'; +import { GlSprintf, GlModal } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; import { TEST_HOST } from 'helpers/test_constants'; -import mountComponent from 'helpers/vue_mount_component_helper'; import axios from '~/lib/utils/axios_utils'; -import { redirectTo } from '~/lib/utils/url_utility'; -import deleteMilestoneModal from '~/milestones/components/delete_milestone_modal.vue'; +import DeleteMilestoneModal from '~/milestones/components/delete_milestone_modal.vue'; import eventHub from '~/milestones/event_hub'; +import { redirectTo } from '~/lib/utils/url_utility'; +import { createAlert } from '~/flash'; -jest.mock('~/lib/utils/url_utility', () => ({ - ...jest.requireActual('~/lib/utils/url_utility'), - redirectTo: jest.fn(), -})); +jest.mock('~/lib/utils/url_utility'); +jest.mock('~/flash'); -describe('delete_milestone_modal.vue', () => { - const Component = Vue.extend(deleteMilestoneModal); - const props = { +describe('Delete milestone modal', () => { + let wrapper; + const mockProps = { issueCount: 1, mergeRequestCount: 2, milestoneId: 3, milestoneTitle: 'my milestone title', milestoneUrl: `${TEST_HOST}/delete_milestone_modal.vue/milestone`, }; - let vm; + + const findModal = () => wrapper.findComponent(GlModal); + + const createComponent = (props) => { + wrapper = shallowMount(DeleteMilestoneModal, { + propsData: { + ...mockProps, + ...props, + }, + stubs: { + GlSprintf, + }, + }); + }; + + beforeEach(() => { + createComponent(); + }); afterEach(() => { - vm.$destroy(); + wrapper.destroy(); }); describe('onSubmit', () => { beforeEach(() => { - vm = mountComponent(Component, props); jest.spyOn(eventHub, '$emit').mockImplementation(() => {}); }); it('deletes milestone and redirects to overview page', async () => { const responseURL = `${TEST_HOST}/delete_milestone_modal.vue/milestoneOverview`; jest.spyOn(axios, 'delete').mockImplementation((url) => { - expect(url).toBe(props.milestoneUrl); + expect(url).toBe(mockProps.milestoneUrl); expect(eventHub.$emit).toHaveBeenCalledWith( 'deleteMilestoneModal.requestStarted', - props.milestoneUrl, + mockProps.milestoneUrl, ); eventHub.$emit.mockReset(); return Promise.resolve({ @@ -47,55 +62,71 @@ describe('delete_milestone_modal.vue', () => { }, }); }); - - await vm.onSubmit(); + await findModal().vm.$emit('primary'); expect(redirectTo).toHaveBeenCalledWith(responseURL); expect(eventHub.$emit).toHaveBeenCalledWith('deleteMilestoneModal.requestFinished', { - milestoneUrl: props.milestoneUrl, + milestoneUrl: mockProps.milestoneUrl, successful: true, }); }); - it('displays error if deleting milestone failed', async () => { - const dummyError = new Error('deleting milestone failed'); - dummyError.response = { status: 418 }; - jest.spyOn(axios, 'delete').mockImplementation((url) => { - expect(url).toBe(props.milestoneUrl); - expect(eventHub.$emit).toHaveBeenCalledWith( - 'deleteMilestoneModal.requestStarted', - props.milestoneUrl, - ); - eventHub.$emit.mockReset(); - return Promise.reject(dummyError); - }); + it.each` + statusCode | alertMessage + ${418} | ${`Failed to delete milestone ${mockProps.milestoneTitle}`} + ${404} | ${`Milestone ${mockProps.milestoneTitle} was not found`} + `( + 'displays error if deleting milestone failed with code $statusCode', + async ({ statusCode, alertMessage }) => { + const dummyError = new Error('deleting milestone failed'); + dummyError.response = { status: statusCode }; + jest.spyOn(axios, 'delete').mockImplementation((url) => { + expect(url).toBe(mockProps.milestoneUrl); + expect(eventHub.$emit).toHaveBeenCalledWith( + 'deleteMilestoneModal.requestStarted', + mockProps.milestoneUrl, + ); + eventHub.$emit.mockReset(); + return Promise.reject(dummyError); + }); - await expect(vm.onSubmit()).rejects.toEqual(dummyError); - expect(redirectTo).not.toHaveBeenCalled(); - expect(eventHub.$emit).toHaveBeenCalledWith('deleteMilestoneModal.requestFinished', { - milestoneUrl: props.milestoneUrl, - successful: false, - }); - }); + await expect(wrapper.vm.onSubmit()).rejects.toEqual(dummyError); + expect(createAlert).toHaveBeenCalledWith({ + message: alertMessage, + }); + expect(redirectTo).not.toHaveBeenCalled(); + expect(eventHub.$emit).toHaveBeenCalledWith('deleteMilestoneModal.requestFinished', { + milestoneUrl: mockProps.milestoneUrl, + successful: false, + }); + }, + ); }); - describe('text', () => { - it('contains the issue and milestone count', () => { - vm = mountComponent(Component, props); - const value = vm.text; + describe('Modal title and description', () => { + const emptyDescription = `You’re about to permanently delete the milestone ${mockProps.milestoneTitle}. This milestone is not currently used in any issues or merge requests.`; + const description = `You’re about to permanently delete the milestone ${mockProps.milestoneTitle} and remove it from 1 issue and 2 merge requests. Once deleted, it cannot be undone or recovered.`; + const title = `Delete milestone ${mockProps.milestoneTitle}?`; - expect(value).toContain('remove it from 1 issue and 2 merge requests'); + it('renders proper title', () => { + const value = findModal().props('title'); + expect(value).toBe(title); }); - it('contains neither issue nor milestone count', () => { - vm = mountComponent(Component, { - ...props, - issueCount: 0, - mergeRequestCount: 0, - }); - - const value = vm.text; + it.each` + statement | descriptionText | issueCount | mergeRequestCount + ${'1 issue and 2 merge requests'} | ${description} | ${1} | ${2} + ${'no issues and merge requests'} | ${emptyDescription} | ${0} | ${0} + `( + 'renders proper description when the milestone contains $statement', + ({ issueCount, mergeRequestCount, descriptionText }) => { + createComponent({ + issueCount, + mergeRequestCount, + }); - expect(value).toContain('is not currently used'); - }); + const value = findModal().text(); + expect(value).toBe(descriptionText); + }, + ); }); }); diff --git a/spec/frontend/work_items/components/work_item_detail_modal_spec.js b/spec/frontend/work_items/components/work_item_detail_modal_spec.js index d55ba318e46..70b1261bdb7 100644 --- a/spec/frontend/work_items/components/work_item_detail_modal_spec.js +++ b/spec/frontend/work_items/components/work_item_detail_modal_spec.js @@ -66,6 +66,7 @@ describe('WorkItemDetailModal component', () => { createComponent(); expect(findWorkItemDetail().props()).toEqual({ + isModal: true, workItemId: '1', workItemParentId: '2', }); @@ -98,6 +99,15 @@ describe('WorkItemDetailModal component', () => { expect(wrapper.emitted('close')).toBeTruthy(); }); + it('hides the modal when WorkItemDetail emits `close` event', () => { + createComponent(); + const closeSpy = jest.spyOn(wrapper.vm.$refs.modal, 'hide'); + + findWorkItemDetail().vm.$emit('close'); + + expect(closeSpy).toHaveBeenCalled(); + }); + describe('delete work item', () => { it('emits workItemDeleted and closes modal', async () => { createComponent(); diff --git a/spec/frontend/work_items/pages/work_item_detail_spec.js b/spec/frontend/work_items/pages/work_item_detail_spec.js index 95a894f0fac..6fb7bb5226e 100644 --- a/spec/frontend/work_items/pages/work_item_detail_spec.js +++ b/spec/frontend/work_items/pages/work_item_detail_spec.js @@ -1,4 +1,4 @@ -import { GlAlert, GlSkeletonLoader } from '@gitlab/ui'; +import { GlAlert, GlButton, GlSkeletonLoader } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; import Vue from 'vue'; import VueApollo from 'vue-apollo'; @@ -26,6 +26,7 @@ describe('WorkItemDetail component', () => { const initialSubscriptionHandler = jest.fn().mockResolvedValue(workItemTitleSubscriptionResponse); const findAlert = () => wrapper.findComponent(GlAlert); + const findCloseButton = () => wrapper.findComponent(GlButton); const findSkeleton = () => wrapper.findComponent(GlSkeletonLoader); const findWorkItemTitle = () => wrapper.findComponent(WorkItemTitle); const findWorkItemState = () => wrapper.findComponent(WorkItemState); @@ -34,6 +35,7 @@ describe('WorkItemDetail component', () => { const findWorkItemWeight = () => wrapper.findComponent(WorkItemWeight); const createComponent = ({ + isModal = false, workItemId = workItemQueryResponse.data.workItem.id, handler = successHandler, subscriptionHandler = initialSubscriptionHandler, @@ -51,7 +53,7 @@ describe('WorkItemDetail component', () => { typePolicies: includeWidgets ? temporaryConfig.cacheConfig.typePolicies : {}, }, ), - propsData: { workItemId }, + propsData: { isModal, workItemId }, provide: { glFeatures: { workItemsMvc2: workItemsMvc2Enabled, @@ -99,6 +101,36 @@ describe('WorkItemDetail component', () => { }); }); + describe('close button', () => { + describe('when isModal prop is false', () => { + it('does not render', async () => { + createComponent({ isModal: false }); + await waitForPromises(); + + expect(findCloseButton().exists()).toBe(false); + }); + }); + + describe('when isModal prop is true', () => { + it('renders', async () => { + createComponent({ isModal: true }); + await waitForPromises(); + + expect(findCloseButton().props('icon')).toBe('close'); + expect(findCloseButton().attributes('aria-label')).toBe('Close'); + }); + + it('emits `close` event when clicked', async () => { + createComponent({ isModal: true }); + await waitForPromises(); + + findCloseButton().vm.$emit('click'); + + expect(wrapper.emitted('close')).toEqual([[]]); + }); + }); + }); + describe('description', () => { it('does not show description widget if loading description fails', () => { createComponent(); diff --git a/spec/frontend/work_items/pages/work_item_root_spec.js b/spec/frontend/work_items/pages/work_item_root_spec.js index 3c5da94114e..d9372f2bcf0 100644 --- a/spec/frontend/work_items/pages/work_item_root_spec.js +++ b/spec/frontend/work_items/pages/work_item_root_spec.js @@ -52,6 +52,7 @@ describe('Work items root component', () => { createComponent(); expect(findWorkItemDetail().props()).toEqual({ + isModal: false, workItemId: 'gid://gitlab/WorkItem/1', workItemParentId: null, }); diff --git a/spec/lib/gitlab/gpg/commit_spec.rb b/spec/lib/gitlab/gpg/commit_spec.rb index 9c399e78d80..919335bc9fa 100644 --- a/spec/lib/gitlab/gpg/commit_spec.rb +++ b/spec/lib/gitlab/gpg/commit_spec.rb @@ -3,6 +3,34 @@ require 'spec_helper' RSpec.describe Gitlab::Gpg::Commit do + let_it_be(:project) { create(:project, :repository, path: 'sample-project') } + + let(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' } + let(:committer_email) { GpgHelpers::User1.emails.first } + let(:user_email) { committer_email } + let(:public_key) { GpgHelpers::User1.public_key } + let(:user) { create(:user, email: user_email) } + let(:commit) { create(:commit, project: project, sha: commit_sha, committer_email: committer_email) } + let(:crypto) { instance_double(GPGME::Crypto) } + let(:mock_signature_data?) { true } + # gpg_keys must be pre-loaded so that they can be found during signature verification. + let!(:gpg_key) { create(:gpg_key, key: public_key, user: user) } + + let(:signature_data) do + [ + GpgHelpers::User1.signed_commit_signature, + GpgHelpers::User1.signed_commit_base_data + ] + end + + before do + if mock_signature_data? + allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily) + .with(Gitlab::Git::Repository, commit_sha) + .and_return(signature_data) + end + end + describe '#signature' do shared_examples 'returns the cached signature on second call' do it 'returns the cached signature on second call' do @@ -17,11 +45,8 @@ RSpec.describe Gitlab::Gpg::Commit do end end - let!(:project) { create :project, :repository, path: 'sample-project' } - let!(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' } - context 'unsigned commit' do - let!(:commit) { create :commit, project: project, sha: commit_sha } + let(:signature_data) { nil } it 'returns nil' do expect(described_class.new(commit).signature).to be_nil @@ -29,20 +54,12 @@ RSpec.describe Gitlab::Gpg::Commit do end context 'invalid signature' do - let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User1.emails.first } - - let!(:user) { create(:user, email: GpgHelpers::User1.emails.first) } - - before do - allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily) - .with(Gitlab::Git::Repository, commit_sha) - .and_return( - [ - # Corrupt the key - GpgHelpers::User1.signed_commit_signature.tr('=', 'a'), - GpgHelpers::User1.signed_commit_base_data - ] - ) + let(:signature_data) do + [ + # Corrupt the key + GpgHelpers::User1.signed_commit_signature.tr('=', 'a'), + GpgHelpers::User1.signed_commit_base_data + ] end it 'returns nil' do @@ -53,25 +70,6 @@ RSpec.describe Gitlab::Gpg::Commit do context 'known key' do context 'user matches the key uid' do context 'user email matches the email committer' do - let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User1.emails.first } - - let!(:user) { create(:user, email: GpgHelpers::User1.emails.first) } - - let!(:gpg_key) do - create :gpg_key, key: GpgHelpers::User1.public_key, user: user - end - - before do - allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily) - .with(Gitlab::Git::Repository, commit_sha) - .and_return( - [ - GpgHelpers::User1.signed_commit_signature, - GpgHelpers::User1.signed_commit_base_data - ] - ) - end - it 'returns a valid signature' do signature = described_class.new(commit).signature @@ -112,32 +110,13 @@ RSpec.describe Gitlab::Gpg::Commit do end context 'valid key signed using recent version of Gnupg' do - let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User1.emails.first } - - let!(:user) { create(:user, email: GpgHelpers::User1.emails.first) } - - let!(:gpg_key) do - create :gpg_key, key: GpgHelpers::User1.public_key, user: user - end - - let!(:crypto) { instance_double(GPGME::Crypto) } - before do - fake_signature = [ - GpgHelpers::User1.signed_commit_signature, - GpgHelpers::User1.signed_commit_base_data - ] - - allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily) - .with(Gitlab::Git::Repository, commit_sha) - .and_return(fake_signature) - end - - it 'returns a valid signature' do verified_signature = double('verified-signature', fingerprint: GpgHelpers::User1.fingerprint, valid?: true) allow(GPGME::Crypto).to receive(:new).and_return(crypto) allow(crypto).to receive(:verify).and_yield(verified_signature) + end + it 'returns a valid signature' do signature = described_class.new(commit).signature expect(signature).to have_attributes( @@ -153,33 +132,14 @@ RSpec.describe Gitlab::Gpg::Commit do end context 'valid key signed using older version of Gnupg' do - let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User1.emails.first } - - let!(:user) { create(:user, email: GpgHelpers::User1.emails.first) } - - let!(:gpg_key) do - create :gpg_key, key: GpgHelpers::User1.public_key, user: user - end - - let!(:crypto) { instance_double(GPGME::Crypto) } - before do - fake_signature = [ - GpgHelpers::User1.signed_commit_signature, - GpgHelpers::User1.signed_commit_base_data - ] - - allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily) - .with(Gitlab::Git::Repository, commit_sha) - .and_return(fake_signature) - end - - it 'returns a valid signature' do keyid = GpgHelpers::User1.fingerprint.last(16) verified_signature = double('verified-signature', fingerprint: keyid, valid?: true) allow(GPGME::Crypto).to receive(:new).and_return(crypto) allow(crypto).to receive(:verify).and_yield(verified_signature) + end + it 'returns a valid signature' do signature = described_class.new(commit).signature expect(signature).to have_attributes( @@ -195,32 +155,13 @@ RSpec.describe Gitlab::Gpg::Commit do end context 'commit with multiple signatures' do - let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User1.emails.first } - - let!(:user) { create(:user, email: GpgHelpers::User1.emails.first) } - - let!(:gpg_key) do - create :gpg_key, key: GpgHelpers::User1.public_key, user: user - end - - let!(:crypto) { instance_double(GPGME::Crypto) } - before do - fake_signature = [ - GpgHelpers::User1.signed_commit_signature, - GpgHelpers::User1.signed_commit_base_data - ] - - allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily) - .with(Gitlab::Git::Repository, commit_sha) - .and_return(fake_signature) - end - - it 'returns an invalid signatures error' do verified_signature = double('verified-signature', fingerprint: GpgHelpers::User1.fingerprint, valid?: true) allow(GPGME::Crypto).to receive(:new).and_return(crypto) allow(crypto).to receive(:verify).and_yield(verified_signature).and_yield(verified_signature) + end + it 'returns an invalid signatures error' do signature = described_class.new(commit).signature expect(signature).to have_attributes( @@ -236,27 +177,18 @@ RSpec.describe Gitlab::Gpg::Commit do end context 'commit signed with a subkey' do - let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User3.emails.first } - - let!(:user) { create(:user, email: GpgHelpers::User3.emails.first) } - - let!(:gpg_key) do - create :gpg_key, key: GpgHelpers::User3.public_key, user: user - end + let(:committer_email) { GpgHelpers::User3.emails.first } + let(:public_key) { GpgHelpers::User3.public_key } let(:gpg_key_subkey) do gpg_key.subkeys.find_by(fingerprint: GpgHelpers::User3.subkey_fingerprints.last) end - before do - allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily) - .with(Gitlab::Git::Repository, commit_sha) - .and_return( - [ - GpgHelpers::User3.signed_commit_signature, - GpgHelpers::User3.signed_commit_base_data - ] - ) + let(:signature_data) do + [ + GpgHelpers::User3.signed_commit_signature, + GpgHelpers::User3.signed_commit_base_data + ] end it 'returns a valid signature' do @@ -275,7 +207,7 @@ RSpec.describe Gitlab::Gpg::Commit do end context 'user email does not match the committer email, but is the same user' do - let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User2.emails.first } + let(:committer_email) { GpgHelpers::User2.emails.first } let(:user) do create(:user, email: GpgHelpers::User1.emails.first).tap do |user| @@ -283,21 +215,6 @@ RSpec.describe Gitlab::Gpg::Commit do end end - let!(:gpg_key) do - create :gpg_key, key: GpgHelpers::User1.public_key, user: user - end - - before do - allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily) - .with(Gitlab::Git::Repository, commit_sha) - .and_return( - [ - GpgHelpers::User1.signed_commit_signature, - GpgHelpers::User1.signed_commit_base_data - ] - ) - end - it 'returns an invalid signature' do expect(described_class.new(commit).signature).to have_attributes( commit_sha: commit_sha, @@ -314,24 +231,8 @@ RSpec.describe Gitlab::Gpg::Commit do end context 'user email does not match the committer email' do - let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: GpgHelpers::User2.emails.first } - - let(:user) { create(:user, email: GpgHelpers::User1.emails.first) } - - let!(:gpg_key) do - create :gpg_key, key: GpgHelpers::User1.public_key, user: user - end - - before do - allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily) - .with(Gitlab::Git::Repository, commit_sha) - .and_return( - [ - GpgHelpers::User1.signed_commit_signature, - GpgHelpers::User1.signed_commit_base_data - ] - ) - end + let(:committer_email) { GpgHelpers::User2.emails.first } + let(:user_email) { GpgHelpers::User1.emails.first } it 'returns an invalid signature' do expect(described_class.new(commit).signature).to have_attributes( @@ -350,24 +251,8 @@ RSpec.describe Gitlab::Gpg::Commit do end context 'user does not match the key uid' do - let!(:commit) { create :commit, project: project, sha: commit_sha } - - let(:user) { create(:user, email: GpgHelpers::User2.emails.first) } - - let!(:gpg_key) do - create :gpg_key, key: GpgHelpers::User1.public_key, user: user - end - - before do - allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily) - .with(Gitlab::Git::Repository, commit_sha) - .and_return( - [ - GpgHelpers::User1.signed_commit_signature, - GpgHelpers::User1.signed_commit_base_data - ] - ) - end + let(:user_email) { GpgHelpers::User2.emails.first } + let(:public_key) { GpgHelpers::User1.public_key } it 'returns an invalid signature' do expect(described_class.new(commit).signature).to have_attributes( @@ -386,18 +271,7 @@ RSpec.describe Gitlab::Gpg::Commit do end context 'unknown key' do - let!(:commit) { create :commit, project: project, sha: commit_sha } - - before do - allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily) - .with(Gitlab::Git::Repository, commit_sha) - .and_return( - [ - GpgHelpers::User1.signed_commit_signature, - GpgHelpers::User1.signed_commit_base_data - ] - ) - end + let(:gpg_key) { nil } it 'returns an invalid signature' do expect(described_class.new(commit).signature).to have_attributes( @@ -415,15 +289,15 @@ RSpec.describe Gitlab::Gpg::Commit do end context 'multiple commits with signatures' do - let(:first_signature) { create(:gpg_signature) } - - let(:gpg_key) { create(:gpg_key, key: GpgHelpers::User2.public_key) } - let(:second_signature) { create(:gpg_signature, gpg_key: gpg_key) } + let(:mock_signature_data?) { false } + let!(:first_signature) { create(:gpg_signature) } + let!(:gpg_key) { create(:gpg_key, key: GpgHelpers::User2.public_key) } + let!(:second_signature) { create(:gpg_signature, gpg_key: gpg_key) } let!(:first_commit) { create(:commit, project: project, sha: first_signature.commit_sha) } let!(:second_commit) { create(:commit, project: project, sha: second_signature.commit_sha) } - let(:commits) do + let!(:commits) do [first_commit, second_commit].map do |commit| gpg_commit = described_class.new(commit) @@ -442,4 +316,21 @@ RSpec.describe Gitlab::Gpg::Commit do end end end + + describe '#update_signature!' do + let!(:gpg_key) { nil } + + let(:signature) { described_class.new(commit).signature } + + it 'updates signature record' do + signature + + create(:gpg_key, key: public_key, user: user) + + stored_signature = CommitSignatures::GpgSignature.find_by_commit_sha(commit_sha) + expect { described_class.new(commit).update_signature!(stored_signature) }.to( + change { signature.reload.verification_status }.from('unknown_key').to('verified') + ) + end + end end diff --git a/spec/lib/gitlab/x509/commit_spec.rb b/spec/lib/gitlab/x509/commit_spec.rb index a81955b995e..c7d56e49fab 100644 --- a/spec/lib/gitlab/x509/commit_spec.rb +++ b/spec/lib/gitlab/x509/commit_spec.rb @@ -2,14 +2,21 @@ require 'spec_helper' RSpec.describe Gitlab::X509::Commit do - describe '#signature' do - let(:signature) { described_class.new(commit).signature } + let(:commit_sha) { '189a6c924013fc3fe40d6f1ec1dc20214183bc97' } + let(:user) { create(:user, email: X509Helpers::User1.certificate_email) } + let(:project) { create(:project, :repository, path: X509Helpers::User1.path, creator: user) } + let(:commit) { project.commit_by(oid: commit_sha ) } + let(:signature) { Gitlab::X509::Commit.new(commit).signature } + let(:store) { OpenSSL::X509::Store.new } + let(:certificate) { OpenSSL::X509::Certificate.new(X509Helpers::User1.trust_cert) } - context 'returns the cached signature' do - let(:commit_sha) { '189a6c924013fc3fe40d6f1ec1dc20214183bc97' } - let(:project) { create(:project, :public, :repository) } - let(:commit) { create(:commit, project: project, sha: commit_sha) } + before do + store.add_cert(certificate) if certificate + allow(OpenSSL::X509::Store).to receive(:new).and_return(store) + end + describe '#signature' do + context 'returns the cached signature' do it 'on second call' do allow_any_instance_of(described_class).to receive(:new).and_call_original expect_any_instance_of(described_class).to receive(:create_cached_signature!).and_call_original @@ -23,13 +30,29 @@ RSpec.describe Gitlab::X509::Commit do end context 'unsigned commit' do - let!(:project) { create :project, :repository, path: X509Helpers::User1.path } - let!(:commit_sha) { X509Helpers::User1.commit } - let!(:commit) { create :commit, project: project, sha: commit_sha } + let(:project) { create :project, :repository, path: X509Helpers::User1.path } + let(:commit_sha) { X509Helpers::User1.commit } + let(:commit) { create :commit, project: project, sha: commit_sha } it 'returns nil' do expect(signature).to be_nil end end end + + describe '#update_signature!' do + let(:certificate) { nil } + + it 'updates verification status' do + signature + + cert = OpenSSL::X509::Certificate.new(X509Helpers::User1.trust_cert) + store.add_cert(cert) + + stored_signature = CommitSignatures::X509CommitSignature.find_by_commit_sha(commit_sha) + expect { described_class.new(commit).update_signature!(stored_signature) }.to( + change { signature.reload.verification_status }.from('unverified').to('verified') + ) + end + end end diff --git a/spec/support/helpers/features/invite_members_modal_helper.rb b/spec/support/helpers/features/invite_members_modal_helper.rb index 7ed64615020..b56ac5b32c6 100644 --- a/spec/support/helpers/features/invite_members_modal_helper.rb +++ b/spec/support/helpers/features/invite_members_modal_helper.rb @@ -9,18 +9,28 @@ module Spec click_on 'Invite members' page.within invite_modal_selector do - Array.wrap(names).each do |name| - find(member_dropdown_selector).set(name) + select_members(names) + choose_options(role, expires_at) + click_button 'Invite' + end - wait_for_requests - click_button name - end + page.refresh if refresh + end - choose_options(role, expires_at) + def input_invites(names) + click_on 'Invite members' - click_button 'Invite' + page.within invite_modal_selector do + select_members(names) + end + end + + def select_members(names) + Array.wrap(names).each do |name| + find(member_dropdown_selector).set(name) - page.refresh if refresh + wait_for_requests + click_button name end end @@ -64,6 +74,24 @@ module Spec '[data-testid="invite-modal"]' end + def member_token_error_selector(id) + "[data-testid='error-icon-#{id}']" + end + + def member_token_avatar_selector + "[data-testid='token-avatar']" + end + + def member_token_selector(id) + "[data-token-id='#{id}']" + end + + def remove_token(id) + page.within member_token_selector(id) do + find('[data-testid="close-icon"]').click + end + end + def expect_to_have_group(group) expect(page).to have_selector("[entity-id='#{group.id}']") end diff --git a/spec/support/helpers/features/source_editor_spec_helpers.rb b/spec/support/helpers/features/source_editor_spec_helpers.rb index 57057b47fbb..cdc59f9cbe1 100644 --- a/spec/support/helpers/features/source_editor_spec_helpers.rb +++ b/spec/support/helpers/features/source_editor_spec_helpers.rb @@ -15,13 +15,6 @@ module Spec execute_script("monaco.editor.getModel('#{uri}').setValue('#{escape_javascript(value)}')") end - - def editor_get_value - editor = find('.monaco-editor') - uri = editor['data-uri'] - - evaluate_script("monaco.editor.getModel('#{uri}').getValue()") - end end end end diff --git a/spec/support/helpers/features/web_ide_spec_helpers.rb b/spec/support/helpers/features/web_ide_spec_helpers.rb index d5d9fed0f79..70dedc3ac50 100644 --- a/spec/support/helpers/features/web_ide_spec_helpers.rb +++ b/spec/support/helpers/features/web_ide_spec_helpers.rb @@ -100,10 +100,6 @@ module WebIdeSpecHelpers editor_set_value(value) end - def ide_editor_value - editor_get_value - end - def ide_commit_tab_selector ide_tab_selector('commit') end diff --git a/spec/support/shared_examples/features/inviting_members_shared_examples.rb b/spec/support/shared_examples/features/inviting_members_shared_examples.rb index 5d643658ab4..bca0e02fcdd 100644 --- a/spec/support/shared_examples/features/inviting_members_shared_examples.rb +++ b/spec/support/shared_examples/features/inviting_members_shared_examples.rb @@ -23,6 +23,22 @@ RSpec.shared_examples 'inviting members' do |snowplow_invite_label| ) end + it 'displays the user\'s avatar in the member input token', :js do + visit members_page_path + + input_invites(user2.name) + + expect(page).to have_selector(member_token_avatar_selector) + end + + it 'does not display an avatar in the member input token for an email address', :js do + visit members_page_path + + input_invites('test@example.com') + + expect(page).not_to have_selector(member_token_avatar_selector) + end + it 'invites user by email', :js, :snowplow, :aggregate_failures do visit members_page_path @@ -79,11 +95,13 @@ RSpec.shared_examples 'inviting members' do |snowplow_invite_label| context 'when member is already a member by email' do it 'updates the member for that email', :js do + email = 'test@example.com' + visit members_page_path - invite_member('test@example.com', role: 'Developer') + invite_member(email, role: 'Developer') - invite_member('test@example.com', role: 'Reporter', refresh: false) + invite_member(email, role: 'Reporter', refresh: false) expect(page).not_to have_selector(invite_modal_selector) @@ -91,7 +109,7 @@ RSpec.shared_examples 'inviting members' do |snowplow_invite_label| click_link 'Invited' - page.within find_invited_member_row('test@example.com') do + page.within find_invited_member_row(email) do expect(page).to have_button('Reporter') end end @@ -130,8 +148,8 @@ RSpec.shared_examples 'inviting members' do |snowplow_invite_label| invite_member(user2.name, role: role, refresh: false) expect(page).to have_selector(invite_modal_selector) - expect(page).to have_content "Access level should be greater than or equal to Developer inherited membership " \ - "from group #{group.name}" + expect(page).to have_content "#{user2.name}: Access level should be greater than or equal to Developer " \ + "inherited membership from group #{group.name}" page.refresh @@ -148,13 +166,31 @@ RSpec.shared_examples 'inviting members' do |snowplow_invite_label| group.add_maintainer(user3) end - it 'only shows the first user error', :js do + it 'shows the user errors and then removes them from the form', :js do visit subentity_members_page_path invite_member([user2.name, user3.name], role: role, refresh: false) expect(page).to have_selector(invite_modal_selector) - expect(page).to have_text("Access level should be greater than or equal to", count: 1) + expect(page).to have_selector(member_token_error_selector(user2.id)) + expect(page).to have_selector(member_token_error_selector(user3.id)) + expect(page).to have_text("The following 2 members couldn't be invited") + expect(page).to have_text("#{user2.name}: Access level should be greater than or equal to") + expect(page).to have_text("#{user3.name}: Access level should be greater than or equal to") + + remove_token(user2.id) + + expect(page).not_to have_selector(member_token_error_selector(user2.id)) + expect(page).to have_selector(member_token_error_selector(user3.id)) + expect(page).to have_text("The following member couldn't be invited") + expect(page).not_to have_text("#{user2.name}: Access level should be greater than or equal to") + + remove_token(user3.id) + + expect(page).not_to have_selector(member_token_error_selector(user3.id)) + expect(page).not_to have_text("The following member couldn't be invited") + expect(page).not_to have_text("Review the invite errors and try again") + expect(page).not_to have_text("#{user3.name}: Access level should be greater than or equal to") page.refresh @@ -168,6 +204,19 @@ RSpec.shared_examples 'inviting members' do |snowplow_invite_label| expect(page).not_to have_button('Maintainer') end end + + it 'only shows the error for an invalid formatted email and does not display other member errors', :js do + visit subentity_members_page_path + + invite_member([user2.name, user3.name, 'bad@email'], role: role, refresh: false) + + expect(page).to have_selector(invite_modal_selector) + expect(page).to have_text('email contains an invalid email address') + expect(page).not_to have_text("The following 2 members couldn't be invited") + expect(page).not_to have_text("Review the invite errors and try again") + expect(page).not_to have_text("#{user2.name}: Access level should be greater than or equal to") + expect(page).not_to have_text("#{user3.name}: Access level should be greater than or equal to") + end end end end |