Merge branch 'ce-backport-3772-total-weight' into 'master'

[Backport] View summed weights of issues in board column

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

12 files changed, 113 insertions, 46 deletions

diff --git a/app/assets/javascripts/boards/components/board.js b/app/assets/javascripts/boards/components/board.js
index a2355d7fd5c..9ad451fa375 100644
--- a/app/assets/javascripts/boards/components/board.js
+++ b/app/assets/javascripts/boards/components/board.js
@@ -2,6 +2,9 @@
 
 import Sortable from 'sortablejs';
 import Vue from 'vue';
+import { n__ } from '~/locale';
+import Icon from '~/vue_shared/components/icon.vue';
+import Tooltip from '~/vue_shared/directives/tooltip';
 import AccessorUtilities from '../../lib/utils/accessor';
 import boardList from './board_list.vue';
 import BoardBlankState from './board_blank_state.vue';
@@ -17,6 +20,10 @@ gl.issueBoards.Board = Vue.extend({
     boardList,
     'board-delete': gl.issueBoards.BoardDelete,
     BoardBlankState,
+    Icon,
+  },
+  directives: {
+    Tooltip,
   },
   props: {
     list: {
@@ -46,6 +53,12 @@ gl.issueBoards.Board = Vue.extend({
       filter: Store.filter,
     };
   },
+  computed: {
+    counterTooltip() {
+      const { issuesSize } = this.list;
+      return `${n__('%d issue', '%d issues', issuesSize)}`;
+    },
+  },
   watch: {
     filter: {
       handler() {
diff --git a/app/assets/javascripts/boards/models/list.js b/app/assets/javascripts/boards/models/list.js
index 4f05a0e4282..050cbd8db48 100644
--- a/app/assets/javascripts/boards/models/list.js
+++ b/app/assets/javascripts/boards/models/list.js
@@ -136,6 +136,8 @@ class List {
         }
 
         this.createIssues(data.issues);
+
+        return data;
       });
   }
 
diff --git a/app/assets/javascripts/boards/stores/boards_store.js b/app/assets/javascripts/boards/stores/boards_store.js
index 333338489bc..76467564608 100644
--- a/app/assets/javascripts/boards/stores/boards_store.js
+++ b/app/assets/javascripts/boards/stores/boards_store.js
@@ -125,11 +125,17 @@ gl.issueBoards.BoardsStore = {
     } else if (listTo.type === 'backlog' && listFrom.type === 'assignee') {
       issue.removeAssignee(listFrom.assignee);
       listFrom.removeIssue(issue);
-    } else if ((listTo.type !== 'label' && listFrom.type === 'assignee') ||
-               (listTo.type !== 'assignee' && listFrom.type === 'label')) {
+    } else if (this.shouldRemoveIssue(listFrom, listTo)) {
       listFrom.removeIssue(issue);
     }
   },
+  shouldRemoveIssue(listFrom, listTo) {
+    return (
+      (listTo.type !== 'label' && listFrom.type === 'assignee') ||
+      (listTo.type !== 'assignee' && listFrom.type === 'label') ||
+      (listFrom.type === 'backlog')
+    );
+  },
   moveIssueInList (list, issue, oldIndex, newIndex, idArray) {
     const beforeId = parseInt(idArray[newIndex - 1], 10) || null;
     const afterId = parseInt(idArray[newIndex + 1], 10) || null;
diff --git a/app/assets/stylesheets/framework/typography.scss b/app/assets/stylesheets/framework/typography.scss
index a2789021ab4..473ca408c04 100644
--- a/app/assets/stylesheets/framework/typography.scss
+++ b/app/assets/stylesheets/framework/typography.scss
@@ -444,3 +444,5 @@ textarea {
     color: $placeholder-text-color;
   }
 }
+
+.lh-100 { line-height: 1; }
diff --git a/app/assets/stylesheets/pages/boards.scss b/app/assets/stylesheets/pages/boards.scss
index 7347da2ae61..a68b47b1d02 100644
--- a/app/assets/stylesheets/pages/boards.scss
+++ b/app/assets/stylesheets/pages/boards.scss
@@ -205,7 +205,7 @@
 
 .board-title {
   margin: 0;
-  padding: 12px $gl-padding;
+  padding: $gl-padding-8 $gl-padding;
   font-size: 1em;
   border-bottom: 1px solid $border-color;
   display: flex;
diff --git a/app/assets/stylesheets/pages/issues/issue_count_badge.scss b/app/assets/stylesheets/pages/issues/issue_count_badge.scss
index ccb62bfed18..4fba89e956b 100644
--- a/app/assets/stylesheets/pages/issues/issue_count_badge.scss
+++ b/app/assets/stylesheets/pages/issues/issue_count_badge.scss
@@ -1,29 +1,11 @@
 .issue-count-badge {
   display: inline-flex;
-  align-items: stretch;
-  height: 24px;
-}
-
-.issue-count-badge-count {
-  display: flex;
-  align-items: center;
-  padding-right: 10px;
-  padding-left: 10px;
-  border: 1px solid $border-color;
   border-radius: $border-radius-base;
-  line-height: 1;
-
-  &.has-btn {
-    border-right: 0;
-    border-top-right-radius: 0;
-    border-bottom-right-radius: 0;
-  }
+  border: 1px solid $border-color;
+  padding: 5px $gl-padding-8;
 }
 
-.issue-count-badge-add-button {
-  display: flex;
+.issue-count-badge-count {
+  display: inline-flex;
   align-items: center;
-  border: 1px solid $border-color;
-  border-radius: 0 $border-radius-base $border-radius-base 0;
-  line-height: 1;
 }
diff --git a/app/controllers/boards/issues_controller.rb b/app/controllers/boards/issues_controller.rb
index 09e143c23e8..7dd19f87ef5 100644
--- a/app/controllers/boards/issues_controller.rb
+++ b/app/controllers/boards/issues_controller.rb
@@ -12,8 +12,9 @@ module Boards
     skip_before_action :authenticate_user!, only: [:index]
 
     def index
-      issues = Boards::Issues::ListService.new(board_parent, current_user, filter_params).execute
-      issues = issues.page(params[:page]).per(params[:per] || 20)
+      list_service = Boards::Issues::ListService.new(board_parent, current_user, filter_params)
+      issues = list_service.execute
+      issues = issues.page(params[:page]).per(params[:per] || 20).without_count
       make_sure_position_is_set(issues) if Gitlab::Database.read_write?
       issues = issues.preload(:project,
                               :milestone,
@@ -22,10 +23,7 @@ module Boards
                               notes: [:award_emoji, :author]
                              )
 
-      render json: {
-        issues: serialize_as_json(issues),
-        size: issues.total_count
-      }
+      render_issues(issues, list_service.metadata)
     end
 
     def create
@@ -51,6 +49,13 @@ module Boards
 
     private
 
+    def render_issues(issues, metadata)
+      data = { issues: serialize_as_json(issues) }
+      data.merge!(metadata)
+
+      render json: data
+    end
+
     def make_sure_position_is_set(issues)
       issues.each do |issue|
         issue.move_to_end && issue.save unless issue.relative_position
diff --git a/app/services/boards/issues/list_service.rb b/app/services/boards/issues/list_service.rb
index 50c11be0d15..0db1418b37a 100644
--- a/app/services/boards/issues/list_service.rb
+++ b/app/services/boards/issues/list_service.rb
@@ -3,14 +3,35 @@
 module Boards
   module Issues
     class ListService < Boards::BaseService
+      include Gitlab::Utils::StrongMemoize
+
       def execute
-        issues = IssuesFinder.new(current_user, filter_params).execute
-        issues = filter(issues)
-        issues.order_by_position_and_priority
+        fetch_issues.order_by_position_and_priority
+      end
+
+      def metadata
+        keys = metadata_fields.keys
+        columns = metadata_fields.values_at(*keys).join(', ')
+        results = Issue.where(id: fetch_issues.select('issues.id')).pluck(columns)
+
+        Hash[keys.zip(results.flatten)]
       end
 
       private
 
+      def metadata_fields
+        { size: 'COUNT(*)' }
+      end
+
+      # We memoize the query here since the finder methods we use are quite complex. This does not memoize the result of the query.
+      def fetch_issues
+        strong_memoize(:fetch_issues) do
+          issues = IssuesFinder.new(current_user, filter_params).execute
+
+          filter(issues).reorder(nil)
+        end
+      end
+
       def filter(issues)
         issues = without_board_labels(issues) unless list&.movable? || list&.closed?
         issues = with_list_label(issues) if list&.label?
diff --git a/app/views/shared/boards/components/_board.html.haml b/app/views/shared/boards/components/_board.html.haml
index 03e008f5fa0..b35877e5518 100644
--- a/app/views/shared/boards/components/_board.html.haml
+++ b/app/views/shared/boards/components/_board.html.haml
@@ -32,17 +32,21 @@
             "v-if" => "!list.preset && list.id" }
             %button.board-delete.has-tooltip.float-right{ type: "button", title: _("Delete list"), "aria-label" => _("Delete list"), data: { placement: "bottom" }, "@click.stop" => "deleteBoard" }
               = icon("trash")
-        .issue-count-badge.clearfix{ "v-if" => 'list.type !== "blank" && list.type !== "promotion"' }
-          %span.issue-count-badge-count.float-left{ ":class" => '{ "has-btn": list.type !== "closed" && !disabled }' }
+        .issue-count-badge.text-secondary{ "v-if" => 'list.type !== "blank" && list.type !== "promotion"', ":title": "counterTooltip", "v-tooltip": true, data: { placement: "top" } }
+          %span.issue-count-badge-count
+            %icon.mr-1{ name: "issues" }
             {{ list.issuesSize }}
-          - if can?(current_user, :admin_list, current_board_parent)
-            %button.issue-count-badge-add-button.btn.btn-sm.btn-default.has-tooltip.js-no-trigger-collapse{ type: "button",
-              "@click" => "showNewIssueForm",
-              "v-if" => 'list.type !== "closed"',
-              "aria-label" => _("New issue"),
-              "title" => _("New issue"),
-              data: { placement: "top", container: "body" } }
-              = icon("plus", class: "js-no-trigger-collapse")
+          = render_if_exists "shared/boards/components/list_weight"
+
+        - if can?(current_user, :admin_list, current_board_parent)
+          %button.issue-count-badge-add-button.btn.btn-sm.btn-default.ml-1.has-tooltip.js-no-trigger-collapse{ type: "button",
+            "@click" => "showNewIssueForm",
+            "v-if" => 'list.type !== "closed"',
+            "aria-label" => _("New issue"),
+            "title" => _("New issue"),
+            data: { placement: "top", container: "body" } }
+            = icon("plus", class: "js-no-trigger-collapse")
+
     %board-list{ "v-if" => 'list.type !== "blank" && list.type !== "promotion"',
       ":list" => "list",
       ":issues" => "list.issues",
diff --git a/spec/controllers/boards/issues_controller_spec.rb b/spec/controllers/boards/issues_controller_spec.rb
index ce7762691c9..d98e6ff0df8 100644
--- a/spec/controllers/boards/issues_controller_spec.rb
+++ b/spec/controllers/boards/issues_controller_spec.rb
@@ -42,7 +42,7 @@ describe Boards::IssuesController do
           parsed_response = JSON.parse(response.body)
 
           expect(response).to match_response_schema('issues')
-          expect(parsed_response.length).to eq 2
+          expect(parsed_response['issues'].length).to eq 2
           expect(development.issues.map(&:relative_position)).not_to include(nil)
         end
 
@@ -80,7 +80,7 @@ describe Boards::IssuesController do
         parsed_response = JSON.parse(response.body)
 
         expect(response).to match_response_schema('issues')
-        expect(parsed_response.length).to eq 2
+        expect(parsed_response['issues'].length).to eq 2
       end
     end
 
diff --git a/spec/javascripts/boards/boards_store_spec.js b/spec/javascripts/boards/boards_store_spec.js
index f7af099b3bf..1ee6f4cf680 100644
--- a/spec/javascripts/boards/boards_store_spec.js
+++ b/spec/javascripts/boards/boards_store_spec.js
@@ -161,6 +161,28 @@ describe('Store', () => {
       }, 0);
     });
 
+    it('moves an issue from backlog to a list', (done) => {
+      const backlog = gl.issueBoards.BoardsStore.addList({
+        ...listObj,
+        list_type: 'backlog',
+      });
+      const listTwo = gl.issueBoards.BoardsStore.addList(listObjDuplicate);
+
+      expect(gl.issueBoards.BoardsStore.state.lists.length).toBe(2);
+
+      setTimeout(() => {
+        expect(backlog.issues.length).toBe(1);
+        expect(listTwo.issues.length).toBe(1);
+
+        gl.issueBoards.BoardsStore.moveIssueToList(backlog, listTwo, backlog.findIssue(1));
+
+        expect(backlog.issues.length).toBe(0);
+        expect(listTwo.issues.length).toBe(1);
+
+        done();
+      }, 0);
+    });
+
     it('moves issue to top of another list', (done) => {
       const listOne = gl.issueBoards.BoardsStore.addList(listObj);
       const listTwo = gl.issueBoards.BoardsStore.addList(listObjDuplicate);
diff --git a/spec/support/shared_examples/services/boards/issues_list_service.rb b/spec/support/shared_examples/services/boards/issues_list_service.rb
index 3e744323cea..8b879cef084 100644
--- a/spec/support/shared_examples/services/boards/issues_list_service.rb
+++ b/spec/support/shared_examples/services/boards/issues_list_service.rb
@@ -7,6 +7,16 @@ shared_examples 'issues list service' do
     described_class.new(parent, user, params).execute
   end
 
+  context '#metadata' do
+    it 'returns issues count for list' do
+      params = { board_id: board.id, id: list1.id }
+
+      metadata = described_class.new(parent, user, params).metadata
+
+      expect(metadata[:size]).to eq(3)
+    end
+  end
+
   context 'issues are ordered by priority' do
     it 'returns opened issues when list_id is missing' do
       params = { board_id: board.id }