Polish reusing of controllers for project boards

app/assets/javascripts/boards/services/board_service.js

@@ -4,8 +4,8 @@ import Vue from 'vue';
 
 class BoardService {
   constructor (root, listsEndpoint, bulkUpdatePath, boardId) {
-    alert(listsEndpoint)
-    alert(root)
+    // alert(listsEndpoint)
+    // alert(root)
     this.boards = Vue.resource(`${root}{/id}.json`, {}, {
       issues: {
         method: 'GET',
@@ -18,8 +18,8 @@ class BoardService {
         url: `${listsEndpoint}/generate.json`
       }
     });
-    this.issue = Vue.resource(`${root}/${boardId}/issues{/id}`, {});
-    this.issues = Vue.resource(`${root}/${boardId}/lists{/id}/issues`, {}, {
+    this.issue = Vue.resource(`/boards/${boardId}/issues{/id}`, {});
+    this.issues = Vue.resource(`${listsEndpoint}{/id}/issues`, {}, {
       bulkUpdate: {
         method: 'POST',
         url: bulkUpdatePath,

app/controllers/boards/application_controller.rb

-
 module Boards
   class ApplicationController < ::ApplicationController
     respond_to :json
@@ -7,6 +6,14 @@ module Boards
 
     private
 
+    def board
+      @board ||= Board.find(params[:board_id])
+    end
+
+    def board_parent
+      @board_parent ||= board.parent
+    end
+
     def record_not_found(exception)
       render json: { error: exception.message }, status: :not_found
     end

app/controllers/boards/issues_controller.rb

 module Boards
   class IssuesController < Boards::ApplicationController
+    include BoardsAuthorizations
+
     before_action :authorize_read_issue!, only: [:index]
     before_action :authorize_create_issue!, only: [:create]
     before_action :authorize_update_issue!, only: [:update]
 
     def index
-      issues = ::Boards::Issues::ListService.new(project, current_user, filter_params).execute
+      issues = Boards::Issues::ListService.new(board_parent, current_user, filter_params).execute
       issues = issues.page(params[:page]).per(params[:per] || 20)
       make_sure_position_is_set(issues) unless Gitlab::Geo.secondary?
 
@@ -16,7 +18,7 @@ module Boards
     end
 
     def create
-      service = ::Boards::Issues::CreateService.new(project, current_user, issue_params)
+      service = Boards::Issues::CreateService.new(board_parent, current_user, issue_params)
       issue = service.execute
 
       if issue.valid?
@@ -27,7 +29,7 @@ module Boards
     end
 
     def update
-      service = ::Boards::Issues::MoveService.new(project, current_user, move_params)
+      service = Boards::Issues::MoveService.new(board_parent, current_user, move_params)
 
       if service.execute(issue)
         head :ok
@@ -46,24 +48,12 @@ module Boards
 
     def issue
       @issue ||=
-        IssuesFinder.new(current_user, project_id: project.id)
+        IssuesFinder.new(current_user, project_id: board_parent.id)
                     .execute
                     .where(iid: params[:id])
                     .first!
     end
 
-    def authorize_read_issue!
-      return render_403 unless can?(current_user, :read_issue, project)
-    end
-
-    def authorize_create_issue!
-      return render_403 unless can?(current_user, :admin_issue, project)
-    end
-
-    def authorize_update_issue!
-      return render_403 unless can?(current_user, :update_issue, issue)
-    end
-
     def filter_params
       params.merge(board_id: params[:board_id], id: params[:list_id]).compact
     end

app/controllers/boards/lists_controller.rb

 module Boards
   class ListsController < Boards::ApplicationController
+    include BoardsAuthorizations
+
     #before_action :authorize_admin_list!, only: [:create, :update, :destroy, :generate]
-    #before_action :authorize_read_list!, only: [:index]
+    before_action :authorize_read_list!, only: [:index]
 
     def index
-      lists = ::Boards::Lists::ListService.new(board.parent, current_user).execute(board)
+      lists = Boards::Lists::ListService.new(board.parent, current_user).execute(board)
 
       render json: serialize_as_json(lists)
     end
@@ -21,7 +23,7 @@ module Boards
 
     def update
       list = board.lists.movable.find(params[:id])
-      service = ::Boards::Lists::MoveService.new(project, current_user, move_params)
+      service = Boards::Lists::MoveService.new(project, current_user, move_params)
 
       if service.execute(list)
         head :ok
@@ -32,7 +34,7 @@ module Boards
 
     def destroy
       list = board.lists.destroyable.find(params[:id])
-      service = ::Boards::Lists::DestroyService.new(project, current_user)
+      service = Boards::Lists::DestroyService.new(project, current_user)
 
       if service.execute(list)
         head :ok
@@ -42,7 +44,7 @@ module Boards
     end
 
     def generate
-      service = ::Boards::Lists::GenerateService.new(project, current_user)
+      service = Boards::Lists::GenerateService.new(board_parent, current_user)
 
       if service.execute(board)
         render json: serialize_as_json(board.lists.movable)
@@ -57,14 +59,6 @@ module Boards
       return render_403 unless can?(current_user, :admin_list, project)
     end
 
-    def authorize_read_list!
-      return render_403 unless can?(current_user, :read_list, project)
-    end
-
-    def board
-      @board ||= Board.find(params[:board_id])
-    end
-
     def list_params
       params.require(:list).permit(:label_id)
     end

app/controllers/concerns/boards_authorizations.rb

+module BoardsAuthorizations
+  # Shared authorizations between projects and groups which
+  # have different policies.
+  def authorize_read_list!
+    ability = board.is_group_board? ? :read_group : :read_list
+
+    return render_403 unless action_allowed_for?(board.parent, ability)
+  end
+
+  def authorize_read_issue!
+    ability = board.is_group_board? ? :read_group : :read_issue
+
+    return render_403 unless action_allowed_for?(board.parent, ability)
+  end
+
+  def authorize_update_issue!
+    return render_403 unless action_allowed_for?(issue, :admin_issue)
+  end
+
+  def authorize_create_issue!
+    return render_403 unless action_allowed_for?(board.parent, :admin_issue)
+  end
+
+  def action_allowed_for?(resource, ability)
+    can?(current_user, ability, resource)
+  end
+end

app/models/ee/board.rb

@@ -25,6 +25,10 @@ module EE
       group || project
     end
 
+    def is_group_board?
+      group_id.present?
+    end
+
     def as_json(options = {})
       milestone_attrs = options.fetch(:include, {})
                           .extract!(:milestone)

config/routes.rb

@@ -82,6 +82,19 @@ Rails.application.routes.draw do
   # Notification settings
   resources :notification_settings, only: [:create, :update]
 
+  # Boards resources shared between group and projects
+  resources :boards do
+    resources :lists, module: :boards, only: [:index, :create, :update, :destroy] do
+      collection do
+        post :generate
+      end
+
+      resources :issues, only: [:index, :create, :update]
+    end
+
+    resources :issues, module: :boards, only: [:index, :update]
+  end
+
   draw :import
   draw :uploads
   draw :explore
@@ -96,15 +109,5 @@ Rails.application.routes.draw do
 
   draw :test if Rails.env.test?
 
-  resources :boards do
-    resources :lists, module: :boards, only: [:index, :create, :update, :destroy] do
-      collection do
-        post :generate
-      end
-
-      resources :issues, only: [:index, :create, :update]
-    end
-  end
-
   get '*unmatched_route', to: 'application#route_not_found'
 end

config/routes/project.rb

@@ -379,11 +379,7 @@ constraints(ProjectUrlConstrainer.new) do
 
       get 'noteable/:target_type/:target_id/notes' => 'notes#index', as: 'noteable_notes'
 
-      resources :boards, only: [:index, :show, :create, :update, :destroy] do
-        scope module: :boards do
-          resources :issues, only: [:index, :update]
-        end
-      end
+      resources :boards, only: [:index, :show, :create, :update, :destroy]
 
       resources :todos, only: [:create]