Commit 7fa98743 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'refactor-boards-actions' into 'master'

Refactor groups and projects boards actions

See merge request gitlab-org/gitlab-ce!25568
parents 7b5cf9e8 6e3bb1e3
# frozen_string_literal: true
module BoardsActions
include Gitlab::Utils::StrongMemoize
extend ActiveSupport::Concern
included do
include BoardsResponses
before_action :boards, only: :index
before_action :board, only: :show
end
def index
respond_with_boards
end
def show
# Add / update the board in the recent visits table
Boards::Visits::CreateService.new(parent, current_user).execute(board) if request.format.html?
respond_with_board
end
private
def boards
strong_memoize(:boards) do
Boards::ListService.new(parent, current_user).execute
end
end
def board
strong_memoize(:board) do
boards.find(params[:id])
end
end
end
# frozen_string_literal: true # frozen_string_literal: true
class Groups::BoardsController < Groups::ApplicationController class Groups::BoardsController < Groups::ApplicationController
include BoardsResponses include BoardsActions
include RecordUserLastActivity include RecordUserLastActivity
before_action :assign_endpoint_vars before_action :assign_endpoint_vars
before_action :boards, only: :index
before_action :redirect_to_recent_board, only: :index
def index
respond_with_boards
end
def show
@board = boards.find(params[:id])
# add/update the board in the recent visited table
Boards::Visits::CreateService.new(@board.group, current_user).execute(@board) if request.format.html?
respond_with_board
end
private private
def boards
@boards ||= Boards::ListService.new(group, current_user).execute
end
def assign_endpoint_vars def assign_endpoint_vars
@boards_endpoint = group_boards_url(group) @boards_endpoint = group_boards_url(group)
@namespace_path = group.to_param @namespace_path = group.to_param
@labels_endpoint = group_labels_url(group) @labels_endpoint = group_labels_url(group)
end end
def serialize_as_json(resource)
resource.as_json(only: [:id])
end
def includes_board?(board_id)
boards.any? { |board| board.id == board_id }
end
def redirect_to_recent_board
return if request.format.json?
recently_visited = Boards::Visits::LatestService.new(group, current_user).execute
if recently_visited && includes_board?(recently_visited.board_id)
redirect_to(group_board_path(id: recently_visited.board_id), status: :found)
end
end
end end
# frozen_string_literal: true # frozen_string_literal: true
class Projects::BoardsController < Projects::ApplicationController class Projects::BoardsController < Projects::ApplicationController
include BoardsResponses include BoardsActions
include IssuableCollections include IssuableCollections
before_action :check_issues_available! before_action :check_issues_available!
before_action :authorize_read_board!, only: [:index, :show] before_action :authorize_read_board!, only: [:index, :show]
before_action :boards, only: :index
before_action :assign_endpoint_vars before_action :assign_endpoint_vars
before_action :redirect_to_recent_board, only: :index
def index
respond_with_boards
end
def show
@board = boards.find(params[:id])
# add/update the board in the recent visited table
Boards::Visits::CreateService.new(@board.project, current_user).execute(@board) if request.format.html?
respond_with_board
end
private private
def boards
@boards ||= Boards::ListService.new(project, current_user).execute
end
def assign_endpoint_vars def assign_endpoint_vars
@boards_endpoint = project_boards_path(project) @boards_endpoint = project_boards_path(project)
@bulk_issues_path = bulk_update_project_issues_path(project) @bulk_issues_path = bulk_update_project_issues_path(project)
...@@ -39,22 +20,4 @@ class Projects::BoardsController < Projects::ApplicationController ...@@ -39,22 +20,4 @@ class Projects::BoardsController < Projects::ApplicationController
def authorize_read_board! def authorize_read_board!
access_denied! unless can?(current_user, :read_board, project) access_denied! unless can?(current_user, :read_board, project)
end end
def serialize_as_json(resource)
resource.as_json(only: [:id])
end
def includes_board?(board_id)
boards.any? { |board| board.id == board_id }
end
def redirect_to_recent_board
return if request.format.json?
recently_visited = Boards::Visits::LatestService.new(project, current_user).execute
if recently_visited && includes_board?(recently_visited.board_id)
redirect_to(namespace_project_board_path(id: recently_visited.board_id), status: :found)
end
end
end end
...@@ -38,7 +38,6 @@ class Project < ActiveRecord::Base ...@@ -38,7 +38,6 @@ class Project < ActiveRecord::Base
BoardLimitExceeded = Class.new(StandardError) BoardLimitExceeded = Class.new(StandardError)
STATISTICS_ATTRIBUTE = 'repositories_count'.freeze STATISTICS_ATTRIBUTE = 'repositories_count'.freeze
NUMBER_OF_PERMITTED_BOARDS = 1
UNKNOWN_IMPORT_URL = 'http://unknown.git'.freeze UNKNOWN_IMPORT_URL = 'http://unknown.git'.freeze
# Hashed Storage versions handle rolling out new storage to project and dependents models: # Hashed Storage versions handle rolling out new storage to project and dependents models:
# nil: legacy # nil: legacy
...@@ -137,7 +136,7 @@ class Project < ActiveRecord::Base ...@@ -137,7 +136,7 @@ class Project < ActiveRecord::Base
alias_attribute :parent_id, :namespace_id alias_attribute :parent_id, :namespace_id
has_one :last_event, -> {order 'events.created_at DESC'}, class_name: 'Event' has_one :last_event, -> {order 'events.created_at DESC'}, class_name: 'Event'
has_many :boards, before_add: :validate_board_limit has_many :boards
# Project services # Project services
has_one :campfire_service has_one :campfire_service
...@@ -2192,17 +2191,6 @@ class Project < ActiveRecord::Base ...@@ -2192,17 +2191,6 @@ class Project < ActiveRecord::Base
"projects/#{id}/pushes_since_gc" "projects/#{id}/pushes_since_gc"
end end
# Similar to the normal callbacks that hook into the life cycle of an
# Active Record object, you can also define callbacks that get triggered
# when you add an object to an association collection. If any of these
# callbacks throw an exception, the object will not be added to the
# collection. Before you add a new board to the boards collection if you
# already have 1, 2, or n it will fail, but it if you have 0 that is lower
# than the number of permitted boards per project it won't fail.
def validate_board_limit(board)
raise BoardLimitExceeded, 'Number of permitted boards exceeded' if boards.size >= NUMBER_OF_PERMITTED_BOARDS
end
def update_project_statistics def update_project_statistics
stats = statistics || build_statistics stats = statistics || build_statistics
stats.update(namespace_id: namespace_id) stats.update(namespace_id: namespace_id)
......
...@@ -22,28 +22,6 @@ describe Groups::BoardsController do ...@@ -22,28 +22,6 @@ describe Groups::BoardsController do
expect(response.content_type).to eq 'text/html' expect(response.content_type).to eq 'text/html'
end end
it 'redirects to latest visited board' do
board = create(:board, group: group)
create(:board_group_recent_visit, group: board.group, board: board, user: user)
list_boards
expect(response).to redirect_to(group_board_path(id: board.id))
end
it 'renders template if visited board is not found' do
temporary_board = create(:board, group: group)
visited = create(:board_group_recent_visit, group: temporary_board.group, board: temporary_board, user: user)
temporary_board.delete
allow_any_instance_of(Boards::Visits::LatestService).to receive(:execute).and_return(visited)
list_boards
expect(response).to render_template :index
expect(response.content_type).to eq 'text/html'
end
context 'with unauthorized user' do context 'with unauthorized user' do
before do before do
allow(Ability).to receive(:allowed?).with(user, :read_cross_project, :global).and_return(true) allow(Ability).to receive(:allowed?).with(user, :read_cross_project, :global).and_return(true)
......
...@@ -28,28 +28,6 @@ describe Projects::BoardsController do ...@@ -28,28 +28,6 @@ describe Projects::BoardsController do
expect(response.content_type).to eq 'text/html' expect(response.content_type).to eq 'text/html'
end end
it 'redirects to latest visited board' do
board = create(:board, project: project)
create(:board_project_recent_visit, project: board.project, board: board, user: user)
list_boards
expect(response).to redirect_to(namespace_project_board_path(id: board.id))
end
it 'renders template if visited board is not found' do
temporary_board = create(:board, project: project)
visited = create(:board_project_recent_visit, project: temporary_board.project, board: temporary_board, user: user)
temporary_board.delete
allow_any_instance_of(Boards::Visits::LatestService).to receive(:execute).and_return(visited)
list_boards
expect(response).to render_template :index
expect(response.content_type).to eq 'text/html'
end
context 'with unauthorized user' do context 'with unauthorized user' do
before do before do
allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true) allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(true)
......
...@@ -136,15 +136,6 @@ describe Project do ...@@ -136,15 +136,6 @@ describe Project do
end end
end end
describe '#boards' do
it 'raises an error when attempting to add more than one board to the project' do
subject.boards.build
expect { subject.boards.build }.to raise_error(Project::BoardLimitExceeded, 'Number of permitted boards exceeded')
expect(subject.boards.size).to eq 1
end
end
describe 'ci_pipelines association' do describe 'ci_pipelines association' do
it 'returns only pipelines from ci_sources' do it 'returns only pipelines from ci_sources' do
expect(Ci::Pipeline).to receive(:ci_sources).and_call_original expect(Ci::Pipeline).to receive(:ci_sources).and_call_original
......
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