From 0bfa39d5bdb9f53bfc319b9351230b3eb405b619 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Thu, 29 Sep 2016 01:03:28 -0300 Subject: [PATCH] Remove scopes/types for labels --- app/assets/stylesheets/pages/labels.scss | 10 +--- app/controllers/projects/labels_controller.rb | 19 +++---- app/helpers/labels_helper.rb | 20 ------- app/views/groups/labels/edit.html.haml | 3 +- app/views/groups/labels/index.html.haml | 8 +-- app/views/groups/labels/new.html.haml | 5 +- app/views/projects/labels/edit.html.haml | 3 +- app/views/projects/labels/index.html.haml | 52 ++++++++----------- app/views/projects/labels/new.html.haml | 3 +- app/views/shared/_label_row.html.haml | 2 - .../projects/labels_controller_spec.rb | 38 +++----------- .../labels/update_prioritization_spec.rb | 10 ++-- 12 files changed, 50 insertions(+), 123 deletions(-) diff --git a/app/assets/stylesheets/pages/labels.scss b/app/assets/stylesheets/pages/labels.scss index cbd009ccd07..9bac6d46355 100644 --- a/app/assets/stylesheets/pages/labels.scss +++ b/app/assets/stylesheets/pages/labels.scss @@ -140,10 +140,6 @@ color: $gray-light; } - .label-type { - opacity: 0.3; - } - li:hover { .draggable-handler { display: inline-block; @@ -152,11 +148,7 @@ } } -.group-labels + .project-labels { - margin-top: 30px; -} - -.group-labels, .project-labels { +.other-labels { .remove-priority { display: none; } diff --git a/app/controllers/projects/labels_controller.rb b/app/controllers/projects/labels_controller.rb index 919c6f239cb..3154a4435f6 100644 --- a/app/controllers/projects/labels_controller.rb +++ b/app/controllers/projects/labels_controller.rb @@ -3,7 +3,7 @@ class Projects::LabelsController < Projects::ApplicationController before_action :module_enabled before_action :label, only: [:edit, :update, :destroy] - before_action :labels, only: [:index] + before_action :find_labels, only: [:index, :set_priorities, :remove_priority] before_action :authorize_read_label! before_action :authorize_admin_labels!, only: [:new, :create, :edit, :update, :generate, :destroy, :remove_priority, @@ -12,9 +12,8 @@ class Projects::LabelsController < Projects::ApplicationController respond_to :js, :html def index - @prioritized_labels = @labels.prioritized - @group_labels = @project.group.labels.unprioritized if @project.group.present? - @project_labels = @project.labels.unprioritized.page(params[:page]) + @prioritized_labels = @available_labels.prioritized + @labels = @available_labels.unprioritized.page(params[:page]) respond_to do |format| format.html @@ -70,7 +69,7 @@ class Projects::LabelsController < Projects::ApplicationController def destroy @label.destroy - @labels = labels + @labels = find_labels respond_to do |format| format.html do @@ -83,7 +82,7 @@ class Projects::LabelsController < Projects::ApplicationController def remove_priority respond_to do |format| - label = labels.find(params[:id]) + label = @available_labels.find(params[:id]) if label.update_attribute(:priority, nil) format.json { render json: label } @@ -96,8 +95,10 @@ class Projects::LabelsController < Projects::ApplicationController def set_priorities Label.transaction do + label_ids = @available_labels.where(id: params[:label_ids]).pluck(:id) + params[:label_ids].each_with_index do |label_id, index| - next unless labels.where(id: label_id).any? + next unless label_ids.include?(label_id.to_i) Label.where(id: label_id).update_all(priority: index) end @@ -125,8 +126,8 @@ class Projects::LabelsController < Projects::ApplicationController end alias_method :subscribable_resource, :label - def labels - @labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute + def find_labels + @available_labels ||= LabelsFinder.new(current_user, project_id: @project.id).execute end def authorize_admin_labels! diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb index 6d0d33b84fb..e8992c114b0 100644 --- a/app/helpers/labels_helper.rb +++ b/app/helpers/labels_helper.rb @@ -81,26 +81,6 @@ module LabelsHelper end end - def label_type_icon(label, options = {}) - title, icon = - case label - when GroupLabel then ['Group', 'folder-open'] - when ProjectLabel then ['Project', 'bookmark'] - end - - options[:class] ||= '' - options[:class] << ' has-tooltip js-label-type' - options[:class] << ' hidden' if options.fetch(:hidden, false) - - content_tag :span, - class: options[:class], - data: { 'placement' => 'top' }, - title: title, - aria: { label: title } do - icon(icon, base: true) - end - end - def render_colored_label(label, label_suffix = '', tooltip: true) label_color = label.color || Label::DEFAULT_COLOR text_color = text_color_for_bg(label_color) diff --git a/app/views/groups/labels/edit.html.haml b/app/views/groups/labels/edit.html.haml index e247393abd5..28471f407ad 100644 --- a/app/views/groups/labels/edit.html.haml +++ b/app/views/groups/labels/edit.html.haml @@ -1,8 +1,7 @@ - page_title 'Edit', @label.name, 'Labels' %h3.page-title - = icon('folder-open') - Edit Group Label + Edit Label %hr = render 'form', url: group_label_path(@group, @label) diff --git a/app/views/groups/labels/index.html.haml b/app/views/groups/labels/index.html.haml index 8e93ea4f625..6c69e3465f4 100644 --- a/app/views/groups/labels/index.html.haml +++ b/app/views/groups/labels/index.html.haml @@ -10,13 +10,9 @@ New label .labels - - hide = @group.labels.empty? || (params[:page].present? && params[:page] != '1') - .group-labels - %h5{ class: ('hide' if hide) } - = icon('folder-open') - Group Labels + .other-labels - if @labels.present? - %ul.content-list.manage-labels-list.js-group-labels + %ul.content-list.manage-labels-list.js-other-labels = render partial: 'shared/label', collection: @labels, as: :label = paginate @labels, theme: 'gitlab' - else diff --git a/app/views/groups/labels/new.html.haml b/app/views/groups/labels/new.html.haml index 01a50607db4..257ae97de03 100644 --- a/app/views/groups/labels/new.html.haml +++ b/app/views/groups/labels/new.html.haml @@ -1,9 +1,8 @@ -- page_title 'New Group Label' +- page_title 'New Label' - header_title group_title(@group, 'Labels', group_labels_path(@group)) %h3.page-title - = icon('folder-open') - New Group Label + New Label %hr = render 'form', url: group_labels_path diff --git a/app/views/projects/labels/edit.html.haml b/app/views/projects/labels/edit.html.haml index 372abcb8773..49adb593559 100644 --- a/app/views/projects/labels/edit.html.haml +++ b/app/views/projects/labels/edit.html.haml @@ -4,7 +4,6 @@ %div{ class: container_class } %h3.page-title - = icon('bookmark') - Edit Project Label + Edit Label %hr = render 'form', url: namespace_project_label_path(@project.namespace.becomes(Namespace), @project, @label) diff --git a/app/views/projects/labels/index.html.haml b/app/views/projects/labels/index.html.haml index 99f8e8095ad..c1ec9cabc40 100644 --- a/app/views/projects/labels/index.html.haml +++ b/app/views/projects/labels/index.html.haml @@ -14,36 +14,26 @@ New label .labels - - unless @labels.empty? + - if can?(current_user, :admin_label, @project) -# Only show it in the first page - - hide = params[:page].present? && params[:page] != '1' + - 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 partial: 'shared/label', collection: @prioritized_labels, as: :label + + .other-labels - if can?(current_user, :admin_label, @project) - .prioritized-labels{ class: ('hidden' 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: 'shared/label', collection: @prioritized_labels, as: :label - - .group-labels{ class: ('hidden' if hide || @project.group.blank? || @group_labels.empty?) } - %h5 - = icon('folder-open') - Group Labels - %ul.content-list.manage-labels-list.js-group-labels - - if @group_labels.present? - = render partial: 'shared/label', collection: @group_labels, as: :label - - .project-labels{ class: ('hidden' if @project_labels.empty?) } - %h5{ class: ('hidden' if hide) } - = icon('bookmark') - Project Labels - %ul.content-list.manage-labels-list.js-project-labels - - if @project_labels.present? - = render partial: 'shared/label', collection: @project_labels, as: :label - = paginate @project_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. + %h5{ class: ('hide' if hide) } Other Labels + %ul.content-list.manage-labels-list.js-other-labels + - if @labels.present? + = render partial: 'shared/label', collection: @labels, as: :label + = paginate @labels, theme: 'gitlab' + - if @labels.blank? + .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 diff --git a/app/views/projects/labels/new.html.haml b/app/views/projects/labels/new.html.haml index f170c41bfc4..0c177feb43c 100644 --- a/app/views/projects/labels/new.html.haml +++ b/app/views/projects/labels/new.html.haml @@ -4,7 +4,6 @@ %div{ class: container_class } %h3.page-title - = icon('bookmark') - New Project Label + New Label %hr = render 'form', url: namespace_project_labels_path(@project.namespace.becomes(Namespace), @project) diff --git a/app/views/shared/_label_row.html.haml b/app/views/shared/_label_row.html.haml index a623bbc6b11..813ce4f1405 100644 --- a/app/views/shared/_label_row.html.haml +++ b/app/views/shared/_label_row.html.haml @@ -10,8 +10,6 @@ = icon('star') %span.label-name = link_to_label(label, subject: @project, tooltip: false) - - if can?(current_user, :admin_label, @project) - = label_type_icon(label, hidden: label.priority.blank?) - if label.description %span.label-description = markdown_field(label, :description) diff --git a/spec/controllers/projects/labels_controller_spec.rb b/spec/controllers/projects/labels_controller_spec.rb index 2b39f9cf0d1..29251f49810 100644 --- a/spec/controllers/projects/labels_controller_spec.rb +++ b/spec/controllers/projects/labels_controller_spec.rb @@ -33,55 +33,29 @@ describe Projects::LabelsController do end it 'is sorted by priority, then label title' do - expect(assigns(:prioritized_labels)).to match_array [group_label_2, label_1, label_3, group_label_1, label_2] + expect(assigns(:prioritized_labels)).to eq [group_label_2, label_1, label_3, group_label_1, label_2] end end - context '@group_labels' do - it 'contains only group labels' do - list_labels - - expect(assigns(:group_labels)).to all(have_attributes(group_id: a_value > 0)) - end - + context '@labels' do it 'contains only unprioritized labels' do list_labels - expect(assigns(:group_labels)).to all(have_attributes(priority: nil)) + expect(assigns(:labels)).to all(have_attributes(priority: nil)) end it 'is sorted by label title' do list_labels - expect(assigns(:group_labels)).to match_array [group_label_3, group_label_4] + expect(assigns(:labels)).to eq [group_label_3, group_label_4, label_4, label_5] end - it 'is nil when project does not belong to a group' do + it 'does not include group labels when project does not belong to a group' do project.update(namespace: create(:namespace)) list_labels - expect(assigns(:group_labels)).to be_nil - end - end - - context '@project_labels' do - before do - list_labels - end - - it 'contains only project labels' do - list_labels - - expect(assigns(:project_labels)).to all(have_attributes(project_id: a_value > 0)) - end - - it 'contains only unprioritized labels' do - expect(assigns(:project_labels)).to all(have_attributes(priority: nil)) - end - - it 'is sorted by label title' do - expect(assigns(:project_labels)).to match_array [label_4, label_5] + expect(assigns(:labels)).not_to include(group_label_3, group_label_4) end end diff --git a/spec/features/projects/labels/update_prioritization_spec.rb b/spec/features/projects/labels/update_prioritization_spec.rb index 21896f0a787..84a12a38c26 100644 --- a/spec/features/projects/labels/update_prioritization_spec.rb +++ b/spec/features/projects/labels/update_prioritization_spec.rb @@ -22,8 +22,8 @@ feature 'Prioritize labels', feature: true do expect(page).to have_content('No prioritized labels yet') - page.within('.group-labels') do - first('.js-toggle-priority').click + page.within('.other-labels') do + all('.js-toggle-priority')[1].click wait_for_ajax expect(page).not_to have_content('feature') end @@ -47,7 +47,7 @@ feature 'Prioritize labels', feature: true do expect(page).not_to have_content('bug') end - page.within('.group-labels') do + page.within('.other-labels') do expect(page).to have_content('feature') end end @@ -57,7 +57,7 @@ feature 'Prioritize labels', feature: true do expect(page).to have_content('No prioritized labels yet') - page.within('.project-labels') do + page.within('.other-labels') do first('.js-toggle-priority').click wait_for_ajax expect(page).not_to have_content('bug') @@ -82,7 +82,7 @@ feature 'Prioritize labels', feature: true do expect(page).not_to have_content('bug') end - page.within('.project-labels') do + page.within('.other-labels') do expect(page).to have_content('bug') expect(page).to have_content('wontfix') end