diff --git a/CHANGELOG b/CHANGELOG index 18efe0572993e5df10a7f5d2b072fe814deceb4f..0a1e47be2c8670db02310c371faf40544a6334a8 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -49,6 +49,7 @@ v 8.12.0 (unreleased) v 8.11.4 (unreleased) - Fix broken gitlab:backup:restore because of bad permissions on repo storage !6098 (Dirk Hörner) + - Creating an issue through our API now emails label subscribers !5720 v 8.11.4 (unreleased) - Fix resolving conflicts on forks diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index e06c37c323ed3bc4e47a67cf64e028f5945e9948..4c8d93999a7049f218ea3a273a4a7dd47b03e041 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -45,6 +45,7 @@ class IssuableBaseService < BaseService unless can?(current_user, ability, project) params.delete(:milestone_id) + params.delete(:labels) params.delete(:add_label_ids) params.delete(:remove_label_ids) params.delete(:label_ids) @@ -72,6 +73,7 @@ class IssuableBaseService < BaseService filter_labels_in_param(:add_label_ids) filter_labels_in_param(:remove_label_ids) filter_labels_in_param(:label_ids) + find_or_create_label_ids end def filter_labels_in_param(key) @@ -80,6 +82,17 @@ class IssuableBaseService < BaseService params[key] = project.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 + end + end + def process_label_ids(attributes, existing_label_ids: nil) label_ids = attributes.delete(:label_ids) add_label_ids = attributes.delete(:add_label_ids) @@ -162,7 +175,12 @@ class IssuableBaseService < BaseService if params.present? && update_issuable(issuable, params) issuable.reset_events_cache - handle_common_system_notes(issuable, old_labels: old_labels) + + # We do not touch as it will affect a update on updated_at field + ActiveRecord::Base.no_touching do + handle_common_system_notes(issuable, old_labels: old_labels) + end + handle_changes(issuable, old_labels: old_labels, old_mentioned_users: old_mentioned_users) issuable.create_new_cross_references!(current_user) execute_hooks(issuable, 'update') diff --git a/lib/api/issues.rb b/lib/api/issues.rb index 077258faee19235a01025d992e04fe9e528b6cd5..d0bc7243e548490b23f5df6e967823c5fb63d277 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -154,21 +154,15 @@ module API render_api_error!({ labels: errors }, 400) end - project = user_project + attrs[:labels] = params[:labels] if params[:labels] - issue = ::Issues::CreateService.new(project, current_user, attrs.merge(request: request, api: true)).execute + issue = ::Issues::CreateService.new(user_project, current_user, attrs.merge(request: request, api: true)).execute if issue.spam? render_api_error!({ error: 'Spam detected' }, 400) end if issue.valid? - # Find or create labels and attach to issue. Labels are valid because - # we already checked its name, so there can't be an error here - if params[:labels].present? - issue.add_labels_by_names(params[:labels].split(',')) - end - present issue, with: Entities::Issue, current_user: current_user else render_validation_error!(issue) @@ -202,17 +196,11 @@ module API render_api_error!({ labels: errors }, 400) end + attrs[:labels] = params[:labels] if params[:labels] + issue = ::Issues::UpdateService.new(user_project, current_user, attrs).execute(issue) if issue.valid? - # Find or create labels and attach to issue. Labels are valid because - # we already checked its name, so there can't be an error here - if params[:labels] && can?(current_user, :admin_issue, user_project) - issue.remove_labels - # Create and add labels to the new created issue - issue.add_labels_by_names(params[:labels].split(',')) - end - present issue, with: Entities::Issue, current_user: current_user else render_validation_error!(issue) diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index b8038fc85a19774feb57e5ae0bd7db5de60d5383..3362a88d798b37e3855cb311e044a470ede4b8de 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' describe API::API, api: true do include ApiHelpers + let(:user) { create(:user) } let(:user2) { create(:user) } let(:non_member) { create(:user) } @@ -478,6 +479,18 @@ describe API::API, api: true do expect(json_response['labels']).to eq(['label', 'label2']) end + it "sends notifications for subscribers of newly added labels" do + label = project.labels.first + label.toggle_subscription(user2) + + perform_enqueued_jobs do + post api("/projects/#{project.id}/issues", user), + title: 'new issue', labels: label.title + end + + should_email(user2) + end + it "returns a 400 bad request if title not given" do post api("/projects/#{project.id}/issues", user), labels: 'label, label2' expect(response).to have_http_status(400) @@ -633,6 +646,18 @@ describe API::API, api: true do expect(json_response['labels']).to eq([label.title]) end + it "sends notifications for subscribers of newly added labels when issue is updated" do + label = create(:label, title: 'foo', color: '#FFAABB', project: project) + label.toggle_subscription(user2) + + perform_enqueued_jobs do + put api("/projects/#{project.id}/issues/#{issue.id}", user), + title: 'updated title', labels: label.title + end + + should_email(user2) + end + it 'removes all labels' do put api("/projects/#{project.id}/issues/#{issue.id}", user), labels: ''