Improve code

3 files changed, 22 insertions, 17 deletions

diff --git a/app/controllers/groups/milestones_controller.rb b/app/controllers/groups/milestones_controller.rb
index 21fc329f233..fcf19e8066a 100644
--- a/app/controllers/groups/milestones_controller.rb
+++ b/app/controllers/groups/milestones_controller.rb
@@ -24,7 +24,7 @@ class Groups::MilestonesController < Groups::ApplicationController
     if create_milestones(project_ids, title)
       redirect_to milestone_path(title)
     else
-      render_new_with_error(@error)
+      render_new_with_error(project_ids.empty?)
     end
   end
 
@@ -42,28 +42,25 @@ class Groups::MilestonesController < Groups::ApplicationController
   private
 
   def create_milestones(project_ids, title)
-    unless project_ids.present?
-      @error = "Please select at least one project."
-      return false
-    end
+    return false unless project_ids.present?
 
-    begin
-      ActiveRecord::Base.transaction do
-        @projects.where(id: project_ids).each do |project|
-          Milestones::CreateService.new(project, current_user, milestone_params).execute
-        end
+    ActiveRecord::Base.transaction do
+      @projects.where(id: project_ids).each do |project|
+        Milestones::CreateService.new(project, current_user, milestone_params).execute
       end
+    end
+
+    true
 
-      true
     rescue => e
-      @error = "Error creating milestone: #{e.message}."
-      false
-    end
+
+    flash.now[:alert] = "An error occurred while creating the milestone: #{e.message}"
+    false
   end
 
-  def render_new_with_error(error)
+  def render_new_with_error(empty_project_ids)
     @milestone = Milestone.new(milestone_params)
-    flash[:alert] = error
+    @milestone.errors.add(:project_id, "Please select at least one project.") if empty_project_ids
     render :new
   end
 
diff --git a/app/views/groups/milestones/new.html.haml b/app/views/groups/milestones/new.html.haml
index a8e1ed77da9..4290e0bf72e 100644
--- a/app/views/groups/milestones/new.html.haml
+++ b/app/views/groups/milestones/new.html.haml
@@ -10,6 +10,14 @@
 
 = form_for @milestone, url: group_milestones_path(@group), html: { class: 'form-horizontal milestone-form gfm-form js-quick-submit js-requires-input' }  do |f|
   .row
+    - if @milestone.errors.any?
+      #error_explanation
+        .alert.alert-danger
+          %ul
+            - @milestone.errors.full_messages.each do |msg|
+              %li
+                = msg
+
     .col-md-6
       .form-group
         = f.label :title, "Title", class: "control-label"
diff --git a/spec/controllers/groups/milestones_controller_spec.rb b/spec/controllers/groups/milestones_controller_spec.rb
index f258d6fa0c7..9c7b5c74b8e 100644
--- a/spec/controllers/groups/milestones_controller_spec.rb
+++ b/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(flash[:alert]).to_not be_nil
+      expect(assigns(:milestone).errors).to_not be_nil
     end
   end
 end