From 823c9460964d90ea442042354f4113fd8e3e9f2b Mon Sep 17 00:00:00 2001 From: Kushal Pandya <kushalspandya@gmail.com> Date: Thu, 22 Oct 2020 18:55:00 +0530 Subject: [PATCH] Update `issuable_list` app Updates `issuable_list` app with following enhancements; - Custom reference and status support via slots. - Assignees support. - Labels permalink support. - Ability to update page URL using provided params. - Tab counts are now optional. - Handle external URLs for Issuable items. --- .../components/issuable_item.vue | 79 ++++++++-- .../components/issuable_list_root.vue | 45 +++++- .../components/issuable_tabs.vue | 5 +- .../javascripts/issuable_list/constants.js | 49 ++++++ locale/gitlab.pot | 6 + .../components/issuable_item_spec.js | 139 +++++++++++++++++- .../components/issuable_list_root_spec.js | 27 ++++ spec/frontend/issuable_list/mock_data.js | 1 + 8 files changed, 330 insertions(+), 21 deletions(-) create mode 100644 app/assets/javascripts/issuable_list/constants.js diff --git a/app/assets/javascripts/issuable_list/components/issuable_item.vue b/app/assets/javascripts/issuable_list/components/issuable_item.vue index d8cb1ab07cd..283e49c287a 100644 --- a/app/assets/javascripts/issuable_list/components/issuable_item.vue +++ b/app/assets/javascripts/issuable_list/components/issuable_item.vue @@ -1,15 +1,20 @@ <script> -import { GlLink, GlLabel, GlTooltipDirective } from '@gitlab/ui'; +import { GlLink, GlIcon, GlLabel, GlTooltipDirective } from '@gitlab/ui'; import { __, sprintf } from '~/locale'; +import { getIdFromGraphQLId } from '~/graphql_shared/utils'; import { getTimeago } from '~/lib/utils/datetime_utility'; import { isScopedLabel } from '~/lib/utils/common_utils'; import timeagoMixin from '~/vue_shared/mixins/timeago'; +import IssuableAssignees from '~/vue_shared/components/issue/issue_assignees.vue'; + export default { components: { GlLink, + GlIcon, GlLabel, + IssuableAssignees, }, directives: { GlTooltip: GlTooltipDirective, @@ -24,25 +29,33 @@ export default { type: Object, required: true, }, + enableLabelPermalinks: { + type: Boolean, + required: true, + }, }, computed: { author() { return this.issuable.author; }, authorId() { - const id = parseInt(this.author.id, 10); - - if (Number.isNaN(id)) { - return this.author.id.includes('gid') - ? this.author.id.split('gid://gitlab/User/').pop() - : ''; + return getIdFromGraphQLId(`${this.author.id}`); + }, + isIssuableUrlExternal() { + // Check if URL is relative, which means it is internal. + if (!/^https?:\/\//g.test(this.issuable.webUrl)) { + return false; } - - return id; + // In case URL is absolute, it may or may not be internal, + // hence use `gon.gitlab_url` which is current instance domain. + return !this.issuable.webUrl.includes(gon.gitlab_url); }, labels() { return this.issuable.labels?.nodes || this.issuable.labels || []; }, + assignees() { + return this.issuable.assignees || []; + }, createdAt() { return sprintf(__('created %{timeAgo}'), { timeAgo: getTimeago().format(this.issuable.createdAt), @@ -53,11 +66,33 @@ export default { timeAgo: getTimeago().format(this.issuable.updatedAt), }); }, + issuableTitleProps() { + if (this.isIssuableUrlExternal) { + return { + target: '_blank', + }; + } + return {}; + }, }, methods: { + hasSlotContents(slotName) { + return Boolean(this.$slots[slotName]); + }, scopedLabel(label) { return isScopedLabel(label); }, + labelTitle(label) { + return label.title || label.name; + }, + labelTarget(label) { + if (this.enableLabelPermalinks) { + const key = encodeURIComponent('label_name[]'); + const value = encodeURIComponent(this.labelTitle(label)); + return `?${key}=${value}`; + } + return '#'; + }, /** * This is needed as an independent method since * when user changes current page, `$refs.authorLink` @@ -74,17 +109,20 @@ export default { </script> <template> - <li class="issue"> + <li class="issue px-3"> <div class="issue-box"> <div class="issuable-info-container"> <div class="issuable-main-info"> <div data-testid="issuable-title" class="issue-title title"> <span class="issue-title-text" dir="auto"> - <gl-link :href="issuable.webUrl">{{ issuable.title }}</gl-link> + <gl-link :href="issuable.webUrl" v-bind="issuableTitleProps" + >{{ issuable.title }}<gl-icon v-if="isIssuableUrlExternal" name="external-link" + /></gl-link> </span> </div> <div class="issuable-info"> - <span data-testid="issuable-reference" class="issuable-reference" + <slot v-if="hasSlotContents('reference')" name="reference"></slot> + <span v-else data-testid="issuable-reference" class="issuable-reference" >{{ issuableSymbol }}{{ issuable.iid }}</span > <span class="issuable-authored d-none d-sm-inline-block"> @@ -113,15 +151,30 @@ export default { v-for="(label, index) in labels" :key="index" :background-color="label.color" - :title="label.title" + :title="labelTitle(label)" :description="label.description" :scoped="scopedLabel(label)" + :target="labelTarget(label)" :class="{ 'gl-ml-2': index }" size="sm" /> </div> </div> <div class="issuable-meta"> + <ul v-if="hasSlotContents('status') || issuable.assignees" class="controls"> + <li v-if="hasSlotContents('status')" class="issuable-status"> + <slot name="status"></slot> + </li> + <li v-if="assignees.length" class="gl-display-flex"> + <issuable-assignees + :assignees="issuable.assignees" + :icon-size="16" + :max-visible="4" + img-css-classes="gl-mr-2!" + class="gl-align-items-center gl-display-flex gl-ml-3" + /> + </li> + </ul> <div data-testid="issuable-updated-at" class="float-right issuable-updated-at d-none d-sm-inline-block" diff --git a/app/assets/javascripts/issuable_list/components/issuable_list_root.vue b/app/assets/javascripts/issuable_list/components/issuable_list_root.vue index 7535203dea1..52ebb9acdc3 100644 --- a/app/assets/javascripts/issuable_list/components/issuable_list_root.vue +++ b/app/assets/javascripts/issuable_list/components/issuable_list_root.vue @@ -1,6 +1,7 @@ <script> import { GlLoadingIcon, GlPagination } from '@gitlab/ui'; +import { updateHistory, setUrlParams } from '~/lib/utils/url_utility'; import FilteredSearchBar from '~/vue_shared/components/filtered_search_bar/filtered_search_bar_root.vue'; import IssuableTabs from './issuable_tabs.vue'; @@ -35,6 +36,11 @@ export default { type: Array, required: true, }, + urlParams: { + type: Object, + required: false, + default: () => ({}), + }, initialFilterValue: { type: Array, required: false, @@ -55,7 +61,8 @@ export default { }, tabCounts: { type: Object, - required: true, + required: false, + default: null, }, currentTab: { type: String, @@ -81,6 +88,11 @@ export default { required: false, default: 20, }, + totalPages: { + type: Number, + required: false, + default: 0, + }, currentPage: { type: Number, required: false, @@ -96,6 +108,26 @@ export default { required: false, default: 2, }, + enableLabelPermalinks: { + type: Boolean, + required: false, + default: true, + }, + }, + watch: { + urlParams: { + deep: true, + immediate: true, + handler(params) { + if (Object.keys(params).length) { + updateHistory({ + url: setUrlParams(params, window.location.href, true), + title: document.title, + replace: true, + }); + } + }, + }, }, }; </script> @@ -135,12 +167,21 @@ export default { :key="issuable.id" :issuable-symbol="issuableSymbol" :issuable="issuable" - /> + :enable-label-permalinks="enableLabelPermalinks" + > + <template #reference> + <slot name="reference" :issuable="issuable"></slot> + </template> + <template #status> + <slot name="status" :issuable="issuable"></slot> + </template> + </issuable-item> </ul> <slot v-if="!issuablesLoading && !issuables.length" name="empty-state"></slot> <gl-pagination v-if="showPaginationControls" :per-page="defaultPageSize" + :total-items="totalPages" :value="currentPage" :prev-page="previousPage" :next-page="nextPage" diff --git a/app/assets/javascripts/issuable_list/components/issuable_tabs.vue b/app/assets/javascripts/issuable_list/components/issuable_tabs.vue index df544ce69e7..d9aab004077 100644 --- a/app/assets/javascripts/issuable_list/components/issuable_tabs.vue +++ b/app/assets/javascripts/issuable_list/components/issuable_tabs.vue @@ -14,7 +14,8 @@ export default { }, tabCounts: { type: Object, - required: true, + required: false, + default: null, }, currentTab: { type: String, @@ -40,7 +41,7 @@ export default { > <template #title> <span :title="tab.titleTooltip">{{ tab.title }}</span> - <gl-badge variant="neutral" size="sm" class="gl-px-2 gl-py-1!">{{ + <gl-badge v-if="tabCounts" variant="neutral" size="sm" class="gl-px-2 gl-py-1!">{{ tabCounts[tab.name] }}</gl-badge> </template> diff --git a/app/assets/javascripts/issuable_list/constants.js b/app/assets/javascripts/issuable_list/constants.js new file mode 100644 index 00000000000..4d3ce61366a --- /dev/null +++ b/app/assets/javascripts/issuable_list/constants.js @@ -0,0 +1,49 @@ +import { __ } from '~/locale'; + +export const IssuableStates = { + Opened: 'opened', + Closed: 'closed', + All: 'all', +}; + +export const IssuableListTabs = [ + { + id: 'state-opened', + name: IssuableStates.Opened, + title: __('Open'), + titleTooltip: __('Filter by issues that are currently opened.'), + }, + { + id: 'state-closed', + name: IssuableStates.Closed, + title: __('Closed'), + titleTooltip: __('Filter by issues that are currently closed.'), + }, + { + id: 'state-all', + name: IssuableStates.All, + title: __('All'), + titleTooltip: __('Show all issues.'), + }, +]; + +export const AvailableSortOptions = [ + { + id: 1, + title: __('Created date'), + sortDirection: { + descending: 'created_desc', + ascending: 'created_asc', + }, + }, + { + id: 2, + title: __('Last updated'), + sortDirection: { + descending: 'updated_desc', + ascending: 'updated_asc', + }, + }, +]; + +export const DEFAULT_PAGE_SIZE = 20; diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f6fdfb1706f..fa909b6638e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -11520,6 +11520,9 @@ msgstr "" msgid "Filter by issues that are currently closed." msgstr "" +msgid "Filter by issues that are currently opened." +msgstr "" + msgid "Filter by label" msgstr "" @@ -24249,6 +24252,9 @@ msgstr "" msgid "Show all activity" msgstr "" +msgid "Show all issues." +msgstr "" + msgid "Show all members" msgstr "" diff --git a/spec/frontend/issuable_list/components/issuable_item_spec.js b/spec/frontend/issuable_list/components/issuable_item_spec.js index a96a4e15e6c..5c8b679e4e1 100644 --- a/spec/frontend/issuable_list/components/issuable_item_spec.js +++ b/spec/frontend/issuable_list/components/issuable_item_spec.js @@ -2,28 +2,34 @@ import { shallowMount } from '@vue/test-utils'; import { GlLink, GlLabel } from '@gitlab/ui'; import IssuableItem from '~/issuable_list/components/issuable_item.vue'; +import IssuableAssignees from '~/vue_shared/components/issue/issue_assignees.vue'; import { mockIssuable, mockRegularLabel, mockScopedLabel } from '../mock_data'; -const createComponent = ({ issuableSymbol = '#', issuable = mockIssuable } = {}) => +const createComponent = ({ issuableSymbol = '#', issuable = mockIssuable, slots = {} } = {}) => shallowMount(IssuableItem, { propsData: { issuableSymbol, issuable, + enableLabelPermalinks: true, }, + slots, }); describe('IssuableItem', () => { const mockLabels = mockIssuable.labels.nodes; const mockAuthor = mockIssuable.author; + const originalUrl = gon.gitlab_url; let wrapper; beforeEach(() => { + gon.gitlab_url = 'http://0.0.0.0:3000'; wrapper = createComponent(); }); afterEach(() => { wrapper.destroy(); + gon.gitlab_url = originalUrl; }); describe('computed', () => { @@ -38,8 +44,8 @@ describe('IssuableItem', () => { authorId | returnValue ${1} | ${1} ${'1'} | ${1} - ${'gid://gitlab/User/1'} | ${'1'} - ${'foo'} | ${''} + ${'gid://gitlab/User/1'} | ${1} + ${'foo'} | ${null} `( 'returns $returnValue when value of `issuable.author.id` is $authorId', async ({ authorId, returnValue }) => { @@ -60,6 +66,30 @@ describe('IssuableItem', () => { ); }); + describe('isIssuableUrlExternal', () => { + it.each` + issuableWebUrl | urlType | returnValue + ${'/gitlab-org/gitlab-test/-/issues/2'} | ${'relative'} | ${false} + ${'http://0.0.0.0:3000/gitlab-org/gitlab-test/-/issues/1'} | ${'absolute and internal'} | ${false} + ${'http://jira.atlassian.net/browse/IG-1'} | ${'external'} | ${true} + ${'https://github.com/gitlabhq/gitlabhq/issues/1'} | ${'external'} | ${true} + `( + 'returns $returnValue when `issuable.webUrl` is $urlType', + async ({ issuableWebUrl, returnValue }) => { + wrapper.setProps({ + issuable: { + ...mockIssuable, + webUrl: issuableWebUrl, + }, + }); + + await wrapper.vm.$nextTick(); + + expect(wrapper.vm.isIssuableUrlExternal).toBe(returnValue); + }, + ); + }); + describe('labels', () => { it('returns `issuable.labels.nodes` reference when it is available', () => { expect(wrapper.vm.labels).toEqual(mockLabels); @@ -92,6 +122,12 @@ describe('IssuableItem', () => { }); }); + describe('assignees', () => { + it('returns `issuable.assignees` reference when it is available', () => { + expect(wrapper.vm.assignees).toBe(mockIssuable.assignees); + }); + }); + describe('createdAt', () => { it('returns string containing timeago string based on `issuable.createdAt`', () => { expect(wrapper.vm.createdAt).toContain('created'); @@ -120,6 +156,34 @@ describe('IssuableItem', () => { }, ); }); + + describe('labelTitle', () => { + it.each` + label | propWithTitle | returnValue + ${{ title: 'foo' }} | ${'title'} | ${'foo'} + ${{ name: 'foo' }} | ${'name'} | ${'foo'} + `('returns string value of `label.$propWithTitle`', ({ label, returnValue }) => { + expect(wrapper.vm.labelTitle(label)).toBe(returnValue); + }); + }); + + describe('labelTarget', () => { + it('returns target string for a provided label param when `enableLabelPermalinks` is true', () => { + expect(wrapper.vm.labelTarget(mockRegularLabel)).toBe( + '?label_name%5B%5D=Documentation%20Update', + ); + }); + + it('returns string "#" for a provided label param when `enableLabelPermalinks` is false', async () => { + wrapper.setProps({ + enableLabelPermalinks: false, + }); + + await wrapper.vm.$nextTick(); + + expect(wrapper.vm.labelTarget(mockRegularLabel)).toBe('#'); + }); + }); }); describe('template', () => { @@ -128,9 +192,28 @@ describe('IssuableItem', () => { expect(titleEl.exists()).toBe(true); expect(titleEl.find(GlLink).attributes('href')).toBe(mockIssuable.webUrl); + expect(titleEl.find(GlLink).attributes('target')).not.toBeDefined(); expect(titleEl.find(GlLink).text()).toBe(mockIssuable.title); }); + it('renders issuable title with `target` set as "_blank" when issuable.webUrl is external', async () => { + wrapper.setProps({ + issuable: { + ...mockIssuable, + webUrl: 'http://jira.atlassian.net/browse/IG-1', + }, + }); + + await wrapper.vm.$nextTick(); + + expect( + wrapper + .find('[data-testid="issuable-title"]') + .find(GlLink) + .attributes('target'), + ).toBe('_blank'); + }); + it('renders issuable reference', () => { const referenceEl = wrapper.find('[data-testid="issuable-reference"]'); @@ -138,6 +221,24 @@ describe('IssuableItem', () => { expect(referenceEl.text()).toBe(`#${mockIssuable.iid}`); }); + it('renders issuable reference via slot', () => { + const wrapperWithRefSlot = createComponent({ + issuableSymbol: '#', + issuable: mockIssuable, + slots: { + reference: ` + <b class="js-reference">${mockIssuable.iid}</b> + `, + }, + }); + const referenceEl = wrapperWithRefSlot.find('.js-reference'); + + expect(referenceEl.exists()).toBe(true); + expect(referenceEl.text()).toBe(`${mockIssuable.iid}`); + + wrapperWithRefSlot.destroy(); + }); + it('renders issuable createdAt info', () => { const createdAtEl = wrapper.find('[data-testid="issuable-created-at"]'); @@ -151,7 +252,7 @@ describe('IssuableItem', () => { expect(authorEl.exists()).toBe(true); expect(authorEl.attributes()).toMatchObject({ - 'data-user-id': wrapper.vm.authorId, + 'data-user-id': `${wrapper.vm.authorId}`, 'data-username': mockAuthor.username, 'data-name': mockAuthor.name, 'data-avatar-url': mockAuthor.avatarUrl, @@ -170,10 +271,40 @@ describe('IssuableItem', () => { title: mockLabels[0].title, description: mockLabels[0].description, scoped: false, + target: wrapper.vm.labelTarget(mockLabels[0]), size: 'sm', }); }); + it('renders issuable status via slot', () => { + const wrapperWithStatusSlot = createComponent({ + issuableSymbol: '#', + issuable: mockIssuable, + slots: { + status: ` + <b class="js-status">${mockIssuable.state}</b> + `, + }, + }); + const statusEl = wrapperWithStatusSlot.find('.js-status'); + + expect(statusEl.exists()).toBe(true); + expect(statusEl.text()).toBe(`${mockIssuable.state}`); + + wrapperWithStatusSlot.destroy(); + }); + + it('renders issuable-assignees component', () => { + const assigneesEl = wrapper.find(IssuableAssignees); + + expect(assigneesEl.exists()).toBe(true); + expect(assigneesEl.props()).toMatchObject({ + assignees: mockIssuable.assignees, + iconSize: 16, + maxVisible: 4, + }); + }); + it('renders issuable updatedAt info', () => { const updatedAtEl = wrapper.find('[data-testid="issuable-updated-at"]'); diff --git a/spec/frontend/issuable_list/components/issuable_list_root_spec.js b/spec/frontend/issuable_list/components/issuable_list_root_spec.js index 34184522b55..e560b0915bc 100644 --- a/spec/frontend/issuable_list/components/issuable_list_root_spec.js +++ b/spec/frontend/issuable_list/components/issuable_list_root_spec.js @@ -1,6 +1,8 @@ import { mount } from '@vue/test-utils'; import { GlLoadingIcon, GlPagination } from '@gitlab/ui'; +import { TEST_HOST } from 'helpers/test_constants'; + import IssuableListRoot from '~/issuable_list/components/issuable_list_root.vue'; import IssuableTabs from '~/issuable_list/components/issuable_tabs.vue'; import IssuableItem from '~/issuable_list/components/issuable_item.vue'; @@ -32,6 +34,29 @@ describe('IssuableListRoot', () => { wrapper.destroy(); }); + describe('watch', () => { + describe('urlParams', () => { + it('updates window URL reflecting props within `urlParams`', async () => { + const urlParams = { + state: 'closed', + sort: 'updated_asc', + page: 1, + search: 'foo', + }; + + wrapper.setProps({ + urlParams, + }); + + await wrapper.vm.$nextTick(); + + expect(global.window.location.href).toBe( + `${TEST_HOST}/?state=${urlParams.state}&sort=${urlParams.sort}&page=${urlParams.page}&search=${urlParams.search}`, + ); + }); + }); + }); + describe('template', () => { it('renders component container element with class "issuable-list-container"', () => { expect(wrapper.classes()).toContain('issuable-list-container'); @@ -114,6 +139,7 @@ describe('IssuableListRoot', () => { it('renders gl-pagination when `showPaginationControls` prop is true', async () => { wrapper.setProps({ showPaginationControls: true, + totalPages: 10, }); await wrapper.vm.$nextTick(); @@ -125,6 +151,7 @@ describe('IssuableListRoot', () => { value: 1, prevPage: 0, nextPage: 2, + totalItems: 10, align: 'center', }); }); diff --git a/spec/frontend/issuable_list/mock_data.js b/spec/frontend/issuable_list/mock_data.js index 8eab2ca6f94..b18e49e2102 100644 --- a/spec/frontend/issuable_list/mock_data.js +++ b/spec/frontend/issuable_list/mock_data.js @@ -51,6 +51,7 @@ export const mockIssuable = { labels: { nodes: mockLabels, }, + assignees: [mockAuthor], }; export const mockIssuables = [ -- 2.30.9