Merge branch 'dz-labels-subscribe-filter' into 'master'

Add subscribe filter to labels page

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

app/controllers/groups/labels_controller.rb

@@ -12,7 +12,8 @@ class Groups::LabelsController < Groups::ApplicationController
   def index
     respond_to do |format|
       format.html do
-        @labels = GroupLabelsFinder.new(@group, params.merge(sort: sort)).execute
+        @labels = GroupLabelsFinder
+          .new(current_user, @group, params.merge(sort: sort)).execute
       end
       format.json do
         render json: LabelSerializer.new.represent_appearance(available_labels)

app/controllers/projects/labels_controller.rb

@@ -163,6 +163,7 @@ class Projects::LabelsController < Projects::ApplicationController
                        project_id: @project.id,
                        include_ancestor_groups: params[:include_ancestor_groups],
                        search: params[:search],
+                       subscribed: params[:subscribed],
                        sort: sort).execute
   end
 

app/finders/group_labels_finder.rb

 # frozen_string_literal: true
 
 class GroupLabelsFinder
-  attr_reader :group, :params
+  attr_reader :current_user, :group, :params
 
-  def initialize(group, params = {})
+  def initialize(current_user, group, params = {})
+    @current_user = current_user
     @group = group
     @params = params
   end
 
   def execute
     group.labels
+      .optionally_subscribed_by(subscriber_id)
       .optionally_search(params[:search])
       .order_by(params[:sort])
       .page(params[:page])
   end
+
+  private
+
+  def subscriber_id
+    current_user&.id if subscribed?
+  end
+
+  def subscribed?
+    params[:subscribed] == 'true'
+  end
 end

app/finders/labels_finder.rb

@@ -17,6 +17,7 @@ class LabelsFinder < UnionFinder
     @skip_authorization = skip_authorization
     items = find_union(label_ids, Label) || Label.none
     items = with_title(items)
+    items = by_subscription(items)
     items = by_search(items)
     sort(items)
   end
@@ -84,6 +85,18 @@ class LabelsFinder < UnionFinder
     labels.search(params[:search])
   end
 
+  def by_subscription(labels)
+    labels.optionally_subscribed_by(subscriber_id)
+  end
+
+  def subscriber_id
+    current_user&.id if subscribed?
+  end
+
+  def subscribed?
+    params[:subscribed] == 'true'
+  end
+
   # Gets redacted array of group ids
   # which can include the ancestors and descendants of the requested group.
   def group_ids_for(group)

app/helpers/boards_helper.rb

@@ -59,8 +59,8 @@ module BoardsHelper
 
     {
       toggle: "dropdown",
-      list_labels_path: labels_filter_path(true, include_ancestor_groups: true),
-      labels: labels_filter_path(true, include_descendant_groups: include_descendant_groups),
+      list_labels_path: labels_filter_path_with_defaults(only_group_labels: true, include_ancestor_groups: true),
+      labels: labels_filter_path_with_defaults(only_group_labels: true, include_descendant_groups: include_descendant_groups),
       labels_endpoint: @labels_endpoint,
       namespace_path: @namespace_path,
       project_path: @project&.path,

app/helpers/labels_helper.rb

@@ -131,20 +131,26 @@ module LabelsHelper
     end
   end
 
-  def labels_filter_path(only_group_labels = false, include_ancestor_groups: true, include_descendant_groups: false)
-    project = @target_project || @project
-
+  def labels_filter_path_with_defaults(only_group_labels: false, include_ancestor_groups: true, include_descendant_groups: false)
     options = {}
     options[:include_ancestor_groups] = include_ancestor_groups if include_ancestor_groups
     options[:include_descendant_groups] = include_descendant_groups if include_descendant_groups
+    options[:only_group_labels] = only_group_labels if only_group_labels && @group
+    options[:format] = :json
+
+    labels_filter_path(options)
+  end
+
+  def labels_filter_path(options = {})
+    project = @target_project || @project
+    format = options.delete(:format) || :html
 
     if project
-      project_labels_path(project, :json, options)
+      project_labels_path(project, format, options)
     elsif @group
-      options[:only_group_labels] = only_group_labels if only_group_labels
-      group_labels_path(@group, :json, options)
+      group_labels_path(@group, format, options)
     else
-      dashboard_labels_path(:json)
+      dashboard_labels_path(format, options)
     end
   end
 

app/models/label.rb

@@ -45,6 +45,7 @@ class Label < ActiveRecord::Base
   scope :on_project_boards, ->(project_id) { with_lists_and_board.where(boards: { project_id: project_id }) }
   scope :order_name_asc, -> { reorder(title: :asc) }
   scope :order_name_desc, -> { reorder(title: :desc) }
+  scope :subscribed_by, ->(user_id) { joins(:subscriptions).where(subscriptions: { user_id: user_id, subscribed: true }) }
 
   def self.prioritized(project)
     joins(:priorities)
@@ -74,6 +75,14 @@ class Label < ActiveRecord::Base
     joins(label_priorities)
   end
 
+  def self.optionally_subscribed_by(user_id)
+    if user_id
+      subscribed_by(user_id)
+    else
+      all
+    end
+  end
+
   alias_attribute :name, :title
 
   def self.reference_prefix

app/views/groups/labels/index.html.haml

@@ -3,29 +3,23 @@
 - can_admin_label = can?(current_user, :admin_label, @group)
 - issuables = ['issues', 'merge requests']
 - search = params[:search]
+- subscribed = params[:subscribed]
+- labels_or_filters = @labels.exists? || search.present? || subscribed.present?
 
 - if can_admin_label
   - content_for(:header_content) do
     .nav-controls
       = link_to _('New label'), new_group_label_path(@group), class: "btn btn-success"
 
-- if @labels.exists? || search.present?
+- if labels_or_filters
   #promote-label-modal
   %div{ class: container_class }
-    .top-area.adjust
-      .nav-text
-        = _('Labels can be applied to %{features}. Group labels are available for any project within the group.') % { features: issuables.to_sentence }
-      .nav-controls
-        = form_tag group_labels_path(@group), method: :get do
-          .input-group
-            = search_field_tag :search, params[:search], { placeholder: _('Filter'), id: 'label-search', class: 'form-control search-text-input input-short', spellcheck: false }
-            %span.input-group-append
-              %button.btn.btn-default{ type: "submit", "aria-label" => _('Submit search') }
-                = icon("search")
-        = render 'shared/labels/sort_dropdown'
+    = render 'shared/labels/nav'
 
     .labels-container.prepend-top-5
       - if @labels.any?
+        .text-muted
+          = _('Labels can be applied to %{features}. Group labels are available for any project within the group.') % { features: issuables.to_sentence }
         .other-labels
           %h5= _('Labels')
           %ul.content-list.manage-labels-list.js-other-labels
@@ -34,6 +28,9 @@
       - elsif search.present?
         .nothing-here-block
           = _('No labels with such name or description')
+      - elsif subscribed.present?
+        .nothing-here-block
+          = _('You do not have any subscriptions yet')
 - else
   = render 'shared/empty_states/labels'
 

app/views/projects/labels/index.html.haml

@@ -2,32 +2,25 @@
 - page_title "Labels"
 - can_admin_label = can?(current_user, :admin_label, @project)
 - search = params[:search]
+- subscribed = params[:subscribed]
+- labels_or_filters = @labels.exists? || @prioritized_labels.exists? || search.present? || subscribed.present?
 
 - if can_admin_label
   - content_for(:header_content) do
     .nav-controls
       = link_to _('New label'), new_project_label_path(@project), class: "btn btn-success"
 
-- if @labels.exists? || @prioritized_labels.exists? || search.present?
+- if labels_or_filters
   #promote-label-modal
   %div{ class: container_class }
-    .top-area.adjust
-      .nav-text
-        = _('Labels can be applied to issues and merge requests.')
-
-      .nav-controls
-        = form_tag project_labels_path(@project), method: :get do
-          .input-group
-            = search_field_tag :search, params[:search], { placeholder: _('Filter'), id: 'label-search', class: 'form-control search-text-input input-short', spellcheck: false }
-            %span.input-group-append
-              %button.btn.btn-default{ type: "submit", "aria-label" => _('Submit search') }
-                = icon("search")
-        = render 'shared/labels/sort_dropdown'
+    = render 'shared/labels/nav'
 
     .labels-container.prepend-top-10
       - if can_admin_label
         - if search.blank?
           %p.text-muted
+            = _('Labels can be applied to issues and merge requests.')
+            %br
             = _('Star a label to make it a priority label. Order the prioritized labels to change their relative priority, by dragging.')
         -# Only show it in the first page
         - hide = @available_labels.empty? || (params[:page].present? && params[:page] != '1')
@@ -59,7 +52,9 @@
           - else
             .nothing-here-block
               = _('No labels with such name or description')
-
+      - elsif subscribed.present?
+        .nothing-here-block
+          = _('You do not have any subscriptions yet')
 
 - else
   = render 'shared/empty_states/labels'

app/views/shared/boards/_show.html.haml

@@ -33,7 +33,7 @@
   - if @project
     %board-add-issues-modal{ "new-issue-path" => new_project_issue_path(@project),
       "milestone-path" => milestones_filter_dropdown_path,
-      "label-path" => labels_filter_path,
+      "label-path" => labels_filter_path_with_defaults,
       "empty-state-svg" => image_path('illustrations/issues.svg'),
       ":issue-link-base" => "issueLinkBase",
       ":root-path" => "rootPath",

app/views/shared/boards/components/sidebar/_labels.html.haml

@@ -25,7 +25,7 @@
             show_no: "true",
             show_any: "true",
             project_id: @project&.try(:id),
-            labels: labels_filter_path(false),
+            labels: labels_filter_path_with_defaults,
             namespace_path: @namespace_path,
             project_path: @project.try(:path) } }
           %span.dropdown-toggle-text

app/views/shared/issuable/_label_dropdown.html.haml

@@ -8,7 +8,7 @@
 - classes = local_assigns.fetch(:classes, [])
 - selected = local_assigns.fetch(:selected, nil)
 - dropdown_title = local_assigns.fetch(:dropdown_title, "Filter by label")
-- dropdown_data = {toggle: 'dropdown', field_name: "label_name[]", show_no: "true", show_any: "true", namespace_path: @project.try(:namespace).try(:full_path), project_path: @project.try(:path), labels: labels_filter_path, default_label: "Labels"}
+- dropdown_data = {toggle: 'dropdown', field_name: "label_name[]", show_no: "true", show_any: "true", namespace_path: @project.try(:namespace).try(:full_path), project_path: @project.try(:path), labels: labels_filter_path_with_defaults, default_label: "Labels"}
 - dropdown_data.merge!(data_options)
 - label_name = local_assigns.fetch(:label_name, "Labels")
 - no_default_styles = local_assigns.fetch(:no_default_styles, false)

app/views/shared/issuable/_sidebar.html.haml

@@ -109,7 +109,7 @@
             - selected_labels.each do |label|
               = hidden_field_tag "#{issuable.to_ability_name}[label_names][]", label.id, id: nil
             .dropdown
-              %button.dropdown-menu-toggle.js-label-select.js-multiselect.js-label-sidebar-dropdown{ type: "button", data: {toggle: "dropdown", default_label: "Labels", field_name: "#{issuable.to_ability_name}[label_names][]", ability_name: issuable.to_ability_name, show_no: "true", show_any: "true", namespace_path: @project.try(:namespace).try(:full_path), project_path: @project.try(:path), issue_update: issuable_json_path(issuable), labels: (labels_filter_path(false) if @project), display: 'static' } }
+              %button.dropdown-menu-toggle.js-label-select.js-multiselect.js-label-sidebar-dropdown{ type: "button", data: {toggle: "dropdown", default_label: "Labels", field_name: "#{issuable.to_ability_name}[label_names][]", ability_name: issuable.to_ability_name, show_no: "true", show_any: "true", namespace_path: @project.try(:namespace).try(:full_path), project_path: @project.try(:path), issue_update: issuable_json_path(issuable), labels: (labels_filter_path_with_defaults if @project), display: 'static' } }
                 %span.dropdown-toggle-text{ class: ("is-default" if selected_labels.empty?) }
                   = multi_label_name(selected_labels, "Labels")
                 = icon('chevron-down', 'aria-hidden': 'true')

app/views/shared/labels/_nav.html.haml

+- subscribed = params[:subscribed]
+
+.top-area.adjust
+  %ul.nav-links.nav.nav-tabs
+    %li{ class: active_when(subscribed != 'true') }>
+      = link_to labels_filter_path do
+        = _('All')
+    - if current_user
+      %li{ class: active_when(subscribed == 'true') }>
+        = link_to labels_filter_path(subscribed: 'true') do
+          = _('Subscribed')
+  .nav-controls
+    = form_tag labels_filter_path, method: :get do
+      = hidden_field_tag :subscribed, params[:subscribed]
+      .input-group
+        = search_field_tag :search, params[:search], { placeholder: _('Filter'), id: 'label-search', class: 'form-control search-text-input input-short', spellcheck: false }
+        %span.input-group-append
+          %button.btn.btn-default{ type: "submit", "aria-label" => _('Submit search') }
+            = icon("search")
+    = render 'shared/labels/sort_dropdown'

app/views/shared/labels/_sort_dropdown.html.haml

@@ -6,4 +6,4 @@
   %ul.dropdown-menu.dropdown-menu-right.dropdown-menu-sort
     %li
       - label_sort_options_hash.each do |value, title|
-        = sortable_item(title, page_filter_path(sort: value, label: true), sort_title)
+        = sortable_item(title, page_filter_path(sort: value, label: true, subscribed: params[:subscribed]), sort_title)

changelogs/unreleased/dz-labels-subscribe-filter.yml

+---
+title: Add subscribe filter to group and project labels pages
+merge_request: 21965
+author:
+type: added

locale/gitlab.pot

@@ -5786,6 +5786,9 @@ msgstr ""
 msgid "Subscribe at project level"
 msgstr ""
 
+msgid "Subscribed"
+msgstr ""
+
 msgid "Summary of issues, merge requests, push events, and comments (Timezone: %{utcFormatted})"
 msgstr ""
 
@@ -6956,6 +6959,9 @@ msgstr ""
 msgid "You cannot write to this read-only GitLab instance."
 msgstr ""
 
+msgid "You do not have any subscriptions yet"
+msgstr ""
+
 msgid "You don't have any applications"
 msgstr ""
 

spec/features/groups/labels/subscription_spec.rb

@@ -3,7 +3,8 @@ require 'spec_helper'
 describe 'Labels subscription' do
   let(:user)     { create(:user) }
   let(:group)    { create(:group) }
-  let!(:feature) { create(:group_label, group: group, title: 'feature') }
+  let!(:label1)  { create(:group_label, group: group, title: 'foo') }
+  let!(:label2)  { create(:group_label, group: group, title: 'bar') }
 
   context 'when signed in' do
     before do
@@ -14,9 +15,9 @@ describe 'Labels subscription' do
     it 'users can subscribe/unsubscribe to group labels', :js do
       visit group_labels_path(group)
 
-      expect(page).to have_content('feature')
+      expect(page).to have_content(label1.title)
 
-      within "#group_label_#{feature.id}" do
+      within "#group_label_#{label1.id}" do
         expect(page).not_to have_button 'Unsubscribe'
 
         click_button 'Subscribe'
@@ -30,15 +31,48 @@ describe 'Labels subscription' do
         expect(page).not_to have_button 'Unsubscribe'
       end
     end
+
+    context 'subscription filter' do
+      before do
+        visit group_labels_path(group)
+      end
+
+      it 'shows only subscribed labels' do
+        label1.subscribe(user)
+
+        click_subscribed_tab
+
+        page.within('.labels-container') do
+          expect(page).to have_content label1.title
+        end
+      end
+
+      it 'shows no subscribed labels message' do
+        click_subscribed_tab
+
+        page.within('.labels-container') do
+          expect(page).not_to have_content label1.title
+          expect(page).to have_content('You do not have any subscriptions yet')
+        end
+      end
+    end
   end
 
   context 'when not signed in' do
-    it 'users can not subscribe/unsubscribe to labels' do
+    before do
       visit group_labels_path(group)
+    end
 
-      expect(page).to have_content 'feature'
+    it 'users can not subscribe/unsubscribe to labels' do
+      expect(page).to have_content label1.title
       expect(page).not_to have_button('Subscribe')
     end
+
+    it 'does not show subscribed tab' do
+      page.within('.nav-tabs') do
+        expect(page).not_to have_link 'Subscribed'
+      end
+    end
   end
 
   def click_link_on_dropdown(text)
@@ -48,4 +82,10 @@ describe 'Labels subscription' do
       find('a.js-subscribe-button', text: text).click
     end
   end
+
+  def click_subscribed_tab
+    page.within('.nav-tabs') do
+      click_link 'Subscribed'
+    end
+  end
 end

spec/finders/group_labels_finder_spec.rb

@@ -4,29 +4,38 @@ require 'spec_helper'
 
 describe GroupLabelsFinder, '#execute' do
   let!(:group) { create(:group) }
+  let!(:user)  { create(:user) }
   let!(:label1) { create(:group_label, title: 'Foo', description: 'Lorem ipsum', group: group) }
   let!(:label2) { create(:group_label, title: 'Bar', description: 'Fusce consequat', group: group) }
 
   it 'returns all group labels sorted by name if no params' do
-    result = described_class.new(group).execute
+    result = described_class.new(user, group).execute
 
     expect(result.to_a).to match_array([label2, label1])
   end
 
   it 'returns all group labels sorted by name desc' do
-    result = described_class.new(group, sort: 'name_desc').execute
+    result = described_class.new(user, group, sort: 'name_desc').execute
 
     expect(result.to_a).to match_array([label2, label1])
   end
 
-  it 'returns group labels that march search' do
-    result = described_class.new(group, search: 'Foo').execute
+  it 'returns group labels that match search' do
+    result = described_class.new(user, group, search: 'Foo').execute
 
     expect(result.to_a).to match_array([label1])
   end
 
+  it 'returns group labels user subscribed to' do
+    label2.subscribe(user)
+
+    result = described_class.new(user, group, subscribed: 'true').execute
+
+    expect(result.to_a).to match_array([label2])
+  end
+
   it 'returns second page of labels' do
-    result = described_class.new(group, page: '2').execute
+    result = described_class.new(user, group, page: '2').execute
 
     expect(result.to_a).to match_array([])
   end

spec/finders/labels_finder_spec.rb

@@ -210,5 +210,15 @@ describe LabelsFinder do
         expect(finder.execute).to eq [project_label_1]
       end
     end
+
+    context 'filter by subscription' do
+      it 'returns labels user subscribed to' do
+        project_label_1.subscribe(user)
+
+        finder = described_class.new(user, subscribed: 'true')
+
+        expect(finder.execute).to eq [project_label_1]
+      end
+    end
   end
 end

spec/models/label_spec.rb

@@ -155,4 +155,40 @@ describe Label do
       expect(described_class.search('feature')).to be_empty
     end
   end
+
+  describe '.subscribed_by' do
+    let!(:user)   { create(:user) }
+    let!(:label)  { create(:label) }
+    let!(:label2) { create(:label) }
+
+    before do
+      label.subscribe(user)
+    end
+
+    it 'returns subscribed labels' do
+      expect(described_class.subscribed_by(user.id)).to eq([label])
+    end
+
+    it 'returns nothing' do
+      expect(described_class.subscribed_by(0)).to be_empty
+    end
+  end
+
+  describe '.optionally_subscribed_by' do
+    let!(:user)   { create(:user) }
+    let!(:label)  { create(:label) }
+    let!(:label2) { create(:label) }
+
+    before do
+      label.subscribe(user)
+    end
+
+    it 'returns subscribed labels' do
+      expect(described_class.optionally_subscribed_by(user.id)).to eq([label])
+    end
+
+    it 'returns all labels if user_id is nil' do
+      expect(described_class.optionally_subscribed_by(nil)).to match_array([label, label2])
+    end
+  end
 end