diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2016-10-13 17:21:14 +0300 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2016-10-13 17:21:14 +0300 |
commit | 11e93ad59d6209bf9805a6f7053719e8052cc508 (patch) | |
tree | 0770038bcfb327e45381e8f45ba6629c02a09f12 /spec/services | |
parent | 36dafbbb02201ad36ca7e59f699cbf727494129e (diff) | |
parent | 25c82c6faf067a82e99bad590357ad57618475d9 (diff) |
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/services')
-rw-r--r-- | spec/services/boards/create_service_spec.rb | 22 | ||||
-rw-r--r-- | spec/services/boards/issues/create_service_spec.rb | 12 | ||||
-rw-r--r-- | spec/services/boards/issues/list_service_spec.rb | 33 | ||||
-rw-r--r-- | spec/services/boards/issues/move_service_spec.rb | 36 | ||||
-rw-r--r-- | spec/services/boards/list_service_spec.rb | 37 | ||||
-rw-r--r-- | spec/services/boards/lists/create_service_spec.rb | 14 | ||||
-rw-r--r-- | spec/services/boards/lists/destroy_service_spec.rb | 12 | ||||
-rw-r--r-- | spec/services/boards/lists/generate_service_spec.rb | 12 | ||||
-rw-r--r-- | spec/services/boards/lists/list_service_spec.rb | 16 | ||||
-rw-r--r-- | spec/services/boards/lists/move_service_spec.rb | 8 |
10 files changed, 137 insertions, 65 deletions
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 |