Commit 600a10b9 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Add subscribe filter to labels page

Signed-off-by: default avatarDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
parent 2243e4d4
...@@ -12,7 +12,8 @@ class Groups::LabelsController < Groups::ApplicationController ...@@ -12,7 +12,8 @@ class Groups::LabelsController < Groups::ApplicationController
def index def index
respond_to do |format| respond_to do |format|
format.html do format.html do
@labels = GroupLabelsFinder.new(@group, params.merge(sort: sort)).execute @labels = GroupLabelsFinder
.new(current_user, @group, params.merge(sort: sort)).execute
end end
format.json do format.json do
render json: LabelSerializer.new.represent_appearance(available_labels) render json: LabelSerializer.new.represent_appearance(available_labels)
......
...@@ -163,6 +163,7 @@ class Projects::LabelsController < Projects::ApplicationController ...@@ -163,6 +163,7 @@ class Projects::LabelsController < Projects::ApplicationController
project_id: @project.id, project_id: @project.id,
include_ancestor_groups: params[:include_ancestor_groups], include_ancestor_groups: params[:include_ancestor_groups],
search: params[:search], search: params[:search],
subscribed: params[:subscribed],
sort: sort).execute sort: sort).execute
end end
......
# frozen_string_literal: true # frozen_string_literal: true
class GroupLabelsFinder 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 @group = group
@params = params @params = params
end end
def execute def execute
group.labels group.labels
.optionally_subscribed_by(subscriber_id)
.optionally_search(params[:search]) .optionally_search(params[:search])
.order_by(params[:sort]) .order_by(params[:sort])
.page(params[:page]) .page(params[:page])
end end
private
def subscriber_id
current_user&.id if subscribed?
end
def subscribed?
params[:subscribed] == 'true'
end
end end
...@@ -17,6 +17,7 @@ class LabelsFinder < UnionFinder ...@@ -17,6 +17,7 @@ class LabelsFinder < UnionFinder
@skip_authorization = skip_authorization @skip_authorization = skip_authorization
items = find_union(label_ids, Label) || Label.none items = find_union(label_ids, Label) || Label.none
items = with_title(items) items = with_title(items)
items = by_subscription(items)
items = by_search(items) items = by_search(items)
sort(items) sort(items)
end end
...@@ -84,6 +85,18 @@ class LabelsFinder < UnionFinder ...@@ -84,6 +85,18 @@ class LabelsFinder < UnionFinder
labels.search(params[:search]) labels.search(params[:search])
end 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 # Gets redacted array of group ids
# which can include the ancestors and descendants of the requested group. # which can include the ancestors and descendants of the requested group.
def group_ids_for(group) def group_ids_for(group)
......
...@@ -45,6 +45,7 @@ class Label < ActiveRecord::Base ...@@ -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 :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_asc, -> { reorder(title: :asc) }
scope :order_name_desc, -> { reorder(title: :desc) } 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) def self.prioritized(project)
joins(:priorities) joins(:priorities)
...@@ -74,6 +75,14 @@ class Label < ActiveRecord::Base ...@@ -74,6 +75,14 @@ class Label < ActiveRecord::Base
joins(label_priorities) joins(label_priorities)
end end
def self.optionally_subscribed_by(user_id)
if user_id
subscribed_by(user_id)
else
all
end
end
alias_attribute :name, :title alias_attribute :name, :title
def self.reference_prefix def self.reference_prefix
......
...@@ -3,20 +3,29 @@ ...@@ -3,20 +3,29 @@
- can_admin_label = can?(current_user, :admin_label, @group) - can_admin_label = can?(current_user, :admin_label, @group)
- issuables = ['issues', 'merge requests'] - issuables = ['issues', 'merge requests']
- search = params[:search] - search = params[:search]
- subscribed = params[:subscribed]
- labels_or_filters = @labels.exists? || search.present? || subscribed.present?
- if can_admin_label - if can_admin_label
- content_for(:header_content) do - content_for(:header_content) do
.nav-controls .nav-controls
= link_to _('New label'), new_group_label_path(@group), class: "btn btn-success" = 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 #promote-label-modal
%div{ class: container_class } %div{ class: container_class }
.top-area.adjust .top-area.adjust
.nav-text %ul.nav-links.nav.nav-tabs
= _('Labels can be applied to %{features}. Group labels are available for any project within the group.') % { features: issuables.to_sentence } %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 .nav-controls
= form_tag group_labels_path(@group), method: :get do = form_tag group_labels_path(@group), method: :get do
= hidden_field_tag :subscribed, params[:subscribed]
.input-group .input-group
= search_field_tag :search, params[:search], { placeholder: _('Filter'), id: 'label-search', class: 'form-control search-text-input input-short', spellcheck: false } = 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 %span.input-group-append
...@@ -26,6 +35,8 @@ ...@@ -26,6 +35,8 @@
.labels-container.prepend-top-5 .labels-container.prepend-top-5
- if @labels.any? - 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 .other-labels
%h5= _('Labels') %h5= _('Labels')
%ul.content-list.manage-labels-list.js-other-labels %ul.content-list.manage-labels-list.js-other-labels
...@@ -34,6 +45,9 @@ ...@@ -34,6 +45,9 @@
- elsif search.present? - elsif search.present?
.nothing-here-block .nothing-here-block
= _('No labels with such name or description') = _('No labels with such name or description')
- elsif subscribed.present?
.nothing-here-block
= _('You do not have any subscriptions yet')
- else - else
= render 'shared/empty_states/labels' = render 'shared/empty_states/labels'
......
...@@ -2,21 +2,30 @@ ...@@ -2,21 +2,30 @@
- page_title "Labels" - page_title "Labels"
- can_admin_label = can?(current_user, :admin_label, @project) - can_admin_label = can?(current_user, :admin_label, @project)
- search = params[:search] - search = params[:search]
- subscribed = params[:subscribed]
- labels_or_filters = @labels.exists? || @prioritized_labels.exists? || search.present? || subscribed.present?
- if can_admin_label - if can_admin_label
- content_for(:header_content) do - content_for(:header_content) do
.nav-controls .nav-controls
= link_to _('New label'), new_project_label_path(@project), class: "btn btn-success" = 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 #promote-label-modal
%div{ class: container_class } %div{ class: container_class }
.top-area.adjust .top-area.adjust
.nav-text %ul.nav-links.nav.nav-tabs
= _('Labels can be applied to issues and merge requests.') %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 .nav-controls
= form_tag project_labels_path(@project), method: :get do = form_tag project_labels_path(@project), method: :get do
= hidden_field_tag :subscribed, params[:subscribed]
.input-group .input-group
= search_field_tag :search, params[:search], { placeholder: _('Filter'), id: 'label-search', class: 'form-control search-text-input input-short', spellcheck: false } = 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 %span.input-group-append
...@@ -28,6 +37,8 @@ ...@@ -28,6 +37,8 @@
- if can_admin_label - if can_admin_label
- if search.blank? - if search.blank?
%p.text-muted %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.') = _('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 -# Only show it in the first page
- hide = @available_labels.empty? || (params[:page].present? && params[:page] != '1') - hide = @available_labels.empty? || (params[:page].present? && params[:page] != '1')
...@@ -59,7 +70,9 @@ ...@@ -59,7 +70,9 @@
- else - else
.nothing-here-block .nothing-here-block
= _('No labels with such name or description') = _('No labels with such name or description')
- elsif subscribed.present?
.nothing-here-block
= _('You do not have any subscriptions yet')
- else - else
= render 'shared/empty_states/labels' = render 'shared/empty_states/labels'
......
...@@ -6,4 +6,4 @@ ...@@ -6,4 +6,4 @@
%ul.dropdown-menu.dropdown-menu-right.dropdown-menu-sort %ul.dropdown-menu.dropdown-menu-right.dropdown-menu-sort
%li %li
- label_sort_options_hash.each do |value, title| - 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)
---
title: Add subscribe filter to group and project labels pages
merge_request: 21965
author:
type: added
...@@ -5786,6 +5786,9 @@ msgstr "" ...@@ -5786,6 +5786,9 @@ msgstr ""
msgid "Subscribe at project level" msgid "Subscribe at project level"
msgstr "" msgstr ""
msgid "Subscribed"
msgstr ""
msgid "Summary of issues, merge requests, push events, and comments (Timezone: %{utcFormatted})" msgid "Summary of issues, merge requests, push events, and comments (Timezone: %{utcFormatted})"
msgstr "" msgstr ""
...@@ -6962,6 +6965,9 @@ msgstr "" ...@@ -6962,6 +6965,9 @@ msgstr ""
msgid "You cannot write to this read-only GitLab instance." msgid "You cannot write to this read-only GitLab instance."
msgstr "" msgstr ""
msgid "You do not have any subscriptions yet"
msgstr ""
msgid "You don't have any applications" msgid "You don't have any applications"
msgstr "" msgstr ""
......
...@@ -3,7 +3,8 @@ require 'spec_helper' ...@@ -3,7 +3,8 @@ require 'spec_helper'
describe 'Labels subscription' do describe 'Labels subscription' do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:group) { create(:group) } 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 context 'when signed in' do
before do before do
...@@ -14,9 +15,9 @@ describe 'Labels subscription' do ...@@ -14,9 +15,9 @@ describe 'Labels subscription' do
it 'users can subscribe/unsubscribe to group labels', :js do it 'users can subscribe/unsubscribe to group labels', :js do
visit group_labels_path(group) 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' expect(page).not_to have_button 'Unsubscribe'
click_button 'Subscribe' click_button 'Subscribe'
...@@ -30,15 +31,48 @@ describe 'Labels subscription' do ...@@ -30,15 +31,48 @@ describe 'Labels subscription' do
expect(page).not_to have_button 'Unsubscribe' expect(page).not_to have_button 'Unsubscribe'
end end
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 end
context 'when not signed in' do context 'when not signed in' do
it 'users can not subscribe/unsubscribe to labels' do before do
visit group_labels_path(group) 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') expect(page).not_to have_button('Subscribe')
end end
it 'does not show subscribed tab' do
page.within('.nav-tabs') do
expect(page).not_to have_link 'Subscribed'
end
end
end end
def click_link_on_dropdown(text) def click_link_on_dropdown(text)
...@@ -48,4 +82,10 @@ describe 'Labels subscription' do ...@@ -48,4 +82,10 @@ describe 'Labels subscription' do
find('a.js-subscribe-button', text: text).click find('a.js-subscribe-button', text: text).click
end end
end end
def click_subscribed_tab
page.within('.nav-tabs') do
click_link 'Subscribed'
end
end
end end
...@@ -4,29 +4,38 @@ require 'spec_helper' ...@@ -4,29 +4,38 @@ require 'spec_helper'
describe GroupLabelsFinder, '#execute' do describe GroupLabelsFinder, '#execute' do
let!(:group) { create(:group) } let!(:group) { create(:group) }
let!(:user) { create(:user) }
let!(:label1) { create(:group_label, title: 'Foo', description: 'Lorem ipsum', group: group) } let!(:label1) { create(:group_label, title: 'Foo', description: 'Lorem ipsum', group: group) }
let!(:label2) { create(:group_label, title: 'Bar', description: 'Fusce consequat', 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 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]) expect(result.to_a).to match_array([label2, label1])
end end
it 'returns all group labels sorted by name desc' do 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]) expect(result.to_a).to match_array([label2, label1])
end end
it 'returns group labels that march search' do it 'returns group labels that match search' do
result = described_class.new(group, search: 'Foo').execute result = described_class.new(user, group, search: 'Foo').execute
expect(result.to_a).to match_array([label1]) expect(result.to_a).to match_array([label1])
end 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 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([]) expect(result.to_a).to match_array([])
end end
......
...@@ -210,5 +210,15 @@ describe LabelsFinder do ...@@ -210,5 +210,15 @@ describe LabelsFinder do
expect(finder.execute).to eq [project_label_1] expect(finder.execute).to eq [project_label_1]
end end
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
end end
...@@ -155,4 +155,40 @@ describe Label do ...@@ -155,4 +155,40 @@ describe Label do
expect(described_class.search('feature')).to be_empty expect(described_class.search('feature')).to be_empty
end end
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 end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment