Commit 0f6c42c5 authored by Alexandru Croitor's avatar Alexandru Croitor

Move Multiple Issue Boards for Projects to Core

Refactor code to allow multiple issue boards management for projects
in CE
parent 2b9ddc2f
...@@ -3,8 +3,9 @@ ...@@ -3,8 +3,9 @@
module BoardsResponses module BoardsResponses
include Gitlab::Utils::StrongMemoize include Gitlab::Utils::StrongMemoize
# Overridden on EE module
def board_params def board_params
params.require(:board).permit(:name, :weight, :milestone_id, :assignee_id, label_ids: []) params.require(:board).permit(:name)
end end
def parent def parent
......
# frozen_string_literal: true
module MultipleBoardsActions
include Gitlab::Utils::StrongMemoize
extend ActiveSupport::Concern
included do
include BoardsActions
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
def recent
recent_visits = ::Boards::VisitsFinder.new(parent, current_user).latest(4)
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.persisted?
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)
respond_to do |format|
format.json do
if service.execute(board)
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? || !latest_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!
check_multiple_group_issue_boards_available! if group?
end
def authorize_admin_board!
return render_404 unless can?(current_user, :admin_board, parent)
end
def serializer
BoardSerializer.new(current_user: current_user)
end
def serialize_as_json(resource)
serializer.represent(resource, serializer: 'board', include_full_project_path: board.group_board?)
end
end
# frozen_string_literal: true # frozen_string_literal: true
class Projects::BoardsController < Projects::ApplicationController class Projects::BoardsController < Projects::ApplicationController
include BoardsActions include MultipleBoardsActions
include IssuableCollections include IssuableCollections
before_action :check_issues_available! before_action :check_issues_available!
......
# 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
...@@ -15,7 +15,8 @@ module BoardsHelper ...@@ -15,7 +15,8 @@ module BoardsHelper
root_path: root_path, root_path: root_path,
bulk_update_path: @bulk_issues_path, bulk_update_path: @bulk_issues_path,
default_avatar: image_path(default_avatar), default_avatar: image_path(default_avatar),
time_tracking_limit_to_hours: Gitlab::CurrentSettings.time_tracking_limit_to_hours.to_s time_tracking_limit_to_hours: Gitlab::CurrentSettings.time_tracking_limit_to_hours.to_s,
recent_boards_endpoint: recent_boards_path
} }
end end
...@@ -87,6 +88,18 @@ module BoardsHelper ...@@ -87,6 +88,18 @@ module BoardsHelper
end end
def boards_link_text def boards_link_text
s_("IssueBoards|Board") if current_board_parent.multiple_issue_boards_available?
s_("IssueBoards|Boards")
else
s_("IssueBoards|Board")
end
end
def recent_boards_path
recent_project_boards_path(@project) if current_board_parent.is_a?(Project)
end
def current_board_json
board.to_json
end end
end end
...@@ -1949,9 +1949,8 @@ class Project < ApplicationRecord ...@@ -1949,9 +1949,8 @@ class Project < ApplicationRecord
end end
end end
# Overridden on EE module
def multiple_issue_boards_available? def multiple_issue_boards_available?
false true
end end
def full_path_before_last_save def full_path_before_last_save
......
...@@ -196,6 +196,7 @@ class ProjectPolicy < BasePolicy ...@@ -196,6 +196,7 @@ class ProjectPolicy < BasePolicy
rule { guest & can?(:read_container_image) }.enable :build_read_container_image rule { guest & can?(:read_container_image) }.enable :build_read_container_image
rule { can?(:reporter_access) }.policy do rule { can?(:reporter_access) }.policy do
enable :admin_board
enable :download_code enable :download_code
enable :read_statistics enable :read_statistics
enable :download_wiki_code enable :download_wiki_code
...@@ -240,6 +241,7 @@ class ProjectPolicy < BasePolicy ...@@ -240,6 +241,7 @@ class ProjectPolicy < BasePolicy
rule { can?(:developer_access) & can?(:create_issue) }.enable :import_issues rule { can?(:developer_access) & can?(:create_issue) }.enable :import_issues
rule { can?(:developer_access) }.policy do rule { can?(:developer_access) }.policy do
enable :admin_board
enable :admin_merge_request enable :admin_merge_request
enable :admin_milestone enable :admin_milestone
enable :update_merge_request enable :update_merge_request
...@@ -266,6 +268,7 @@ class ProjectPolicy < BasePolicy ...@@ -266,6 +268,7 @@ class ProjectPolicy < BasePolicy
end end
rule { can?(:maintainer_access) }.policy do rule { can?(:maintainer_access) }.policy do
enable :admin_board
enable :push_to_delete_protected_branch enable :push_to_delete_protected_branch
enable :update_project_snippet enable :update_project_snippet
enable :update_environment enable :update_environment
......
...@@ -2,4 +2,5 @@ ...@@ -2,4 +2,5 @@
class BoardSimpleEntity < Grape::Entity class BoardSimpleEntity < Grape::Entity
expose :id expose :id
expose :name
end end
...@@ -9,7 +9,7 @@ module Boards ...@@ -9,7 +9,7 @@ module Boards
private private
def can_create_board? def can_create_board?
parent.boards.empty? parent.boards.empty? || parent.multiple_issue_boards_available?
end end
def create_board! def create_board!
......
# 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
# frozen_string_literal: true
module Boards
class UpdateService < Boards::BaseService
def execute(board)
board.update(params)
end
end
end
# 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
---
title: Move Multiple Issue Boards for Projects to Core
merge_request:
author:
type: added
...@@ -155,7 +155,11 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do ...@@ -155,7 +155,11 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
end end
end end
resources :boards, only: [:index, :show], constraints: { id: /\d+/ } resources :boards, only: [:index, :show, :create, :update, :destroy], constraints: { id: /\d+/ } do
collection do
get :recent
end
end
resources :releases, only: [:index] resources :releases, only: [:index]
resources :forks, only: [:index, :new, :create] resources :forks, only: [:index, :new, :create]
resources :group_links, only: [:index, :create, :update, :destroy], constraints: { id: /\d+/ } resources :group_links, only: [:index, :create, :update, :destroy], constraints: { id: /\d+/ }
......
...@@ -5479,6 +5479,9 @@ msgstr "" ...@@ -5479,6 +5479,9 @@ msgstr ""
msgid "IssueBoards|Board" msgid "IssueBoards|Board"
msgstr "" msgstr ""
msgid "IssueBoards|Boards"
msgstr ""
msgid "Issues" msgid "Issues"
msgstr "" msgstr ""
......
...@@ -59,7 +59,7 @@ describe Groups::BoardsController do ...@@ -59,7 +59,7 @@ describe Groups::BoardsController do
it 'return an array with one group board' do it 'return an array with one group board' do
create(:board, group: group) create(:board, group: group)
expect(Boards::Visits::LatestService).not_to receive(:new) expect(Boards::VisitsFinder).not_to receive(:new)
list_boards format: :json list_boards format: :json
......
...@@ -65,7 +65,7 @@ describe Projects::BoardsController do ...@@ -65,7 +65,7 @@ describe Projects::BoardsController do
it 'returns a list of project boards' do it 'returns a list of project boards' do
create_list(:board, 2, project: project) 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 list_boards format: :json
......
...@@ -2,32 +2,32 @@ ...@@ -2,32 +2,32 @@
require 'spec_helper' require 'spec_helper'
describe Boards::Visits::LatestService do describe Boards::VisitsFinder do
describe '#execute' do describe '#latest' do
let(:user) { create(:user) } let(:user) { create(:user) }
context 'when a project board' do context 'when a project board' do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:project_board) { create(:board, project: 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 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 end
it 'queries for most recent visit' do it 'queries for most recent visit' do
expect(BoardProjectRecentVisit).to receive(:latest).once expect(BoardProjectRecentVisit).to receive(:latest).once
service.execute finder.execute
end end
it 'queries for last N visits' do it 'queries for last N visits' do
expect(BoardProjectRecentVisit).to receive(:latest).with(user, project, count: 5).once 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
end end
...@@ -35,24 +35,24 @@ describe Boards::Visits::LatestService do ...@@ -35,24 +35,24 @@ describe Boards::Visits::LatestService do
let(:group) { create(:group) } let(:group) { create(:group) }
let(:group_board) { create(:board, group: 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 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 end
it 'queries for most recent visit' do it 'queries for most recent visit' do
expect(BoardGroupRecentVisit).to receive(:latest).once expect(BoardGroupRecentVisit).to receive(:latest).once
service.execute finder.latest
end end
it 'queries for last N visits' do it 'queries for last N visits' do
expect(BoardGroupRecentVisit).to receive(:latest).with(user, group, count: 5).once 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 end
end end
......
...@@ -17,7 +17,7 @@ describe 'layouts/nav/sidebar/_project' do ...@@ -17,7 +17,7 @@ describe 'layouts/nav/sidebar/_project' do
it 'has board tab' do it 'has board tab' do
render render
expect(rendered).to have_css('a[title="Board"]') expect(rendered).to have_css('a[title="Boards"]')
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment