diff options
| -rw-r--r-- | app/assets/stylesheets/pages/labels.scss | 10 | ||||
| -rw-r--r-- | app/controllers/groups/labels_controller.rb | 20 | ||||
| -rw-r--r-- | app/controllers/projects/application_controller.rb | 4 | ||||
| -rw-r--r-- | app/controllers/projects/issues_controller.rb | 4 | ||||
| -rw-r--r-- | app/controllers/projects/labels_controller.rb | 11 | ||||
| -rw-r--r-- | app/controllers/projects/merge_requests_controller.rb | 2 | ||||
| -rw-r--r-- | app/helpers/labels_helper.rb | 43 | ||||
| -rw-r--r-- | app/models/label.rb | 24 | ||||
| -rw-r--r-- | app/views/groups/labels/_form.html.haml | 2 | ||||
| -rw-r--r-- | app/views/groups/labels/_label.html.haml | 2 | ||||
| -rw-r--r-- | app/views/projects/labels/_form.html.haml | 2 | ||||
| -rw-r--r-- | app/views/projects/labels/_label.html.haml | 23 | ||||
| -rw-r--r-- | app/views/projects/labels/destroy.js.haml | 2 | ||||
| -rw-r--r-- | app/views/projects/labels/edit.html.haml | 3 | ||||
| -rw-r--r-- | app/views/projects/labels/index.html.haml | 53 | ||||
| -rw-r--r-- | app/views/projects/labels/new.html.haml | 3 | ||||
| -rw-r--r-- | app/views/shared/_label_row.html.haml | 3 | ||||
| -rw-r--r-- | spec/features/projects/labels/update_prioritization_spec.rb | 104 |
18 files changed, 226 insertions, 89 deletions
diff --git a/app/assets/stylesheets/pages/labels.scss b/app/assets/stylesheets/pages/labels.scss index 9bac6d46355..cbd009ccd07 100644 --- a/app/assets/stylesheets/pages/labels.scss +++ b/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; } diff --git a/app/controllers/groups/labels_controller.rb b/app/controllers/groups/labels_controller.rb index 449298f51a8..0ec2fcda0ef 100644 --- a/app/controllers/groups/labels_controller.rb +++ b/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 diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index b2ff36f6538..5daf4311cc8 100644 --- a/app/controllers/projects/application_controller.rb +++ b/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 diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 4f6d7ca80df..1558426e1a4 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/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 diff --git a/app/controllers/projects/labels_controller.rb b/app/controllers/projects/labels_controller.rb index 35224f965bf..3db3c091da6 100644 --- a/app/controllers/projects/labels_controller.rb +++ b/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 diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index c8970155497..291c3f64914 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/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 diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb index 540eb6dd493..3f0e502fbc9 100644 --- a/app/helpers/labels_helper.rb +++ b/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 diff --git a/app/models/label.rb b/app/models/label.rb index 295c5bfaf70..f43bebbf71b 100644 --- a/app/models/label.rb +++ b/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 diff --git a/app/views/groups/labels/_form.html.haml b/app/views/groups/labels/_form.html.haml index 008b5fb9ba1..a0b44b0dcfb 100644 --- a/app/views/groups/labels/_form.html.haml +++ b/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' diff --git a/app/views/groups/labels/_label.html.haml b/app/views/groups/labels/_label.html.haml index b9aab76f057..9faf90c303e 100644 --- a/app/views/groups/labels/_label.html.haml +++ b/app/views/groups/labels/_label.html.haml @@ -1,6 +1,6 @@ - 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 diff --git a/app/views/projects/labels/_form.html.haml b/app/views/projects/labels/_form.html.haml index 6ab6ae50389..5f7be074f25 100644 --- a/app/views/projects/labels/_form.html.haml +++ b/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' diff --git a/app/views/projects/labels/_label.html.haml b/app/views/projects/labels/_label.html.haml index 71f7f354d72..2b7b79390f7 100644 --- a/app/views/projects/labels/_label.html.haml +++ b/app/views/projects/labels/_label.html.haml @@ -1,4 +1,7 @@ - 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') diff --git a/app/views/projects/labels/destroy.js.haml b/app/views/projects/labels/destroy.js.haml index d59563b122a..0b9937c4808 100644 --- a/app/views/projects/labels/destroy.js.haml +++ b/app/views/projects/labels/destroy.js.haml @@ -1,2 +1,2 @@ -- if @project.labels.size == 0 +- if @project_labels.none? $('.labels').load(document.URL + ' .nothing-here-block').hide().fadeIn(1000) diff --git a/app/views/projects/labels/edit.html.haml b/app/views/projects/labels/edit.html.haml index 52b187e7e58..c9ec371c3e1 100644 --- a/app/views/projects/labels/edit.html.haml +++ b/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' diff --git a/app/views/projects/labels/index.html.haml b/app/views/projects/labels/index.html.haml index db66a0edbd8..286b58f57c2 100644 --- a/app/views/projects/labels/index.html.haml +++ b/app/views/projects/labels/index.html.haml @@ -14,25 +14,38 @@ 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') - .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 + - hide = params[:page].present? && params[:page] != '1' - if can?(current_user, :admin_label, @project) - %h5{ class: ('hide' if hide) } Other Labels - - if @labels.present? - %ul.content-list.manage-labels-list.js-other-labels - = render @labels - = paginate @labels, theme: 'gitlab' - - else - .nothing-here-block - - 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 + .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 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? + = render @labels + = paginate @labels, theme: 'gitlab' + - else + .nothing-here-block + - 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 yet. diff --git a/app/views/projects/labels/new.html.haml b/app/views/projects/labels/new.html.haml index a1bb66cfb6c..a1e2df6c55d 100644 --- a/app/views/projects/labels/new.html.haml +++ b/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' diff --git a/app/views/shared/_label_row.html.haml b/app/views/shared/_label_row.html.haml index 6f593e8dff9..751b2d1c158 100644 --- a/app/views/shared/_label_row.html.haml +++ b/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) diff --git a/spec/features/projects/labels/update_prioritization_spec.rb b/spec/features/projects/labels/update_prioritization_spec.rb index cb7495da8eb..21896f0a787 100644 --- a/spec/features/projects/labels/update_prioritization_spec.rb +++ b/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(:user) { create(:user) } + 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('bug') + 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) - - project.labels << bug - project.labels << wontfix + bug.update(priority: 1) + feature.update(priority: 2) - 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 |