Implement review suggestions

CHANGELOG

@@ -20,6 +20,7 @@ v 8.7.0 (unreleased)
   - Ensure empty recipients are rejected in BuildsEmailService
   - API: Ability to filter milestones by state `active` and `closed` (Robert Schilling)
   - Implement 'Groups View' as an option for dashboard preferences !3379 (Elias W.)
+  - Better errors handling when creating milestones inside groups
   - Implement 'TODOs View' as an option for dashboard preferences !3379 (Elias W.)
   - Gracefully handle notes on deleted commits in merge requests (Stan Hu)
   - Fix creation of merge requests for orphaned branches (Stan Hu)

app/controllers/groups/milestones_controller.rb

@@ -21,7 +21,7 @@ class Groups::MilestonesController < Groups::ApplicationController
     project_ids = params[:milestone][:project_ids].reject(&:blank?)
     title = milestone_params[:title]
 
-    if create_milestones(project_ids, title)
+    if create_milestones(project_ids)
       redirect_to milestone_path(title)
     else
       render_new_with_error(project_ids.empty?)
@@ -41,7 +41,7 @@ class Groups::MilestonesController < Groups::ApplicationController
 
   private
 
-  def create_milestones(project_ids, title)
+  def create_milestones(project_ids)
     return false unless project_ids.present?
 
     ActiveRecord::Base.transaction do
@@ -51,9 +51,7 @@ class Groups::MilestonesController < Groups::ApplicationController
     end
 
     true
-
-    rescue => e
-
+  rescue ActiveRecord::ActiveRecordError => e
     flash.now[:alert] = "An error occurred while creating the milestone: #{e.message}"
     false
   end

app/services/milestones/create_service.rb

@@ -3,7 +3,7 @@ module Milestones
     def execute
       milestone = project.milestones.new(params)
 
-      if milestone.save
+      if milestone.save!
         event_service.open_milestone(milestone, current_user)
       end
 

spec/controllers/groups/milestones_controller_spec.rb

@@ -27,7 +27,7 @@ describe Groups::MilestonesController do
     it "redirects to new when there are no project ids" do
       post :create, group_id: group.id, milestone: { title: title, project_ids: [""] }
       expect(response).to render_template :new
-      expect(assigns(:milestone).errors).to_not be_nil
+      expect(assigns(:milestone).errors).not_to be_nil
     end
   end
 end