diff options
author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2018-09-27 17:19:28 +0300 |
---|---|---|
committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2018-10-04 11:24:50 +0300 |
commit | 600a10b9d14c0d581efd270a68944957af762a17 (patch) | |
tree | d7f4fc6bf5abc9f1199b4256eb70f099f2e8c33c | |
parent | 2243e4d4ca4bbd96a05059d463d283d468a3fc57 (diff) |
Add subscribe filter to labels page
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
-rw-r--r-- | app/controllers/groups/labels_controller.rb | 3 | ||||
-rw-r--r-- | app/controllers/projects/labels_controller.rb | 1 | ||||
-rw-r--r-- | app/finders/group_labels_finder.rb | 16 | ||||
-rw-r--r-- | app/finders/labels_finder.rb | 13 | ||||
-rw-r--r-- | app/models/label.rb | 9 | ||||
-rw-r--r-- | app/views/groups/labels/index.html.haml | 20 | ||||
-rw-r--r-- | app/views/projects/labels/index.html.haml | 21 | ||||
-rw-r--r-- | app/views/shared/labels/_sort_dropdown.html.haml | 2 | ||||
-rw-r--r-- | changelogs/unreleased/dz-labels-subscribe-filter.yml | 5 | ||||
-rw-r--r-- | locale/gitlab.pot | 6 | ||||
-rw-r--r-- | spec/features/groups/labels/subscription_spec.rb | 50 | ||||
-rw-r--r-- | spec/finders/group_labels_finder_spec.rb | 19 | ||||
-rw-r--r-- | spec/finders/labels_finder_spec.rb | 10 | ||||
-rw-r--r-- | spec/models/label_spec.rb | 36 |
14 files changed, 190 insertions, 21 deletions
diff --git a/app/controllers/groups/labels_controller.rb b/app/controllers/groups/labels_controller.rb index cb9ab35de85..26768c628ca 100644 --- a/app/controllers/groups/labels_controller.rb +++ b/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) diff --git a/app/controllers/projects/labels_controller.rb b/app/controllers/projects/labels_controller.rb index a0ce3b08d9f..640038818f2 100644 --- a/app/controllers/projects/labels_controller.rb +++ b/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 diff --git a/app/finders/group_labels_finder.rb b/app/finders/group_labels_finder.rb index 903023033ed..a668a0f0fae 100644 --- a/app/finders/group_labels_finder.rb +++ b/app/finders/group_labels_finder.rb @@ -1,17 +1,29 @@ # 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 diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb index 08fc2968e77..82e0b2ed9e1 100644 --- a/app/finders/labels_finder.rb +++ b/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) diff --git a/app/models/label.rb b/app/models/label.rb index 9ef57a05b3e..43b49445765 100644 --- a/app/models/label.rb +++ b/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 diff --git a/app/views/groups/labels/index.html.haml b/app/views/groups/labels/index.html.haml index 003bd25dd06..77be21ae2d0 100644 --- a/app/views/groups/labels/index.html.haml +++ b/app/views/groups/labels/index.html.haml @@ -3,20 +3,29 @@ - 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 } + %ul.nav-links.nav.nav-tabs + %li{ class: active_when(subscribed != 'true') }> + = link_to group_labels_path(@group) do + = _('All') + - if current_user + %li{ class: active_when(subscribed == 'true') }> + = link_to group_labels_path(@group, subscribed: 'true') do + = _('Subscribed') .nav-controls = form_tag group_labels_path(@group), 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 @@ -26,6 +35,8 @@ .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 +45,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' diff --git a/app/views/projects/labels/index.html.haml b/app/views/projects/labels/index.html.haml index 683dda4f166..c6b98c03ce5 100644 --- a/app/views/projects/labels/index.html.haml +++ b/app/views/projects/labels/index.html.haml @@ -2,21 +2,30 @@ - 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.') + %ul.nav-links.nav.nav-tabs + %li{ class: active_when(subscribed != 'true') }> + = link_to project_labels_path(@project) do + = _('All') + - if current_user + %li{ class: active_when(subscribed == 'true') }> + = link_to project_labels_path(@project, subscribed: 'true') do + = _('Subscribed') .nav-controls = form_tag project_labels_path(@project), 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 @@ -28,6 +37,8 @@ - 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 +70,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' diff --git a/app/views/shared/labels/_sort_dropdown.html.haml b/app/views/shared/labels/_sort_dropdown.html.haml index ff6e2947ffd..8a7d037e15b 100644 --- a/app/views/shared/labels/_sort_dropdown.html.haml +++ b/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) diff --git a/changelogs/unreleased/dz-labels-subscribe-filter.yml b/changelogs/unreleased/dz-labels-subscribe-filter.yml new file mode 100644 index 00000000000..768c20c77c7 --- /dev/null +++ b/changelogs/unreleased/dz-labels-subscribe-filter.yml @@ -0,0 +1,5 @@ +--- +title: Add subscribe filter to group and project labels pages +merge_request: 21965 +author: +type: added diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 93c7ff32946..47ea026a447 100644 --- a/locale/gitlab.pot +++ b/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 "" @@ -6962,6 +6965,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 "" diff --git a/spec/features/groups/labels/subscription_spec.rb b/spec/features/groups/labels/subscription_spec.rb index d9543bfa97f..22b51b297a6 100644 --- a/spec/features/groups/labels/subscription_spec.rb +++ b/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 diff --git a/spec/finders/group_labels_finder_spec.rb b/spec/finders/group_labels_finder_spec.rb index ef68fc105e4..7bdd312eff0 100644 --- a/spec/finders/group_labels_finder_spec.rb +++ b/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 diff --git a/spec/finders/labels_finder_spec.rb b/spec/finders/labels_finder_spec.rb index f5cec8e349a..9abc52aa664 100644 --- a/spec/finders/labels_finder_spec.rb +++ b/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 diff --git a/spec/models/label_spec.rb b/spec/models/label_spec.rb index 99670af786a..3fc6c06b7fa 100644 --- a/spec/models/label_spec.rb +++ b/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 |