From 274987d5c0f7b04a9b7510c318c8df6dfab477df Mon Sep 17 00:00:00 2001
From: Douglas Barbosa Alexandre <dbalexandre@gmail.com>
Date: Fri, 27 Jan 2017 16:21:11 -0200
Subject: [PATCH] Reuse endpoint to list issues for a list instead of create a
new one
---
.../boards/components/modal/index.js.es6 | 2 +-
.../boards/services/board_service.js.es6 | 6 +-
.../projects/boards/issues_controller.rb | 2 +-
app/controllers/projects/boards_controller.rb | 23 +-----
app/services/boards/issues/list_service.rb | 14 +++-
config/routes/project.rb | 4 +-
.../projects/boards/issues_controller_spec.rb | 76 ++++++++++++-------
.../boards/issues/list_service_spec.rb | 12 +++
8 files changed, 79 insertions(+), 60 deletions(-)
diff --git a/app/assets/javascripts/boards/components/modal/index.js.es6 b/app/assets/javascripts/boards/components/modal/index.js.es6
index eedfd826787..ababe3af0e3 100644
--- a/app/assets/javascripts/boards/components/modal/index.js.es6
+++ b/app/assets/javascripts/boards/components/modal/index.js.es6
@@ -48,7 +48,7 @@
}).then((res) => {
const data = res.json();
- data.forEach((issueObj) => {
+ data.issues.forEach((issueObj) => {
const issue = new ListIssue(issueObj);
const foundSelectedIssue = ModalStore.findSelectedIssue(issue);
issue.selected = foundSelectedIssue !== undefined;
diff --git a/app/assets/javascripts/boards/services/board_service.js.es6 b/app/assets/javascripts/boards/services/board_service.js.es6
index 20df8f2b373..993f8599fd4 100644
--- a/app/assets/javascripts/boards/services/board_service.js.es6
+++ b/app/assets/javascripts/boards/services/board_service.js.es6
@@ -4,9 +4,9 @@
class BoardService {
constructor (root, boardId) {
this.boards = Vue.resource(`${root}{/id}.json`, {}, {
- backlog: {
+ issues: {
method: 'GET',
- url: `${root}/${boardId}/backlog.json`
+ url: `${root}/${boardId}/issues.json`
}
});
this.lists = Vue.resource(`${root}/${boardId}/lists{/id}`, {}, {
@@ -73,7 +73,7 @@ class BoardService {
}
getBacklog(data) {
- return this.boards.backlog(data);
+ return this.boards.issues(data);
}
}
diff --git a/app/controllers/projects/boards/issues_controller.rb b/app/controllers/projects/boards/issues_controller.rb
index dc33e1405f2..a061b575e21 100644
--- a/app/controllers/projects/boards/issues_controller.rb
+++ b/app/controllers/projects/boards/issues_controller.rb
@@ -59,7 +59,7 @@ module Projects
end
def filter_params
- params.merge(board_id: params[:board_id], id: params[:list_id])
+ params.merge(board_id: params[:board_id], id: params[:list_id]).compact
end
def move_params
diff --git a/app/controllers/projects/boards_controller.rb b/app/controllers/projects/boards_controller.rb
index 1eaa2ffdf29..808affa4f98 100644
--- a/app/controllers/projects/boards_controller.rb
+++ b/app/controllers/projects/boards_controller.rb
@@ -1,7 +1,7 @@
class Projects::BoardsController < Projects::ApplicationController
include IssuableCollections
- # before_action :authorize_read_board!, only: [:index, :show, :backlog]
+ before_action :authorize_read_board!, only: [:index, :show]
def index
@boards = ::Boards::ListService.new(project, current_user).execute
@@ -25,27 +25,6 @@ class Projects::BoardsController < Projects::ApplicationController
end
end
- def backlog
- board = project.boards.find(params[:id])
-
- @issues = issues_collection
- @issues = @issues.where.not(
- LabelLink.where("label_links.target_type = 'Issue' AND label_links.target_id = issues.id")
- .where(label_id: board.lists.movable.pluck(:label_id)).limit(1).arel.exists
- )
- @issues = @issues.page(params[:page]).per(params[:per])
-
- render json: @issues.as_json(
- labels: true,
- only: [:id, :iid, :title, :confidential, :due_date],
- include: {
- assignee: { only: [:id, :name, :username], methods: [:avatar_url] },
- milestone: { only: [:id, :title] }
- },
- user: current_user
- )
- end
-
private
def authorize_read_board!
diff --git a/app/services/boards/issues/list_service.rb b/app/services/boards/issues/list_service.rb
index fd4a462c7b2..8a94c54b6ab 100644
--- a/app/services/boards/issues/list_service.rb
+++ b/app/services/boards/issues/list_service.rb
@@ -3,8 +3,8 @@ module Boards
class ListService < BaseService
def execute
issues = IssuesFinder.new(current_user, filter_params).execute
- issues = without_board_labels(issues) unless list.movable?
- issues = with_list_label(issues) if list.movable?
+ issues = without_board_labels(issues) unless movable_list?
+ issues = with_list_label(issues) if movable_list?
issues
end
@@ -15,7 +15,13 @@ module Boards
end
def list
- @list ||= board.lists.find(params[:id])
+ return @list if defined?(@list)
+
+ @list = board.lists.find(params[:id]) if params.key?(:id)
+ end
+
+ def movable_list?
+ @movable_list ||= list.present? && list.movable?
end
def filter_params
@@ -40,7 +46,7 @@ module Boards
end
def set_state
- params[:state] = list.done? ? 'closed' : 'opened'
+ params[:state] = list && list.done? ? 'closed' : 'opened'
end
def board_label_ids
diff --git a/config/routes/project.rb b/config/routes/project.rb
index 46d6530333d..7cd4a73b1a0 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -266,10 +266,8 @@ constraints(ProjectUrlConstrainer.new) do
end
resources :boards, only: [:index, :show] do
- get :backlog, on: :member
-
scope module: :boards do
- resources :issues, only: [:update]
+ resources :issues, only: [:index, :update]
resources :lists, only: [:index, :create, :update, :destroy] do
collection do
diff --git a/spec/controllers/projects/boards/issues_controller_spec.rb b/spec/controllers/projects/boards/issues_controller_spec.rb
index 299d2c981d3..ad15e3942a5 100644
--- a/spec/controllers/projects/boards/issues_controller_spec.rb
+++ b/spec/controllers/projects/boards/issues_controller_spec.rb
@@ -18,23 +18,7 @@ describe Projects::Boards::IssuesController do
end
describe 'GET index' do
- context 'with valid list id' do
- it 'returns issues that have the list label applied' do
- johndoe = create(:user, avatar: fixture_file_upload(File.join(Rails.root, 'spec/fixtures/dk.png')))
- issue = create(:labeled_issue, project: project, labels: [planning])
- create(:labeled_issue, project: project, labels: [planning])
- create(:labeled_issue, project: project, labels: [development], due_date: Date.tomorrow)
- create(:labeled_issue, project: project, labels: [development], assignee: johndoe)
- issue.subscribe(johndoe, project)
-
- list_issues user: user, board: board, list: list2
-
- parsed_response = JSON.parse(response.body)
-
- expect(response).to match_response_schema('issues')
- expect(parsed_response.length).to eq 2
- end
- end
+ let(:johndoe) { create(:user, avatar: fixture_file_upload(File.join(Rails.root, 'spec/fixtures/dk.png'))) }
context 'with invalid board id' do
it 'returns a not found 404 response' do
@@ -44,11 +28,47 @@ describe Projects::Boards::IssuesController do
end
end
- context 'with invalid list id' do
- it 'returns a not found 404 response' do
- list_issues user: user, board: board, list: 999
+ context 'when list id is present' do
+ context 'with valid list id' do
+ it 'returns issues that have the list label applied' do
+ issue = create(:labeled_issue, project: project, labels: [planning])
+ create(:labeled_issue, project: project, labels: [planning])
+ create(:labeled_issue, project: project, labels: [development], due_date: Date.tomorrow)
+ create(:labeled_issue, project: project, labels: [development], assignee: johndoe)
+ issue.subscribe(johndoe, project)
- expect(response).to have_http_status(404)
+ list_issues user: user, board: board, list: list2
+
+ parsed_response = JSON.parse(response.body)
+
+ expect(response).to match_response_schema('issues')
+ expect(parsed_response.length).to eq 2
+ end
+ end
+
+ context 'with invalid list id' do
+ it 'returns a not found 404 response' do
+ list_issues user: user, board: board, list: 999
+
+ expect(response).to have_http_status(404)
+ end
+ end
+ end
+
+ context 'when list id is missing' do
+ it 'returns opened issues without board labels applied' do
+ bug = create(:label, project: project, name: 'Bug')
+ create(:issue, project: project)
+ create(:labeled_issue, project: project, labels: [planning])
+ create(:labeled_issue, project: project, labels: [development])
+ create(:labeled_issue, project: project, labels: [bug])
+
+ list_issues user: user, board: board
+
+ parsed_response = JSON.parse(response.body)
+
+ expect(response).to match_response_schema('issues')
+ expect(parsed_response.length).to eq 2
end
end
@@ -65,13 +85,17 @@ describe Projects::Boards::IssuesController do
end
end
- def list_issues(user:, board:, list:)
+ def list_issues(user:, board:, list: nil)
sign_in(user)
- get :index, namespace_id: project.namespace.to_param,
- project_id: project.to_param,
- board_id: board.to_param,
- list_id: list.to_param
+ params = {
+ namespace_id: project.namespace.to_param,
+ project_id: project.to_param,
+ board_id: board.to_param,
+ list_id: list.try(:to_param)
+ }
+
+ get :index, params.compact
end
end
diff --git a/spec/services/boards/issues/list_service_spec.rb b/spec/services/boards/issues/list_service_spec.rb
index 5d53c54254c..305278843f5 100644
--- a/spec/services/boards/issues/list_service_spec.rb
+++ b/spec/services/boards/issues/list_service_spec.rb
@@ -17,6 +17,10 @@ describe Boards::Issues::ListService, services: true do
let!(:list2) { create(:list, board: board, label: testing, position: 1) }
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]) }
+ let!(:reopened_issue1) { create(:issue, :reopened, project: project) }
+
let!(:list1_issue1) { create(:labeled_issue, project: project, labels: [p2, development]) }
let!(:list1_issue2) { create(:labeled_issue, project: project, labels: [development]) }
let!(:list1_issue3) { create(:labeled_issue, project: project, labels: [development, p1]) }
@@ -40,6 +44,14 @@ describe Boards::Issues::ListService, services: true do
end
context 'sets default order to priority' do
+ it 'returns opened issues when list id is missing' do
+ params = { board_id: board.id }
+
+ issues = described_class.new(project, user, params).execute
+
+ expect(issues).to eq [opened_issue2, reopened_issue1, opened_issue1]
+ end
+
it 'returns closed issues when listing issues from Done' do
params = { board_id: board.id, id: done.id }
--
GitLab