Merge branch '39167-async-boards-sidebar' into 'master'

Make Issue Boards sidebar subscriptions async

Closes #39167 and #40094

See merge request gitlab-org/gitlab-ce!15364

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'));
+            });
+        }
       }
     },
   });

app/assets/javascripts/boards/components/board_card.vue

@@ -1,25 +1,11 @@
+<script>
 import './issue_card_inner';
+import eventHub from '../eventhub';
 
 const Store = gl.issueBoards.BoardsStore;
 
 export default {
   name: 'BoardsIssueCard',
-  template: `
-    <li class="card"
-      :class="{ 'user-can-drag': !disabled && issue.id, 'is-disabled': disabled || !issue.id, 'is-active': issueDetailVisible }"
-      :index="index"
-      :data-issue-id="issue.id"
-      @mousedown="mouseDown"
-      @mousemove="mouseMove"
-      @mouseup="showIssue($event)">
-      <issue-card-inner
-        :list="list"
-        :issue="issue"
-        :issue-link-base="issueLinkBase"
-        :root-path="rootPath"
-        :update-filters="true" />
-    </li>
-  `,
   components: {
     'issue-card-inner': gl.issueBoards.IssueCardInner,
   },
@@ -56,12 +42,30 @@ export default {
         this.showDetail = false;
 
         if (Store.detail.issue && Store.detail.issue.id === this.issue.id) {
-          Store.detail.issue = {};
+          eventHub.$emit('clearDetailIssue');
         } else {
-          Store.detail.issue = this.issue;
+          eventHub.$emit('newDetailIssue', this.issue);
           Store.detail.list = this.list;
         }
       }
     },
   },
 };
+</script>
+
+<template>
+  <li class="card"
+    :class="{ 'user-can-drag': !disabled && issue.id, 'is-disabled': disabled || !issue.id, 'is-active': issueDetailVisible }"
+    :index="index"
+    :data-issue-id="issue.id"
+    @mousedown="mouseDown"
+    @mousemove="mouseMove"
+    @mouseup="showIssue($event)">
+    <issue-card-inner
+      :list="list"
+      :issue="issue"
+      :issue-link-base="issueLinkBase"
+      :root-path="rootPath"
+      :update-filters="true" />
+  </li>
+</template>

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';
 

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,
   },
 });

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: {

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;

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();
 };

app/assets/javascripts/main.js

@@ -79,7 +79,6 @@ import './right_sidebar';
 import './search';
 import './search_autocomplete';
 import './smart_interval';
-import './subscription';
 import './subscription_select';
 import initBreadcrumbs from './breadcrumb';
 

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'));
         });
     },
   },

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);
     },
   },
 };

app/assets/javascripts/subscription.js

@@ -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;

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] },

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(

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" }

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

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

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" }

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');
+      vm.detailIssue.issue = vm.issue;
 
-      setTimeout(() => {
-        expect(vm.$el.classList.contains('is-active')).toBe(true);
-        done();
-      }, 0);
+      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');
     });
   });
 });

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) => {

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');