diff options
| author | Sean McGivern <sean@mcgivern.me.uk> | 2016-10-13 14:21:14 +0000 |
|---|---|---|
| committer | Sean McGivern <sean@mcgivern.me.uk> | 2016-10-13 14:21:14 +0000 |
| commit | 11e93ad59d6209bf9805a6f7053719e8052cc508 (patch) | |
| tree | 0770038bcfb327e45381e8f45ba6629c02a09f12 /spec | |
| parent | 36dafbbb02201ad36ca7e59f699cbf727494129e (diff) | |
| parent | 25c82c6faf067a82e99bad590357ad57618475d9 (diff) | |
| download | gitlab-ce-11e93ad59d6209bf9805a6f7053719e8052cc508.tar.gz | |
Merge branch 'feature/issues-board' into 'master'
Refactoring Issues Board
## What does this MR do?
This MR aims to minimize conflicts between the CE issues board feature with EE multiple boards feature.
## Are there points in the code the reviewer needs to double check?
## Why was this MR needed?
To avoid a lot of conflicts with EE multiple boards feature.
## Screenshots (if relevant)
## Does this MR meet the acceptance criteria?
- [ ] ~~[CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added~~
- [ ] ~~[Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~
- [x] API support added
- Tests
- [X] Added for this feature/bug
- [ ] All builds are passing
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [X] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [ ] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
## What are the relevant issue numbers?
https://gitlab.com/gitlab-org/gitlab-ee/issues/929
https://gitlab.com/gitlab-org/gitlab-ee/issues/1084
See merge request !6727
Diffstat (limited to 'spec')
26 files changed, 344 insertions, 156 deletions
diff --git a/spec/controllers/projects/boards/issues_controller_spec.rb b/spec/controllers/projects/boards/issues_controller_spec.rb index 566658b508d..da59642f24d 100644 --- a/spec/controllers/projects/boards/issues_controller_spec.rb +++ b/spec/controllers/projects/boards/issues_controller_spec.rb @@ -1,15 +1,16 @@ require 'spec_helper' describe Projects::Boards::IssuesController do - let(:project) { create(:project_with_board) } + let(:project) { create(:empty_project) } + let(:board) { create(:board, project: project) } let(:user) { create(:user) } let(:guest) { create(:user) } let(:planning) { create(:label, project: project, name: 'Planning') } let(:development) { create(:label, project: project, name: 'Development') } - let!(:list1) { create(:list, board: project.board, label: planning, position: 0) } - let!(:list2) { create(:list, board: project.board, label: development, position: 1) } + let!(:list1) { create(:list, board: board, label: planning, position: 0) } + let!(:list2) { create(:list, board: board, label: development, position: 1) } before do project.team << [user, :master] @@ -24,7 +25,7 @@ describe Projects::Boards::IssuesController do create(:labeled_issue, project: project, labels: [development]) create(:labeled_issue, project: project, labels: [development], assignee: johndoe) - list_issues user: user, list_id: list2 + list_issues user: user, board: board, list: list2 parsed_response = JSON.parse(response.body) @@ -33,9 +34,17 @@ describe Projects::Boards::IssuesController do end end + context 'with invalid board id' do + it 'returns a not found 404 response' do + list_issues user: user, board: 999, list: list2 + + expect(response).to have_http_status(404) + end + end + context 'with invalid list id' do it 'returns a not found 404 response' do - list_issues user: user, list_id: 999 + list_issues user: user, board: board, list: 999 expect(response).to have_http_status(404) end @@ -47,32 +56,33 @@ describe Projects::Boards::IssuesController do allow(Ability).to receive(:allowed?).with(user, :read_issue, project).and_return(false) end - it 'returns a successful 403 response' do - list_issues user: user, list_id: list2 + it 'returns a forbidden 403 response' do + list_issues user: user, board: board, list: list2 expect(response).to have_http_status(403) end end - def list_issues(user:, list_id:) + def list_issues(user:, board:, list:) sign_in(user) get :index, namespace_id: project.namespace.to_param, project_id: project.to_param, - list_id: list_id.to_param + board_id: board.to_param, + list_id: list.to_param end end describe 'POST create' do context 'with valid params' do it 'returns a successful 200 response' do - create_issue user: user, list: list1, title: 'New issue' + create_issue user: user, board: board, list: list1, title: 'New issue' expect(response).to have_http_status(200) end it 'returns the created issue' do - create_issue user: user, list: list1, title: 'New issue' + create_issue user: user, board: board, list: list1, title: 'New issue' expect(response).to match_response_schema('issue') end @@ -81,7 +91,7 @@ describe Projects::Boards::IssuesController do context 'with invalid params' do context 'when title is nil' do it 'returns an unprocessable entity 422 response' do - create_issue user: user, list: list1, title: nil + create_issue user: user, board: board, list: list1, title: nil expect(response).to have_http_status(422) end @@ -91,7 +101,7 @@ describe Projects::Boards::IssuesController do it 'returns a not found 404 response' do list = create(:list) - create_issue user: user, list: list, title: 'New issue' + create_issue user: user, board: board, list: list, title: 'New issue' expect(response).to have_http_status(404) end @@ -100,17 +110,18 @@ describe Projects::Boards::IssuesController do context 'with unauthorized user' do it 'returns a forbidden 403 response' do - create_issue user: guest, list: list1, title: 'New issue' + create_issue user: guest, board: board, list: list1, title: 'New issue' expect(response).to have_http_status(403) end end - def create_issue(user:, list:, title:) + def create_issue(user:, board:, list:, title:) sign_in(user) post :create, namespace_id: project.namespace.to_param, project_id: project.to_param, + board_id: board.to_param, list_id: list.to_param, issue: { title: title }, format: :json @@ -122,13 +133,13 @@ describe Projects::Boards::IssuesController do context 'with valid params' do it 'returns a successful 200 response' do - move user: user, issue: issue, from_list_id: list1.id, to_list_id: list2.id + move user: user, board: board, issue: issue, from_list_id: list1.id, to_list_id: list2.id expect(response).to have_http_status(200) end it 'moves issue to the desired list' do - move user: user, issue: issue, from_list_id: list1.id, to_list_id: list2.id + move user: user, board: board, issue: issue, from_list_id: list1.id, to_list_id: list2.id expect(issue.reload.labels).to contain_exactly(development) end @@ -136,31 +147,44 @@ describe Projects::Boards::IssuesController do context 'with invalid params' do it 'returns a unprocessable entity 422 response for invalid lists' do - move user: user, issue: issue, from_list_id: nil, to_list_id: nil + move user: user, board: board, issue: issue, from_list_id: nil, to_list_id: nil expect(response).to have_http_status(422) end + it 'returns a not found 404 response for invalid board id' do + move user: user, board: 999, issue: issue, from_list_id: list1.id, to_list_id: list2.id + + expect(response).to have_http_status(404) + end + it 'returns a not found 404 response for invalid issue id' do - move user: user, issue: 999, from_list_id: list1.id, to_list_id: list2.id + move user: user, board: board, issue: 999, from_list_id: list1.id, to_list_id: list2.id expect(response).to have_http_status(404) end end context 'with unauthorized user' do + let(:guest) { create(:user) } + + before do + project.team << [guest, :guest] + end + it 'returns a forbidden 403 response' do - move user: guest, issue: issue, from_list_id: list1.id, to_list_id: list2.id + move user: guest, board: board, issue: issue, from_list_id: list1.id, to_list_id: list2.id expect(response).to have_http_status(403) end end - def move(user:, issue:, from_list_id:, to_list_id:) + def move(user:, board:, issue:, from_list_id:, to_list_id:) sign_in(user) patch :update, namespace_id: project.namespace.to_param, project_id: project.to_param, + board_id: board.to_param, id: issue.to_param, from_list_id: from_list_id, to_list_id: to_list_id, diff --git a/spec/controllers/projects/boards/lists_controller_spec.rb b/spec/controllers/projects/boards/lists_controller_spec.rb index 709006a3601..34d6119429d 100644 --- a/spec/controllers/projects/boards/lists_controller_spec.rb +++ b/spec/controllers/projects/boards/lists_controller_spec.rb @@ -1,8 +1,8 @@ require 'spec_helper' describe Projects::Boards::ListsController do - let(:project) { create(:project_with_board) } - let(:board) { project.board } + let(:project) { create(:empty_project) } + let(:board) { create(:board, project: project) } let(:user) { create(:user) } let(:guest) { create(:user) } @@ -13,7 +13,7 @@ describe Projects::Boards::ListsController do describe 'GET index' do it 'returns a successful 200 response' do - read_board_list user: user + read_board_list user: user, board: board expect(response).to have_http_status(200) expect(response.content_type).to eq 'application/json' @@ -22,7 +22,7 @@ describe Projects::Boards::ListsController do it 'returns a list of board lists' do create(:list, board: board) - read_board_list user: user + read_board_list user: user, board: board parsed_response = JSON.parse(response.body) @@ -37,17 +37,18 @@ describe Projects::Boards::ListsController do end it 'returns a forbidden 403 response' do - read_board_list user: user + read_board_list user: user, board: board expect(response).to have_http_status(403) end end - def read_board_list(user:) + def read_board_list(user:, board:) sign_in(user) get :index, namespace_id: project.namespace.to_param, project_id: project.to_param, + board_id: board.to_param, format: :json end end @@ -57,13 +58,13 @@ describe Projects::Boards::ListsController do let(:label) { create(:label, project: project, name: 'Development') } it 'returns a successful 200 response' do - create_board_list user: user, label_id: label.id + create_board_list user: user, board: board, label_id: label.id expect(response).to have_http_status(200) end it 'returns the created list' do - create_board_list user: user, label_id: label.id + create_board_list user: user, board: board, label_id: label.id expect(response).to match_response_schema('list') end @@ -72,7 +73,7 @@ describe Projects::Boards::ListsController do context 'with invalid params' do context 'when label is nil' do it 'returns a not found 404 response' do - create_board_list user: user, label_id: nil + create_board_list user: user, board: board, label_id: nil expect(response).to have_http_status(404) end @@ -82,7 +83,7 @@ describe Projects::Boards::ListsController do it 'returns a not found 404 response' do label = create(:label, name: 'Development') - create_board_list user: user, label_id: label.id + create_board_list user: user, board: board, label_id: label.id expect(response).to have_http_status(404) end @@ -93,17 +94,18 @@ describe Projects::Boards::ListsController do it 'returns a forbidden 403 response' do label = create(:label, project: project, name: 'Development') - create_board_list user: guest, label_id: label.id + create_board_list user: guest, board: board, label_id: label.id expect(response).to have_http_status(403) end end - def create_board_list(user:, label_id:) + def create_board_list(user:, board:, label_id:) sign_in(user) post :create, namespace_id: project.namespace.to_param, project_id: project.to_param, + board_id: board.to_param, list: { label_id: label_id }, format: :json end @@ -115,13 +117,13 @@ describe Projects::Boards::ListsController do context 'with valid position' do it 'returns a successful 200 response' do - move user: user, list: planning, position: 1 + move user: user, board: board, list: planning, position: 1 expect(response).to have_http_status(200) end it 'moves the list to the desired position' do - move user: user, list: planning, position: 1 + move user: user, board: board, list: planning, position: 1 expect(planning.reload.position).to eq 1 end @@ -129,7 +131,7 @@ describe Projects::Boards::ListsController do context 'with invalid position' do it 'returns an unprocessable entity 422 response' do - move user: user, list: planning, position: 6 + move user: user, board: board, list: planning, position: 6 expect(response).to have_http_status(422) end @@ -137,7 +139,7 @@ describe Projects::Boards::ListsController do context 'with invalid list id' do it 'returns a not found 404 response' do - move user: user, list: 999, position: 1 + move user: user, board: board, list: 999, position: 1 expect(response).to have_http_status(404) end @@ -145,17 +147,18 @@ describe Projects::Boards::ListsController do context 'with unauthorized user' do it 'returns a forbidden 403 response' do - move user: guest, list: planning, position: 6 + move user: guest, board: board, list: planning, position: 6 expect(response).to have_http_status(403) end end - def move(user:, list:, position:) + def move(user:, board:, list:, position:) sign_in(user) patch :update, namespace_id: project.namespace.to_param, project_id: project.to_param, + board_id: board.to_param, id: list.to_param, list: { position: position }, format: :json @@ -167,19 +170,19 @@ describe Projects::Boards::ListsController do context 'with valid list id' do it 'returns a successful 200 response' do - remove_board_list user: user, list: planning + remove_board_list user: user, board: board, list: planning expect(response).to have_http_status(200) end it 'removes list from board' do - expect { remove_board_list user: user, list: planning }.to change(board.lists, :size).by(-1) + expect { remove_board_list user: user, board: board, list: planning }.to change(board.lists, :size).by(-1) end end context 'with invalid list id' do it 'returns a not found 404 response' do - remove_board_list user: user, list: 999 + remove_board_list user: user, board: board, list: 999 expect(response).to have_http_status(404) end @@ -187,17 +190,18 @@ describe Projects::Boards::ListsController do context 'with unauthorized user' do it 'returns a forbidden 403 response' do - remove_board_list user: guest, list: planning + remove_board_list user: guest, board: board, list: planning expect(response).to have_http_status(403) end end - def remove_board_list(user:, list:) + def remove_board_list(user:, board:, list:) sign_in(user) delete :destroy, namespace_id: project.namespace.to_param, project_id: project.to_param, + board_id: board.to_param, id: list.to_param, format: :json end @@ -206,13 +210,13 @@ describe Projects::Boards::ListsController do describe 'POST generate' do context 'when board lists is empty' do it 'returns a successful 200 response' do - generate_default_board_lists user: user + generate_default_lists user: user, board: board expect(response).to have_http_status(200) end it 'returns the defaults lists' do - generate_default_board_lists user: user + generate_default_lists user: user, board: board expect(response).to match_response_schema('lists') end @@ -222,7 +226,7 @@ describe Projects::Boards::ListsController do it 'returns an unprocessable entity 422 response' do create(:list, board: board) - generate_default_board_lists user: user + generate_default_lists user: user, board: board expect(response).to have_http_status(422) end @@ -230,17 +234,18 @@ describe Projects::Boards::ListsController do context 'with unauthorized user' do it 'returns a forbidden 403 response' do - generate_default_board_lists user: guest + generate_default_lists user: guest, board: board expect(response).to have_http_status(403) end end - def generate_default_board_lists(user:) + def generate_default_lists(user:, board:) sign_in(user) post :generate, namespace_id: project.namespace.to_param, project_id: project.to_param, + board_id: board.to_param, format: :json end end diff --git a/spec/controllers/projects/boards_controller_spec.rb b/spec/controllers/projects/boards_controller_spec.rb index 6f6e608e1f3..cc19035740e 100644 --- a/spec/controllers/projects/boards_controller_spec.rb +++ b/spec/controllers/projects/boards_controller_spec.rb @@ -9,16 +9,71 @@ describe Projects::BoardsController do sign_in(user) end + describe 'GET index' do + it 'creates a new project board when project does not have one' do + expect { list_boards }.to change(project.boards, :count).by(1) + end + + context 'when format is HTML' do + it 'renders template' do + list_boards + + expect(response).to render_template :index + expect(response.content_type).to eq 'text/html' + end + end + + context 'when format is JSON' do + it 'returns a list of project boards' do + create_list(:board, 2, project: project) + + 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 + 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, :read_board, project).and_return(false) + end + + it 'returns a not found 404 response' do + list_boards + + expect(response).to have_http_status(404) + end + end + + def list_boards(format: :html) + get :index, namespace_id: project.namespace.to_param, + project_id: project.to_param, + format: format + end + end + describe 'GET show' do - it 'creates a new board when project does not have one' do - expect { read_board }.to change(Board, :count).by(1) + let!(:board) { create(:board, project: project) } + + context 'when format is HTML' do + it 'renders template' do + read_board board: board + + expect(response).to render_template :show + expect(response.content_type).to eq 'text/html' + end end - it 'renders HTML template' do - read_board + context 'when format is JSON' do + it 'returns project board' do + read_board board: board, format: :json - expect(response).to render_template :show - expect(response.content_type).to eq 'text/html' + expect(response).to match_response_schema('board') + end end context 'with unauthorized user' do @@ -27,16 +82,27 @@ describe Projects::BoardsController do allow(Ability).to receive(:allowed?).with(user, :read_board, project).and_return(false) end - it 'returns a successful 404 response' do - read_board + it 'returns a not found 404 response' do + read_board board: board + + expect(response).to have_http_status(404) + end + end + + context 'when board does not belong to project' do + it 'returns a not found 404 response' do + another_board = create(:board) + + read_board board: another_board expect(response).to have_http_status(404) end end - def read_board(format: :html) + def read_board(board:, format: :html) get :show, namespace_id: project.namespace.to_param, project_id: project.to_param, + id: board.to_param, format: format end end diff --git a/spec/factories/boards.rb b/spec/factories/boards.rb index 35c4a0b6f08..ec46146d9b5 100644 --- a/spec/factories/boards.rb +++ b/spec/factories/boards.rb @@ -1,5 +1,10 @@ FactoryGirl.define do factory :board do project factory: :empty_project + + after(:create) do |board| + board.lists.create(list_type: :backlog) + board.lists.create(list_type: :done) + end end end diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index 331172445e4..719ef17f57e 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -124,12 +124,4 @@ FactoryGirl.define do ) end end - - factory :project_with_board, parent: :empty_project do - after(:create) do |project| - project.create_board - project.board.lists.create(list_type: :backlog) - project.board.lists.create(list_type: :done) - end - end end diff --git a/spec/features/boards/boards_spec.rb b/spec/features/boards/boards_spec.rb index 470e2bdbb9b..0fb1608a0a3 100644 --- a/spec/features/boards/boards_spec.rb +++ b/spec/features/boards/boards_spec.rb @@ -4,7 +4,8 @@ describe 'Issue Boards', feature: true, js: true do include WaitForAjax include WaitForVueResource - let(:project) { create(:project_with_board, :public) } + let(:project) { create(:empty_project, :public) } + let(:board) { create(:board, project: project) } let(:user) { create(:user) } let!(:user2) { create(:user) } @@ -17,7 +18,7 @@ describe 'Issue Boards', feature: true, js: true do context 'no lists' do before do - visit namespace_project_board_path(project.namespace, project) + visit namespace_project_board_path(project.namespace, project, board) wait_for_vue_resource expect(page).to have_selector('.board', count: 3) end @@ -60,8 +61,8 @@ describe 'Issue Boards', feature: true, js: true do let!(:done) { create(:label, project: project, name: 'Done') } let!(:accepting) { create(:label, project: project, name: 'Accepting Merge Requests') } - let!(:list1) { create(:list, board: project.board, label: planning, position: 0) } - let!(:list2) { create(:list, board: project.board, label: development, position: 1) } + let!(:list1) { create(:list, board: board, label: planning, position: 0) } + let!(:list2) { create(:list, board: board, label: development, position: 1) } let!(:confidential_issue) { create(:issue, :confidential, project: project, author: user) } let!(:issue1) { create(:issue, project: project, assignee: user) } @@ -75,7 +76,7 @@ describe 'Issue Boards', feature: true, js: true do let!(:issue9) { create(:labeled_issue, project: project, labels: [testing, bug, accepting]) } before do - visit namespace_project_board_path(project.namespace, project) + visit namespace_project_board_path(project.namespace, project, board) wait_for_vue_resource @@ -169,7 +170,7 @@ describe 'Issue Boards', feature: true, js: true do create(:issue, project: project) end - visit namespace_project_board_path(project.namespace, project) + visit namespace_project_board_path(project.namespace, project, board) wait_for_vue_resource page.within(find('.board', match: :first)) do @@ -468,7 +469,7 @@ describe 'Issue Boards', feature: true, js: true do it 'removes filtered labels' do wait_for_vue_resource - + page.within '.labels-filter' do click_button('Label') wait_for_ajax @@ -603,7 +604,7 @@ describe 'Issue Boards', feature: true, js: true do context 'keyboard shortcuts' do before do - visit namespace_project_board_path(project.namespace, project) + visit namespace_project_board_path(project.namespace, project, board) wait_for_vue_resource end @@ -616,7 +617,7 @@ describe 'Issue Boards', feature: true, js: true do context 'signed out user' do before do logout - visit namespace_project_board_path(project.namespace, project) + visit namespace_project_board_path(project.namespace, project, board) wait_for_vue_resource end @@ -632,7 +633,7 @@ describe 'Issue Boards', feature: true, js: true do project.team << [user_guest, :guest] logout login_as(user_guest) - visit namespace_project_board_path(project.namespace, project) + visit namespace_project_board_path(project.namespace, project, board) wait_for_vue_resource end diff --git a/spec/features/boards/keyboard_shortcut_spec.rb b/spec/features/boards/keyboard_shortcut_spec.rb index 7ef68e9eb8d..a5fc766401f 100644 --- a/spec/features/boards/keyboard_shortcut_spec.rb +++ b/spec/features/boards/keyboard_shortcut_spec.rb @@ -6,9 +6,7 @@ describe 'Issue Boards shortcut', feature: true, js: true do let(:project) { create(:empty_project) } before do - project.create_board - project.board.lists.create(list_type: :backlog) - project.board.lists.create(list_type: :done) + create(:board, project: project) login_as :admin diff --git a/spec/features/boards/new_issue_spec.rb b/spec/features/boards/new_issue_spec.rb index c046e6b8d79..67d6da5f39a 100644 --- a/spec/features/boards/new_issue_spec.rb +++ b/spec/features/boards/new_issue_spec.rb @@ -4,7 +4,8 @@ describe 'Issue Boards new issue', feature: true, js: true do include WaitForAjax include WaitForVueResource - let(:project) { create(:project_with_board, :public) } + let(:project) { create(:empty_project, :public) } + let(:board) { create(:board, project: project) } let(:user) { create(:user) } context 'authorized user' do @@ -13,7 +14,7 @@ describe 'Issue Boards new issue', feature: true, js: true do login_as(user) - visit namespace_project_board_path(project.namespace, project) + visit namespace_project_board_path(project.namespace, project, board) wait_for_vue_resource expect(page).to have_selector('.board', count: 3) @@ -69,7 +70,7 @@ describe 'Issue Boards new issue', feature: true, js: true do context 'unauthorized user' do before do - visit namespace_project_board_path(project.namespace, project) + visit namespace_project_board_path(project.namespace, project, board) wait_for_vue_resource end diff --git a/spec/fixtures/api/schemas/board.json b/spec/fixtures/api/schemas/board.json new file mode 100644 index 00000000000..03aca4a3cc0 --- /dev/null +++ b/spec/fixtures/api/schemas/board.json @@ -0,0 +1,11 @@ +{ + "type": "object", + "required" : [ + "id" + ], + "properties" : { + "id": { "type": "integer" }, + "name": { "type": "string" } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/api/schemas/boards.json b/spec/fixtures/api/schemas/boards.json new file mode 100644 index 00000000000..117564ef77a --- /dev/null +++ b/spec/fixtures/api/schemas/boards.json @@ -0,0 +1,4 @@ +{ + "type": "array", + "items": { "$ref": "board.json" } +} diff --git a/spec/javascripts/boards/boards_store_spec.js.es6 b/spec/javascripts/boards/boards_store_spec.js.es6 index 078e4b00023..15c305ce321 100644 --- a/spec/javascripts/boards/boards_store_spec.js.es6 +++ b/spec/javascripts/boards/boards_store_spec.js.es6 @@ -14,7 +14,7 @@ (() => { beforeEach(() => { - gl.boardService = new BoardService('/test/issue-boards/board'); + gl.boardService = new BoardService('/test/issue-boards/board', '1'); gl.issueBoards.BoardsStore.create(); $.cookie('issue_board_welcome_hidden', 'false'); diff --git a/spec/javascripts/boards/issue_spec.js.es6 b/spec/javascripts/boards/issue_spec.js.es6 index 3569d1b98bd..328c6f82ab5 100644 --- a/spec/javascripts/boards/issue_spec.js.es6 +++ b/spec/javascripts/boards/issue_spec.js.es6 @@ -16,7 +16,7 @@ describe('Issue model', () => { let issue; beforeEach(() => { - gl.boardService = new BoardService('/test/issue-boards/board'); + gl.boardService = new BoardService('/test/issue-boards/board', '1'); gl.issueBoards.BoardsStore.create(); issue = new ListIssue({ diff --git a/spec/javascripts/boards/list_spec.js.es6 b/spec/javascripts/boards/list_spec.js.es6 index 1688b996162..ec78d82e919 100644 --- a/spec/javascripts/boards/list_spec.js.es6 +++ b/spec/javascripts/boards/list_spec.js.es6 @@ -16,7 +16,7 @@ describe('List model', () => { let list; beforeEach(() => { - gl.boardService = new BoardService('/test/issue-boards/board'); + gl.boardService = new BoardService('/test/issue-boards/board', '1'); gl.issueBoards.BoardsStore.create(); list = new List(listObj); diff --git a/spec/javascripts/boards/mock_data.js.es6 b/spec/javascripts/boards/mock_data.js.es6 index f3797ed44d4..052455f2ca6 100644 --- a/spec/javascripts/boards/mock_data.js.es6 +++ b/spec/javascripts/boards/mock_data.js.es6 @@ -26,7 +26,7 @@ const listObjDuplicate = { const BoardsMockData = { 'GET': { - '/test/issue-boards/board/lists{/id}/issues': { + '/test/issue-boards/board/1/lists{/id}/issues': { issues: [{ title: 'Testing', iid: 1, @@ -37,13 +37,13 @@ const BoardsMockData = { } }, 'POST': { - '/test/issue-boards/board/lists{/id}': listObj + '/test/issue-boards/board/1/lists{/id}': listObj }, 'PUT': { - '/test/issue-boards/board/lists{/id}': {} + '/test/issue-boards/board/1/lists{/id}': {} }, 'DELETE': { - '/test/issue-boards/board/lists{/id}': {} + '/test/issue-boards/board/1/lists{/id}': {} } }; diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 006569254a6..5d5836e9bee 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -118,7 +118,7 @@ project: - creator - group - namespace -- board +- boards - last_event - services - campfire_service diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index dae546a0cdc..308a00db9cd 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -24,7 +24,7 @@ describe Project, models: true do it { is_expected.to have_one(:slack_service).dependent(:destroy) } it { is_expected.to have_one(:pushover_service).dependent(:destroy) } it { is_expected.to have_one(:asana_service).dependent(:destroy) } - it { is_expected.to have_one(:board).dependent(:destroy) } + it { is_expected.to have_many(:boards).dependent(:destroy) } it { is_expected.to have_one(:campfire_service).dependent(:destroy) } it { is_expected.to have_one(:drone_ci_service).dependent(:destroy) } it { is_expected.to have_one(:emails_on_push_service).dependent(:destroy) } @@ -94,6 +94,15 @@ describe Project, models: true do 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 end describe 'modules' do diff --git a/spec/services/boards/create_service_spec.rb b/spec/services/boards/create_service_spec.rb index a1a4dd4c57c..fde807cc410 100644 --- a/spec/services/boards/create_service_spec.rb +++ b/spec/services/boards/create_service_spec.rb @@ -2,33 +2,31 @@ require 'spec_helper' describe Boards::CreateService, services: true do describe '#execute' do + let(:project) { create(:empty_project) } + subject(:service) { described_class.new(project, double) } context 'when project does not have a board' do - let(:project) { create(:empty_project, board: nil) } - it 'creates a new board' do expect { service.execute }.to change(Board, :count).by(1) end it 'creates default lists' do - service.execute + board = service.execute - expect(project.board.lists.size).to eq 2 - expect(project.board.lists.first).to be_backlog - expect(project.board.lists.last).to be_done + expect(board.lists.size).to eq 2 + expect(board.lists.first).to be_backlog + expect(board.lists.last).to be_done end end context 'when project has a board' do - let!(:project) { create(:project_with_board) } - - it 'does not create a new board' do - expect { service.execute }.not_to change(Board, :count) + before do + create(:board, project: project) end - it 'does not create board lists' do - expect { service.execute }.not_to change(project.board.lists, :count) + it 'does not create a new board' do + expect { service.execute }.not_to change(project.boards, :count) end end end diff --git a/spec/services/boards/issues/create_service_spec.rb b/spec/services/boards/issues/create_service_spec.rb index 33e10e79f6d..360ee398f77 100644 --- a/spec/services/boards/issues/create_service_spec.rb +++ b/spec/services/boards/issues/create_service_spec.rb @@ -2,13 +2,13 @@ require 'spec_helper' describe Boards::Issues::CreateService, services: true do describe '#execute' do - let(:project) { create(:project_with_board) } - let(:board) { project.board } + let(:project) { create(:empty_project) } + let(:board) { create(:board, project: project) } let(:user) { create(:user) } let(:label) { create(:label, project: project, name: 'in-progress') } let!(:list) { create(:list, board: board, label: label, position: 0) } - subject(:service) { described_class.new(project, user, title: 'New issue') } + subject(:service) { described_class.new(project, user, board_id: board.id, list_id: list.id, title: 'New issue') } before do project.team << [user, :developer] @@ -17,15 +17,15 @@ describe Boards::Issues::CreateService, services: true do it 'delegates the create proceedings to Issues::CreateService' do expect_any_instance_of(Issues::CreateService).to receive(:execute).once - service.execute(list) + service.execute end it 'creates a new issue' do - expect { service.execute(list) }.to change(project.issues, :count).by(1) + expect { service.execute }.to change(project.issues, :count).by(1) end it 'adds the label of the list to the issue' do - issue = service.execute(list) + issue = service.execute expect(issue.labels).to eq [label] end diff --git a/spec/services/boards/issues/list_service_spec.rb b/spec/services/boards/issues/list_service_spec.rb index 5b9f454fd2d..7c206cf3ce7 100644 --- a/spec/services/boards/issues/list_service_spec.rb +++ b/spec/services/boards/issues/list_service_spec.rb @@ -3,8 +3,8 @@ require 'spec_helper' describe Boards::Issues::ListService, services: true do describe '#execute' do let(:user) { create(:user) } - let(:project) { create(:project_with_board) } - let(:board) { project.board } + let(:project) { create(:empty_project) } + let(:board) { create(:board, project: project) } let(:bug) { create(:label, project: project, name: 'Bug') } let(:development) { create(:label, project: project, name: 'Development') } @@ -13,10 +13,10 @@ describe Boards::Issues::ListService, services: true do let(:p2) { create(:label, title: 'P2', project: project, priority: 2) } let(:p3) { create(:label, title: 'P3', project: project, priority: 3) } - let!(:backlog) { project.board.backlog_list } + let!(:backlog) { create(:backlog_list, board: board) } let!(:list1) { create(:list, board: board, label: development, position: 0) } let!(:list2) { create(:list, board: board, label: testing, position: 1) } - let!(:done) { project.board.done_list } + let!(:done) { create(:done_list, board: board) } let!(:opened_issue1) { create(:labeled_issue, project: project, labels: [bug]) } let!(:opened_issue2) { create(:labeled_issue, project: project, labels: [p2]) } @@ -37,7 +37,7 @@ describe Boards::Issues::ListService, services: true do end it 'delegates search to IssuesFinder' do - params = { id: list1.id } + params = { board_id: board.id, id: list1.id } expect_any_instance_of(IssuesFinder).to receive(:execute).once.and_call_original @@ -46,7 +46,7 @@ describe Boards::Issues::ListService, services: true do context 'sets default order to priority' do it 'returns opened issues when listing issues from Backlog' do - params = { id: backlog.id } + params = { board_id: board.id, id: backlog.id } issues = described_class.new(project, user, params).execute @@ -54,7 +54,7 @@ describe Boards::Issues::ListService, services: true do end it 'returns closed issues when listing issues from Done' do - params = { id: done.id } + params = { board_id: board.id, id: done.id } issues = described_class.new(project, user, params).execute @@ -62,12 +62,29 @@ describe Boards::Issues::ListService, services: true do end it 'returns opened issues that have label list applied when listing issues from a label list' do - params = { id: list1.id } + params = { board_id: board.id, id: list1.id } issues = described_class.new(project, user, params).execute expect(issues).to eq [list1_issue3, list1_issue1, list1_issue2] end end + + context 'with list that does not belong to the board' do + it 'raises an error' do + list = create(:list) + service = described_class.new(project, user, board_id: board.id, id: list.id) + + expect { service.execute }.to raise_error(ActiveRecord::RecordNotFound) + end + end + + context 'with invalid list id' do + it 'raises an error' do + service = described_class.new(project, user, board_id: board.id, id: nil) + + expect { service.execute }.to raise_error(ActiveRecord::RecordNotFound) + end + end end end diff --git a/spec/services/boards/issues/move_service_spec.rb b/spec/services/boards/issues/move_service_spec.rb index 180f1b08631..c43b2aec490 100644 --- a/spec/services/boards/issues/move_service_spec.rb +++ b/spec/services/boards/issues/move_service_spec.rb @@ -3,17 +3,17 @@ require 'spec_helper' describe Boards::Issues::MoveService, services: true do describe '#execute' do let(:user) { create(:user) } - let(:project) { create(:project_with_board) } - let(:board) { project.board } + let(:project) { create(:empty_project) } + let(:board1) { create(:board, project: project) } let(:bug) { create(:label, project: project, name: 'Bug') } let(:development) { create(:label, project: project, name: 'Development') } let(:testing) { create(:label, project: project, name: 'Testing') } - let!(:backlog) { project.board.backlog_list } - let!(:list1) { create(:list, board: board, label: development, position: 0) } - let!(:list2) { create(:list, board: board, label: testing, position: 1) } - let!(:done) { project.board.done_list } + let!(:backlog) { create(:backlog_list, board: board1) } + let!(:list1) { create(:list, board: board1, label: development, position: 0) } + let!(:list2) { create(:list, board: board1, label: testing, position: 1) } + let!(:done) { create(:done_list, board: board1) } before do project.team << [user, :developer] @@ -22,7 +22,7 @@ describe Boards::Issues::MoveService, services: true do context 'when moving from backlog' do it 'adds the label of the list it goes to' do issue = create(:labeled_issue, project: project, labels: [bug]) - params = { from_list_id: backlog.id, to_list_id: list1.id } + params = { board_id: board1.id, from_list_id: backlog.id, to_list_id: list1.id } described_class.new(project, user, params).execute(issue) @@ -33,7 +33,7 @@ describe Boards::Issues::MoveService, services: true do context 'when moving to backlog' do it 'removes all list-labels' do issue = create(:labeled_issue, project: project, labels: [bug, development, testing]) - params = { from_list_id: list1.id, to_list_id: backlog.id } + params = { board_id: board1.id, from_list_id: list1.id, to_list_id: backlog.id } described_class.new(project, user, params).execute(issue) @@ -44,7 +44,7 @@ describe Boards::Issues::MoveService, services: true do context 'when moving from backlog to done' do it 'closes the issue' do issue = create(:labeled_issue, project: project, labels: [bug]) - params = { from_list_id: backlog.id, to_list_id: done.id } + params = { board_id: board1.id, from_list_id: backlog.id, to_list_id: done.id } described_class.new(project, user, params).execute(issue) issue.reload @@ -56,7 +56,7 @@ describe Boards::Issues::MoveService, services: true do context 'when moving an issue between lists' do let(:issue) { create(:labeled_issue, project: project, labels: [bug, development]) } - let(:params) { { from_list_id: list1.id, to_list_id: list2.id } } + let(:params) { { board_id: board1.id, from_list_id: list1.id, to_list_id: list2.id } } it 'delegates the label changes to Issues::UpdateService' do expect_any_instance_of(Issues::UpdateService).to receive(:execute).with(issue).once @@ -72,8 +72,12 @@ describe Boards::Issues::MoveService, services: true do end context 'when moving to done' do - let(:issue) { create(:labeled_issue, project: project, labels: [bug, development, testing]) } - let(:params) { { from_list_id: list2.id, to_list_id: done.id } } + let(:board2) { create(:board, project: project) } + let(:regression) { create(:label, project: project, name: 'Regression') } + let!(:list3) { create(:list, board: board2, label: regression, position: 1) } + + let(:issue) { create(:labeled_issue, project: project, labels: [bug, development, testing, regression]) } + let(:params) { { board_id: board1.id, from_list_id: list2.id, to_list_id: done.id } } it 'delegates the close proceedings to Issues::CloseService' do expect_any_instance_of(Issues::CloseService).to receive(:execute).with(issue).once @@ -81,7 +85,7 @@ describe Boards::Issues::MoveService, services: true do described_class.new(project, user, params).execute(issue) end - it 'removes all list-labels and close the issue' do + it 'removes all list-labels from project boards and close the issue' do described_class.new(project, user, params).execute(issue) issue.reload @@ -92,7 +96,7 @@ describe Boards::Issues::MoveService, services: true do context 'when moving from done' do let(:issue) { create(:labeled_issue, :closed, project: project, labels: [bug]) } - let(:params) { { from_list_id: done.id, to_list_id: list2.id } } + let(:params) { { board_id: board1.id, from_list_id: done.id, to_list_id: list2.id } } it 'delegates the re-open proceedings to Issues::ReopenService' do expect_any_instance_of(Issues::ReopenService).to receive(:execute).with(issue).once @@ -112,7 +116,7 @@ describe Boards::Issues::MoveService, services: true do context 'when moving from done to backlog' do it 'reopens the issue' do issue = create(:labeled_issue, :closed, project: project, labels: [bug]) - params = { from_list_id: done.id, to_list_id: backlog.id } + params = { board_id: board1.id, from_list_id: done.id, to_list_id: backlog.id } described_class.new(project, user, params).execute(issue) issue.reload @@ -124,7 +128,7 @@ describe Boards::Issues::MoveService, services: true do context 'when moving to same list' do let(:issue) { create(:labeled_issue, project: project, labels: [bug, development]) } - let(:params) { { from_list_id: list1.id, to_list_id: list1.id } } + let(:params) { { board_id: board1.id, from_list_id: list1.id, to_list_id: list1.id } } it 'returns false' do expect(described_class.new(project, user, params).execute(issue)).to eq false diff --git a/spec/services/boards/list_service_spec.rb b/spec/services/boards/list_service_spec.rb new file mode 100644 index 00000000000..dff33e4bcbb --- /dev/null +++ b/spec/services/boards/list_service_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +describe Boards::ListService, services: true do + describe '#execute' do + let(:project) { create(:empty_project) } + + subject(:service) { described_class.new(project, double) } + + context 'when project does not have a board' do + it 'creates a new project board' do + expect { service.execute }.to change(project.boards, :count).by(1) + end + + it 'delegates the project board creation to Boards::CreateService' do + expect_any_instance_of(Boards::CreateService).to receive(:execute).once + + service.execute + end + end + + context 'when project has a board' do + before do + create(:board, project: project) + end + + it 'does not create a new board' do + expect { service.execute }.not_to change(project.boards, :count) + end + end + + it 'returns project boards' do + board = create(:board, project: project) + + expect(service.execute).to match_array [board] + end + end +end diff --git a/spec/services/boards/lists/create_service_spec.rb b/spec/services/boards/lists/create_service_spec.rb index bff9c1fd1fe..e7806add916 100644 --- a/spec/services/boards/lists/create_service_spec.rb +++ b/spec/services/boards/lists/create_service_spec.rb @@ -2,8 +2,8 @@ require 'spec_helper' describe Boards::Lists::CreateService, services: true do describe '#execute' do - let(:project) { create(:project_with_board) } - let(:board) { project.board } + let(:project) { create(:empty_project) } + let(:board) { create(:board, project: project) } let(:user) { create(:user) } let(:label) { create(:label, project: project, name: 'in-progress') } @@ -11,7 +11,7 @@ describe Boards::Lists::CreateService, services: true do context 'when board lists is empty' do it 'creates a new list at beginning of the list' do - list = service.execute + list = service.execute(board) expect(list.position).to eq 0 end @@ -19,7 +19,7 @@ describe Boards::Lists::CreateService, services: true do context 'when board lists has backlog, and done lists' do it 'creates a new list at beginning of the list' do - list = service.execute + list = service.execute(board) expect(list.position).to eq 0 end @@ -30,7 +30,7 @@ describe Boards::Lists::CreateService, services: true do create(:list, board: board, position: 0) create(:list, board: board, position: 1) - list = service.execute + list = service.execute(board) expect(list.position).to eq 2 end @@ -40,7 +40,7 @@ describe Boards::Lists::CreateService, services: true do it 'creates a new list at end of the label lists' do list1 = create(:list, board: board, position: 0) - list2 = service.execute + list2 = service.execute(board) expect(list1.reload.position).to eq 0 expect(list2.reload.position).to eq 1 @@ -52,7 +52,7 @@ describe Boards::Lists::CreateService, services: true do label = create(:label, name: 'in-development') service = described_class.new(project, user, label_id: label.id) - expect { service.execute }.to raise_error(ActiveRecord::RecordNotFound) + expect { service.execute(board) }.to raise_error(ActiveRecord::RecordNotFound) end end end diff --git a/spec/services/boards/lists/destroy_service_spec.rb b/spec/services/boards/lists/destroy_service_spec.rb index 474c4512471..628caf03476 100644 --- a/spec/services/boards/lists/destroy_service_spec.rb +++ b/spec/services/boards/lists/destroy_service_spec.rb @@ -2,8 +2,8 @@ require 'spec_helper' describe Boards::Lists::DestroyService, services: true do describe '#execute' do - let(:project) { create(:project_with_board) } - let(:board) { project.board } + let(:project) { create(:empty_project) } + let(:board) { create(:board, project: project) } let(:user) { create(:user) } context 'when list type is label' do @@ -15,11 +15,11 @@ describe Boards::Lists::DestroyService, services: true do end it 'decrements position of higher lists' do - backlog = project.board.backlog_list + backlog = board.backlog_list development = create(:list, board: board, position: 0) review = create(:list, board: board, position: 1) staging = create(:list, board: board, position: 2) - done = project.board.done_list + done = board.done_list described_class.new(project, user).execute(development) @@ -31,14 +31,14 @@ describe Boards::Lists::DestroyService, services: true do end it 'does not remove list from board when list type is backlog' do - list = project.board.backlog_list + list = board.backlog_list service = described_class.new(project, user) expect { service.execute(list) }.not_to change(board.lists, :count) end it 'does not remove list from board when list type is done' do - list = project.board.done_list + list = board.done_list service = described_class.new(project, user) expect { service.execute(list) }.not_to change(board.lists, :count) diff --git a/spec/services/boards/lists/generate_service_spec.rb b/spec/services/boards/lists/generate_service_spec.rb index 4171e4d816c..8b2f5e81338 100644 --- a/spec/services/boards/lists/generate_service_spec.rb +++ b/spec/services/boards/lists/generate_service_spec.rb @@ -2,15 +2,15 @@ require 'spec_helper' describe Boards::Lists::GenerateService, services: true do describe '#execute' do - let(:project) { create(:project_with_board) } - let(:board) { project.board } + let(:project) { create(:empty_project) } + let(:board) { create(:board, project: project) } let(:user) { create(:user) } subject(:service) { described_class.new(project, user) } context 'when board lists is empty' do it 'creates the default lists' do - expect { service.execute }.to change(board.lists, :count).by(2) + expect { service.execute(board) }.to change(board.lists, :count).by(2) end end @@ -18,13 +18,13 @@ describe Boards::Lists::GenerateService, services: true do it 'does not creates the default lists' do create(:list, board: board) - expect { service.execute }.not_to change(board.lists, :count) + expect { service.execute(board) }.not_to change(board.lists, :count) end end context 'when project labels does not contains any list label' do it 'creates labels' do - expect { service.execute }.to change(project.labels, :count).by(2) + expect { service.execute(board) }.to change(project.labels, :count).by(2) end end @@ -32,7 +32,7 @@ describe Boards::Lists::GenerateService, services: true do it 'creates the missing labels' do create(:label, project: project, name: 'Doing') - expect { service.execute }.to change(project.labels, :count).by(1) + expect { service.execute(board) }.to change(project.labels, :count).by(1) end end end diff --git a/spec/services/boards/lists/list_service_spec.rb b/spec/services/boards/lists/list_service_spec.rb new file mode 100644 index 00000000000..334cee3f06d --- /dev/null +++ b/spec/services/boards/lists/list_service_spec.rb @@ -0,0 +1,16 @@ +require 'spec_helper' + +describe Boards::Lists::ListService, services: true do + describe '#execute' do + it "returns board's lists" do + project = create(:empty_project) + board = create(:board, project: project) + label = create(:label, project: project) + list = create(:list, board: board, label: label) + + service = described_class.new(project, double) + + expect(service.execute(board)).to eq [board.backlog_list, list, board.done_list] + end + end +end diff --git a/spec/services/boards/lists/move_service_spec.rb b/spec/services/boards/lists/move_service_spec.rb index 102ed67449d..63fa0bb8c5f 100644 --- a/spec/services/boards/lists/move_service_spec.rb +++ b/spec/services/boards/lists/move_service_spec.rb @@ -2,16 +2,16 @@ require 'spec_helper' describe Boards::Lists::MoveService, services: true do describe '#execute' do - let(:project) { create(:project_with_board) } - let(:board) { project.board } + let(:project) { create(:empty_project) } + let(:board) { create(:board, project: project) } let(:user) { create(:user) } - let!(:backlog) { project.board.backlog_list } + let!(:backlog) { create(:backlog_list, board: board) } let!(:planning) { create(:list, board: board, position: 0) } let!(:development) { create(:list, board: board, position: 1) } let!(:review) { create(:list, board: board, position: 2) } let!(:staging) { create(:list, board: board, position: 3) } - let!(:done) { project.board.done_list } + let!(:done) { create(:done_list, board: board) } context 'when list type is set to label' do it 'keeps position of lists when new position is nil' do |
