Merge branch 'bvl-license-check-multiple-issue-boards' into 'master'

Add a license check on multiple issue boards

Closes #2567

See merge request !2317

app/controllers/ee/projects/boards_controller.rb

+module EE
+  module Projects
+    module BoardsController
+      extend ActiveSupport::Concern
+      prepended do
+        before_action :check_multiple_issue_boards_available!, only: [:create]
+        before_action :authorize_admin_board!, only: [:create, :update, :destroy]
+        before_action :find_board, only: [:update, :destroy]
+      end
+
+      def create
+        board = ::Boards::CreateService.new(project, current_user, board_params).execute
+
+        respond_to do |format|
+          format.json do
+            if board.valid?
+              render json: serialize_as_json(board)
+            else
+              render json: board.errors, status: :unprocessable_entity
+            end
+          end
+        end
+      end
+
+      def update
+        service = ::Boards::UpdateService.new(project, current_user, board_params)
+
+        service.execute(@board)
+
+        respond_to do |format|
+          format.json do
+            if @board.valid?
+              render json: serialize_as_json(@board)
+            else
+              render json: @board.errors, status: :unprocessable_entity
+            end
+          end
+        end
+      end
+
+      def destroy
+        service = ::Boards::DestroyService.new(project, current_user)
+        service.execute(@board)
+
+        respond_to do |format|
+          format.html { redirect_to project_boards_path(@project), status: 302 }
+        end
+      end
+
+      private
+
+      def authorize_admin_board!
+        return render_404 unless can?(current_user, :admin_board, project)
+      end
+
+      def board_params
+        params.require(:board).permit(:name, :milestone_id)
+      end
+
+      def find_board
+        @board = project.boards.find(params[:id])
+      end
+
+      def serialize_as_json(resource)
+        resource.as_json(
+          only: [:id, :name],
+          include: {
+            milestone: { only: [:id, :title] }
+          }
+        )
+      end
+    end
+  end
+end

app/controllers/projects/boards_controller.rb

 class Projects::BoardsController < Projects::ApplicationController
+  prepend EE::Projects::BoardsController
   include IssuableCollections
 
   before_action :authorize_read_board!, only: [:index, :show]
-  before_action :authorize_admin_board!, only: [:create, :update, :destroy]
-  before_action :find_board, only: [:show, :update, :destroy]
 
   def index
     @boards = ::Boards::ListService.new(project, current_user).execute
@@ -17,77 +16,23 @@ class Projects::BoardsController < Projects::ApplicationController
   end
 
   def show
-    respond_to do |format|
-      format.html
-      format.json do
-        render json: serialize_as_json(@board)
-      end
-    end
-  end
-
-  def create
-    board = ::Boards::CreateService.new(project, current_user, board_params).execute
-
-    respond_to do |format|
-      format.json do
-        if board.valid?
-          render json: serialize_as_json(board)
-        else
-          render json: board.errors, status: :unprocessable_entity
-        end
-      end
-    end
-  end
-
-  def update
-    service = ::Boards::UpdateService.new(project, current_user, board_params)
-
-    service.execute(@board)
+    @board = project.boards.find(params[:id])
 
     respond_to do |format|
+      format.html
       format.json do
-        if @board.valid?
         render json: serialize_as_json(@board)
-        else
-          render json: @board.errors, status: :unprocessable_entity
-        end
       end
     end
   end
 
-  def destroy
-    service = ::Boards::DestroyService.new(project, current_user)
-    service.execute(@board)
-
-    respond_to do |format|
-      format.html { redirect_to project_boards_path(@project), status: 302 }
-    end
-  end
-
   private
 
-  def authorize_admin_board!
-    return render_404 unless can?(current_user, :admin_board, project)
-  end
-
   def authorize_read_board!
-    return render_404 unless can?(current_user, :read_board, project)
-  end
-
-  def board_params
-    params.require(:board).permit(:name, :milestone_id)
-  end
-
-  def find_board
-    @board = project.boards.find(params[:id])
+    return access_denied! unless can?(current_user, :read_board, project)
   end
 
   def serialize_as_json(resource)
-    resource.as_json(
-      only: [:id, :name],
-      include: {
-        milestone: { only: [:id, :title] }
-      }
-    )
+    resource.as_json(only: [:id])
   end
 end

app/models/license.rb

@@ -19,6 +19,7 @@ class License < ActiveRecord::Base
   MERGE_REQUEST_REBASE_FEATURE = 'GitLab_MergeRequestRebase'.freeze
   MERGE_REQUEST_SQUASH_FEATURE = 'GitLab_MergeRequestSquash'.freeze
   MULTIPLE_ISSUE_ASSIGNEES_FEATURE = 'GitLab_MultipleIssueAssignees'.freeze
+  MULTIPLE_ISSUE_BOARDS_FEATURE = 'GitLab_MultipleIssueBoards'.freeze
   OBJECT_STORAGE_FEATURE = 'GitLab_ObjectStorage'.freeze
   PUSH_RULES_FEATURE = 'GitLab_PushRules'.freeze
   RELATED_ISSUES_FEATURE = 'GitLab_RelatedIssues'.freeze
@@ -50,6 +51,7 @@ class License < ActiveRecord::Base
     merge_request_rebase: MERGE_REQUEST_REBASE_FEATURE,
     merge_request_squash: MERGE_REQUEST_SQUASH_FEATURE,
     multiple_issue_assignees: MULTIPLE_ISSUE_ASSIGNEES_FEATURE,
+    multiple_issue_boards: MULTIPLE_ISSUE_BOARDS_FEATURE,
     push_rules: PUSH_RULES_FEATURE
   }.freeze
 
@@ -73,6 +75,7 @@ class License < ActiveRecord::Base
     { MERGE_REQUEST_REBASE_FEATURE => 1 },
     { MERGE_REQUEST_SQUASH_FEATURE => 1 },
     { MULTIPLE_ISSUE_ASSIGNEES_FEATURE => 1 },
+    { MULTIPLE_ISSUE_BOARDS_FEATURE => 1 },
     { PUSH_RULES_FEATURE => 1 },
     { RELATED_ISSUES_FEATURE => 1 }
   ].freeze
@@ -117,6 +120,7 @@ class License < ActiveRecord::Base
     { MERGE_REQUEST_REBASE_FEATURE => 1 },
     { MERGE_REQUEST_SQUASH_FEATURE => 1 },
     { MULTIPLE_ISSUE_ASSIGNEES_FEATURE => 1 },
+    { MULTIPLE_ISSUE_BOARDS_FEATURE => 1 },
     { OBJECT_STORAGE_FEATURE => 1 },
     { PUSH_RULES_FEATURE => 1 },
     { SERVICE_DESK_FEATURE => 1 }

app/services/boards/create_service.rb

 module Boards
   class CreateService < BaseService
+    prepend EE::Boards::CreateService
+
     def execute
+      create_board! if can_create_board?
+    end
+
+    private
+
+    def can_create_board?
+      project.boards.size == 0
+    end
+
+    def create_board!
       board = project.boards.create(params)
 
       if board.persisted?

app/services/boards/list_service.rb

 module Boards
   class ListService < BaseService
+    prepend EE::Boards::ListService
+
     def execute
       create_board! if project.boards.empty?
       project.boards

app/services/ee/boards/create_service.rb

+module EE
+  module Boards
+    module CreateService
+      def can_create_board?
+        raise NotImplementedError unless defined?(super)
+
+        project.feature_available?(:multiple_issue_boards) || super
+      end
+    end
+  end
+end

app/services/ee/boards/list_service.rb

+module EE
+  module Boards
+    module ListService
+      def execute
+        raise NotImplementedError unless defined?(super)
+
+        if project.feature_available?(:multiple_issue_boards, current_user)
+          super
+        else
+          super.limit(1)
+        end
+      end
+    end
+  end
+end

app/views/projects/boards/_switcher.html.haml

@@ -22,6 +22,9 @@
           %li{ "v-for" => "board in boards" }
             %a{ ":href" => "'#{project_boards_path(@project)}/' + board.id" }
               {{ board.name }}
+          - if !@project.feature_available?(:multiple_issue_boards) && @project.boards.size > 1
+            %li.small
+              Some of your boards are hidden, activate a license to see them again.
       .dropdown-loading{ "v-if" => "loading" }
         = icon("spin spinner")
       - if can?(current_user, :admin_board, @project)
@@ -43,6 +46,7 @@
       - if can?(current_user, :admin_board, @project)
         .dropdown-footer{ "v-if" => "currentPage === ''" }
           %ul.dropdown-footer-list
+            - if @project.feature_available?(:multiple_issue_boards)
               %li
                 %a{ "href" => "#", "@click.stop.prevent" => "showPage('new')" }
                   Create new board

changelogs/unreleased-ee/bvl-license-check-multiple-issue-boards.yml

+---
+title: Add license checks for multiple issue boards
+merge_request: 2317
+author:

spec/controllers/ee/projects/boards_controller_spec.rb

+require 'spec_helper'
+
+describe Projects::BoardsController do # rubocop:disable RSpec/FilePath
+  let(:project) { create(:empty_project) }
+  let(:user)    { create(:user) }
+
+  before do
+    project.team << [user, :master]
+    sign_in(user)
+  end
+
+  describe 'GET index' do
+    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
+
+      parsed_response = JSON.parse(response.body)
+
+      expect(response).to match_response_schema('boards')
+      expect(parsed_response.length).to eq 2
+    end
+
+    def list_boards(format: :html)
+      get :index, namespace_id: project.namespace,
+                  project_id: project,
+                  format: format
+    end
+  end
+
+  describe 'POST create' do
+    context 'with the multiple issue boards available' do
+      before do
+        stub_licensed_features(multiple_issue_boards: true)
+      end
+
+      context 'with valid params' do
+        it 'returns a successful 200 response' do
+          create_board name: 'Backend'
+
+          expect(response).to have_http_status(200)
+        end
+
+        it 'returns the created board' do
+          create_board name: 'Backend'
+
+          expect(response).to match_response_schema('board')
+        end
+      end
+
+      context 'with invalid params' do
+        it 'returns an unprocessable entity 422 response' do
+          create_board name: nil
+
+          expect(response).to have_http_status(422)
+        end
+      end
+
+      context 'with unauthorized user' do
+        before do
+          allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true)
+          allow(Ability).to receive(:allowed?).with(user, :admin_board, project).and_return(false)
+        end
+
+        it 'returns a not found 404 response' do
+          create_board name: 'Backend'
+
+          expect(response.content_type).to eq 'application/json'
+          expect(response).to have_http_status(404)
+        end
+      end
+    end
+
+    it 'renders a 404 when multiple issue boards are not available' do
+      stub_licensed_features(multiple_issue_boards: false)
+
+      create_board name: 'Backend'
+
+      expect(response).to have_http_status(404)
+    end
+
+    def create_board(name:)
+      post :create, namespace_id: project.namespace.to_param,
+                    project_id: project.to_param,
+                    board: { name: name },
+                    format: :json
+    end
+  end
+
+  describe 'PATCH update' do
+    let(:board) { create(:board, project: project, name: 'Backend') }
+
+    context 'with valid params' do
+      it 'returns a successful 200 response' do
+        update_board board: board, name: 'Frontend'
+
+        expect(response).to have_http_status(200)
+      end
+
+      it 'returns the updated board' do
+        update_board board: board, name: 'Frontend'
+
+        expect(response).to match_response_schema('board')
+      end
+    end
+
+    context 'with invalid params' do
+      it 'returns an unprocessable entity 422 response' do
+        update_board board: board, name: nil
+
+        expect(response).to have_http_status(422)
+      end
+    end
+
+    context 'with invalid board id' do
+      it 'returns a not found 404 response' do
+        update_board board: 999, name: 'Frontend'
+
+        expect(response).to have_http_status(404)
+      end
+    end
+
+    context 'with unauthorized user' do
+      before do
+        allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true)
+        allow(Ability).to receive(:allowed?).with(user, :admin_board, project).and_return(false)
+      end
+
+      it 'returns a not found 404 response' do
+        update_board board: board, name: 'Backend'
+
+        expect(response.content_type).to eq 'application/json'
+        expect(response).to have_http_status(404)
+      end
+    end
+
+    def update_board(board:, name:)
+      patch :update, namespace_id: project.namespace.to_param,
+                     project_id: project.to_param,
+                     id: board.to_param,
+                     board: { name: name },
+                     format: :json
+    end
+  end
+
+  describe 'DELETE destroy' do
+    let!(:boards) { create_pair(:board, project: project) }
+    let(:board)   { project.boards.first }
+
+    context 'with valid board id' do
+      it 'redirects to the issue boards page' do
+        remove_board board: board
+
+        expect(response).to redirect_to(project_boards_path(project))
+      end
+
+      it 'removes board from project' do
+        expect { remove_board board: board }.to change(project.boards, :size).by(-1)
+      end
+    end
+
+    context 'with invalid board id' do
+      it 'returns a not found 404 response' do
+        remove_board board: 999
+
+        expect(response).to have_http_status(404)
+      end
+    end
+
+    context 'with unauthorized user' do
+      before do
+        allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true)
+        allow(Ability).to receive(:allowed?).with(user, :admin_board, project).and_return(false)
+      end
+
+      it 'returns a not found 404 response' do
+        remove_board board: board
+
+        expect(response).to have_http_status(404)
+      end
+    end
+
+    def remove_board(board:)
+      delete :destroy, namespace_id: project.namespace.to_param,
+                       project_id: project.to_param,
+                       id: board.to_param,
+                       format: :html
+    end
+  end
+end

spec/controllers/projects/boards_controller_spec.rb

@@ -137,150 +137,4 @@ describe Projects::BoardsController do
                  format: format
     end
   end
-
-  describe 'POST create' do
-    context 'with valid params' do
-      it 'returns a successful 200 response' do
-        create_board name: 'Backend'
-
-        expect(response).to have_http_status(200)
-      end
-
-      it 'returns the created board' do
-        create_board name: 'Backend'
-
-        expect(response).to match_response_schema('board')
-      end
-    end
-
-    context 'with invalid params' do
-      it 'returns an unprocessable entity 422 response' do
-        create_board name: nil
-
-        expect(response).to have_http_status(422)
-      end
-    end
-
-    context 'with unauthorized user' do
-      before do
-        allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true)
-        allow(Ability).to receive(:allowed?).with(user, :admin_board, project).and_return(false)
-      end
-
-      it 'returns a not found 404 response' do
-        create_board name: 'Backend'
-
-        expect(response.content_type).to eq 'application/json'
-        expect(response).to have_http_status(404)
-      end
-    end
-
-    def create_board(name:)
-      post :create, namespace_id: project.namespace.to_param,
-                    project_id: project.to_param,
-                    board: { name: name },
-                    format: :json
-    end
-  end
-
-  describe 'PATCH update' do
-    let(:board) { create(:board, project: project, name: 'Backend') }
-
-    context 'with valid params' do
-      it 'returns a successful 200 response' do
-        update_board board: board, name: 'Frontend'
-
-        expect(response).to have_http_status(200)
-      end
-
-      it 'returns the updated board' do
-        update_board board: board, name: 'Frontend'
-
-        expect(response).to match_response_schema('board')
-      end
-    end
-
-    context 'with invalid params' do
-      it 'returns an unprocessable entity 422 response' do
-        update_board board: board, name: nil
-
-        expect(response).to have_http_status(422)
-      end
-    end
-
-    context 'with invalid board id' do
-      it 'returns a not found 404 response' do
-        update_board board: 999, name: 'Frontend'
-
-        expect(response).to have_http_status(404)
-      end
-    end
-
-    context 'with unauthorized user' do
-      before do
-        allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true)
-        allow(Ability).to receive(:allowed?).with(user, :admin_board, project).and_return(false)
-      end
-
-      it 'returns a not found 404 response' do
-        update_board board: board, name: 'Backend'
-
-        expect(response.content_type).to eq 'application/json'
-        expect(response).to have_http_status(404)
-      end
-    end
-
-    def update_board(board:, name:)
-      patch :update, namespace_id: project.namespace.to_param,
-                     project_id: project.to_param,
-                     id: board.to_param,
-                     board: { name: name },
-                     format: :json
-    end
-  end
-
-  describe 'DELETE destroy' do
-    let!(:boards) { create_pair(:board, project: project) }
-    let(:board)   { project.boards.first }
-
-    context 'with valid board id' do
-      it 'redirects to the issue boards page' do
-        remove_board board: board
-
-        expect(response).to redirect_to(project_boards_path(project))
-      end
-
-      it 'removes board from project' do
-        expect { remove_board board: board }.to change(project.boards, :size).by(-1)
-      end
-    end
-
-    context 'with invalid board id' do
-      it 'returns a not found 404 response' do
-        remove_board board: 999
-
-        expect(response).to have_http_status(404)
-      end
-    end
-
-    context 'with unauthorized user' do
-      before do
-        allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true)
-        allow(Ability).to receive(:allowed?).with(user, :admin_board, project).and_return(false)
-      end
-
-      it 'returns a not found 404 response' do
-        remove_board board: board
-
-        expect(response).to have_http_status(404)
-      end
-    end
-
-    def remove_board(board:)
-      delete :destroy, namespace_id: project.namespace.to_param,
-                       project_id: project.to_param,
-                       id: board.to_param,
-                       format: :html
-    end
-  end
 end

spec/features/boards/multiple_boards_spec.rb

@@ -7,11 +7,16 @@ describe 'Multiple Issue Boards', feature: true, js: true do
   let!(:board)    { create(:board, project: project) }
   let!(:board2)   { create(:board, project: project) }
 
+  context 'with multiple issue boards enabled' do
+    before do
+      stub_licensed_features(multiple_issue_boards: true)
+    end
+
     context 'authorized user' do
       before do
         project.team << [user, :master]
 
-      gitlab_sign_in(user)
+        login_as(user)
 
         visit project_boards_path(project)
         wait_for_requests
@@ -50,18 +55,16 @@ describe 'Multiple Issue Boards', feature: true, js: true do
         click_button board.name
 
         page.within('.dropdown-menu') do
-        click_link 'Edit board name'
+          click_link 'Create new board'
 
-        fill_in 'board-new-name', with: 'Testing'
+          fill_in 'board-new-name', with: 'This is a new board'
 
-        click_button 'Save'
+          click_button 'Create'
         end
 
         wait_for_requests
 
-      page.within('.dropdown-menu') do
-        expect(page).to have_content('Testing')
-      end
+        expect(page).to have_button('This is a new board')
       end
 
       it 'edits board name' do
@@ -158,4 +161,38 @@ describe 'Multiple Issue Boards', feature: true, js: true do
         end
       end
     end
+  end
+
+  context 'with multiple issue boards disabled' do
+    before do
+      stub_licensed_features(multiple_issue_boards: false)
+      project.team << [user, :master]
+
+      login_as(user)
+    end
+
+    it 'hides the link to create a new board' do
+      visit project_boards_path(project)
+      wait_for_requests
+
+      click_button board.name
+
+      page.within('.dropdown-menu') do
+        expect(page).to have_content('Edit board name')
+        expect(page).not_to have_content('Create new board')
+        expect(page).not_to have_content('Delete board')
+      end
+    end
+
+    it 'shows a mention that boards are hidden when multiple boards are created' do
+      create(:board, project: project)
+
+      visit project_boards_path(project)
+      wait_for_requests
+
+      click_button board.name
+
+      expect(page).to have_content('Some of your boards are hidden, activate a license to see them again.')
+    end
+  end
 end

spec/services/boards/create_service_spec.rb

@@ -4,11 +4,11 @@ describe Boards::CreateService, services: true do
   describe '#execute' do
     let(:project) { create(:empty_project) }
 
-    context 'with valid params' do
-      subject(:service) { described_class.new(project, double, name: 'Backend') }
+    subject(:service) { described_class.new(project, double) }
 
-      it 'creates a new project board' do
-        expect { service.execute }.to change(project.boards, :count).by(1)
+    context 'when project does not have a board' do
+      it 'creates a new board' do
+        expect { service.execute }.to change(Board, :count).by(1)
       end
 
       it 'creates the default lists' do
@@ -20,32 +20,15 @@ describe Boards::CreateService, services: true do
       end
     end
 
-    context 'with invalid params' do
-      subject(:service) { described_class.new(project, double, name: nil) }
-
-      it 'does not create a new project board' do
-        expect { service.execute }.not_to change(project.boards, :count)
-      end
-
-      it "does not create board's default lists" do
-        board = service.execute
-
-        expect(board.lists.size).to eq 0
-      end
-    end
-
-    context 'without params' do
-      subject(:service) { described_class.new(project, double) }
-
-      it 'creates a new project board' do
-        expect { service.execute }.to change(project.boards, :count).by(1)
+    context 'when project has a board' do
+      before do
+        create(:board, project: project)
       end
 
-      it "creates board's default lists" do
-        board = service.execute
+      it 'does not create a new board' do
+        expect(service).to receive(:can_create_board?) { false }
 
-        expect(board.lists.size).to eq 2
-        expect(board.lists.last).to be_closed
+        expect { service.execute }.not_to change(project.boards, :count)
       end
     end
   end

spec/services/ee/boards/create_service_spec.rb

+require 'spec_helper'
+
+describe Boards::CreateService, services: true do
+  describe '#execute' do
+    let(:project) { create(:empty_project) }
+
+    context 'With the feature available' do
+      before do
+        stub_licensed_features(multiple_issue_boards: true)
+      end
+
+      context 'with valid params' do
+        subject(:service) { described_class.new(project, double, name: 'Backend') }
+
+        it 'creates a new project board' do
+          expect { service.execute }.to change(project.boards, :count).by(1)
+        end
+
+        it 'creates the default lists' do
+          board = service.execute
+
+          expect(board.lists.size).to eq 2
+          expect(board.lists.first).to be_backlog
+          expect(board.lists.last).to be_closed
+        end
+      end
+
+      context 'with invalid params' do
+        subject(:service) { described_class.new(project, double, name: nil) }
+
+        it 'does not create a new project board' do
+          expect { service.execute }.not_to change(project.boards, :count)
+        end
+
+        it "does not create board's default lists" do
+          board = service.execute
+
+          expect(board.lists.size).to eq 0
+        end
+      end
+
+      context 'without params' do
+        subject(:service) { described_class.new(project, double) }
+
+        it 'creates a new project board' do
+          expect { service.execute }.to change(project.boards, :count).by(1)
+        end
+
+        it "creates board's default lists" do
+          board = service.execute
+
+          expect(board.lists.size).to eq 2
+          expect(board.lists.last).to be_closed
+        end
+      end
+    end
+
+    it 'skips creating a second board when the feature is not available' do
+      stub_licensed_features(multiple_issue_boards: false)
+      service = described_class.new(project, double)
+
+      expect(service.execute).not_to be_nil
+
+      expect { service.execute }.not_to change(project.boards, :count)
+    end
+  end
+end

spec/services/ee/boards/list_service_spec.rb

+require 'spec_helper'
+
+describe Boards::ListService do
+  let(:project) { create(:empty_project) }
+  let(:service) { described_class.new(project, double) }
+
+  before do
+    create_list(:board, 2, project: project)
+  end
+
+  describe '#execute' do
+    it 'returns all issue boards when `multiple_issue_boards` is enabled' do
+      stub_licensed_features(multiple_issue_boards: true)
+
+      expect(service.execute.size).to eq(2)
+    end
+
+    it 'returns the first issue board when `multiple_issue_boards` is disabled' do
+      stub_licensed_features(multiple_issue_boards: false)
+
+      expect(service.execute.size).to eq(1)
+    end
+  end
+end