From 398ab263fd08a5d9d7b19c5b3d06f33814a474eb Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 19 Sep 2016 17:21:39 -0300 Subject: [PATCH] Allow users to apply group labels on Issues/MRs --- .../dashboard/labels_controller.rb | 5 ++++- app/controllers/projects/issues_controller.rb | 9 +++++--- app/controllers/projects/labels_controller.rb | 2 +- .../projects/merge_requests_controller.rb | 5 ++++- app/finders/issuable_finder.rb | 4 +++- app/models/label.rb | 6 +++++- app/services/issuable_base_service.rb | 15 ++++++++----- app/services/projects/autocomplete_service.rb | 2 +- app/views/shared/issuable/_filter.html.haml | 9 ++++---- app/views/shared/issuable/_form.html.haml | 2 +- spec/services/issues/create_service_spec.rb | 21 +++++++++++++++++++ 11 files changed, 60 insertions(+), 20 deletions(-) diff --git a/app/controllers/dashboard/labels_controller.rb b/app/controllers/dashboard/labels_controller.rb index 2a88350a4ca..797f8503b2d 100644 --- a/app/controllers/dashboard/labels_controller.rb +++ b/app/controllers/dashboard/labels_controller.rb @@ -1,6 +1,9 @@ class Dashboard::LabelsController < Dashboard::ApplicationController def index - labels = Label.where(project_id: projects).select(:id, :title, :color).uniq(:title) + labels = LabelsFinder.new(current_user, project_id: projects) + .execute + .select(:id, :title, :color) + .uniq(:title) respond_to do |format| format.json { render json: labels } diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 96041b07647..d85bc85092f 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -25,8 +25,7 @@ class Projects::IssuesController < Projects::ApplicationController def index @issues = issues_collection @issues = @issues.page(params[:page]) - - @labels = @project.labels.where(title: params[:label_name]) + @labels = LabelsFinder.new(current_user, project_id: @project.id, title: params[:label_name]).execute if params[:label_name].presence respond_to do |format| format.html @@ -45,11 +44,15 @@ class Projects::IssuesController < Projects::ApplicationController assignee_id: "" ) - @issue = @noteable = @project.issues.new(issue_params) + @issue = @noteable = @project.issues.new(issue_params) + @labels = LabelsFinder.new(current_user, project_id: @project.id).execute + respond_with(@issue) end def edit + @labels = LabelsFinder.new(current_user, project_id: @project.id).execute + respond_with(@issue) end diff --git a/app/controllers/projects/labels_controller.rb b/app/controllers/projects/labels_controller.rb index a6626df4826..35224f965bf 100644 --- a/app/controllers/projects/labels_controller.rb +++ b/app/controllers/projects/labels_controller.rb @@ -17,7 +17,7 @@ class Projects::LabelsController < Projects::ApplicationController respond_to do |format| format.html format.json do - render json: @project.labels + render json: LabelsFinder.new(current_user, project_id: @project.id).execute end end end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 04386258c19..c8970155497 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -40,7 +40,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController @merge_requests = @merge_requests.page(params[:page]) @merge_requests = @merge_requests.preload(:target_project) - @labels = @project.labels.where(title: params[:label_name]) + @labels = LabelsFinder.new(current_user, project_id: @project.id, title: params[:label_name]).execute if params[:label_name].presence respond_to do |format| format.html @@ -263,6 +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 end def update @@ -575,6 +576,8 @@ class Projects::MergeRequestsController < Projects::ApplicationController @note_counts = Note.where(commit_id: @commits.map(&:id)). group(:commit_id).count + @labels = LabelsFinder.new(current_user, project_id: @project.id).execute + define_pipelines_vars end diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 9f170428100..37151f8d134 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -274,8 +274,10 @@ class IssuableFinder items = items.without_label else items = items.with_label(label_names, params[:sort]) + if projects - items = items.where(labels: { project_id: projects }) + label_ids = LabelsFinder.new(current_user, project_id: projects).execute.select(:id) + items = items.where(labels: { id: label_ids }) end end end diff --git a/app/models/label.rb b/app/models/label.rb index e8e12e2904e..295c5bfaf70 100644 --- a/app/models/label.rb +++ b/app/models/label.rb @@ -23,7 +23,7 @@ class Label < ActiveRecord::Base has_many :merge_requests, through: :label_links, source: :target, source_type: 'MergeRequest' validates :color, color: true, allow_blank: false - validates :project, presence: true, unless: Proc.new { |service| service.template? } + validates :project, presence: true, if: :project_label? # Don't allow ',' for label titles validates :title, @@ -127,6 +127,10 @@ class Label < ActiveRecord::Base private + def project_label? + type.blank? && !template? + end + def label_format_reference(format = :id) raise StandardError, 'Unknown format' unless [:id, :name].include?(format) diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 57d521f2fea..b3e4f8dcf27 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -80,17 +80,18 @@ class IssuableBaseService < BaseService def filter_labels_in_param(key) return if params[key].to_a.empty? - params[key] = project.labels.where(id: params[key]).pluck(:id) + params[key] = available_labels.where(id: params[key]).pluck(:id) end def find_or_create_label_ids labels = params.delete(:labels) return unless labels - params[:label_ids] = labels.split(",").map do |label_name| - project.labels.create_with(color: Label::DEFAULT_COLOR) - .find_or_create_by(title: label_name.strip) - .id + params[:label_ids] = labels.split(',').map do |label_name| + label = available_labels.find_by(title: title).select(:id) + label ||= project.labels.create(title: label_name.strip, color: Label::DEFAULT_COLOR) + + label.id end end @@ -111,6 +112,10 @@ class IssuableBaseService < BaseService new_label_ids end + def available_labels + LabelsFinder.new(current_user, project_id: @project.id).execute + end + def merge_slash_commands_into_params!(issuable) description, command_params = SlashCommands::InterpretService.new(project, current_user). diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb index f578f8dbea2..015f2828921 100644 --- a/app/services/projects/autocomplete_service.rb +++ b/app/services/projects/autocomplete_service.rb @@ -13,7 +13,7 @@ module Projects end def labels - @project.labels.select([:title, :color]) + LabelsFinder.new(current_user, project_id: project.id).execute.select([:title, :color]) end def commands(noteable, type) diff --git a/app/views/shared/issuable/_filter.html.haml b/app/views/shared/issuable/_filter.html.haml index 31620297be0..8c2036a1cde 100644 --- a/app/views/shared/issuable/_filter.html.haml +++ b/app/views/shared/issuable/_filter.html.haml @@ -77,11 +77,10 @@ = hidden_field_tag :state_event, params[:state_event] .filter-item.inline = button_tag "Update #{type.to_s.humanize(capitalize: false)}", class: "btn update_selected_issues btn-save" - - - if !@labels.nil? - .row-content-block.second-block.filtered-labels{ class: ("hidden" if !@labels.any?) } - - if @labels.any? - = render "shared/labels_row", labels: @labels + - has_labels = @labels && @labels.any? + .row-content-block.second-block.filtered-labels{ class: ("hidden" unless has_labels) } + - if has_labels + = render 'shared/labels_row', labels: @labels :javascript new UsersSelect(); diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml index a7944a60130..34c66a17303 100644 --- a/app/views/shared/issuable/_form.html.haml +++ b/app/views/shared/issuable/_form.html.haml @@ -95,7 +95,7 @@ .issuable-form-select-holder = render "shared/issuable/milestone_dropdown", selected: issuable.milestone, name: "#{issuable.class.model_name.param_key}[milestone_id]", show_any: false, show_menu_above: true, show_upcoming: false, extra_class: "js-issuable-form-dropdown js-dropdown-keep-input", dropdown_title: "Select milestone" .form-group - - has_labels = issuable.project.labels.any? + - has_labels = @labels && @labels.any? = f.label :label_ids, "Labels", class: "control-label #{"col-lg-4" if has_due_date}" = f.hidden_field :label_ids, multiple: true, value: '' .col-sm-10{ class: "#{"col-lg-8" if has_due_date} #{'issuable-form-padding-top' if !has_labels}" } diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 1050502fa19..5c0331ebe66 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -67,6 +67,27 @@ describe Issues::CreateService, services: true do expect(Todo.where(attributes).count).to eq 1 end + context 'when label belongs to project group' do + let(:group) { create(:group) } + let(:group_labels) { create_pair(:group_label, group: group) } + + let(:opts) do + { + title: 'Title', + description: 'Description', + label_ids: group_labels.map(&:id) + } + end + + before do + project.update(group: group) + end + + it 'assigns group labels' do + expect(issue.labels).to match_array group_labels + end + end + context 'when label belongs to different project' do let(:label) { create(:label) }