From d2699aea57eaef6adee25bb453a6096b80dda28f Mon Sep 17 00:00:00 2001 From: Oswaldo Ferreira Date: Mon, 13 Nov 2017 21:36:18 -0200 Subject: Add issue sidebar and toggle_subscription endpoint in board issues data --- app/controllers/boards/issues_controller.rb | 1 + app/models/issue.rb | 7 ++++++- spec/fixtures/api/schemas/issue.json | 2 ++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/app/controllers/boards/issues_controller.rb b/app/controllers/boards/issues_controller.rb index 737656b3dcc..f8049b20b9f 100644 --- a/app/controllers/boards/issues_controller.rb +++ b/app/controllers/boards/issues_controller.rb @@ -84,6 +84,7 @@ module Boards resource.as_json( only: [:id, :iid, :project_id, :title, :confidential, :due_date, :relative_position], labels: true, + sidebar_endpoints: true, include: { project: { only: [:id, :path] }, assignees: { only: [:id, :name, :username], methods: [:avatar_url] }, diff --git a/app/models/issue.rb b/app/models/issue.rb index b5abc8f57b0..a9863a50d84 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -246,7 +246,12 @@ class Issue < ActiveRecord::Base def as_json(options = {}) super(options).tap do |json| - json[:subscribed] = subscribed?(options[:user], project) if options.key?(:user) && options[:user] + if options.key?(:sidebar_endpoints) && project + url_helper = Gitlab::Routing.url_helpers + + json.merge!(issue_sidebar_endpoint: url_helper.project_issue_path(project, self, format: :json, serializer: 'sidebar'), + toggle_subscription_endpoint: url_helper.toggle_subscription_project_issue_path(project, self)) + end if options.key?(:labels) json[:labels] = labels.as_json( diff --git a/spec/fixtures/api/schemas/issue.json b/spec/fixtures/api/schemas/issue.json index a55ecaa5697..b579e32c9aa 100644 --- a/spec/fixtures/api/schemas/issue.json +++ b/spec/fixtures/api/schemas/issue.json @@ -13,6 +13,8 @@ "confidential": { "type": "boolean" }, "due_date": { "type": ["date", "null"] }, "relative_position": { "type": "integer" }, + "issue_sidebar_endpoint": { "type": "string" }, + "toggle_subscription_endpoint": { "type": "string" }, "project": { "id": { "type": "integer" }, "path": { "type": "string" } -- cgit v1.2.1 From f494dbc51592766317718c4d656d2b74f9fcb3ab Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 14 Nov 2017 00:05:40 -0600 Subject: Async notification subscriptions in issue boards --- app/assets/javascripts/boards/boards_bundle.js | 52 +++++++++++++++- .../javascripts/boards/components/board_card.js | 67 -------------------- .../javascripts/boards/components/board_card.vue | 71 ++++++++++++++++++++++ .../javascripts/boards/components/board_list.js | 2 +- .../javascripts/boards/components/board_sidebar.js | 11 ++-- app/assets/javascripts/boards/models/issue.js | 13 ++++ .../javascripts/boards/services/board_service.js | 10 ++- app/assets/javascripts/init_issuable_sidebar.js | 1 - app/assets/javascripts/main.js | 1 - .../subscriptions/sidebar_subscriptions.vue | 3 +- .../components/subscriptions/subscriptions.vue | 6 +- app/assets/javascripts/subscription.js | 45 -------------- .../components/sidebar/_notifications.html.haml | 10 ++- .../unreleased/39167-async-boards-sidebar.yml | 5 ++ spec/features/boards/sidebar_spec.rb | 22 ++++++- spec/javascripts/boards/board_card_spec.js | 29 +++++---- spec/javascripts/boards/issue_spec.js | 13 ++++ .../vue_shared/components/loading_button_spec.js | 1 - 18 files changed, 214 insertions(+), 148 deletions(-) delete mode 100644 app/assets/javascripts/boards/components/board_card.js create mode 100644 app/assets/javascripts/boards/components/board_card.vue delete mode 100644 app/assets/javascripts/subscription.js create mode 100644 changelogs/unreleased/39167-async-boards-sidebar.yml diff --git a/app/assets/javascripts/boards/boards_bundle.js b/app/assets/javascripts/boards/boards_bundle.js index ef4093b59e3..20d23162940 100644 --- a/app/assets/javascripts/boards/boards_bundle.js +++ b/app/assets/javascripts/boards/boards_bundle.js @@ -1,12 +1,13 @@ /* eslint-disable one-var, quote-props, comma-dangle, space-before-function-paren */ -/* global BoardService */ import _ from 'underscore'; import Vue from 'vue'; import VueResource from 'vue-resource'; import Flash from '../flash'; +import { __ } from '../locale'; import FilteredSearchBoards from './filtered_search_boards'; import eventHub from './eventhub'; +import sidebarEventHub from '../sidebar/event_hub'; import './models/issue'; import './models/label'; import './models/list'; @@ -14,7 +15,7 @@ import './models/milestone'; import './models/assignee'; import './stores/boards_store'; import './stores/modal_store'; -import './services/board_service'; +import BoardService from './services/board_service'; import './mixins/modal_mixins'; import './mixins/sortable_default_options'; import './filters/due_date_filters'; @@ -77,11 +78,16 @@ $(() => { }); Store.rootPath = this.boardsEndpoint; - // Listen for updateTokens event eventHub.$on('updateTokens', this.updateTokens); + eventHub.$on('newDetailIssue', this.updateDetailIssue); + eventHub.$on('clearDetailIssue', this.clearDetailIssue); + sidebarEventHub.$on('toggleSubscription', this.toggleSubscription); }, beforeDestroy() { eventHub.$off('updateTokens', this.updateTokens); + eventHub.$off('newDetailIssue', this.updateDetailIssue); + eventHub.$off('clearDetailIssue', this.clearDetailIssue); + sidebarEventHub.$off('toggleSubscription', this.toggleSubscription); }, mounted () { this.filterManager = new FilteredSearchBoards(Store.filter, true); @@ -112,6 +118,46 @@ $(() => { methods: { updateTokens() { this.filterManager.updateTokens(); + }, + updateDetailIssue(newIssue) { + const sidebarInfoEndpoint = newIssue.sidebarInfoEndpoint; + if (sidebarInfoEndpoint && newIssue.subscribed === undefined) { + newIssue.setFetchingState('subscriptions', true); + BoardService.getIssueInfo(sidebarInfoEndpoint) + .then(res => res.json()) + .then((data) => { + newIssue.setFetchingState('subscriptions', false); + newIssue.updateData({ + subscribed: data.subscribed, + }); + }) + .catch(() => { + newIssue.setFetchingState('subscriptions', false); + Flash(__('An error occurred while fetching sidebar data')); + }); + } + + Store.detail.issue = newIssue; + }, + clearDetailIssue() { + Store.detail.issue = {}; + }, + toggleSubscription(id) { + const issue = Store.detail.issue; + if (issue.id === id && issue.toggleSubscriptionEndpoint) { + issue.setFetchingState('subscriptions', true); + BoardService.toggleIssueSubscription(issue.toggleSubscriptionEndpoint) + .then(() => { + issue.setFetchingState('subscriptions', false); + issue.updateData({ + subscribed: !issue.subscribed, + }); + }) + .catch(() => { + issue.setFetchingState('subscriptions', false); + Flash(__('An error occurred when toggling the notification subscription')); + }); + } } }, }); diff --git a/app/assets/javascripts/boards/components/board_card.js b/app/assets/javascripts/boards/components/board_card.js deleted file mode 100644 index 079fb6438b9..00000000000 --- a/app/assets/javascripts/boards/components/board_card.js +++ /dev/null @@ -1,67 +0,0 @@ -import './issue_card_inner'; - -const Store = gl.issueBoards.BoardsStore; - -export default { - name: 'BoardsIssueCard', - template: ` -
  • - -
    • - `, - components: { - 'issue-card-inner': gl.issueBoards.IssueCardInner, - }, - props: { - list: Object, - issue: Object, - issueLinkBase: String, - disabled: Boolean, - index: Number, - rootPath: String, - }, - data() { - return { - showDetail: false, - detailIssue: Store.detail, - }; - }, - computed: { - issueDetailVisible() { - return this.detailIssue.issue && this.detailIssue.issue.id === this.issue.id; - }, - }, - methods: { - mouseDown() { - this.showDetail = true; - }, - mouseMove() { - this.showDetail = false; - }, - showIssue(e) { - if (e.target.classList.contains('js-no-trigger')) return; - - if (this.showDetail) { - this.showDetail = false; - - if (Store.detail.issue && Store.detail.issue.id === this.issue.id) { - Store.detail.issue = {}; - } else { - Store.detail.issue = this.issue; - Store.detail.list = this.list; - } - } - }, - }, -}; diff --git a/app/assets/javascripts/boards/components/board_card.vue b/app/assets/javascripts/boards/components/board_card.vue new file mode 100644 index 00000000000..0b220a56e0b --- /dev/null +++ b/app/assets/javascripts/boards/components/board_card.vue @@ -0,0 +1,71 @@ + + + diff --git a/app/assets/javascripts/boards/components/board_list.js b/app/assets/javascripts/boards/components/board_list.js index 6159680f1e6..29aeb8e84aa 100644 --- a/app/assets/javascripts/boards/components/board_list.js +++ b/app/assets/javascripts/boards/components/board_list.js @@ -1,6 +1,6 @@ /* global Sortable */ import boardNewIssue from './board_new_issue'; -import boardCard from './board_card'; +import boardCard from './board_card.vue'; import eventHub from '../eventhub'; import loadingIcon from '../../vue_shared/components/loading_icon.vue'; diff --git a/app/assets/javascripts/boards/components/board_sidebar.js b/app/assets/javascripts/boards/components/board_sidebar.js index 9ae5e270a4b..faa76da964f 100644 --- a/app/assets/javascripts/boards/components/board_sidebar.js +++ b/app/assets/javascripts/boards/components/board_sidebar.js @@ -5,12 +5,13 @@ import Vue from 'vue'; import Flash from '../../flash'; import eventHub from '../../sidebar/event_hub'; -import AssigneeTitle from '../../sidebar/components/assignees/assignee_title'; -import Assignees from '../../sidebar/components/assignees/assignees'; +import assigneeTitle from '../../sidebar/components/assignees/assignee_title'; +import assignees from '../../sidebar/components/assignees/assignees'; import DueDateSelectors from '../../due_date_select'; import './sidebar/remove_issue'; import IssuableContext from '../../issuable_context'; import LabelsSelect from '../../labels_select'; +import subscriptions from '../../sidebar/components/subscriptions/subscriptions.vue'; const Store = gl.issueBoards.BoardsStore; @@ -117,11 +118,11 @@ gl.issueBoards.BoardSidebar = Vue.extend({ new DueDateSelectors(); new LabelsSelect(); new Sidebar(); - gl.Subscription.bindAll('.subscription'); }, components: { + assigneeTitle, + assignees, removeBtn: gl.issueBoards.RemoveIssueBtn, - 'assignee-title': AssigneeTitle, - assignees: Assignees, + subscriptions, }, }); diff --git a/app/assets/javascripts/boards/models/issue.js b/app/assets/javascripts/boards/models/issue.js index 407db176446..10f85c1d676 100644 --- a/app/assets/javascripts/boards/models/issue.js +++ b/app/assets/javascripts/boards/models/issue.js @@ -17,6 +17,11 @@ class ListIssue { this.assignees = []; this.selected = false; this.position = obj.relative_position || Infinity; + this.isFetching = { + subscriptions: true, + }; + this.sidebarInfoEndpoint = obj.issue_sidebar_endpoint; + this.toggleSubscriptionEndpoint = obj.toggle_subscription_endpoint; if (obj.milestone) { this.milestone = new ListMilestone(obj.milestone); @@ -73,6 +78,14 @@ class ListIssue { return gl.issueBoards.BoardsStore.state.lists.filter(list => list.findIssue(this.id)); } + updateData(newData) { + Object.assign(this, newData); + } + + setFetchingState(key, value) { + this.isFetching[key] = value; + } + update (url) { const data = { issue: { diff --git a/app/assets/javascripts/boards/services/board_service.js b/app/assets/javascripts/boards/services/board_service.js index 97e80afa3f8..fa7ddd25e1f 100644 --- a/app/assets/javascripts/boards/services/board_service.js +++ b/app/assets/javascripts/boards/services/board_service.js @@ -2,7 +2,7 @@ import Vue from 'vue'; -class BoardService { +export default class BoardService { constructor ({ boardsEndpoint, listsEndpoint, bulkUpdatePath, boardId }) { this.boards = Vue.resource(`${boardsEndpoint}{/id}.json`, {}, { issues: { @@ -88,6 +88,14 @@ class BoardService { return this.issues.bulkUpdate(data); } + + static getIssueInfo(endpoint) { + return Vue.http.get(endpoint); + } + + static toggleIssueSubscription(endpoint) { + return Vue.http.post(endpoint); + } } window.BoardService = BoardService; diff --git a/app/assets/javascripts/init_issuable_sidebar.js b/app/assets/javascripts/init_issuable_sidebar.js index 1191e0b895e..ada693afc46 100644 --- a/app/assets/javascripts/init_issuable_sidebar.js +++ b/app/assets/javascripts/init_issuable_sidebar.js @@ -14,7 +14,6 @@ export default () => { }); new LabelsSelect(); new IssuableContext(sidebarOptions.currentUser); - gl.Subscription.bindAll('.subscription'); new DueDateSelectors(); window.sidebar = new Sidebar(); }; diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index cef79eec273..8dc0674d9b9 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -80,7 +80,6 @@ import './right_sidebar'; import './search'; import './search_autocomplete'; import './smart_interval'; -import './subscription'; import './subscription_select'; import initBreadcrumbs from './breadcrumb'; diff --git a/app/assets/javascripts/sidebar/components/subscriptions/sidebar_subscriptions.vue b/app/assets/javascripts/sidebar/components/subscriptions/sidebar_subscriptions.vue index 4ad3d469f25..25acc099699 100644 --- a/app/assets/javascripts/sidebar/components/subscriptions/sidebar_subscriptions.vue +++ b/app/assets/javascripts/sidebar/components/subscriptions/sidebar_subscriptions.vue @@ -3,6 +3,7 @@ import Store from '../../stores/sidebar_store'; import Mediator from '../../sidebar_mediator'; import eventHub from '../../event_hub'; import Flash from '../../../flash'; +import { __ } from '../../../locale'; import subscriptions from './subscriptions.vue'; export default { @@ -21,7 +22,7 @@ export default { onToggleSubscription() { this.mediator.toggleSubscription() .catch(() => { - Flash('Error occurred when toggling the notification subscription'); + Flash(__('Error occurred when toggling the notification subscription')); }); }, }, diff --git a/app/assets/javascripts/sidebar/components/subscriptions/subscriptions.vue b/app/assets/javascripts/sidebar/components/subscriptions/subscriptions.vue index a3a8213d63a..940e1764f3d 100644 --- a/app/assets/javascripts/sidebar/components/subscriptions/subscriptions.vue +++ b/app/assets/javascripts/sidebar/components/subscriptions/subscriptions.vue @@ -14,6 +14,10 @@ export default { type: Boolean, required: false, }, + id: { + type: Number, + required: false, + }, }, components: { loadingButton, @@ -32,7 +36,7 @@ export default { }, methods: { toggleSubscription() { - eventHub.$emit('toggleSubscription'); + eventHub.$emit('toggleSubscription', this.id); }, }, }; diff --git a/app/assets/javascripts/subscription.js b/app/assets/javascripts/subscription.js deleted file mode 100644 index bb4d68fcd49..00000000000 --- a/app/assets/javascripts/subscription.js +++ /dev/null @@ -1,45 +0,0 @@ -class Subscription { - constructor(containerElm) { - this.containerElm = containerElm; - - const subscribeButton = containerElm.querySelector('.js-subscribe-button'); - if (subscribeButton) { - // remove class so we don't bind twice - subscribeButton.classList.remove('js-subscribe-button'); - subscribeButton.addEventListener('click', this.toggleSubscription.bind(this)); - } - } - - toggleSubscription(event) { - const button = event.currentTarget; - const buttonSpan = button.querySelector('span'); - if (!buttonSpan || button.classList.contains('disabled')) { - return; - } - button.classList.add('disabled'); - - const isSubscribed = buttonSpan.innerHTML.trim().toLowerCase() !== 'subscribe'; - const toggleActionUrl = this.containerElm.dataset.url; - - $.post(toggleActionUrl, () => { - button.classList.remove('disabled'); - - // hack to allow this to work with the issue boards Vue object - if (document.querySelector('html').classList.contains('issue-boards-page')) { - gl.issueBoards.boardStoreIssueSet( - 'subscribed', - !gl.issueBoards.BoardsStore.detail.issue.subscribed, - ); - } else { - buttonSpan.innerHTML = isSubscribed ? 'Subscribe' : 'Unsubscribe'; - } - }); - } - - static bindAll(selector) { - [].forEach.call(document.querySelectorAll(selector), elm => new Subscription(elm)); - } -} - -window.gl = window.gl || {}; -window.gl.Subscription = Subscription; diff --git a/app/views/shared/boards/components/sidebar/_notifications.html.haml b/app/views/shared/boards/components/sidebar/_notifications.html.haml index 9b989c23cab..333dd1a00b4 100644 --- a/app/views/shared/boards/components/sidebar/_notifications.html.haml +++ b/app/views/shared/boards/components/sidebar/_notifications.html.haml @@ -1,7 +1,5 @@ - if current_user - .block.light.subscription{ ":data-url" => "'#{build_issue_link_base}/' + issue.iid + '/toggle_subscription'" } - %span.issuable-header-text.hide-collapsed.pull-left - Notifications - %button.btn.btn-default.pull-right.js-subscribe-button.issuable-subscribe-button.hide-collapsed{ type: "button" } - %span - {{issue.subscribed ? 'Unsubscribe' : 'Subscribe'}} + .block.subscriptions + %subscriptions{ ":loading" => "issue.isFetching && issue.isFetching.subscriptions", + ":subscribed" => "issue.subscribed", + ":id" => "issue.id" } diff --git a/changelogs/unreleased/39167-async-boards-sidebar.yml b/changelogs/unreleased/39167-async-boards-sidebar.yml new file mode 100644 index 00000000000..dc77f1ad451 --- /dev/null +++ b/changelogs/unreleased/39167-async-boards-sidebar.yml @@ -0,0 +1,5 @@ +--- +title: Update Issue Boards to fetch the notification subscription status asynchronously +merge_request: +author: +type: performance diff --git a/spec/features/boards/sidebar_spec.rb b/spec/features/boards/sidebar_spec.rb index 9137ab82ff4..205900615c4 100644 --- a/spec/features/boards/sidebar_spec.rb +++ b/spec/features/boards/sidebar_spec.rb @@ -331,11 +331,29 @@ describe 'Issue Boards', :js do context 'subscription' do it 'changes issue subscription' do click_card(card) + wait_for_requests - page.within('.subscription') do + page.within('.subscriptions') do click_button 'Subscribe' wait_for_requests - expect(page).to have_content("Unsubscribe") + + expect(page).to have_content('Unsubscribe') + end + end + + it 'has "Unsubscribe" button when already subscribed' do + create(:subscription, user: user, project: project, subscribable: issue2, subscribed: true) + visit project_board_path(project, board) + wait_for_requests + + click_card(card) + wait_for_requests + + page.within('.subscriptions') do + click_button 'Unsubscribe' + wait_for_requests + + expect(page).to have_content('Subscribe') end end end diff --git a/spec/javascripts/boards/board_card_spec.js b/spec/javascripts/boards/board_card_spec.js index 83b13b06dc1..8f607899b20 100644 --- a/spec/javascripts/boards/board_card_spec.js +++ b/spec/javascripts/boards/board_card_spec.js @@ -9,10 +9,11 @@ import Vue from 'vue'; import '~/boards/models/assignee'; +import eventHub from '~/boards/eventhub'; import '~/boards/models/list'; import '~/boards/models/label'; import '~/boards/stores/boards_store'; -import boardCard from '~/boards/components/board_card'; +import boardCard from '~/boards/components/board_card.vue'; import './mock_data'; describe('Board card', () => { @@ -157,33 +158,35 @@ describe('Board card', () => { }); it('sets detail issue to card issue on mouse up', () => { + spyOn(eventHub, '$emit'); + triggerEvent('mousedown'); triggerEvent('mouseup'); - expect(gl.issueBoards.BoardsStore.detail.issue).toEqual(vm.issue); + expect(eventHub.$emit).toHaveBeenCalledWith('newDetailIssue', vm.issue); expect(gl.issueBoards.BoardsStore.detail.list).toEqual(vm.list); }); it('adds active class if detail issue is set', (done) => { - triggerEvent('mousedown'); - triggerEvent('mouseup'); - - setTimeout(() => { - expect(vm.$el.classList.contains('is-active')).toBe(true); - done(); - }, 0); + vm.detailIssue.issue = vm.issue; + + Vue.nextTick() + .then(() => { + expect(vm.$el.classList.contains('is-active')).toBe(true); + }) + .then(done) + .catch(done.fail); }); it('resets detail issue to empty if already set', () => { - triggerEvent('mousedown'); - triggerEvent('mouseup'); + spyOn(eventHub, '$emit'); - expect(gl.issueBoards.BoardsStore.detail.issue).toEqual(vm.issue); + gl.issueBoards.BoardsStore.detail.issue = vm.issue; triggerEvent('mousedown'); triggerEvent('mouseup'); - expect(gl.issueBoards.BoardsStore.detail.issue).toEqual({}); + expect(eventHub.$emit).toHaveBeenCalledWith('clearDetailIssue'); }); }); }); diff --git a/spec/javascripts/boards/issue_spec.js b/spec/javascripts/boards/issue_spec.js index 022d286d5df..ccde657789a 100644 --- a/spec/javascripts/boards/issue_spec.js +++ b/spec/javascripts/boards/issue_spec.js @@ -133,6 +133,19 @@ describe('Issue model', () => { expect(relativePositionIssue.position).toBe(1); }); + it('updates data', () => { + issue.updateData({ subscribed: true }); + expect(issue.subscribed).toBe(true); + }); + + it('sets fetching state', () => { + expect(issue.isFetching.subscriptions).toBe(true); + + issue.setFetchingState('subscriptions', false); + + expect(issue.isFetching.subscriptions).toBe(false); + }); + describe('update', () => { it('passes assignee ids when there are assignees', (done) => { spyOn(Vue.http, 'patch').and.callFake((url, data) => { diff --git a/spec/javascripts/vue_shared/components/loading_button_spec.js b/spec/javascripts/vue_shared/components/loading_button_spec.js index c1eabdede00..49bf8ee6f7c 100644 --- a/spec/javascripts/vue_shared/components/loading_button_spec.js +++ b/spec/javascripts/vue_shared/components/loading_button_spec.js @@ -98,7 +98,6 @@ describe('LoadingButton', function () { it('does not call given callback when disabled because of loading', () => { vm = mountComponent(LoadingButton, { loading: true, - indeterminate: true, }); spyOn(vm, '$emit'); -- cgit v1.2.1