diff options
author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2015-11-13 19:20:48 +0100 |
---|---|---|
committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2015-11-16 14:07:38 +0100 |
commit | 986695e136a8f6afa326048b30be77a9265d3bf7 (patch) | |
tree | 390e60bd0213b3c431094cfe71395be845b77714 | |
parent | 05335a3c8584c48a9317bd0919eccee6948de742 (diff) | |
download | gitlab-ce-986695e136a8f6afa326048b30be77a9265d3bf7.tar.gz |
Refactor global and group milestones logic
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
19 files changed, 145 insertions, 165 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 diff --git a/app/finders/milestones_finder.rb b/app/finders/milestones_finder.rb new file mode 100644 index 00000000000..71f207ca030 --- /dev/null +++ b/app/finders/milestones_finder.rb @@ -0,0 +1,13 @@ +class MilestonesFinder + def execute(projects, params) + milestones = Milestone.of_projects(projects) + milestones = milestones.order("due_date ASC") + + case params[:state] + when 'closed' then milestones.closed + when 'all' then milestones + else milestones.active + end + end +end + diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb index ee04ace35d0..795fb439f25 100644 --- a/app/helpers/labels_helper.rb +++ b/app/helpers/labels_helper.rb @@ -100,7 +100,7 @@ module LabelsHelper Label.where(project_id: @projects) end - grouped_labels = Labels::GroupService.new(labels).execute + grouped_labels = GlobalLabel.build_collection(labels) grouped_labels.unshift(Label::None) grouped_labels.unshift(Label::Any) diff --git a/app/helpers/milestones_helper.rb b/app/helpers/milestones_helper.rb index 37a5b58cce8..ad43892b639 100644 --- a/app/helpers/milestones_helper.rb +++ b/app/helpers/milestones_helper.rb @@ -28,7 +28,7 @@ module MilestonesHelper Milestone.where(project_id: @projects) end.active - grouped_milestones = Milestones::GroupService.new(milestones).execute + grouped_milestones = GlobalMilestone.build_collection(milestones) grouped_milestones.unshift(Milestone::None) grouped_milestones.unshift(Milestone::Any) diff --git a/app/models/global_label.rb b/app/models/global_label.rb new file mode 100644 index 00000000000..0171f7d54b7 --- /dev/null +++ b/app/models/global_label.rb @@ -0,0 +1,17 @@ +class GlobalLabel + attr_accessor :title, :labels + alias_attribute :name, :title + + def self.build_collection(labels) + labels = labels.group_by(&:title) + + labels.map do |title, label| + new(title, label) + end + end + + def initialize(title, labels) + @title = title + @labels = labels + end +end diff --git a/app/models/group_milestone.rb b/app/models/global_milestone.rb index 91844da62e2..f96e9d41c94 100644 --- a/app/models/group_milestone.rb +++ b/app/models/global_milestone.rb @@ -1,7 +1,15 @@ -class GroupMilestone +class GlobalMilestone attr_accessor :title, :milestones alias_attribute :name, :title + def self.build_collection(milestones) + milestones = milestones.group_by(&:title) + + milestones.map do |title, milestones| + new(title, milestones) + end + end + def initialize(title, milestones) @title = title @milestones = milestones @@ -10,7 +18,7 @@ class GroupMilestone def safe_title @title.parameterize end - + def projects milestones.map { |milestone| milestone.project } end @@ -60,15 +68,15 @@ class GroupMilestone end def issues - @group_issues ||= milestones.map(&:issues).flatten.group_by(&:state) + @issues ||= milestones.map(&:issues).flatten.group_by(&:state) end def merge_requests - @group_merge_requests ||= milestones.map(&:merge_requests).flatten.group_by(&:state) + @merge_requests ||= milestones.map(&:merge_requests).flatten.group_by(&:state) end def participants - @group_participants ||= milestones.map(&:participants).flatten.compact.uniq + @participants ||= milestones.map(&:participants).flatten.compact.uniq end def opened_issues diff --git a/app/models/group_label.rb b/app/models/group_label.rb deleted file mode 100644 index 0fc39cb8771..00000000000 --- a/app/models/group_label.rb +++ /dev/null @@ -1,9 +0,0 @@ -class GroupLabel - attr_accessor :title, :labels - alias_attribute :name, :title - - def initialize(title, labels) - @title = title - @labels = labels - end -end diff --git a/app/services/labels/group_service.rb b/app/services/labels/group_service.rb deleted file mode 100644 index b26cee24d56..00000000000 --- a/app/services/labels/group_service.rb +++ /dev/null @@ -1,26 +0,0 @@ -module Labels - class GroupService < ::BaseService - def initialize(project_labels) - @project_labels = project_labels.group_by(&:title) - end - - def execute - build(@project_labels) - end - - def label(title) - if title - group_label = @project_labels[title].group_by(&:title) - build(group_label).first - else - nil - end - end - - private - - def build(label) - label.map { |title, labels| GroupLabel.new(title, labels) } - end - end -end diff --git a/app/services/milestones/group_service.rb b/app/services/milestones/collection_service.rb index 11d702f1e7b..2eefec99e7b 100644 --- a/app/services/milestones/group_service.rb +++ b/app/services/milestones/collection_service.rb @@ -1,5 +1,5 @@ module Milestones - class GroupService < Milestones::BaseService + class CollectionService < Milestones::BaseService def initialize(project_milestones) @project_milestones = project_milestones.group_by(&:title) end diff --git a/app/views/dashboard/milestones/index.html.haml b/app/views/dashboard/milestones/index.html.haml index 21b25c3986e..635251e2374 100644 --- a/app/views/dashboard/milestones/index.html.haml +++ b/app/views/dashboard/milestones/index.html.haml @@ -10,10 +10,10 @@ .milestones %ul.content-list - - if @dashboard_milestones.blank? + - if @milestones.blank? %li .nothing-here-block No milestones to show - else - - @dashboard_milestones.each do |milestone| + - @milestones.each do |milestone| = render 'milestone', milestone: milestone - = paginate @dashboard_milestones, theme: "gitlab" + = paginate @milestones, theme: "gitlab" diff --git a/app/views/dashboard/milestones/show.html.haml b/app/views/dashboard/milestones/show.html.haml index 2fe14c6388c..580db613ed4 100644 --- a/app/views/dashboard/milestones/show.html.haml +++ b/app/views/dashboard/milestones/show.html.haml @@ -1,14 +1,14 @@ -- page_title @dashboard_milestone.title, "Milestones" +- page_title @milestone.title, "Milestones" %h4.page-title - .issue-box{ class: "issue-box-#{@dashboard_milestone.closed? ? 'closed' : 'open'}" } - - if @dashboard_milestone.closed? + .issue-box{ class: "issue-box-#{@milestone.closed? ? 'closed' : 'open'}" } + - if @milestone.closed? Closed - else Open - Milestone #{@dashboard_milestone.title} + Milestone #{@milestone.title} %hr -- if (@dashboard_milestone.total_items_count == @dashboard_milestone.closed_items_count) && @dashboard_milestone.active? +- if (@milestone.total_items_count == @milestone.closed_items_count) && @milestone.active? .alert.alert-success %span All issues for this milestone are closed. You may close the milestone now. @@ -22,7 +22,7 @@ %th Open issues %th State %th Due date - - @dashboard_milestone.milestones.each do |milestone| + - @milestone.milestones.each do |milestone| %tr %td = link_to "#{milestone.project.name_with_namespace}", namespace_project_milestone_path(milestone.project.namespace, milestone.project, milestone) @@ -39,46 +39,46 @@ .context %p.lead Progress: - #{@dashboard_milestone.closed_items_count} closed + #{@milestone.closed_items_count} closed – - #{@dashboard_milestone.open_items_count} open - = milestone_progress_bar(@dashboard_milestone) + #{@milestone.open_items_count} open + = milestone_progress_bar(@milestone) %ul.nav.nav-tabs %li.active = link_to '#tab-issues', 'data-toggle' => 'tab' do Issues - %span.badge= @dashboard_milestone.issue_count + %span.badge= @milestone.issue_count %li = link_to '#tab-merge-requests', 'data-toggle' => 'tab' do Merge Requests - %span.badge= @dashboard_milestone.merge_requests_count + %span.badge= @milestone.merge_requests_count %li = link_to '#tab-participants', 'data-toggle' => 'tab' do Participants - %span.badge= @dashboard_milestone.participants.count + %span.badge= @milestone.participants.count .pull-right - = link_to 'Browse Issues', issues_dashboard_path(milestone_title: @dashboard_milestone.title), class: "btn edit-milestone-link btn-grouped" + = link_to 'Browse Issues', issues_dashboard_path(milestone_title: @milestone.title), class: "btn edit-milestone-link btn-grouped" .tab-content .tab-pane.active#tab-issues .row .col-md-6 - = render 'issues', title: "Open", issues: @dashboard_milestone.opened_issues + = render 'issues', title: "Open", issues: @milestone.opened_issues .col-md-6 - = render 'issues', title: "Closed", issues: @dashboard_milestone.closed_issues + = render 'issues', title: "Closed", issues: @milestone.closed_issues .tab-pane#tab-merge-requests .row .col-md-6 - = render 'merge_requests', title: "Open", merge_requests: @dashboard_milestone.opened_merge_requests + = render 'merge_requests', title: "Open", merge_requests: @milestone.opened_merge_requests .col-md-6 - = render 'merge_requests', title: "Closed", merge_requests: @dashboard_milestone.closed_merge_requests + = render 'merge_requests', title: "Closed", merge_requests: @milestone.closed_merge_requests .tab-pane#tab-participants %ul.bordered-list - - @dashboard_milestone.participants.each do |user| + - @milestone.participants.each do |user| %li = link_to user, title: user.name, class: "darken" do = image_tag avatar_icon(user, 32), class: "avatar s32" diff --git a/app/views/groups/milestones/index.html.haml b/app/views/groups/milestones/index.html.haml index ded4f3713f6..ffd7dd3fc0b 100644 --- a/app/views/groups/milestones/index.html.haml +++ b/app/views/groups/milestones/index.html.haml @@ -14,10 +14,10 @@ group are listed here. .milestones %ul.content-list - - if @group_milestones.blank? + - if @milestones.blank? %li .nothing-here-block No milestones to show - else - - @group_milestones.each do |milestone| + - @milestones.each do |milestone| = render 'milestone', milestone: milestone - = paginate @group_milestones, theme: "gitlab" + = paginate @milestones, theme: "gitlab" diff --git a/app/views/groups/milestones/new.html.haml b/app/views/groups/milestones/new.html.haml index 287f89a7074..4c490d8ccb3 100644 --- a/app/views/groups/milestones/new.html.haml +++ b/app/views/groups/milestones/new.html.haml @@ -5,7 +5,7 @@ This will create milestone in every selected project %hr -= form_for @group_milestone, as: :milestone, url: group_milestones_path(@group), html: { class: 'form-horizontal milestone-form gfm-form js-requires-input' } do |f| += form_for @milestone, url: group_milestones_path(@group), html: { class: 'form-horizontal milestone-form gfm-form js-requires-input' } do |f| .row .col-md-6 .form-group diff --git a/app/views/groups/milestones/show.html.haml b/app/views/groups/milestones/show.html.haml index a92ad5d751b..e609abca08e 100644 --- a/app/views/groups/milestones/show.html.haml +++ b/app/views/groups/milestones/show.html.haml @@ -1,22 +1,22 @@ -- page_title @group_milestone.title, "Milestones" +- page_title @milestone.title, "Milestones" = render "header_title" %h4.page-title - .issue-box{ class: "issue-box-#{@group_milestone.closed? ? 'closed' : 'open'}" } - - if @group_milestone.closed? + .issue-box{ class: "issue-box-#{@milestone.closed? ? 'closed' : 'open'}" } + - if @milestone.closed? Closed - else Open - Milestone #{@group_milestone.title} + Milestone #{@milestone.title} .pull-right - if can?(current_user, :admin_group, @group) - - if @group_milestone.active? - = link_to 'Close Milestone', group_milestone_path(@group, @group_milestone.safe_title, title: @group_milestone.title, milestone: {state_event: :close }), method: :put, class: "btn btn-sm btn-close" + - if @milestone.active? + = link_to 'Close Milestone', group_milestone_path(@group, @milestone.safe_title, title: @milestone.title, milestone: {state_event: :close }), method: :put, class: "btn btn-sm btn-close" - else - = link_to 'Reopen Milestone', group_milestone_path(@group, @group_milestone.safe_title, title: @group_milestone.title, milestone: {state_event: :activate }), method: :put, class: "btn btn-sm btn-grouped btn-reopen" + = link_to 'Reopen Milestone', group_milestone_path(@group, @milestone.safe_title, title: @milestone.title, milestone: {state_event: :activate }), method: :put, class: "btn btn-sm btn-grouped btn-reopen" %hr -- if (@group_milestone.total_items_count == @group_milestone.closed_items_count) && @group_milestone.active? +- if (@milestone.total_items_count == @milestone.closed_items_count) && @milestone.active? .alert.alert-success %span All issues for this milestone are closed. You may close the milestone now. @@ -30,7 +30,7 @@ %th Open issues %th State %th Due date - - @group_milestone.milestones.each do |milestone| + - @milestone.milestones.each do |milestone| %tr %td = link_to "#{milestone.project.name}", namespace_project_milestone_path(milestone.project.namespace, milestone.project, milestone) @@ -47,46 +47,46 @@ .context %p.lead Progress: - #{@group_milestone.closed_items_count} closed + #{@milestone.closed_items_count} closed – - #{@group_milestone.open_items_count} open - = milestone_progress_bar(@group_milestone) + #{@milestone.open_items_count} open + = milestone_progress_bar(@milestone) %ul.nav.nav-tabs %li.active = link_to '#tab-issues', 'data-toggle' => 'tab' do Issues - %span.badge= @group_milestone.issue_count + %span.badge= @milestone.issue_count %li = link_to '#tab-merge-requests', 'data-toggle' => 'tab' do Merge Requests - %span.badge= @group_milestone.merge_requests_count + %span.badge= @milestone.merge_requests_count %li = link_to '#tab-participants', 'data-toggle' => 'tab' do Participants - %span.badge= @group_milestone.participants.count + %span.badge= @milestone.participants.count .pull-right - = link_to 'Browse Issues', issues_group_path(@group, milestone_title: @group_milestone.title), class: "btn edit-milestone-link btn-grouped" + = link_to 'Browse Issues', issues_group_path(@group, milestone_title: @milestone.title), class: "btn edit-milestone-link btn-grouped" .tab-content .tab-pane.active#tab-issues .row .col-md-6 - = render 'issues', title: "Open", issues: @group_milestone.opened_issues + = render 'issues', title: "Open", issues: @milestone.opened_issues .col-md-6 - = render 'issues', title: "Closed", issues: @group_milestone.closed_issues + = render 'issues', title: "Closed", issues: @milestone.closed_issues .tab-pane#tab-merge-requests .row .col-md-6 - = render 'merge_requests', title: "Open", merge_requests: @group_milestone.opened_merge_requests + = render 'merge_requests', title: "Open", merge_requests: @milestone.opened_merge_requests .col-md-6 - = render 'merge_requests', title: "Closed", merge_requests: @group_milestone.closed_merge_requests + = render 'merge_requests', title: "Closed", merge_requests: @milestone.closed_merge_requests .tab-pane#tab-participants %ul.bordered-list - - @group_milestone.participants.each do |user| + - @milestone.participants.each do |user| %li = link_to user, title: user.name, class: "darken" do = image_tag avatar_icon(user, 32), class: "avatar s32" |