Correct authorization for group milestones.

app/controllers/groups/milestones_controller.rb

 class Groups::MilestonesController < ApplicationController
   layout 'group'
 
+  before_filter :authorize_group_milestone!, only: :update
+
   def index
     project_milestones = Milestone.where(project_id: group.projects)
     @group_milestones = Milestones::GroupService.new(project_milestones).execute
@@ -47,4 +49,8 @@ class Groups::MilestonesController < ApplicationController
   def status(state)
     @group_milestones.map{ |milestone| next if milestone.state != state; milestone }.compact
   end
+
+  def authorize_group_milestone!
+    return render_404 unless can?(current_user, :manage_group, group)
+  end
 end

app/models/group_milestone.rb

@@ -76,39 +76,4 @@ class GroupMilestone
   def participants
     milestones.map{ |milestone| milestone.participants.uniq }.reject(&:empty?).flatten
   end
-
-  def filter_by(filter, entity)
-    if entity
-      milestones = self.milestones.sort_by(&:project_id)
-      entities = {}
-      milestones.each do |project_milestone|
-        next unless project_milestone.send(entity).any?
-        project_name = project_milestone.project.name
-        entities_by_state = state_filter(filter, project_milestone.send(entity))
-        entities.store(project_name, entities_by_state)
-      end
-      entities
-    else
-      {}
-    end
-  end
-
-  def state_filter(filter, entities)
-    if entities.present?
-      sorted_entities = entities.sort_by(&:position)
-      entities_by_state =  case filter
-                           when 'active'; sorted_entities.group_by(&:state)['opened']
-                           when 'closed'; sorted_entities.group_by(&:state)['closed']
-                           else sorted_entities
-                           end
-      if entities_by_state.blank?
-        []
-      else
-        entities_by_state
-      end
-    else
-      []
-    end
-  end
-
 end

app/views/groups/milestones/index.html.haml

@@ -24,18 +24,19 @@
           - @group_milestones.each do |milestone|
             %li{class: "milestone milestone-#{milestone.closed? ? 'closed' : 'open'}", id: dom_id(milestone.milestones.first) }
               .pull-right
-                - if milestone.closed?
-                  = link_to 'Reopen Milestone', group_milestone_path(@group, milestone.safe_title, milestone: {state_event: :activate }), method: :put, class: "btn btn-small btn-grouped"
-                - else
-                  = link_to 'Close Milestone', group_milestone_path(@group, milestone.safe_title, milestone: {state_event: :close }), method: :put, class: "btn btn-small btn-remove"
+                - if can?(current_user, :manage_group, @group)
+                  - if milestone.closed?
+                    = link_to 'Reopen Milestone', group_milestone_path(@group, milestone.safe_title, milestone: {state_event: :activate }), method: :put, class: "btn btn-small btn-grouped"
+                  - else
+                    = link_to 'Close Milestone', group_milestone_path(@group, milestone.safe_title, milestone: {state_event: :close }), method: :put, class: "btn btn-small btn-remove"
               %h4
                 = link_to_gfm truncate(milestone.title, length: 100), group_milestone_path(@group, milestone.safe_title)
               %div
                 %div
-                  = link_to group_milestone_path(@group, milestone.safe_title) do
+                  = link_to group_milestone_path(@group, milestone.safe_title, anchor: 'tab-issues') do
                     = pluralize milestone.issue_count, 'Issue'
                    
-                  = link_to group_milestone_path(@group, milestone.safe_title) do
+                  = link_to group_milestone_path(@group, milestone.safe_title, anchor: 'tab-merge-requests') do
                     = pluralize milestone.merge_requests_count, 'Merge Request'
                    
                   %span.light #{milestone.percent_complete}% complete

app/views/groups/milestones/show.html.haml

 %h3.page-title
   Milestone #{@group_milestone.title}
   .pull-right
-    - if @group_milestone.active?
-      = link_to 'Close Milestone', group_milestone_path(@group, @group_milestone.safe_title, milestone: {state_event: :close }), method: :put, class: "btn btn-small btn-remove"
-    - else
-      = link_to 'Reopen Milestone', group_milestone_path(@group, @group_milestone.safe_title, milestone: {state_event: :activate }), method: :put, class: "btn btn-small btn-grouped"
+    - if can?(current_user, :manage_group, @group)
+      - if @group_milestone.active?
+        = link_to 'Close Milestone', group_milestone_path(@group, @group_milestone.safe_title, milestone: {state_event: :close }), method: :put, class: "btn btn-small btn-remove"
+      - else
+        = link_to 'Reopen Milestone', group_milestone_path(@group, @group_milestone.safe_title, milestone: {state_event: :activate }), method: :put, class: "btn btn-small btn-grouped"
 
 - if (@group_milestone.total_items_count == @group_milestone.closed_items_count) && @group_milestone.active?
   .alert.alert-success