diff --git a/app/assets/javascripts/sidebar/components/assignees/assignee_avatar.vue b/app/assets/javascripts/sidebar/components/assignees/assignee_avatar.vue new file mode 100644 index 0000000000000000000000000000000000000000..0bb4f757de8d634435d7e02795e22b4200bfd512 --- /dev/null +++ b/app/assets/javascripts/sidebar/components/assignees/assignee_avatar.vue @@ -0,0 +1,48 @@ +<script> +import { __, sprintf } from '~/locale'; + +export default { + props: { + user: { + type: Object, + required: true, + }, + imgSize: { + type: Number, + required: true, + }, + issuableType: { + type: String, + require: true, + default: 'issue', + }, + }, + computed: { + assigneeAlt() { + return sprintf(__("%{userName}'s avatar"), { userName: this.user.name }); + }, + avatarUrl() { + return this.user.avatar || this.user.avatar_url || gon.default_avatar_url; + }, + isMergeRequest() { + return this.issuableType === 'merge_request'; + }, + hasMergeIcon() { + return this.isMergeRequest && !this.user.can_merge; + }, + }, +}; +</script> + +<template> + <span class="position-relative"> + <img + :alt="assigneeAlt" + :src="avatarUrl" + :width="imgSize" + :class="`s${imgSize}`" + class="avatar avatar-inline m-0" + /> + <i v-if="hasMergeIcon" aria-hidden="true" class="fa fa-exclamation-triangle merge-icon"></i> + </span> +</template> diff --git a/app/assets/javascripts/sidebar/components/assignees/assignee_avatar_link.vue b/app/assets/javascripts/sidebar/components/assignees/assignee_avatar_link.vue new file mode 100644 index 0000000000000000000000000000000000000000..6563d1d341078d22cb8cf81cf4bd52c53875ae38 --- /dev/null +++ b/app/assets/javascripts/sidebar/components/assignees/assignee_avatar_link.vue @@ -0,0 +1,74 @@ +<script> +import { __, sprintf } from '~/locale'; +import { GlTooltipDirective, GlLink } from '@gitlab/ui'; +import { joinPaths } from '~/lib/utils/url_utility'; +import AssigneeAvatar from './assignee_avatar.vue'; + +export default { + components: { + AssigneeAvatar, + GlLink, + }, + directives: { + GlTooltip: GlTooltipDirective, + }, + props: { + user: { + type: Object, + required: true, + }, + rootPath: { + type: String, + required: true, + }, + tooltipPlacement: { + type: String, + default: 'bottom', + required: false, + }, + tooltipHasName: { + type: Boolean, + default: true, + required: false, + }, + issuableType: { + type: String, + default: 'issue', + required: false, + }, + }, + computed: { + cannotMerge() { + return this.issuableType === 'merge_request' && !this.user.can_merge; + }, + tooltipTitle() { + if (this.cannotMerge && this.tooltipHasName) { + return sprintf(__('%{userName} (cannot merge)'), { userName: this.user.name }); + } else if (this.cannotMerge) { + return __('Cannot merge'); + } else if (this.tooltipHasName) { + return this.user.name; + } + + return ''; + }, + tooltipOption() { + return { + container: 'body', + placement: this.tooltipPlacement, + boundary: 'viewport', + }; + }, + assigneeUrl() { + return joinPaths(`${this.rootPath}`, `${this.user.username}`); + }, + }, +}; +</script> + +<template> + <gl-link v-gl-tooltip="tooltipOption" :href="assigneeUrl" :title="tooltipTitle" class="d-flex"> + <assignee-avatar :user="user" :img-size="32" :issuable-type="issuableType" /> + <slot :user="user"></slot> + </gl-link> +</template> diff --git a/app/assets/javascripts/sidebar/components/assignees/assignees.vue b/app/assets/javascripts/sidebar/components/assignees/assignees.vue index 631e2e28d4d67e85068ac31e5d319df65d4c3fae..ca4090eee960b35c590c4926c0ca796fa836e64c 100644 --- a/app/assets/javascripts/sidebar/components/assignees/assignees.vue +++ b/app/assets/javascripts/sidebar/components/assignees/assignees.vue @@ -1,13 +1,14 @@ <script> -import { __, sprintf } from '~/locale'; -import tooltip from '~/vue_shared/directives/tooltip'; +import CollapsedAssigneeList from '../assignees/collapsed_assignee_list.vue'; +import UncollapsedAssigneeList from '../assignees/uncollapsed_assignee_list.vue'; export default { // name: 'Assignees' is a false positive: https://gitlab.com/gitlab-org/frontend/eslint-plugin-i18n/issues/26#possible-false-positives // eslint-disable-next-line @gitlab/i18n/no-non-i18n-strings name: 'Assignees', - directives: { - tooltip, + components: { + CollapsedAssigneeList, + UncollapsedAssigneeList, }, props: { rootPath: { @@ -28,167 +29,30 @@ export default { default: 'issue', }, }, - data() { - return { - defaultRenderCount: 5, - defaultMaxCounter: 99, - showLess: true, - }; - }, computed: { - firstUser() { - return this.users[0]; - }, - hasMoreThanTwoAssignees() { - return this.users.length > 2; - }, - hasMoreThanOneAssignee() { - return this.users.length > 1; - }, - hasAssignees() { - return this.users.length > 0; - }, hasNoUsers() { return !this.users.length; }, - hasOneUser() { - return this.users.length === 1; - }, - renderShowMoreSection() { - return this.users.length > this.defaultRenderCount; - }, - numberOfHiddenAssignees() { - return this.users.length - this.defaultRenderCount; - }, - isHiddenAssignees() { - return this.numberOfHiddenAssignees > 0; - }, - hiddenAssigneesLabel() { - const { numberOfHiddenAssignees } = this; - return sprintf(__('+ %{numberOfHiddenAssignees} more'), { numberOfHiddenAssignees }); - }, - collapsedTooltipTitle() { - const maxRender = Math.min(this.defaultRenderCount, this.users.length); - const renderUsers = this.users.slice(0, maxRender); - const names = renderUsers.map(u => u.name); - - if (this.users.length > maxRender) { - names.push(`+ ${this.users.length - maxRender} more`); - } - - if (!this.users.length) { - const emptyTooltipLabel = __('Assignee(s)'); - names.push(emptyTooltipLabel); - } - - return names.join(', '); - }, - sidebarAvatarCounter() { - let counter = `+${this.users.length - 1}`; - - if (this.users.length > this.defaultMaxCounter) { - counter = `${this.defaultMaxCounter}+`; - } + sortedAssigness() { + const canMergeUsers = this.users.filter(user => user.can_merge); + const canNotMergeUsers = this.users.filter(user => !user.can_merge); - return counter; - }, - mergeNotAllowedTooltipMessage() { - const assigneesCount = this.users.length; - - if (this.issuableType !== 'merge_request' || assigneesCount === 0) { - return null; - } - - const cannotMergeCount = this.users.filter(u => u.can_merge === false).length; - const canMergeCount = assigneesCount - cannotMergeCount; - - if (canMergeCount === assigneesCount) { - // Everyone can merge - return null; - } else if (cannotMergeCount === assigneesCount && assigneesCount > 1) { - return __('No one can merge'); - } else if (assigneesCount === 1) { - return __('Cannot merge'); - } - - return sprintf(__('%{canMergeCount}/%{assigneesCount} can merge'), { - canMergeCount, - assigneesCount, - }); + return [...canMergeUsers, ...canNotMergeUsers]; }, }, methods: { assignSelf() { this.$emit('assign-self'); }, - toggleShowLess() { - this.showLess = !this.showLess; - }, - renderAssignee(index) { - return !this.showLess || (index < this.defaultRenderCount && this.showLess); - }, - avatarUrl(user) { - return user.avatar || user.avatar_url || gon.default_avatar_url; - }, - assigneeUrl(user) { - return `${this.rootPath}${user.username}`; - }, - assigneeAlt(user) { - return sprintf(__("%{userName}'s avatar"), { userName: user.name }); - }, - assigneeUsername(user) { - return `@${user.username}`; - }, - shouldRenderCollapsedAssignee(index) { - const firstTwo = this.users.length <= 2 && index <= 2; - - return index === 0 || firstTwo; - }, }, }; </script> <template> <div> - <div - v-tooltip - :class="{ 'multiple-users': hasMoreThanOneAssignee }" - :title="collapsedTooltipTitle" - class="sidebar-collapsed-icon sidebar-collapsed-user" - data-container="body" - data-placement="left" - data-boundary="viewport" - > - <i v-if="hasNoUsers" :aria-label="__('None')" class="fa fa-user"> </i> - <button - v-for="(user, index) in users" - v-if="shouldRenderCollapsedAssignee(index)" - :key="user.id" - type="button" - class="btn-link" - > - <img - :alt="assigneeAlt(user)" - :src="avatarUrl(user)" - width="24" - class="avatar avatar-inline s24" - /> - <span class="author"> {{ user.name }} </span> - </button> - <button v-if="hasMoreThanTwoAssignees" class="btn-link" type="button"> - <span class="avatar-counter sidebar-avatar-counter"> {{ sidebarAvatarCounter }} </span> - </button> - </div> + <collapsed-assignee-list :users="sortedAssigness" :issuable-type="issuableType" /> + <div class="value hide-collapsed"> - <span - v-if="mergeNotAllowedTooltipMessage" - v-tooltip - :title="mergeNotAllowedTooltipMessage" - data-placement="left" - class="float-right cannot-be-merged" - > - <i aria-hidden="true" data-hidden="true" class="fa fa-exclamation-triangle"></i> - </span> <template v-if="hasNoUsers"> <span class="assign-yourself no-value qa-assign-yourself"> {{ __('None') }} @@ -200,51 +64,13 @@ export default { </template> </span> </template> - <template v-else-if="hasOneUser"> - <a :href="assigneeUrl(firstUser)" class="author-link bold"> - <img - :alt="assigneeAlt(firstUser)" - :src="avatarUrl(firstUser)" - width="32" - class="avatar avatar-inline s32" - /> - <span class="author"> {{ firstUser.name }} </span> - <span class="username"> {{ assigneeUsername(firstUser) }} </span> - </a> - </template> - <template v-else> - <div class="user-list"> - <div - v-for="(user, index) in users" - v-if="renderAssignee(index)" - :key="user.id" - class="user-item" - > - <a - :href="assigneeUrl(user)" - :data-title="user.name" - class="user-link has-tooltip" - data-container="body" - data-placement="bottom" - > - <img - :alt="assigneeAlt(user)" - :src="avatarUrl(user)" - width="32" - class="avatar avatar-inline s32" - /> - </a> - </div> - </div> - <div v-if="renderShowMoreSection" class="user-list-more"> - <button type="button" class="btn-link" @click="toggleShowLess"> - <template v-if="showLess"> - {{ hiddenAssigneesLabel }} - </template> - <template v-else>{{ __('- show less') }}</template> - </button> - </div> - </template> + + <uncollapsed-assignee-list + v-else + :users="sortedAssigness" + :root-path="rootPath" + :issuable-type="issuableType" + /> </div> </div> </template> diff --git a/app/assets/javascripts/sidebar/components/assignees/collapsed_assignee.vue b/app/assets/javascripts/sidebar/components/assignees/collapsed_assignee.vue new file mode 100644 index 0000000000000000000000000000000000000000..3f3626092a8c9ef12516062a50417ef73db87bc9 --- /dev/null +++ b/app/assets/javascripts/sidebar/components/assignees/collapsed_assignee.vue @@ -0,0 +1,27 @@ +<script> +import AssigneeAvatar from './assignee_avatar.vue'; + +export default { + components: { + AssigneeAvatar, + }, + props: { + user: { + type: Object, + required: true, + }, + issuableType: { + type: String, + require: true, + default: 'issue', + }, + }, +}; +</script> + +<template> + <button type="button" class="btn-link"> + <assignee-avatar :user="user" :img-size="24" :issuable-type="issuableType" /> + <span class="author"> {{ user.name }} </span> + </button> +</template> diff --git a/app/assets/javascripts/sidebar/components/assignees/collapsed_assignee_list.vue b/app/assets/javascripts/sidebar/components/assignees/collapsed_assignee_list.vue new file mode 100644 index 0000000000000000000000000000000000000000..6db198f1c1e82290085760b11b011787b1e2d3b7 --- /dev/null +++ b/app/assets/javascripts/sidebar/components/assignees/collapsed_assignee_list.vue @@ -0,0 +1,121 @@ +<script> +import { __, sprintf } from '~/locale'; +import { GlTooltipDirective } from '@gitlab/ui'; +import CollapsedAssignee from './collapsed_assignee.vue'; + +const DEFAULT_MAX_COUNTER = 99; +const DEFAULT_RENDER_COUNT = 5; + +export default { + directives: { + GlTooltip: GlTooltipDirective, + }, + components: { + CollapsedAssignee, + }, + props: { + users: { + type: Array, + required: true, + }, + issuableType: { + type: String, + require: true, + default: 'issue', + }, + }, + computed: { + isMergeRequest() { + return this.issuableType === 'merge_request'; + }, + hasNoUsers() { + return !this.users.length; + }, + hasMoreThanOneAssignee() { + return this.users.length > 1; + }, + hasMoreThanTwoAssignees() { + return this.users.length > 2; + }, + allAssigneesCanMerge() { + return this.users.every(user => user.can_merge); + }, + sidebarAvatarCounter() { + if (this.users.length > DEFAULT_MAX_COUNTER) { + return `${DEFAULT_MAX_COUNTER}+`; + } + + return `+${this.users.length - 1}`; + }, + collapsedUsers() { + const collapsedLength = this.hasMoreThanTwoAssignees ? 1 : this.users.length; + + return this.users.slice(0, collapsedLength); + }, + tooltipTitleMergeStatus() { + if (!this.isMergeRequest) { + return ''; + } + + const mergeLength = this.users.filter(u => u.can_merge).length; + + if (mergeLength === this.users.length) { + return ''; + } else if (mergeLength > 0) { + return sprintf(__('%{mergeLength}/%{usersLength} can merge'), { + mergeLength, + usersLength: this.users.length, + }); + } + + return this.users.length === 1 ? __('cannot merge') : __('no one can merge'); + }, + tooltipTitle() { + const maxRender = Math.min(DEFAULT_RENDER_COUNT, this.users.length); + const renderUsers = this.users.slice(0, maxRender); + const names = renderUsers.map(u => u.name); + + if (!this.users.length) { + return __('Assignee(s)'); + } + + if (this.users.length > names.length) { + names.push(sprintf(__('+ %{amount} more'), { amount: this.users.length - names.length })); + } + + const text = names.join(', '); + + return this.tooltipTitleMergeStatus ? `${text} (${this.tooltipTitleMergeStatus})` : text; + }, + + tooltipOptions() { + return { container: 'body', placement: 'left', boundary: 'viewport' }; + }, + }, +}; +</script> + +<template> + <div + v-gl-tooltip="tooltipOptions" + :class="{ 'multiple-users': hasMoreThanOneAssignee }" + :title="tooltipTitle" + class="sidebar-collapsed-icon sidebar-collapsed-user" + > + <i v-if="hasNoUsers" :aria-label="__('None')" class="fa fa-user"> </i> + <collapsed-assignee + v-for="user in collapsedUsers" + :key="user.id" + :user="user" + :issuable-type="issuableType" + /> + <button v-if="hasMoreThanTwoAssignees" class="btn-link" type="button"> + <span class="avatar-counter sidebar-avatar-counter"> {{ sidebarAvatarCounter }} </span> + <i + v-if="isMergeRequest && !allAssigneesCanMerge" + aria-hidden="true" + class="fa fa-exclamation-triangle merge-icon" + ></i> + </button> + </div> +</template> diff --git a/app/assets/javascripts/sidebar/components/assignees/uncollapsed_assignee_list.vue b/app/assets/javascripts/sidebar/components/assignees/uncollapsed_assignee_list.vue new file mode 100644 index 0000000000000000000000000000000000000000..e84b323c7bdf920105d581dfd314cab05f2a53d7 --- /dev/null +++ b/app/assets/javascripts/sidebar/components/assignees/uncollapsed_assignee_list.vue @@ -0,0 +1,96 @@ +<script> +import { __, sprintf } from '~/locale'; +import AssigneeAvatarLink from './assignee_avatar_link.vue'; + +const DEFAULT_RENDER_COUNT = 5; + +export default { + components: { + AssigneeAvatarLink, + }, + props: { + users: { + type: Array, + required: true, + }, + rootPath: { + type: String, + required: true, + }, + issuableType: { + type: String, + require: true, + default: 'issue', + }, + }, + data() { + return { + showLess: true, + }; + }, + computed: { + firstUser() { + return this.users[0]; + }, + hasOneUser() { + return this.users.length === 1; + }, + hiddenAssigneesLabel() { + const { numberOfHiddenAssignees } = this; + return sprintf(__('+ %{numberOfHiddenAssignees} more'), { numberOfHiddenAssignees }); + }, + renderShowMoreSection() { + return this.users.length > DEFAULT_RENDER_COUNT; + }, + numberOfHiddenAssignees() { + return this.users.length - DEFAULT_RENDER_COUNT; + }, + uncollapsedUsers() { + const uncollapsedLength = this.showLess + ? Math.min(this.users.length, DEFAULT_RENDER_COUNT) + : this.users.length; + return this.showLess ? this.users.slice(0, uncollapsedLength) : this.users; + }, + username() { + return `@${this.firstUser.username}`; + }, + }, + methods: { + toggleShowLess() { + this.showLess = !this.showLess; + }, + }, +}; +</script> + +<template> + <assignee-avatar-link + v-if="hasOneUser" + v-slot="{ user }" + tooltip-placement="left" + :tooltip-has-name="false" + :user="firstUser" + :root-path="rootPath" + :issuable-type="issuableType" + > + <div class="ml-2"> + <span class="author"> {{ user.name }} </span> + <span class="username"> {{ username }} </span> + </div> + </assignee-avatar-link> + <div v-else> + <div class="user-list"> + <div v-for="user in uncollapsedUsers" :key="user.id" class="user-item"> + <assignee-avatar-link :user="user" :root-path="rootPath" :issuable-type="issuableType" /> + </div> + </div> + <div v-if="renderShowMoreSection" class="user-list-more"> + <button type="button" class="btn-link" @click="toggleShowLess"> + <template v-if="showLess"> + {{ hiddenAssigneesLabel }} + </template> + <template v-else>{{ __('- show less') }}</template> + </button> + </div> + </div> +</template> diff --git a/app/assets/javascripts/users_select.js b/app/assets/javascripts/users_select.js index 33cedf78331fe6d1388a5f24ba3b2b2e7b1cd6da..12c939aa70fd4eaa873915f39d06f7d3ea6fee07 100644 --- a/app/assets/javascripts/users_select.js +++ b/app/assets/javascripts/users_select.js @@ -62,6 +62,8 @@ function UsersSelect(currentUser, els, options = {}) { options.showCurrentUser = $dropdown.data('currentUser'); options.todoFilter = $dropdown.data('todoFilter'); options.todoStateFilter = $dropdown.data('todoStateFilter'); + options.iid = $dropdown.data('iid'); + options.issuableType = $dropdown.data('issuableType'); showNullUser = $dropdown.data('nullUser'); defaultNullUser = $dropdown.data('nullUserDefault'); showMenuAbove = $dropdown.data('showMenuAbove'); @@ -239,7 +241,7 @@ function UsersSelect(currentUser, els, options = {}) { '<% if( avatar ) { %> <a class="author-link" href="/<%- username %>"> <img width="24" class="avatar avatar-inline s24" alt="" src="<%- avatar %>"> </a> <% } else { %> <i class="fa fa-user"></i> <% } %>', ); assigneeTemplate = _.template( - `<% if (username) { %> <a class="author-link bold" href="/<%- username %>"> <% if( avatar ) { %> <img width="32" class="avatar avatar-inline s32" alt="" src="<%- avatar %>"> <% } %> <span class="author"><%- name %></span> <span class="username"> @<%- username %> </span> </a> <% } else { %> <span class="no-value assign-yourself"> + `<% if (username) { %> <a class="author-link bold" href="/<%- username %>"> <% if( avatar ) { %> <img width="32" class="avatar avatar-inline s32" alt="" src="<%- avatar %>"> <% } %> <span class="author"><%- name %></span> <span class="username"> @<%- username %> </span> </a> <% } else { %> <span class="no-value assign-yourself"> ${sprintf(s__('UsersSelect|No assignee - %{openingTag} assign yourself %{closingTag}'), { openingTag: '<a href="#" class="js-assign-yourself">', closingTag: '</a>', @@ -423,6 +425,8 @@ function UsersSelect(currentUser, els, options = {}) { const { $el, e, isMarking } = options; const user = options.selectedObj; + $el.tooltip('dispose'); + if ($dropdown.hasClass('js-multiselect')) { const isActive = $el.hasClass('is-active'); const previouslySelected = $dropdown @@ -570,20 +574,11 @@ function UsersSelect(currentUser, els, options = {}) { user.name, )}</a></li>`; } else { - img = "<img src='" + avatar + "' class='avatar avatar-inline' width='32' />"; + // 0 margin, because it's now handled by a wrapper + img = "<img src='" + avatar + "' class='avatar avatar-inline m-0' width='32' />"; } - return ` - <li data-user-id=${user.id}> - <a href='#' class='dropdown-menu-user-link ${selected === true ? 'is-active' : ''}'> - ${img} - <strong class='dropdown-menu-user-full-name'> - ${_.escape(user.name)} - </strong> - ${username ? `<span class='dropdown-menu-user-username'>${username}</span>` : ''} - </a> - </li> - `; + return _this.renderRow(options.issuableType, user, selected, username, img); }, }); }; @@ -764,6 +759,11 @@ UsersSelect.prototype.users = function(query, options, callback) { author_id: options.authorId || null, skip_users: options.skipUsers || null, }; + + if (options.issuableType === 'merge_request') { + params.merge_request_iid = options.iid || null; + } + return axios.get(url, { params }).then(({ data }) => { callback(data); }); @@ -776,4 +776,44 @@ UsersSelect.prototype.buildUrl = function(url) { return url; }; +UsersSelect.prototype.renderRow = function(issuableType, user, selected, username, img) { + const tooltip = issuableType === 'merge_request' && !user.can_merge ? __('Cannot merge') : ''; + const tooltipClass = tooltip ? `has-tooltip` : ''; + const selectedClass = selected === true ? 'is-active' : ''; + const linkClasses = `${selectedClass} ${tooltipClass}`; + const tooltipAttributes = tooltip + ? `data-container="body" data-placement="left" data-title="${tooltip}"` + : ''; + + return ` + <li data-user-id=${user.id}> + <a href="#" class="dropdown-menu-user-link d-flex align-items-center ${linkClasses}" ${tooltipAttributes}> + ${this.renderRowAvatar(issuableType, user, img)} + <span class="d-flex flex-column overflow-hidden"> + <strong class="dropdown-menu-user-full-name"> + ${_.escape(user.name)} + </strong> + ${username ? `<span class="dropdown-menu-user-username">${username}</span>` : ''} + </span> + </a> + </li> + `; +}; + +UsersSelect.prototype.renderRowAvatar = function(issuableType, user, img) { + if (user.beforeDivider) { + return img; + } + + const mergeIcon = + issuableType === 'merge_request' && !user.can_merge + ? '<i class="fa fa-exclamation-triangle merge-icon"></i>' + : ''; + + return `<span class="position-relative mr-2"> + ${img} + ${mergeIcon} + </span>`; +}; + export default UsersSelect; diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index fa52ce6402d10245e150dbc0dc13d5105c1d0a6b..0e844b0e4a5d1e7f98bea6779cfeec19c3cb9a75 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -126,6 +126,16 @@ } } +.assignee { + .merge-icon { + color: $orange-500; + position: absolute; + bottom: 0; + right: 0; + text-shadow: -1px -1px 0 $white-light, 1px -1px 0 $white-light, -1px 1px 0 $white-light, 1px 1px 0 $white-light; + } +} + .right-sidebar { position: fixed; top: $header-height; @@ -202,7 +212,6 @@ &.assignee { .author-link { display: block; - padding-left: 42px; position: relative; &:hover { @@ -210,12 +219,6 @@ text-decoration: underline; } } - - .avatar { - left: 0; - position: absolute; - top: 0; - } } } } @@ -354,13 +357,6 @@ margin-top: 0; } - .assignee .avatar { - float: left; - margin-right: 10px; - margin-bottom: 0; - margin-left: 0; - } - .assignee .user-list .avatar { margin: 0; } @@ -521,6 +517,10 @@ display: none; } + .merge-icon { + font-size: 10px; + } + .multiple-users { position: relative; height: 24px; diff --git a/app/serializers/issuable_sidebar_basic_entity.rb b/app/serializers/issuable_sidebar_basic_entity.rb index 065fffb6e47d82f1fd357f29436f0d2c174496a1..498cfe5930d094a654dcaf7076a52f9ec74240be 100644 --- a/app/serializers/issuable_sidebar_basic_entity.rb +++ b/app/serializers/issuable_sidebar_basic_entity.rb @@ -4,6 +4,7 @@ class IssuableSidebarBasicEntity < Grape::Entity include RequestAwareEntity expose :id + expose :iid expose :type do |issuable| issuable.to_ability_name end diff --git a/app/serializers/merge_request_serializer.rb b/app/serializers/merge_request_serializer.rb index 8ad1df5dfe0177e24aaf418312b704d83b6e3b84..bd2e682a122d98e949f92350ea63b07060427fc2 100644 --- a/app/serializers/merge_request_serializer.rb +++ b/app/serializers/merge_request_serializer.rb @@ -8,7 +8,7 @@ class MergeRequestSerializer < BaseSerializer entity ||= case opts[:serializer] when 'sidebar' - IssuableSidebarBasicEntity + MergeRequestSidebarBasicEntity when 'sidebar_extras' MergeRequestSidebarExtrasEntity when 'basic' diff --git a/app/serializers/merge_request_sidebar_basic_entity.rb b/app/serializers/merge_request_sidebar_basic_entity.rb new file mode 100644 index 0000000000000000000000000000000000000000..fdd0cdc50d293a98a44cd956a4f4678455ca0236 --- /dev/null +++ b/app/serializers/merge_request_sidebar_basic_entity.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +class MergeRequestSidebarBasicEntity < IssuableSidebarBasicEntity +end + +MergeRequestSidebarBasicEntity.prepend_if_ee('EE::MergeRequestSidebarBasicEntity') diff --git a/app/views/shared/issuable/_sidebar_assignees.html.haml b/app/views/shared/issuable/_sidebar_assignees.html.haml index ab01094ed6e4f1581cdd5b9ff212bc64db662baa..1dc538826dcd6aa2ca6f19b1513f05585ddf8eb7 100644 --- a/app/views/shared/issuable/_sidebar_assignees.html.haml +++ b/app/views/shared/issuable/_sidebar_assignees.html.haml @@ -20,6 +20,8 @@ placeholder: _('Search users'), data: { first_user: issuable_sidebar.dig(:current_user, :username), current_user: true, + iid: issuable_sidebar[:iid], + issuable_type: issuable_type, project_id: issuable_sidebar[:project_id], author_id: issuable_sidebar[:author_id], field_name: "#{issuable_type}[assignee_ids][]", diff --git a/ee/app/serializers/ee/merge_request_sidebar_basic_entity.rb b/ee/app/serializers/ee/merge_request_sidebar_basic_entity.rb new file mode 100644 index 0000000000000000000000000000000000000000..a8e639aeef4d965953608a3e5914bcea1eeb113a --- /dev/null +++ b/ee/app/serializers/ee/merge_request_sidebar_basic_entity.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module EE + module MergeRequestSidebarBasicEntity + extend ActiveSupport::Concern + + prepended do + expose :current_user, if: lambda { |_issuable| current_user } do + expose :can_merge do |merge_request| + merge_request.can_be_merged_by?(current_user) + end + end + end + end +end diff --git a/ee/changelogs/unreleased/22058-improve-ux-multi-assignees-in-mr.yml b/ee/changelogs/unreleased/22058-improve-ux-multi-assignees-in-mr.yml new file mode 100644 index 0000000000000000000000000000000000000000..5a2549c6e80d7c4c2128317dadefe82ba286c90d --- /dev/null +++ b/ee/changelogs/unreleased/22058-improve-ux-multi-assignees-in-mr.yml @@ -0,0 +1,5 @@ +--- +title: Improve UX multi assignees in MR +merge_request: 14851 +author: +type: changed diff --git a/ee/spec/serializers/ee/merge_request_sidebar_basic_entity_spec.rb b/ee/spec/serializers/ee/merge_request_sidebar_basic_entity_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..b364b1a330657c7d0d7c8e346baccbb641aa9af2 --- /dev/null +++ b/ee/spec/serializers/ee/merge_request_sidebar_basic_entity_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe MergeRequestSidebarBasicEntity do + let(:project) { create :project, :repository } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let(:user) { create(:user) } + + let(:request) { double('request', current_user: user, project: project) } + + let(:entity) { described_class.new(merge_request, request: request).as_json } + + describe '#current_user' do + it 'contains attributes related to the current user' do + expect(entity[:current_user].keys).to contain_exactly( + :id, :name, :username, :state, :avatar_url, :web_url, :todo, + :can_edit, :can_move, :can_admin_label, :can_merge + ) + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 6dc4e4b6bc461e8a4d3a07cc22924c7123e87b13..cb8e5dfae52a847c56ddb7a5c5a2d25c4e6ad9ef 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -176,9 +176,6 @@ msgstr "" msgid "%{authorsName}'s thread" msgstr "" -msgid "%{canMergeCount}/%{assigneesCount} can merge" -msgstr "" - msgid "%{commit_author_link} authored %{commit_timeago}" msgstr "" @@ -281,6 +278,9 @@ msgstr "" msgid "%{lock_path} is locked by GitLab User %{lock_user_id}" msgstr "" +msgid "%{mergeLength}/%{usersLength} can merge" +msgstr "" + msgid "%{mrText}, this issue will be closed automatically." msgstr "" @@ -367,6 +367,9 @@ msgstr "" msgid "%{usage_ping_link_start}Learn more%{usage_ping_link_end} about what information is shared with GitLab Inc." msgstr "" +msgid "%{userName} (cannot merge)" +msgstr "" + msgid "%{userName}'s avatar" msgstr "" @@ -408,6 +411,9 @@ msgstr "" msgid "(external source)" msgstr "" +msgid "+ %{amount} more" +msgstr "" + msgid "+ %{count} more" msgstr "" @@ -9986,9 +9992,6 @@ msgstr "" msgid "No milestones to show" msgstr "" -msgid "No one can merge" -msgstr "" - msgid "No other labels with such name or description" msgstr "" @@ -17689,6 +17692,9 @@ msgstr "" msgid "cannot itself be blocked" msgstr "" +msgid "cannot merge" +msgstr "" + msgid "ciReport|%{linkStartTag}Learn more about Container Scanning %{linkEndTag}" msgstr "" @@ -18515,6 +18521,9 @@ msgstr "" msgid "no contributions" msgstr "" +msgid "no one can merge" +msgstr "" + msgid "none" msgstr "" diff --git a/spec/frontend/sidebar/components/assignees/assignee_avatar_link_spec.js b/spec/frontend/sidebar/components/assignees/assignee_avatar_link_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..9e50eefc2281b2b7fd5c8b95cb79131e06e27a70 --- /dev/null +++ b/spec/frontend/sidebar/components/assignees/assignee_avatar_link_spec.js @@ -0,0 +1,84 @@ +import { shallowMount } from '@vue/test-utils'; +import { joinPaths } from '~/lib/utils/url_utility'; +import userDataMock from '../../user_data_mock'; +import AssigneeAvatarLink from '~/sidebar/components/assignees/assignee_avatar_link.vue'; + +const TOOLTIP_PLACEMENT = 'bottom'; +const { name: USER_NAME } = userDataMock(); + +describe('AssigneeAvatarLink component', () => { + let wrapper; + + function createComponent(props = {}) { + const propsData = { + user: userDataMock(), + showLess: true, + rootPath: 'http://localhost:3000/', + tooltipPlacement: TOOLTIP_PLACEMENT, + singleUser: false, + issuableType: 'merge_request', + ...props, + }; + + wrapper = shallowMount(AssigneeAvatarLink, { + propsData, + sync: false, + }); + } + + afterEach(() => { + wrapper.destroy(); + }); + + const findTooltipText = () => wrapper.attributes('data-original-title'); + + it('user who cannot merge has "cannot merge" in tooltip', () => { + createComponent({ + user: { + can_merge: false, + }, + }); + + expect(findTooltipText().includes('cannot merge')).toBe(true); + }); + + it('has the root url present in the assigneeUrl method', () => { + createComponent(); + const assigneeUrl = joinPaths( + `${wrapper.props('rootPath')}`, + `${wrapper.props('user').username}`, + ); + + expect(wrapper.attributes().href).toEqual(assigneeUrl); + }); + + describe.each` + issuableType | tooltipHasName | canMerge | expected + ${'merge_request'} | ${true} | ${true} | ${USER_NAME} + ${'merge_request'} | ${true} | ${false} | ${`${USER_NAME} (cannot merge)`} + ${'merge_request'} | ${false} | ${true} | ${''} + ${'merge_request'} | ${false} | ${false} | ${'Cannot merge'} + ${'issue'} | ${true} | ${true} | ${USER_NAME} + ${'issue'} | ${true} | ${false} | ${USER_NAME} + ${'issue'} | ${false} | ${true} | ${''} + ${'issue'} | ${false} | ${false} | ${''} + `( + 'with $issuableType and tooltipHasName=$tooltipHasName and canMerge=$canMerge', + ({ issuableType, tooltipHasName, canMerge, expected }) => { + beforeEach(() => { + createComponent({ + issuableType, + tooltipHasName, + user: { + ...userDataMock(), + can_merge: canMerge, + }, + }); + }); + + it('sets tooltip', () => { + expect(findTooltipText()).toBe(expected); + }); + }, + ); +}); diff --git a/spec/frontend/sidebar/components/assignees/assignee_avatar_spec.js b/spec/frontend/sidebar/components/assignees/assignee_avatar_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..53e5dbea1dfd88126d1f2557b268c1c2f2dfe0c7 --- /dev/null +++ b/spec/frontend/sidebar/components/assignees/assignee_avatar_spec.js @@ -0,0 +1,78 @@ +import { shallowMount } from '@vue/test-utils'; +import AssigneeAvatar from '~/sidebar/components/assignees/assignee_avatar.vue'; +import { TEST_HOST } from 'helpers/test_constants'; +import userDataMock from '../../user_data_mock'; + +const TEST_AVATAR = `${TEST_HOST}/avatar.png`; +const TEST_DEFAULT_AVATAR_URL = `${TEST_HOST}/default/avatar/url.png`; + +describe('AssigneeAvatar', () => { + let origGon; + let wrapper; + + function createComponent(props = {}) { + const propsData = { + user: userDataMock(), + imgSize: 24, + issuableType: 'merge_request', + ...props, + }; + + wrapper = shallowMount(AssigneeAvatar, { + propsData, + sync: false, + }); + } + + beforeEach(() => { + origGon = window.gon; + window.gon = { default_avatar_url: TEST_DEFAULT_AVATAR_URL }; + }); + + afterEach(() => { + window.gon = origGon; + wrapper.destroy(); + }); + + const findImg = () => wrapper.find('img'); + + it('does not show warning icon if assignee can merge', () => { + createComponent(); + + expect(wrapper.element.querySelector('.merge-icon')).toBeNull(); + }); + + it('shows warning icon if assignee cannot merge', () => { + createComponent({ + user: { + can_merge: false, + }, + }); + + expect(wrapper.element.querySelector('.merge-icon')).not.toBeNull(); + }); + + it('does not show warning icon for issuableType = "issue"', () => { + createComponent({ + issuableType: 'issue', + }); + + expect(wrapper.element.querySelector('.merge-icon')).toBeNull(); + }); + + it.each` + avatar | avatar_url | expected | desc + ${TEST_AVATAR} | ${null} | ${TEST_AVATAR} | ${'with avatar'} + ${null} | ${TEST_AVATAR} | ${TEST_AVATAR} | ${'with avatar_url'} + ${null} | ${null} | ${TEST_DEFAULT_AVATAR_URL} | ${'with no avatar'} + `('$desc', ({ avatar, avatar_url, expected }) => { + createComponent({ + user: { + avatar, + avatar_url, + }, + }); + + expect(findImg().attributes('src')).toEqual(expected); + }); +}); diff --git a/spec/frontend/sidebar/components/assignees/collapsed_assignee_list_spec.js b/spec/frontend/sidebar/components/assignees/collapsed_assignee_list_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..377c0e1d211ac91cc00fb6368141ae2bfbb0dc9c --- /dev/null +++ b/spec/frontend/sidebar/components/assignees/collapsed_assignee_list_spec.js @@ -0,0 +1,201 @@ +import { shallowMount } from '@vue/test-utils'; +import CollapsedAssigneeList from '~/sidebar/components/assignees/collapsed_assignee_list.vue'; +import CollapsedAssignee from '~/sidebar/components/assignees/collapsed_assignee.vue'; +import UsersMockHelper from 'helpers/user_mock_data_helper'; + +const DEFAULT_MAX_COUNTER = 99; + +describe('CollapsedAssigneeList component', () => { + let wrapper; + + function createComponent(props = {}) { + const propsData = { + users: [], + issuableType: 'merge_request', + ...props, + }; + + wrapper = shallowMount(CollapsedAssigneeList, { + propsData, + sync: false, + }); + } + + const findNoUsersIcon = () => wrapper.find('i[aria-label=None]'); + const findAvatarCounter = () => wrapper.find('.avatar-counter'); + const findAssignees = () => wrapper.findAll(CollapsedAssignee); + const getTooltipTitle = () => wrapper.attributes('data-original-title'); + + afterEach(() => { + wrapper.destroy(); + }); + + describe('No assignees/users', () => { + beforeEach(() => { + createComponent({ + users: [], + }); + }); + + it('has no users', () => { + expect(findNoUsersIcon().exists()).toBe(true); + }); + }); + + describe('One assignee/user', () => { + let users; + + beforeEach(() => { + users = UsersMockHelper.createNumberRandomUsers(1); + }); + + it('should not show no users icon', () => { + createComponent({ users }); + + expect(findNoUsersIcon().exists()).toBe(false); + }); + + it('has correct "cannot merge" tooltip when user cannot merge', () => { + users[0].can_merge = false; + + createComponent({ users }); + + expect(getTooltipTitle()).toContain('cannot merge'); + }); + + it('does not have "merge" word in tooltip if user can merge', () => { + users[0].can_merge = true; + + createComponent({ users }); + + expect(getTooltipTitle()).not.toContain('merge'); + }); + }); + + describe('More than one assignees/users', () => { + let users; + + beforeEach(() => { + users = UsersMockHelper.createNumberRandomUsers(2); + }); + + describe('default', () => { + beforeEach(() => { + createComponent({ users }); + }); + + it('has multiple-users class', () => { + expect(wrapper.classes('multiple-users')).toBe(true); + }); + + it('does not display an avatar count', () => { + expect(findAvatarCounter().exists()).toBe(false); + }); + + it('returns just two collapsed users', () => { + expect(findAssignees().length).toBe(2); + }); + }); + + it('has correct "cannot merge" tooltip when no user can merge', () => { + users[0].can_merge = false; + users[1].can_merge = false; + + createComponent({ + users, + }); + + expect(getTooltipTitle()).toEqual(`${users[0].name}, ${users[1].name} (no one can merge)`); + }); + + it('does not have "merge" word in tooltip if everyone can merge', () => { + users[0].can_merge = true; + users[1].can_merge = true; + + createComponent({ + users, + }); + + expect(getTooltipTitle()).toEqual(`${users[0].name}, ${users[1].name}`); + }); + }); + + describe('More than two assignees/users', () => { + let users; + + beforeEach(() => { + users = UsersMockHelper.createNumberRandomUsers(3); + }); + + describe('default', () => { + beforeEach(() => { + createComponent({ users }); + }); + + it('does display an avatar count', () => { + expect(findAvatarCounter().exists()).toBe(true); + expect(findAvatarCounter().text()).toEqual('+2'); + }); + + it('returns one collapsed users', () => { + expect(findAssignees().length).toBe(1); + }); + }); + + it('has correct "cannot merge" tooltip when one user can merge', () => { + users[0].can_merge = true; + users[1].can_merge = false; + users[2].can_merge = false; + + createComponent({ + users, + }); + + expect(getTooltipTitle()).toContain('1/3 can merge'); + }); + + it('has correct "cannot merge" tooltip when more than one user can merge', () => { + users[0].can_merge = false; + users[1].can_merge = true; + users[2].can_merge = true; + + createComponent({ + users, + }); + + expect(getTooltipTitle()).toContain('2/3 can merge'); + }); + + it('does not have "merge" in tooltip if everyone can merge', () => { + users[0].can_merge = true; + users[1].can_merge = true; + users[2].can_merge = true; + + createComponent({ + users, + }); + + expect(getTooltipTitle()).not.toContain('merge'); + }); + + it('displays the correct avatar count via a computed property if less than default max counter', () => { + users = UsersMockHelper.createNumberRandomUsers(5); + + createComponent({ + users, + }); + + expect(findAvatarCounter().text()).toEqual(`+${users.length - 1}`); + }); + + it('displays the correct avatar count via a computed property if more than default max counter', () => { + users = UsersMockHelper.createNumberRandomUsers(100); + + createComponent({ + users, + }); + + expect(findAvatarCounter().text()).toEqual(`${DEFAULT_MAX_COUNTER}+`); + }); + }); +}); diff --git a/spec/frontend/sidebar/components/assignees/collapsed_assignee_spec.js b/spec/frontend/sidebar/components/assignees/collapsed_assignee_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..03b61daa2f8dff619aaf59bd25052a98ad907b0b --- /dev/null +++ b/spec/frontend/sidebar/components/assignees/collapsed_assignee_spec.js @@ -0,0 +1,34 @@ +import { shallowMount } from '@vue/test-utils'; +import CollapsedAssignee from '~/sidebar/components/assignees/collapsed_assignee.vue'; +import userDataMock from '../../user_data_mock'; + +describe('CollapsedAssignee assignee component', () => { + let wrapper; + + function createComponent(props = {}) { + const propsData = { + user: userDataMock(), + ...props, + }; + + wrapper = shallowMount(CollapsedAssignee, { + propsData, + sync: false, + }); + } + + afterEach(() => { + wrapper.destroy(); + }); + + it('has author name', () => { + createComponent(); + + expect( + wrapper + .find('.author') + .text() + .trim(), + ).toEqual(wrapper.vm.user.name); + }); +}); diff --git a/spec/frontend/sidebar/components/assignees/uncollapsed_assignee_list_spec.js b/spec/frontend/sidebar/components/assignees/uncollapsed_assignee_list_spec.js new file mode 100644 index 0000000000000000000000000000000000000000..64170a53a7f996ca662bf14c755b2e737b8e4ddf --- /dev/null +++ b/spec/frontend/sidebar/components/assignees/uncollapsed_assignee_list_spec.js @@ -0,0 +1,135 @@ +import Vue from 'vue'; +import { mount } from '@vue/test-utils'; +import UncollapsedAssigneeList from '~/sidebar/components/assignees/uncollapsed_assignee_list.vue'; +import AssigneeAvatarLink from '~/sidebar/components/assignees/assignee_avatar_link.vue'; +import { TEST_HOST } from 'helpers/test_constants'; +import userDataMock from '../../user_data_mock'; +import UsersMockHelper from '../../../helpers/user_mock_data_helper'; + +const DEFAULT_RENDER_COUNT = 5; + +describe('UncollapsedAssigneeList component', () => { + let wrapper; + + function createComponent(props = {}) { + const propsData = { + users: [], + rootPath: TEST_HOST, + ...props, + }; + + wrapper = mount(UncollapsedAssigneeList, { + sync: false, + propsData, + }); + } + + afterEach(() => { + wrapper.destroy(); + }); + + const findMoreButton = () => wrapper.find('.user-list-more button'); + + describe('One assignee/user', () => { + let user; + + beforeEach(() => { + user = userDataMock(); + + createComponent({ + users: [user], + }); + }); + + it('only has one user', () => { + expect(wrapper.findAll(AssigneeAvatarLink).length).toBe(1); + }); + + it('calls the AssigneeAvatarLink with the proper props', () => { + expect(wrapper.find(AssigneeAvatarLink).exists()).toBe(true); + expect(wrapper.find(AssigneeAvatarLink).props().tooltipPlacement).toEqual('left'); + }); + + it('Shows one user with avatar, username and author name', () => { + expect(wrapper.text()).toContain(user.name); + expect(wrapper.text()).toContain(`@${user.username}`); + }); + }); + + describe('Two or more assignees/users', () => { + beforeEach(() => { + createComponent({ + users: UsersMockHelper.createNumberRandomUsers(3), + }); + }); + + it('more than one user', () => { + expect(wrapper.findAll(AssigneeAvatarLink).length).toBe(3); + }); + + it('shows the "show-less" assignees label', done => { + const users = UsersMockHelper.createNumberRandomUsers(6); + + createComponent({ + users, + }); + + expect(wrapper.vm.$el.querySelectorAll('.user-item').length).toEqual(DEFAULT_RENDER_COUNT); + + expect(wrapper.vm.$el.querySelector('.user-list-more')).not.toBe(null); + const usersLabelExpectation = users.length - DEFAULT_RENDER_COUNT; + + expect(wrapper.vm.$el.querySelector('.user-list-more .btn-link').innerText.trim()).not.toBe( + `+${usersLabelExpectation} more`, + ); + wrapper.vm.toggleShowLess(); + Vue.nextTick(() => { + expect(wrapper.vm.$el.querySelector('.user-list-more .btn-link').innerText.trim()).toBe( + '- show less', + ); + done(); + }); + }); + + it('shows the "show-less" when "n+ more " label is clicked', done => { + createComponent({ + users: UsersMockHelper.createNumberRandomUsers(6), + }); + + wrapper.vm.$el.querySelector('.user-list-more .btn-link').click(); + Vue.nextTick(() => { + expect(wrapper.vm.$el.querySelector('.user-list-more .btn-link').innerText.trim()).toBe( + '- show less', + ); + done(); + }); + }); + + it('does not show n+ more label when less than render count', () => { + expect(findMoreButton().exists()).toBe(false); + }); + }); + + describe('n+ more label', () => { + beforeEach(() => { + createComponent({ + users: UsersMockHelper.createNumberRandomUsers(DEFAULT_RENDER_COUNT + 1), + }); + }); + + it('shows "+1 more" label', () => { + expect(findMoreButton().text()).toBe('+ 1 more'); + expect(wrapper.findAll(AssigneeAvatarLink).length).toBe(DEFAULT_RENDER_COUNT); + }); + + it('shows "show less" label', done => { + findMoreButton().trigger('click'); + + Vue.nextTick(() => { + expect(findMoreButton().text()).toBe('- show less'); + expect(wrapper.findAll(AssigneeAvatarLink).length).toBe(DEFAULT_RENDER_COUNT + 1); + done(); + }); + }); + }); +}); diff --git a/spec/frontend/sidebar/user_data_mock.js b/spec/frontend/sidebar/user_data_mock.js new file mode 100644 index 0000000000000000000000000000000000000000..8ad70bb3499cad11aeb4dcb1fe4e27e9b503961b --- /dev/null +++ b/spec/frontend/sidebar/user_data_mock.js @@ -0,0 +1,9 @@ +export default () => ({ + avatar_url: 'mock_path', + id: 1, + name: 'Root', + state: 'active', + username: 'root', + web_url: '', + can_merge: true, +}); diff --git a/spec/javascripts/sidebar/assignees_spec.js b/spec/javascripts/sidebar/assignees_spec.js index 4ae2141d5f049788535578c2934e43cdc802a3bf..fc731e731ae489c308896ef5307132d24d321b63 100644 --- a/spec/javascripts/sidebar/assignees_spec.js +++ b/spec/javascripts/sidebar/assignees_spec.js @@ -94,115 +94,9 @@ describe('Assignee component', () => { expect(assignee.querySelector('.author').innerText.trim()).toEqual(UsersMock.user.name); }); - - it('Shows one user with avatar, username and author name', () => { - component = new AssigneeComponent({ - propsData: { - rootPath: 'http://localhost:3000/', - users: [UsersMock.user], - editable: true, - }, - }).$mount(); - - expect(component.$el.querySelector('.author-link')).not.toBeNull(); - // The image - expect(component.$el.querySelector('.author-link img').getAttribute('src')).toEqual( - UsersMock.user.avatar, - ); - // Author name - expect(component.$el.querySelector('.author-link .author').innerText.trim()).toEqual( - UsersMock.user.name, - ); - // Username - expect(component.$el.querySelector('.author-link .username').innerText.trim()).toEqual( - `@${UsersMock.user.username}`, - ); - }); - - it('has the root url present in the assigneeUrl method', () => { - component = new AssigneeComponent({ - propsData: { - rootPath: 'http://localhost:3000/', - users: [UsersMock.user], - editable: true, - }, - }).$mount(); - - expect(component.assigneeUrl(UsersMock.user).indexOf('http://localhost:3000/')).not.toEqual( - -1, - ); - }); - - it('has correct "cannot merge" tooltip when user cannot merge', () => { - const user = Object.assign({}, UsersMock.user, { can_merge: false }); - - component = new AssigneeComponent({ - propsData: { - rootPath: 'http://localhost:3000/', - users: [user], - editable: true, - issuableType: 'merge_request', - }, - }).$mount(); - - expect(component.mergeNotAllowedTooltipMessage).toEqual('Cannot merge'); - }); }); describe('Two or more assignees/users', () => { - it('has correct "cannot merge" tooltip when one user can merge', () => { - const users = UsersMockHelper.createNumberRandomUsers(3); - users[0].can_merge = true; - users[1].can_merge = false; - users[2].can_merge = false; - - component = new AssigneeComponent({ - propsData: { - rootPath: 'http://localhost:3000/', - users, - editable: true, - issuableType: 'merge_request', - }, - }).$mount(); - - expect(component.mergeNotAllowedTooltipMessage).toEqual('1/3 can merge'); - }); - - it('has correct "cannot merge" tooltip when no user can merge', () => { - const users = UsersMockHelper.createNumberRandomUsers(2); - users[0].can_merge = false; - users[1].can_merge = false; - - component = new AssigneeComponent({ - propsData: { - rootPath: 'http://localhost:3000/', - users, - editable: true, - issuableType: 'merge_request', - }, - }).$mount(); - - expect(component.mergeNotAllowedTooltipMessage).toEqual('No one can merge'); - }); - - it('has correct "cannot merge" tooltip when more than one user can merge', () => { - const users = UsersMockHelper.createNumberRandomUsers(3); - users[0].can_merge = false; - users[1].can_merge = true; - users[2].can_merge = true; - - component = new AssigneeComponent({ - propsData: { - rootPath: 'http://localhost:3000/', - users, - editable: true, - issuableType: 'merge_request', - }, - }).$mount(); - - expect(component.mergeNotAllowedTooltipMessage).toEqual('2/3 can merge'); - }); - it('has no "cannot merge" tooltip when every user can merge', () => { const users = UsersMockHelper.createNumberRandomUsers(2); users[0].can_merge = true; @@ -217,7 +111,7 @@ describe('Assignee component', () => { }, }).$mount(); - expect(component.mergeNotAllowedTooltipMessage).toEqual(null); + expect(component.collapsedTooltipTitle).not.toContain('cannot merge'); }); it('displays two assignee icons when collapsed', () => { @@ -295,8 +189,12 @@ describe('Assignee component', () => { expect(component.$el.querySelector('.user-list-more')).toBe(null); }); - it('sets tooltip container to body', () => { - const users = UsersMockHelper.createNumberRandomUsers(2); + it('shows sorted assignee where "can merge" users are sorted first', () => { + const users = UsersMockHelper.createNumberRandomUsers(3); + users[0].can_merge = false; + users[1].can_merge = false; + users[2].can_merge = true; + component = new AssigneeComponent({ propsData: { rootPath: 'http://localhost:3000', @@ -305,98 +203,48 @@ describe('Assignee component', () => { }, }).$mount(); - expect(component.$el.querySelector('.user-link').getAttribute('data-container')).toBe('body'); + expect(component.sortedAssigness[0].can_merge).toBe(true); }); - it('Shows the "show-less" assignees label', done => { - const users = UsersMockHelper.createNumberRandomUsers(6); + it('passes the sorted assignees to the uncollapsed-assignee-list', () => { + const users = UsersMockHelper.createNumberRandomUsers(3); + users[0].can_merge = false; + users[1].can_merge = false; + users[2].can_merge = true; + component = new AssigneeComponent({ propsData: { rootPath: 'http://localhost:3000', users, - editable: true, + editable: false, }, }).$mount(); - expect(component.$el.querySelectorAll('.user-item').length).toEqual( - component.defaultRenderCount, - ); - - expect(component.$el.querySelector('.user-list-more')).not.toBe(null); - const usersLabelExpectation = users.length - component.defaultRenderCount; + const userItems = component.$el.querySelectorAll('.user-list .user-item a'); - expect(component.$el.querySelector('.user-list-more .btn-link').innerText.trim()).not.toBe( - `+${usersLabelExpectation} more`, - ); - component.toggleShowLess(); - Vue.nextTick(() => { - expect(component.$el.querySelector('.user-list-more .btn-link').innerText.trim()).toBe( - '- show less', - ); - done(); - }); + expect(userItems.length).toBe(3); + expect(userItems[0].dataset.originalTitle).toBe(users[2].name); + expect(userItems[0].dataset.originalTitle).not.toBe(users[0].name); }); - it('Shows the "show-less" when "n+ more " label is clicked', done => { - const users = UsersMockHelper.createNumberRandomUsers(6); - component = new AssigneeComponent({ - propsData: { - rootPath: 'http://localhost:3000', - users, - editable: true, - }, - }).$mount(); - - component.$el.querySelector('.user-list-more .btn-link').click(); - Vue.nextTick(() => { - expect(component.$el.querySelector('.user-list-more .btn-link').innerText.trim()).toBe( - '- show less', - ); - done(); - }); - }); + it('passes the sorted assignees to the collapsed-assignee-list', () => { + const users = UsersMockHelper.createNumberRandomUsers(3); + users[0].can_merge = false; + users[1].can_merge = false; + users[2].can_merge = true; - it('gets the count of avatar via a computed property ', () => { - const users = UsersMockHelper.createNumberRandomUsers(6); component = new AssigneeComponent({ propsData: { rootPath: 'http://localhost:3000', users, - editable: true, + editable: false, }, }).$mount(); - expect(component.sidebarAvatarCounter).toEqual(`+${users.length - 1}`); - }); + const collapsedButton = component.$el.querySelector('.sidebar-collapsed-user button'); - describe('n+ more label', () => { - beforeEach(() => { - const users = UsersMockHelper.createNumberRandomUsers(6); - component = new AssigneeComponent({ - propsData: { - rootPath: 'http://localhost:3000', - users, - editable: true, - }, - }).$mount(); - }); - - it('shows "+1 more" label', () => { - expect(component.$el.querySelector('.user-list-more .btn-link').innerText.trim()).toBe( - '+ 1 more', - ); - }); - - it('shows "show less" label', done => { - component.toggleShowLess(); - - Vue.nextTick(() => { - expect(component.$el.querySelector('.user-list-more .btn-link').innerText.trim()).toBe( - '- show less', - ); - done(); - }); - }); + expect(collapsedButton.innerText.trim()).toBe(users[2].name); + expect(collapsedButton.innerText.trim()).not.toBe(users[0].name); }); }); }); diff --git a/spec/javascripts/sidebar/mock_data.js b/spec/javascripts/sidebar/mock_data.js index 7f20b0da99178cc3e3eaacb1686717fd0805f82c..107acead3c6b6db827d8ff9386f94741c408b6b3 100644 --- a/spec/javascripts/sidebar/mock_data.js +++ b/spec/javascripts/sidebar/mock_data.js @@ -1,3 +1,13 @@ +export const userDataMock = { + avatar_url: 'mock_path', + id: 1, + name: 'Root', + state: 'active', + username: 'root', + web_url: '', + can_merge: true, +}; + const RESPONSE_MAP = { GET: { '/gitlab-org/gitlab-shell/issues/5.json': { diff --git a/spec/serializers/merge_request_sidebar_basic_entity_spec.rb b/spec/serializers/merge_request_sidebar_basic_entity_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..b364b1a330657c7d0d7c8e346baccbb641aa9af2 --- /dev/null +++ b/spec/serializers/merge_request_sidebar_basic_entity_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe MergeRequestSidebarBasicEntity do + let(:project) { create :project, :repository } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let(:user) { create(:user) } + + let(:request) { double('request', current_user: user, project: project) } + + let(:entity) { described_class.new(merge_request, request: request).as_json } + + describe '#current_user' do + it 'contains attributes related to the current user' do + expect(entity[:current_user].keys).to contain_exactly( + :id, :name, :username, :state, :avatar_url, :web_url, :todo, + :can_edit, :can_move, :can_admin_label, :can_merge + ) + end + end +end