Move Multiple Issue Boards for Projects to Core

Refactor code to allow multiple issue boards management for projects
in CE

app/controllers/concerns/multiple_boards_actions.rb

@@ -14,7 +14,7 @@ module MultipleBoardsActions
   end
 
   def recent
-    recent_visits = ::Boards::Visits::LatestService.new(parent, current_user, count: 4).execute
+    recent_visits = ::Boards::VisitsFinder.new(parent, current_user).latest(4)
     recent_boards = recent_visits.map(&:board)
 
     render json: serialize_as_json(recent_boards)
@@ -25,7 +25,7 @@ module MultipleBoardsActions
 
     respond_to do |format|
       format.json do
-        if board.valid?
+        if board.persisted?
           extra_json = { board_path: board_path(board) }
           render json: serialize_as_json(board).merge(extra_json)
         else
@@ -37,11 +37,10 @@ module MultipleBoardsActions
 
   def update
     service = Boards::UpdateService.new(parent, current_user, board_params)
-    service.execute(board)
 
     respond_to do |format|
       format.json do
-        if board.valid?
+        if service.execute(board)
           extra_json = { board_path: board_path(board) }
           render json: serialize_as_json(board).merge(extra_json)
         else
@@ -64,11 +63,13 @@ module MultipleBoardsActions
   private
 
   def redirect_to_recent_board
-    return if request.format.json? || !parent.multiple_issue_boards_available?
+    return if request.format.json? || !parent.multiple_issue_boards_available? || !latest_visited_board
 
-    if recently_visited = Boards::Visits::LatestService.new(parent, current_user).execute
-      redirect_to board_path(recently_visited.board)
+    redirect_to board_path(latest_visited_board.board)
   end
+
+  def latest_visited_board
+    @latest_visited_board ||= Boards::VisitsFinder.new(parent, current_user).latest
   end
 
   def authorize_create_board!

app/finders/boards/visits_finder.rb

+# frozen_string_literal: true
+
+module Boards
+  class VisitsFinder
+    attr_accessor :params, :current_user, :parent
+
+    def initialize(parent, current_user)
+      @current_user = current_user
+      @parent = parent
+    end
+
+    def execute(count = nil)
+      return unless current_user
+
+      recent_visit_model.latest(current_user, parent, count: count)
+    end
+
+    alias_method :latest, :execute
+
+    private
+
+    def recent_visit_model
+      parent.is_a?(Group) ? BoardGroupRecentVisit : BoardProjectRecentVisit
+    end
+  end
+end

app/services/boards/update_service.rb

@@ -7,3 +7,5 @@ module Boards
     end
   end
 end
+
+Boards::UpdateService.prepend(EE::Boards::UpdateService)

app/services/boards/visits/latest_service.rb

-# frozen_string_literal: true
-
-module Boards
-  module Visits
-    class LatestService < Boards::BaseService
-      def execute
-        return unless current_user
-
-        recent_visit_model.latest(current_user, parent, count: params[:count])
-      end
-
-      private
-
-      def recent_visit_model
-        parent.is_a?(Group) ? BoardGroupRecentVisit : BoardProjectRecentVisit
-      end
-    end
-  end
-end

changelogs/unreleased/53811-issue-boards-to-core-projects-backend-ce.yml

+---
+title: Move Multiple Issue Boards for Projects to Core
+merge_request:
+author:
+type: added

ee/app/controllers/concerns/ee/boards_actions.rb

@@ -5,90 +5,7 @@ module EE
     extend ActiveSupport::Concern
 
     prepended do
-      # We need to include the filters as a separate concern since multiple `included` blocks are not allowed
-      include Filters
-    end
-
-    module Filters
-      extend ActiveSupport::Concern
-
-      included do
-        before_action :redirect_to_recent_board, only: :index
-        before_action :authenticate_user!, only: [:recent]
-        before_action :authorize_create_board!, only: [:create]
-        before_action :authorize_admin_board!, only: [:create, :update, :destroy]
-      end
-    end
-
-    def recent
-      recent_visits = ::Boards::Visits::LatestService.new(parent, current_user, count: 4).execute
-      recent_boards = recent_visits.map(&:board)
-
-      render json: serialize_as_json(recent_boards)
-    end
-
-    def create
-      board = ::Boards::CreateService.new(parent, current_user, board_params).execute
-
-      respond_to do |format|
-        format.json do
-          if board.valid?
-            extra_json = { board_path: board_path(board) }
-            render json: serialize_as_json(board).merge(extra_json)
-          else
-            render json: board.errors, status: :unprocessable_entity
-          end
-        end
-      end
-    end
-
-    def update
-      service = ::Boards::UpdateService.new(parent, current_user, board_params)
-
-      service.execute(board)
-
-      respond_to do |format|
-        format.json do
-          if board.valid?
-            extra_json = { board_path: board_path(board) }
-            render json: serialize_as_json(board).merge(extra_json)
-          else
-            render json: board.errors, status: :unprocessable_entity
-          end
-        end
-      end
-    end
-
-    def destroy
-      service = ::Boards::DestroyService.new(parent, current_user)
-      service.execute(board)
-
-      respond_to do |format|
-        format.json { head :ok }
-        format.html { redirect_to boards_path, status: :found }
-      end
-    end
-
-    private
-
-    def redirect_to_recent_board
-      return if request.format.json? || !parent.multiple_issue_boards_available?
-
-      if recently_visited = ::Boards::Visits::LatestService.new(parent, current_user).execute
-        redirect_to board_path(recently_visited.board)
-      end
-    end
-
-    def authorize_create_board!
-      if group?
-        check_multiple_group_issue_boards_available!
-      else
-        check_multiple_project_issue_boards_available!
-      end
-    end
-
-    def authorize_admin_board!
-      return render_404 unless can?(current_user, :admin_board, parent)
+      include ::MultipleBoardsActions
     end
   end
 end

ee/app/controllers/concerns/ee/boards_responses.rb

@@ -5,6 +5,11 @@ module EE
     extend ActiveSupport::Concern
     extend ::Gitlab::Utils::Override
 
+    override :board_params
+    def board_params
+      params.require(:board).permit(:name, :weight, :milestone_id, :assignee_id, label_ids: [])
+    end
+
     def authorize_read_parent
       authorize_action_for!(board, :read_parent)
     end

ee/app/helpers/ee/boards_helper.rb

@@ -17,8 +17,7 @@ module EE
     override :board_data
     def board_data
       show_feature_promotion = (@project && show_promotions? &&
-                                (!@project.feature_available?(:multiple_project_issue_boards) ||
-                                 !@project.feature_available?(:scoped_issue_board) ||
+                                (!@project.feature_available?(:scoped_issue_board) ||
                                  !@project.feature_available?(:issue_board_focus_mode)))
 
       data = {

ee/app/models/ee/project.rb

@@ -237,11 +237,6 @@ module EE
       end
     end
 
-    override :multiple_issue_boards_available?
-    def multiple_issue_boards_available?
-      feature_available?(:multiple_project_issue_boards)
-    end
-
     def multiple_approval_rules_available?
       feature_available?(:multiple_approval_rules)
     end

ee/app/models/license.rb

@@ -27,7 +27,6 @@ class License < ApplicationRecord
     multiple_ldap_servers
     multiple_issue_assignees
     multiple_merge_request_assignees
-    multiple_project_issue_boards
     push_rules
     protected_refs_for_users
     related_issues
@@ -133,7 +132,6 @@ class License < ApplicationRecord
     jenkins_integration
     merge_request_approvers
     multiple_issue_assignees
-    multiple_project_issue_boards
     multiple_group_issue_boards
     protected_refs_for_users
     push_rules

ee/app/services/boards/destroy_service.rb

-# frozen_string_literal: true
-
-module Boards
-  class DestroyService < Boards::BaseService
-    def execute(board)
-      return false if parent.boards.size == 1
-
-      board.destroy
-    end
-  end
-end

ee/app/services/boards/update_service.rb

-# frozen_string_literal: true
-
-module Boards
-  class UpdateService < Boards::BaseService
-    def execute(board)
-      unless parent.feature_available?(:scoped_issue_board)
-        params.delete(:milestone_id)
-        params.delete(:assignee_id)
-        params.delete(:label_ids)
-        params.delete(:weight)
-      end
-
-      set_assignee
-      set_milestone
-      set_labels
-
-      board.update(params)
-    end
-  end
-end

ee/app/services/ee/boards/create_service.rb

@@ -5,11 +5,6 @@ module EE
     module CreateService
       extend ::Gitlab::Utils::Override
 
-      override :can_create_board?
-      def can_create_board?
-        parent.multiple_issue_boards_available? || super
-      end
-
       override :create_board!
       def create_board!
         set_assignee

ee/app/services/ee/boards/update_service.rb

+# frozen_string_literal: true
+
+module EE
+  module Boards
+    module UpdateService
+      extend ::Gitlab::Utils::Override
+
+      override :execute
+      def execute(board)
+        unless parent.feature_available?(:scoped_issue_board)
+          params.delete(:milestone_id)
+          params.delete(:assignee_id)
+          params.delete(:label_ids)
+          params.delete(:weight)
+        end
+
+        set_assignee
+        set_milestone
+        set_labels
+
+        super
+      end
+    end
+  end
+end

ee/config/routes/project.rb

@@ -12,12 +12,6 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
       # Begin of the /-/ scope.
       # Use this scope for all new project routes.
       scope '-' do
-        resources :boards, only: [:create, :update, :destroy] do
-          collection do
-            get :recent
-          end
-        end
-
         resources :packages, only: [:index, :show, :destroy], module: :packages
         resources :package_files, only: [], module: :packages do
           member do

ee/spec/controllers/concerns/boards_responses_spec.rb

@@ -13,7 +13,6 @@ describe BoardsResponses do
 
   before do
     stub_licensed_features(scoped_issue_board: true)
-    stub_licensed_features(multiple_project_issue_boards: true)
   end
 
   describe '#serialize_as_json' do

ee/spec/controllers/projects/boards_controller_spec.rb

@@ -10,11 +10,13 @@ describe Projects::BoardsController do
   end
 
   describe 'GET index' do
+    let(:parent) { project }
+
     it 'returns a list of project boards including milestones' do
       create(:board, project: project, milestone: create(:milestone, project: project))
       create(:board, project: project, milestone_id: Milestone::Upcoming.id)
 
-      list_boards format: :json
+      list_boards
 
       parsed_response = JSON.parse(response.body)
 
@@ -22,17 +24,7 @@ describe Projects::BoardsController do
       expect(parsed_response.length).to eq 2
     end
 
-    it_behaves_like 'redirects to last visited board' do
-      let(:parent) { project }
-    end
-
-    def list_boards(format: :html)
-      get :index, params: {
-                    namespace_id: project.namespace,
-                    project_id: project
-                  },
-                  format: format
-    end
+    it_behaves_like 'redirects to last visited board'
   end
 
   describe 'GET recent' do
@@ -49,10 +41,6 @@ describe Projects::BoardsController do
 
   describe 'POST create' do
     context 'with the multiple issue boards available' do
-      before do
-        stub_licensed_features(multiple_project_issue_boards: true)
-      end
-
       context 'with valid params' do
         let(:user) { create(:user) }
         let(:milestone) { create(:milestone, project: project) }
@@ -112,14 +100,6 @@ describe Projects::BoardsController do
       end
     end
 
-    it 'renders a 404 when multiple issue boards are not available' do
-      stub_licensed_features(multiple_project_issue_boards: false)
-
-      create_board name: 'Backend'
-
-      expect(response).to have_gitlab_http_status(404)
-    end
-
     def create_board(board_params)
       post :create, params: {
                       namespace_id: project.namespace.to_param,

ee/spec/features/boards/multiple_boards_spec.rb

@@ -146,9 +146,9 @@ describe 'Multiple Issue Boards', :js do
     end
   end
 
+  # todo: remove these when frontend part is done
   context 'with multiple issue boards disabled' do
     before do
-      stub_licensed_features(multiple_project_issue_boards: false)
       project.add_maintainer(user)
 
       login_as(user)
@@ -161,8 +161,8 @@ describe 'Multiple Issue Boards', :js do
       click_button board.name
 
       page.within(dropdown_selector) do
-        expect(page).not_to have_content('Create new board')
-        expect(page).not_to have_content('Delete board')
+        expect(page).to have_content('Create new board')
+        expect(page).to have_content('Delete board')
       end
 
       expect(page).to have_content('Edit board')
@@ -176,7 +176,7 @@ describe 'Multiple Issue Boards', :js do
 
       click_button board.name
 
-      expect(page).to have_content('Some of your boards are hidden, activate a license to see them again.')
+      expect(page).not_to have_content('Some of your boards are hidden, activate a license to see them again.')
     end
   end
 

ee/spec/features/boards/scoped_issue_board_spec.rb

@@ -470,7 +470,7 @@ describe 'Scoped issue boards', :js do
   end
 
   def create_board_weight(weight)
-    create_board_scope('weight', weight)
+    create_board_scope('weight', weight.to_s)
   end
 
   def create_board_assignee(assignee_name)
@@ -492,7 +492,7 @@ describe 'Scoped issue boards', :js do
   end
 
   def update_board_weight(weight)
-    update_board_scope('weight', weight)
+    update_board_scope('weight', weight.to_s)
   end
 
   def create_board_scope(filter, value)

ee/spec/services/boards/create_service_spec.rb

@@ -66,15 +66,6 @@ describe Boards::CreateService, services: true do
         end
       end
     end
-
-    it 'skips creating a second board when the feature is not available' do
-      stub_licensed_features(multiple_project_issue_boards: false)
-      service = described_class.new(parent, double)
-
-      expect(service.execute).not_to be_nil
-
-      expect { service.execute }.not_to change(parent.boards, :count)
-    end
   end
 
   describe '#execute' do
@@ -84,6 +75,14 @@ describe Boards::CreateService, services: true do
 
     it_behaves_like 'boards create service' do
       let(:parent) { create(:group) }
+
+      it 'skips creating a second board when the feature is not available' do
+        stub_licensed_features(multiple_group_issue_boards: false)
+        service = described_class.new(parent, double)
+
+        expect(service.execute).not_to be_nil
+        expect { service.execute }.not_to change(parent.boards, :count)
+      end
     end
 
     it_behaves_like 'board with milestone predefined scope' do

ee/spec/services/boards/list_service_spec.rb

@@ -7,21 +7,11 @@ describe Boards::ListService do
 
     describe '#execute' do
       it 'returns all issue boards when multiple issue boards is enabled' do
-        if parent.is_a?(Group)
         stub_licensed_features(multiple_group_issue_boards: true)
-        end
 
         expect(service.execute.size).to eq(3)
       end
 
-      it 'returns the first issue board when multiple issue boards is disabled' do
-        if parent.is_a?(Project)
-          stub_licensed_features(multiple_project_issue_boards: false)
-        end
-
-        expect(service.execute.size).to eq(1)
-      end
-
       it 'returns boards ordered by name' do
         board_names = ['a-board', 'B-board', 'c-board'].shuffle
         boards.each_with_index { |board, i| board.update_column(:name, board_names[i]) }
@@ -38,5 +28,11 @@ describe Boards::ListService do
 
   it_behaves_like 'boards list service' do
     let(:parent) { create(:group) }
+
+    it 'returns the first issue board when multiple issue boards is disabled' do
+      stub_licensed_features(multiple_group_issue_boards: false)
+
+      expect(service.execute.size).to eq(1)
+    end
   end
 end

ee/spec/support/shared_examples/controllers/multiple_issue_board_show.rb

@@ -22,7 +22,7 @@ shared_examples 'multiple issue boards show' do
 
   context 'when multiple issue boards is disabled' do
     before do
-      stub_licensed_features(multiple_project_issue_boards: false, multiple_group_issue_boards: false)
+      stub_licensed_features(multiple_group_issue_boards: false)
     end
 
     it 'let user view the default shown board' do
@@ -32,12 +32,16 @@ shared_examples 'multiple issue boards show' do
       expect(assigns(:board)).to eq(board2)
     end
 
-    it 'renders 404 when board is not the default' do
+    it 'renders 200 when project board is not the default' do
       show(board1)
 
+      if parent.is_a?(Project)
+        expect(response).to have_gitlab_http_status(200)
+      else
         expect(response).to have_gitlab_http_status(404)
       end
     end
+  end
 
   def show(board)
     params = {}

ee/spec/support/shared_examples/controllers/recent_boards.rb

@@ -38,24 +38,30 @@ shared_examples 'redirects to last visited board' do
 
   context 'when multiple boards are disabled' do
     before do
-      stub_licensed_features(multiple_project_issue_boards: false, multiple_group_issue_boards: false)
+      stub_licensed_features(multiple_group_issue_boards: false)
     end
 
     it 'renders first board' do
-      list_boards
+      list_boards(format: :html)
 
+      if parent.is_a?(Group)
         expect(response).to render_template :index
         expect(response.content_type).to eq 'text/html'
+        expect(response).to have_gitlab_http_status(200)
+      else
+        expect(response.content_type).to eq 'text/html'
+        expect(response).to have_gitlab_http_status(302)
+      end
     end
   end
 
   context 'when multiple boards are enabled' do
     before do
-      stub_licensed_features(multiple_project_issue_boards: true, multiple_group_issue_boards: true)
+      stub_licensed_features(multiple_group_issue_boards: true)
     end
 
     it 'redirects to latest visited board' do
-      list_boards
+      list_boards(format: :html)
 
       board_path = if parent.is_a?(Group)
                      group_board_path(group_id: parent, id: boards[1].id)
@@ -68,7 +74,7 @@ shared_examples 'redirects to last visited board' do
   end
 end
 
-def list_boards(recent: false)
+def list_boards(recent: false, format: :json)
   action = recent ? :recent : :index
   params = if parent.is_a?(Group)
              { group_id: parent }
@@ -76,7 +82,7 @@ def list_boards(recent: false)
              { namespace_id: parent.namespace, project_id: parent }
            end
 
-  get action, params: params, format: :json
+  get action, params: params, format: format
 end
 
 def visit_board(board, time)

ee/spec/support/shared_examples/multiple_and_scoped_issue_boards_.rb

@@ -4,7 +4,7 @@ shared_examples_for 'multiple and scoped issue boards' do |route_definition|
   context 'multiple issue boards' do
     before do
       board_parent.add_reporter(user)
-      stub_licensed_features(multiple_group_issue_boards: true, multiple_project_issue_boards: true)
+      stub_licensed_features(multiple_group_issue_boards: true)
     end
 
     describe "POST #{route_definition}" do

ee/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb

@@ -14,14 +14,13 @@ describe 'layouts/nav/sidebar/_project' do
   end
 
   describe 'issue boards' do
-    it 'has board tab when multiple issue boards is not available' do
+    it 'has boards tab' do
       allow(view).to receive(:can?).and_return(true)
       allow(License).to receive(:feature_available?).and_call_original
-      allow(License).to receive(:feature_available?).with(:multiple_project_issue_boards) { false }
 
       render
 
-      expect(rendered).to have_css('a[title="Board"]')
+      expect(rendered).to have_css('a[title="Boards"]')
     end
   end
 

spec/controllers/groups/boards_controller_spec.rb

@@ -59,7 +59,7 @@ describe Groups::BoardsController do
       it 'return an array with one group board' do
         create(:board, group: group)
 
-        expect(Boards::Visits::LatestService).not_to receive(:new)
+        expect(Boards::VisitsFinder).not_to receive(:new)
 
         list_boards format: :json
 

spec/controllers/projects/boards_controller_spec.rb

@@ -65,7 +65,7 @@ describe Projects::BoardsController do
       it 'returns a list of project boards' do
         create_list(:board, 2, project: project)
 
-        expect(Boards::Visits::LatestService).not_to receive(:new)
+        expect(Boards::VisitsFinder).not_to receive(:new)
 
         list_boards format: :json
 

spec/services/boards/visits/latest_service_spec.rb → spec/finders/boards/visits_finder_spec.rb

@@ -2,32 +2,32 @@
 
 require 'spec_helper'
 
-describe Boards::Visits::LatestService do
-  describe '#execute' do
+describe Boards::VisitsFinder do
+  describe '#latest' do
     let(:user) { create(:user) }
 
     context 'when a project board' do
       let(:project)       { create(:project) }
       let(:project_board) { create(:board, project: project) }
 
-      subject(:service) { described_class.new(project_board.parent, user) }
+      subject(:finder) { described_class.new(project_board.parent, user) }
 
       it 'returns nil when there is no user' do
-        service.current_user = nil
+        finder.current_user = nil
 
-        expect(service.execute).to eq nil
+        expect(finder.execute).to eq nil
       end
 
       it 'queries for most recent visit' do
         expect(BoardProjectRecentVisit).to receive(:latest).once
 
-        service.execute
+        finder.execute
       end
 
       it 'queries for last N visits' do
         expect(BoardProjectRecentVisit).to receive(:latest).with(user, project, count: 5).once
 
-        described_class.new(project_board.parent, user, count: 5).execute
+        described_class.new(project_board.parent, user).latest(5)
       end
     end
 
@@ -35,24 +35,24 @@ describe Boards::Visits::LatestService do
       let(:group)       { create(:group) }
       let(:group_board) { create(:board, group: group) }
 
-      subject(:service) { described_class.new(group_board.parent, user) }
+      subject(:finder) { described_class.new(group_board.parent, user) }
 
       it 'returns nil when there is no user' do
-        service.current_user = nil
+        finder.current_user = nil
 
-        expect(service.execute).to eq nil
+        expect(finder.execute).to eq nil
       end
 
       it 'queries for most recent visit' do
         expect(BoardGroupRecentVisit).to receive(:latest).once
 
-        service.execute
+        finder.latest
       end
 
       it 'queries for last N visits' do
         expect(BoardGroupRecentVisit).to receive(:latest).with(user, group, count: 5).once
 
-        described_class.new(group_board.parent, user, count: 5).execute
+        described_class.new(group_board.parent, user).latest(5)
       end
     end
   end