Reuse endpoint to list issues for a list instead of create a new one

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 }