List group labels on project labels page

app/assets/stylesheets/pages/labels.scss

@@ -140,6 +140,10 @@
     color: $gray-light;
   }
 
+  .label-type {
+    opacity: 0.3;
+  }
+
   li:hover {
     .draggable-handler {
       display: inline-block;
@@ -148,7 +152,11 @@
   }
 }
 
-.other-labels {
+.group-labels + .project-labels {
+  margin-top: 30px;
+}
+
+.group-labels, .project-labels {
   .remove-priority {
     display: none;
   }

app/controllers/groups/labels_controller.rb

@@ -3,6 +3,7 @@ class Groups::LabelsController < Groups::ApplicationController
 
   before_action :label, only: [:edit, :update, :destroy, :toggle_subscription]
   before_action :authorize_admin_labels!, only: [:new, :create, :edit, :update, :destroy]
+  before_action :save_previous_label_path, only: [:edit]
 
   respond_to :html
 
@@ -25,11 +26,12 @@ class Groups::LabelsController < Groups::ApplicationController
   end
 
   def edit
+    @previous_labels_path = previous_labels_path
   end
 
   def update
     if @label.update_attributes(label_params)
-      redirect_to group_labels_path(@group)
+      redirect_back_or_group_labels_path
     else
       render :edit
     end
@@ -64,4 +66,20 @@ class Groups::LabelsController < Groups::ApplicationController
   def label_params
     params.require(:label).permit(:title, :description, :color)
   end
+
+  def redirect_back_or_group_labels_path(options = {})
+    redirect_to previous_labels_path, options
+  end
+
+  def previous_labels_path
+    session.fetch(:previous_labels_path, fallback_path)
+  end
+
+  def fallback_path
+    group_labels_path(@group)
+  end
+
+  def save_previous_label_path
+    session[:previous_labels_path] = URI(request.referer || '').path
+  end
 end

app/controllers/projects/application_controller.rb

@@ -44,6 +44,10 @@ class Projects::ApplicationController < ApplicationController
     @project
   end
 
+  def project_labels
+    @project_labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute
+  end
+
   def repository
     @repository ||= project.repository
   end

app/controllers/projects/issues_controller.rb

@@ -48,13 +48,13 @@ class Projects::IssuesController < Projects::ApplicationController
     )
 
     @issue  = @noteable = @project.issues.new(issue_params)
-    @labels = LabelsFinder.new(current_user, project_id: @project.id).execute
+    @labels = project_labels
 
     respond_with(@issue)
   end
 
   def edit
-    @labels = LabelsFinder.new(current_user, project_id: @project.id).execute
+    @labels = project_labels
 
     respond_with(@issue)
   end

app/controllers/projects/labels_controller.rb

@@ -11,13 +11,15 @@ class Projects::LabelsController < Projects::ApplicationController
   respond_to :js, :html
 
   def index
+    @project_labels = project_labels
+    @prioritized_labels = project_labels.prioritized
+    @group_labels = @project.group.labels.unprioritized if @project.group.present?
     @labels = @project.labels.unprioritized.page(params[:page])
-    @prioritized_labels = @project.labels.prioritized
 
     respond_to do |format|
       format.html
       format.json do
-        render json: LabelsFinder.new(current_user, project_id: @project.id).execute
+        render json: @project_labels
       end
     end
   end
@@ -68,6 +70,7 @@ class Projects::LabelsController < Projects::ApplicationController
 
   def destroy
     @label.destroy
+    @project_labels = project_labels
 
     respond_to do |format|
       format.html do
@@ -80,6 +83,8 @@ class Projects::LabelsController < Projects::ApplicationController
 
   def remove_priority
     respond_to do |format|
+      label = project_labels.find(params[:id])
+
       if label.update_attribute(:priority, nil)
         format.json { render json: label }
       else
@@ -92,7 +97,7 @@ class Projects::LabelsController < Projects::ApplicationController
   def set_priorities
     Label.transaction do
       params[:label_ids].each_with_index do |label_id, index|
-        label = @project.labels.find_by_id(label_id)
+        label = project_labels.find_by_id(label_id)
         label.update_attribute(:priority, index) if label
       end
     end

app/controllers/projects/merge_requests_controller.rb

@@ -263,7 +263,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
     @source_project = @merge_request.source_project
     @target_project = @merge_request.target_project
     @target_branches = @merge_request.target_project.repository.branch_names
-    @labels = LabelsFinder.new(current_user, project_id: @project.id).execute
+    @labels = project_labels
   end
 
   def update

app/helpers/labels_helper.rb

@@ -68,6 +68,49 @@ module LabelsHelper
     end
   end
 
+  def can_admin_label(label)
+    subject =
+      case label
+      when GroupLabel then label.group
+      else label.project
+      end
+
+    can?(current_user, :admin_label, subject)
+  end
+
+  def edit_label_path(label)
+    case label
+    when GroupLabel then edit_group_label_path(label.group, label)
+    else edit_namespace_project_label_path(label.project.namespace, label.project, label)
+    end
+  end
+
+  def destroy_label_path(label)
+    case label
+    when GroupLabel then group_label_path(label.group, label)
+    else namespace_project_label_path(label.project.namespace, label.project, label)
+    end
+  end
+
+  def label_type_icon(label, options = {})
+    title, icon =
+      case label
+      when GroupLabel then ['Group', 'folder-open']
+      else ['Project', 'bookmark']
+      end
+
+    options[:class] ||= ''
+    options[:class] << ' has-tooltip js-label-type'
+
+    content_tag :span,
+      class: options[:class],
+      data: { 'placement' => 'top' },
+      title: title,
+      aria: { label: title } do
+      icon(icon, base: true)
+    end
+  end
+
   def project_label_names
     @project.labels.pluck(:title)
   end

app/models/label.rb

@@ -101,16 +101,16 @@ class Label < ActiveRecord::Base
     end
   end
 
-  def open_issues_count(user = nil)
-    issues.visible_to_user(user).opened.count
+  def open_issues_count(user = nil, project = nil)
+    issues_count(user, project_id: project.try(:id) || project_id, state: 'opened')
   end
 
-  def closed_issues_count(user = nil)
-    issues.visible_to_user(user).closed.count
+  def closed_issues_count(user = nil, project = nil)
+    issues_count(user, project_id: project.try(:id) || project_id, state: 'closed')
   end
 
-  def open_merge_requests_count
-    merge_requests.opened.count
+  def open_merge_requests_count(user = nil, project = nil)
+    merge_requests_count(user, project_id: project.try(:id) || project_id, state: 'opened')
   end
 
   def template?
@@ -127,6 +127,18 @@ class Label < ActiveRecord::Base
 
   private
 
+  def issues_count(user, params = {})
+    IssuesFinder.new(user, { label_name: title, scope: 'all' }.merge(params))
+                .execute
+                .count
+  end
+
+  def merge_requests_count(user, params = {})
+    MergeRequestsFinder.new(user, { label_name: title, scope: 'all' }.merge(params))
+                       .execute
+                       .count
+  end
+
   def project_label?
     type.blank? && !template?
   end

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

@@ -30,4 +30,4 @@
       = f.submit 'Save changes', class: 'btn btn-save js-save-button'
     - else
       = f.submit 'Create Label', class: 'btn btn-create js-save-button'
-    = link_to "Cancel", group_labels_path(@group), class: 'btn btn-cancel'
+    = link_to 'Cancel', @previous_labels_path, class: 'btn btn-cancel'

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

 - label_css_id = dom_id(label)
 - open_issues_count = label.open_issues_count(current_user)
-- open_merge_requests_count = label.open_merge_requests_count
+- open_merge_requests_count = label.open_merge_requests_count(current_user)
 
 %li{id: label_css_id, data: { id: label.id } }
   = render 'label_row', label: label

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

@@ -30,4 +30,4 @@
       = f.submit 'Save changes', class: 'btn btn-save js-save-button'
     - else
       = f.submit 'Create Label', class: 'btn btn-create js-save-button'
-    = link_to "Cancel", namespace_project_labels_path(@project.namespace, @project), class: 'btn btn-cancel'
+    = link_to 'Cancel', namespace_project_labels_path(@project.namespace, @project), class: 'btn btn-cancel'

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

 - label_css_id = dom_id(label)
+- open_issues_count = label.open_issues_count(current_user, @project)
+- open_merge_requests_count = label.open_merge_requests_count(current_user, @project)
+
 %li{id: label_css_id, data: { id: label.id } }
   = render "shared/label_row", label: label
 
@@ -10,25 +13,25 @@
       %ul
         %li
           = link_to_label(label, type: :merge_request) do
-            = pluralize label.open_merge_requests_count, 'merge request'
+            = pluralize open_merge_requests_count, 'merge request'
         %li
           = link_to_label(label) do
-            = pluralize label.open_issues_count(current_user), 'open issue'
+            = pluralize open_issues_count, 'open issue'
         - if current_user
           %li.label-subscription{ data: { url: toggle_subscription_namespace_project_label_path(@project.namespace, @project, label) } }
             %a.js-subscribe-button.label-subscribe-button.subscription-status{ role: "button", href: "#", data: { toggle: "tooltip", status: label_subscription_status(label) } }
               %span= label_subscription_toggle_button_text(label)
-        - if can? current_user, :admin_label, @project
+        - if can_admin_label(label)
           %li
-            = link_to "Edit", edit_namespace_project_label_path(@project.namespace, @project, label)
+            = link_to 'Edit', edit_label_path(label)
           %li
-            = link_to "Delete", namespace_project_label_path(@project.namespace, @project, label), title: "Delete", method: :delete, remote: true, data: {confirm: "Remove this label? Are you sure?"}
+            = link_to 'Delete', destroy_label_path(label), title: 'Delete', method: :delete, remote: true, data: {confirm: 'Remove this label? Are you sure?'}
 
   .pull-right.hidden-xs.hidden-sm.hidden-md
     = link_to_label(label, type: :merge_request, css_class: 'btn btn-transparent btn-action') do
-      = pluralize label.open_merge_requests_count, 'merge request'
+      = pluralize open_merge_requests_count, 'merge request'
     = link_to_label(label, css_class: 'btn btn-transparent btn-action') do
-      = pluralize label.open_issues_count(current_user), 'open issue'
+      = pluralize open_issues_count, 'open issue'
 
     - if current_user
       .label-subscription.inline{ data: { url: toggle_subscription_namespace_project_label_path(@project.namespace, @project, label) } }
@@ -37,11 +40,11 @@
           = icon('eye', class: 'label-subscribe-button-icon')
           = icon('spinner spin', class: 'label-subscribe-button-loading')
 
-    - if can? current_user, :admin_label, @project
-      = link_to edit_namespace_project_label_path(@project.namespace, @project, label), title: "Edit", class: 'btn btn-transparent btn-action', data: {toggle: "tooltip"} do
+    - if can_admin_label(label)
+      = link_to edit_label_path(label), title: "Edit", class: 'btn btn-transparent btn-action', data: {toggle: "tooltip"} do
         %span.sr-only Edit
         = icon('pencil-square-o')
-      = link_to namespace_project_label_path(@project.namespace, @project, label), title: "Delete", class: 'btn btn-transparent btn-action remove-row', method: :delete, remote: true, data: {confirm: "Remove this label? Are you sure?", toggle: "tooltip"} do
+      = link_to destroy_label_path(label), title: "Delete", class: 'btn btn-transparent btn-action remove-row', method: :delete, remote: true, data: {confirm: "Remove this label? Are you sure?", toggle: "tooltip"} do
         %span.sr-only Delete
         = icon('trash-o')
 

app/views/projects/labels/destroy.js.haml

-- if @project.labels.size == 0
+- if @project_labels.none?
   $('.labels').load(document.URL + ' .nothing-here-block').hide().fadeIn(1000)

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

@@ -4,6 +4,7 @@
 
 %div{ class: container_class }
   %h3.page-title
-    Edit Label
+    = icon('bookmark')
+    Edit Project Label
   %hr
   = render 'form'

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

@@ -14,20 +14,33 @@
           New label
 
   .labels
-    - if can?(current_user, :admin_label, @project)
+    - unless @project_labels.empty?
       -# Only show it in the first page
-      - hide = @project.labels.empty? || (params[:page].present? && params[:page] != '1')
+      - hide = params[:page].present? && params[:page] != '1'
+      - if can?(current_user, :admin_label, @project)
         .prioritized-labels{ class: ('hide' if hide) }
           %h5 Prioritized Labels
           %ul.content-list.manage-labels-list.js-prioritized-labels{ "data-url" => set_priorities_namespace_project_labels_path(@project.namespace, @project) }
             %p.empty-message{ class: ('hidden' unless @prioritized_labels.empty?) } No prioritized labels yet
             - if @prioritized_labels.present?
-            = render @prioritized_labels
-    .other-labels
-      - if can?(current_user, :admin_label, @project)
-        %h5{ class: ('hide' if hide) } Other Labels
+              = render partial: 'label', collection: @prioritized_labels, as: :label
+
+      .group-labels{ class: ('hide' if hide || @project.group.blank?) }
+        %h5
+          = icon('folder-open')
+          Group Labels
+        %ul.content-list.manage-labels-list.js-group-labels
+          %p.empty-message{ class: ('hidden' unless @group_labels.empty?) } No group labels
+          - if @group_labels.present?
+            = render partial: 'label', collection: @group_labels, as: :label
+
+      .project-labels
+        %h5{ class: ('hide' if hide) }
+          = icon('bookmark')
+          Project Labels
+        %ul.content-list.manage-labels-list.js-project-labels
+          %p.empty-message{ class: ('hidden' unless @labels.empty?) } No project labels
           - if @labels.present?
-        %ul.content-list.manage-labels-list.js-other-labels
             = render @labels
             = paginate @labels, theme: 'gitlab'
     - else
@@ -35,4 +48,4 @@
         - if can?(current_user, :admin_label, @project)
           Create a label or #{link_to 'generate a default set of labels', generate_namespace_project_labels_path(@project.namespace, @project), method: :post}.
         - else
-            No labels created
+          No labels created yet.

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

@@ -4,6 +4,7 @@
 
 %div{ class: container_class }
   %h3.page-title
-    New Label
+    = icon('bookmark')
+    New Project Label
   %hr
   = render 'form'

app/views/shared/_label_row.html.haml

@@ -3,13 +3,14 @@
     .draggable-handler
       = icon('bars')
     .js-toggle-priority.toggle-priority{ data: { url: remove_priority_namespace_project_label_path(@project.namespace, @project, label),
-      dom_id: dom_id(label) } }
+      dom_id: dom_id(label), type: label.type } }
       %button.add-priority.btn.has-tooltip{ title: 'Prioritize', :'data-placement' => 'top' }
         = icon('star-o')
       %button.remove-priority.btn.has-tooltip{ title: 'Remove priority', :'data-placement' => 'top' }
         = icon('star')
   %span.label-name
     = link_to_label(label, tooltip: false)
+    = label_type_icon(label, class: "#{'hidden' if label.priority.blank?}" )
   - if label.description
     %span.label-description
       = markdown_field(label, :description)

spec/features/projects/labels/update_prioritization_spec.rb

@@ -3,76 +3,106 @@ require 'spec_helper'
 feature 'Prioritize labels', feature: true do
   include WaitForAjax
 
-  context 'when project belongs to user' do
   let(:user)     { create(:user) }
-    let(:project) { create(:project, name: 'test', namespace: user.namespace) }
+  let(:group)    { create(:group) }
+  let(:project)  { create(:empty_project, :public, namespace: group) }
+  let!(:bug)     { create(:label, project: project, title: 'bug') }
+  let!(:wontfix) { create(:label, project: project, title: 'wontfix') }
+  let!(:feature) { create(:group_label, group: group, title: 'feature') }
 
-    scenario 'user can prioritize a label', js: true do
-      bug     = create(:label, title: 'bug')
-      wontfix = create(:label, title: 'wontfix')
-
-      project.labels << bug
-      project.labels << wontfix
+  context 'when user belongs to project team' do
+    before do
+      project.team << [user, :developer]
 
       login_as user
+    end
+
+    scenario 'user can prioritize a group label', js: true do
       visit namespace_project_labels_path(project.namespace, project)
 
       expect(page).to have_content('No prioritized labels yet')
 
-      page.within('.other-labels') do
+      page.within('.group-labels') do
         first('.js-toggle-priority').click
         wait_for_ajax
-        expect(page).not_to have_content('bug')
+        expect(page).not_to have_content('feature')
       end
 
       page.within('.prioritized-labels') do
         expect(page).not_to have_content('No prioritized labels yet')
-        expect(page).to have_content('bug')
+        expect(page).to have_content('feature')
       end
     end
 
-    scenario 'user can unprioritize a label', js: true do
-      bug     = create(:label, title: 'bug', priority: 1)
-      wontfix = create(:label, title: 'wontfix')
+    scenario 'user can unprioritize a group label', js: true do
+      feature.update(priority: 1)
 
-      project.labels << bug
-      project.labels << wontfix
+      visit namespace_project_labels_path(project.namespace, project)
 
-      login_as user
+      page.within('.prioritized-labels') do
+        expect(page).to have_content('feature')
+
+        first('.js-toggle-priority').click
+        wait_for_ajax
+        expect(page).not_to have_content('bug')
+      end
+
+      page.within('.group-labels') do
+        expect(page).to have_content('feature')
+      end
+    end
+
+    scenario 'user can prioritize a project label', js: true do
       visit namespace_project_labels_path(project.namespace, project)
 
+      expect(page).to have_content('No prioritized labels yet')
+
+      page.within('.project-labels') do
+        first('.js-toggle-priority').click
+        wait_for_ajax
+        expect(page).not_to have_content('bug')
+      end
+
+      page.within('.prioritized-labels') do
+        expect(page).not_to have_content('No prioritized labels yet')
         expect(page).to have_content('bug')
+      end
+    end
+
+    scenario 'user can unprioritize a project label', js: true do
+      bug.update(priority: 1)
+
+      visit namespace_project_labels_path(project.namespace, project)
 
       page.within('.prioritized-labels') do
+        expect(page).to have_content('bug')
+
         first('.js-toggle-priority').click
         wait_for_ajax
         expect(page).not_to have_content('bug')
       end
 
-      page.within('.other-labels') do
+      page.within('.project-labels') do
         expect(page).to have_content('bug')
         expect(page).to have_content('wontfix')
       end
     end
 
     scenario 'user can sort prioritized labels and persist across reloads', js: true do
-      bug     = create(:label, title: 'bug', priority: 1)
-      wontfix = create(:label, title: 'wontfix', priority: 2)
+      bug.update(priority: 1)
+      feature.update(priority: 2)
 
-      project.labels << bug
-      project.labels << wontfix
-
-      login_as user
       visit namespace_project_labels_path(project.namespace, project)
 
       expect(page).to have_content 'bug'
+      expect(page).to have_content 'feature'
       expect(page).to have_content 'wontfix'
 
       # Sort labels
-      find("#label_#{bug.id}").drag_to find("#label_#{wontfix.id}")
+      find("#project_label_#{bug.id}").drag_to find("#group_label_#{feature.id}")
 
       page.within('.prioritized-labels') do
-        expect(first('li')).to have_content('wontfix')
+        expect(first('li')).to have_content('feature')
         expect(page.all('li').last).to have_content('bug')
       end
 
@@ -80,7 +110,7 @@ feature 'Prioritize labels', feature: true do
       wait_for_ajax
 
       page.within('.prioritized-labels') do
-        expect(first('li')).to have_content('wontfix')
+        expect(first('li')).to have_content('feature')
         expect(page.all('li').last).to have_content('bug')
       end
     end
@@ -88,28 +118,26 @@ feature 'Prioritize labels', feature: true do
 
   context 'as a guest' do
     it 'does not prioritize labels' do
-      user = create(:user)
       guest = create(:user)
-      project = create(:project, name: 'test', namespace: user.namespace)
-
-      create(:label, title: 'bug')
 
       login_as guest
+
       visit namespace_project_labels_path(project.namespace, project)
 
+      expect(page).to have_content 'bug'
+      expect(page).to have_content 'wontfix'
+      expect(page).to have_content 'feature'
       expect(page).not_to have_css('.prioritized-labels')
     end
   end
 
   context 'as a non signed in user' do
     it 'does not prioritize labels' do
-      user = create(:user)
-      project = create(:project, name: 'test', namespace: user.namespace)
-
-      create(:label, title: 'bug')
-
       visit namespace_project_labels_path(project.namespace, project)
 
+      expect(page).to have_content 'bug'
+      expect(page).to have_content 'wontfix'
+      expect(page).to have_content 'feature'
       expect(page).not_to have_css('.prioritized-labels')
     end
   end