Refactor global and group milestones logic

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

6 files changed, 53 insertions, 76 deletions

diff --git a/app/controllers/concerns/global_milestones.rb b/app/controllers/concerns/global_milestones.rb
new file mode 100644
index 00000000000..b428249acd3
--- /dev/null
+++ b/app/controllers/concerns/global_milestones.rb
@@ -0,0 +1,19 @@
+module GlobalMilestones
+  extend ActiveSupport::Concern
+
+  def milestones
+    @milestones = MilestonesFinder.new.execute(@projects, params)
+    @milestones = GlobalMilestone.build_collection(@milestones)
+    @milestones = Kaminari.paginate_array(@milestones).page(params[:page]).per(ApplicationController::PER_PAGE)
+  end
+
+  def milestone
+    milestones = Milestone.of_projects(@projects).where(title: params[:title])
+
+    if milestones.present?
+      @milestone = GlobalMilestone.new(params[:title], milestones)
+    else
+      render_404
+    end
+  end
+end
diff --git a/app/controllers/dashboard/milestones_controller.rb b/app/controllers/dashboard/milestones_controller.rb
index 53896d4f2c7..2bdce0f8a00 100644
--- a/app/controllers/dashboard/milestones_controller.rb
+++ b/app/controllers/dashboard/milestones_controller.rb
@@ -1,34 +1,19 @@
 class Dashboard::MilestonesController < Dashboard::ApplicationController
-  before_action :load_projects
+  include GlobalMilestones
+
+  before_action :projects
+  before_action :milestones, only: [:index]
+  before_action :milestone, only: [:show]
 
   def index
-    project_milestones = case params[:state]
-                         when 'all'; state
-                         when 'closed'; state('closed')
-                         else state('active')
-                         end
-    @dashboard_milestones = Milestones::GroupService.new(project_milestones).execute
-    @dashboard_milestones = Kaminari.paginate_array(@dashboard_milestones).page(params[:page]).per(PER_PAGE)
   end
 
   def show
-    project_milestones = Milestone.where(project_id: @projects).order("due_date ASC")
-    @dashboard_milestone = Milestones::GroupService.new(project_milestones).milestone(title)
   end
 
   private
 
-  def load_projects
-    @projects = current_user.authorized_projects.sorted_by_activity.non_archived
-  end
-
-  def title
-    params[:title]
-  end
-
-  def state(state = nil)
-    conditions = { project_id: @projects }
-    conditions.reverse_merge!(state: state) if state
-    Milestone.where(conditions).order("title ASC")
+  def projects
+    @projects ||= current_user.authorized_projects.sorted_by_activity.non_archived
   end
 end
diff --git a/app/controllers/groups/application_controller.rb b/app/controllers/groups/application_controller.rb
index 6878d4bc07e..be801858eaf 100644
--- a/app/controllers/groups/application_controller.rb
+++ b/app/controllers/groups/application_controller.rb
@@ -1,8 +1,13 @@
 class Groups::ApplicationController < ApplicationController
   layout 'group'
+  before_action :group
 
   private
-  
+
+  def group
+    @group ||= Group.find_by(path: params[:group_id])
+  end
+
   def authorize_read_group!
     unless @group and can?(current_user, :read_group, @group)
       if current_user.nil?
@@ -12,13 +17,13 @@ class Groups::ApplicationController < ApplicationController
       end
     end
   end
-  
+
   def authorize_admin_group!
     unless can?(current_user, :admin_group, group)
       return render_404
     end
   end
-  
+
   def authorize_admin_group_member!
     unless can?(current_user, :admin_group_member, group)
       return render_403
diff --git a/app/controllers/groups/avatars_controller.rb b/app/controllers/groups/avatars_controller.rb
index 6aa64222f77..f390705dc6a 100644
--- a/app/controllers/groups/avatars_controller.rb
+++ b/app/controllers/groups/avatars_controller.rb
@@ -1,8 +1,6 @@
 class Groups::AvatarsController < ApplicationController
   def destroy
-    @group = Group.find_by(path: params[:group_id])
     @group.remove_avatar!
-
     @group.save
 
     redirect_to edit_group_path(@group)
diff --git a/app/controllers/groups/group_members_controller.rb b/app/controllers/groups/group_members_controller.rb
index 91518c44a98..b25957a06e2 100644
--- a/app/controllers/groups/group_members_controller.rb
+++ b/app/controllers/groups/group_members_controller.rb
@@ -1,6 +1,5 @@
 class Groups::GroupMembersController < Groups::ApplicationController
   skip_before_action :authenticate_user!, only: [:index]
-  before_action :group
 
   # Authorize
   before_action :authorize_read_group!
@@ -80,10 +79,6 @@ class Groups::GroupMembersController < Groups::ApplicationController
 
   protected
 
-  def group
-    @group ||= Group.find_by(path: params[:group_id])
-  end
-
   def member_params
     params.require(:group_member).permit(:access_level, :user_id)
   end
diff --git a/app/controllers/groups/milestones_controller.rb b/app/controllers/groups/milestones_controller.rb
index 8779376d93c..6833a550c9e 100644
--- a/app/controllers/groups/milestones_controller.rb
+++ b/app/controllers/groups/milestones_controller.rb
@@ -1,21 +1,16 @@
 class Groups::MilestonesController < Groups::ApplicationController
-  before_action :authorize_group_milestone!, only: :update
-  before_action :group
+  include GlobalMilestones
 
-  def index
-    project_milestones =
-      case params[:state]
-      when 'all'; state
-      when 'closed'; state('closed')
-      else state('active')
-      end
+  before_action :projects
+  before_action :milestones, only: [:index]
+  before_action :milestone, only: [:show, :update]
+  before_action :authorize_group_milestone!, only: [:create, :update]
 
-    @group_milestones = Milestones::GroupService.new(project_milestones).execute
-    @group_milestones = Kaminari.paginate_array(@group_milestones).page(params[:page]).per(PER_PAGE)
+  def index
   end
 
   def new
-    @group_milestone = OpenStruct.new(title: nil, description: nil)
+    @milestone = Milestone.new
   end
 
   def create
@@ -26,55 +21,35 @@ class Groups::MilestonesController < Groups::ApplicationController
       Milestones::CreateService.new(project, current_user, milestone_params).execute
     end
 
-    redirect_to group_milestone_path(@group, title.parameterize, title: title)
+    redirect_to milestone_path(title)
   end
 
   def show
-    project_milestones = Milestone.where(project_id: group.projects).order("due_date ASC")
-    @group_milestone = Milestones::GroupService.new(project_milestones).milestone(title)
   end
 
   def update
-    project_milestones = Milestone.where(project_id: group.projects).order("due_date ASC")
-    @group_milestones = Milestones::GroupService.new(project_milestones).milestone(title)
-
-    @group_milestones.milestones.each do |milestone|
-      Milestones::UpdateService.new(milestone.project, current_user, params[:milestone]).execute(milestone)
+    @milestone.milestones.each do |milestone|
+      Milestones::UpdateService.new(milestone.project, current_user, milestone_params).execute(milestone)
     end
 
-    respond_to do |format|
-      format.js
-      format.html do
-        redirect_to group_milestones_path(group)
-      end
-    end
+    redirect_back_or_default(default: milestone_path(@milestone.title))
   end
 
   private
 
-  def group
-    @group ||= Group.find_by(path: params[:group_id])
-  end
-
-  def title
-    params[:title]
+  def authorize_group_milestone!
+    return render_404 unless can?(current_user, :admin_group, group)
   end
 
-  def state(state = nil)
-    conditions = { project_id: group.projects }
-    conditions.reverse_merge!(state: state) if state
-    Milestone.where(conditions).order("title ASC")
+  def milestone_params
+    params.require(:milestone).permit(:title, :description, :due_date, :state_event)
   end
 
-  def authorize_group_milestone!
-    return render_404 unless can?(current_user, :admin_group, group)
+  def milestone_path(title)
+    group_milestone_path(@group, title.parameterize, title: title)
   end
 
-  def milestone_params
-    params.require(:milestone).permit(
-      :title,
-      :description,
-      :due_date
-    )
+  def projects
+    @projects ||= @group.projects
   end
 end