From 274987d5c0f7b04a9b7510c318c8df6dfab477df Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Fri, 27 Jan 2017 16:21:11 -0200 Subject: 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 ++++++++++++++-------- spec/services/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 } -- cgit v1.2.1