From 4241c2906c9531ab7ddb43740b222a102f5508fa Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 3 Oct 2016 13:38:12 +0100 Subject: [PATCH 01/16] Add new issue form to lists Closes #21219 --- .../boards/components/board.js.es6 | 8 ++++- .../boards/components/board_list.js.es6 | 7 +++-- .../boards/components/board_new_issue.js.es6 | 24 +++++++++++++++ app/assets/stylesheets/pages/boards.scss | 26 +++++++++++++++++ .../boards/components/_board.html.haml | 29 +++++++++++++++++-- 5 files changed, 89 insertions(+), 5 deletions(-) create mode 100644 app/assets/javascripts/boards/components/board_new_issue.js.es6 diff --git a/app/assets/javascripts/boards/components/board.js.es6 b/app/assets/javascripts/boards/components/board.js.es6 index 7e86f001f44..cacb36a897f 100644 --- a/app/assets/javascripts/boards/components/board.js.es6 +++ b/app/assets/javascripts/boards/components/board.js.es6 @@ -21,7 +21,8 @@ }, data () { return { - filters: Store.state.filters + filters: Store.state.filters, + showIssueForm: false }; }, watch: { @@ -33,6 +34,11 @@ deep: true } }, + methods: { + showNewIssueForm() { + this.showIssueForm = !this.showIssueForm; + } + }, ready () { const options = gl.issueBoards.getBoardSortableDefaultOptions({ disabled: this.disabled, diff --git a/app/assets/javascripts/boards/components/board_list.js.es6 b/app/assets/javascripts/boards/components/board_list.js.es6 index 474805c1437..d2e905ae062 100644 --- a/app/assets/javascripts/boards/components/board_list.js.es6 +++ b/app/assets/javascripts/boards/components/board_list.js.es6 @@ -1,4 +1,5 @@ //= require ./board_card +//= require ./board_new_issue (() => { const Store = gl.issueBoards.BoardsStore; @@ -8,14 +9,16 @@ gl.issueBoards.BoardList = Vue.extend({ components: { - 'board-card': gl.issueBoards.BoardCard + 'board-card': gl.issueBoards.BoardCard, + 'board-new-issue': gl.issueBoards.BoardNewIssue }, props: { disabled: Boolean, list: Object, issues: Array, loading: Boolean, - issueLinkBase: String + issueLinkBase: String, + showIssueForm: Boolean }, data () { return { diff --git a/app/assets/javascripts/boards/components/board_new_issue.js.es6 b/app/assets/javascripts/boards/components/board_new_issue.js.es6 new file mode 100644 index 00000000000..057844f3ebb --- /dev/null +++ b/app/assets/javascripts/boards/components/board_new_issue.js.es6 @@ -0,0 +1,24 @@ +(() => { + window.gl = window.gl || {}; + + gl.issueBoards.BoardNewIssue = Vue.extend({ + props: { + showIssueForm: Boolean + }, + data() { + return { + title: '' + }; + }, + methods: { + submit(e) { + e.preventDefault(); + + this.title = ''; + }, + cancel() { + this.showIssueForm = false; + } + } + }); +})(); diff --git a/app/assets/stylesheets/pages/boards.scss b/app/assets/stylesheets/pages/boards.scss index ecc5b24e360..46e92b8a187 100644 --- a/app/assets/stylesheets/pages/boards.scss +++ b/app/assets/stylesheets/pages/boards.scss @@ -233,3 +233,29 @@ lex margin-right: 5px; } } + +.board-issue-count-holder { + margin-top: -3px; + + .btn { + line-height: 12px; + border-top-left-radius: 0; + border-bottom-left-radius: 0; + } +} + +.board-issue-count { + padding-right: 10px; + padding-left: 10px; + line-height: 21px; + border-radius: $border-radius-base; + border-color: $border-color; + border-style: solid; + border-width: 1px 1px 1px 1px; + + &.has-btn { + border-top-right-radius: 0; + border-bottom-right-radius: 0; + border-width: 1px 0 1px 1px; + } +} diff --git a/app/views/projects/boards/components/_board.html.haml b/app/views/projects/boards/components/_board.html.haml index 73066150fb3..6c95812aebd 100644 --- a/app/views/projects/boards/components/_board.html.haml +++ b/app/views/projects/boards/components/_board.html.haml @@ -12,8 +12,15 @@ %header.board-header{ ":class" => "{ 'has-border': list.label }", ":style" => "{ borderTopColor: (list.label ? list.label.color : null) }" } %h3.board-title.js-board-handle{ ":class" => "{ 'user-can-drag': (!disabled && !list.preset) }" } {{ list.title }} - %span.pull-right{ "v-if" => "list.type !== 'blank'" } - {{ list.issuesSize }} + .board-issue-count-holder.pull-right.clearfix + %span.board-issue-count.pull-left{ "v-if" => "list.type !== 'blank'", + ":class" => "{ 'has-btn': list.type !== 'done' }" } + {{ list.issuesSize }} + - if can? current_user, :create_issue, @project + %button.btn.btn-small.btn-default.pull-right{ type: "button", + "@click" => "showNewIssueForm", + "v-if" => "list.type !== 'done'" } + = icon("plus") - if can?(current_user, :admin_list, @project) %board-delete{ "inline-template" => true, ":list" => "list", @@ -26,12 +33,30 @@ ":issues" => "list.issues", ":loading" => "list.loading", ":disabled" => "disabled", + ":show-issue-form.sync" => "showIssueForm", ":issue-link-base" => "issueLinkBase" } .board-list-loading.text-center{ "v-if" => "loading" } = icon("spinner spin") %ul.board-list{ "v-el:list" => true, "v-show" => "!loading", ":data-board" => "list.id" } + - if can? current_user, :create_issue, @project + %board-new-issue{ "inline-template" => true, + ":show-issue-form.sync" => "showIssueForm", + "v-if" => "list.type !== 'done' && showIssueForm" } + %li.card + %form{ "@submit" => "submit($event)" } + %label.label-light + Title + %input.form-control{ type: "text", + "v-model" => "title" } + .clearfix.prepend-top-10 + %button.btn.btn-success.pull-left{ type: "submit", + ":disabled" => "title === ''" } + Submit issue + %button.btn.btn-default.pull-right{ type: "button", + "@click" => "cancel" } + Cancel = render "projects/boards/components/card" %li.board-list-count.text-center{ "v-if" => "showCount" } = icon("spinner spin", "v-show" => "list.loadingMore" ) From 4d9f76c15115d3fd48d61e998edca86917fb1ccf Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 3 Oct 2016 14:29:21 +0100 Subject: [PATCH 02/16] Added ability to save the new issue --- .../boards/components/board_list.js.es6 | 2 +- .../boards/components/board_new_issue.js.es6 | 15 ++++++++++++++- .../boards/mixins/sortable_default_options.js.es6 | 2 +- app/assets/javascripts/boards/models/list.js.es6 | 11 +++++++++++ .../boards/services/board_service.js.es6 | 8 ++++++++ .../projects/boards/issues_controller.rb | 13 +++++++++++++ .../projects/boards/components/_board.html.haml | 8 +++++--- config/routes/project.rb | 2 +- 8 files changed, 54 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/boards/components/board_list.js.es6 b/app/assets/javascripts/boards/components/board_list.js.es6 index d2e905ae062..208aac504fd 100644 --- a/app/assets/javascripts/boards/components/board_list.js.es6 +++ b/app/assets/javascripts/boards/components/board_list.js.es6 @@ -76,7 +76,7 @@ group: 'issues', sort: false, disabled: this.disabled, - filter: '.board-list-count', + filter: '.board-list-count, .board-new-issue-form', onStart: (e) => { const card = this.$refs.issue[e.oldIndex]; diff --git a/app/assets/javascripts/boards/components/board_new_issue.js.es6 b/app/assets/javascripts/boards/components/board_new_issue.js.es6 index 057844f3ebb..60c13afefeb 100644 --- a/app/assets/javascripts/boards/components/board_new_issue.js.es6 +++ b/app/assets/javascripts/boards/components/board_new_issue.js.es6 @@ -3,6 +3,7 @@ gl.issueBoards.BoardNewIssue = Vue.extend({ props: { + list: Object, showIssueForm: Boolean }, data() { @@ -10,14 +11,26 @@ title: '' }; }, + watch: { + showIssueForm () { + this.$els.input.focus(); + } + }, methods: { submit(e) { e.preventDefault(); + const issue = new ListIssue({ + title: this.title, + labels: [this.list.label] + }); - this.title = ''; + this.list.newIssue(issue); + + this.cancel(); }, cancel() { this.showIssueForm = false; + this.title = ''; } } }); diff --git a/app/assets/javascripts/boards/mixins/sortable_default_options.js.es6 b/app/assets/javascripts/boards/mixins/sortable_default_options.js.es6 index 44addb3ea98..f629d45c587 100644 --- a/app/assets/javascripts/boards/mixins/sortable_default_options.js.es6 +++ b/app/assets/javascripts/boards/mixins/sortable_default_options.js.es6 @@ -21,7 +21,7 @@ fallbackClass: 'is-dragging', fallbackOnBody: true, ghostClass: 'is-ghost', - filter: '.has-tooltip', + filter: '.has-tooltip, .btn', delay: gl.issueBoards.touchEnabled ? 100 : 0, scrollSensitivity: gl.issueBoards.touchEnabled ? 60 : 100, scrollSpeed: 20, diff --git a/app/assets/javascripts/boards/models/list.js.es6 b/app/assets/javascripts/boards/models/list.js.es6 index 91fd620fdb3..14e42db4cd2 100644 --- a/app/assets/javascripts/boards/models/list.js.es6 +++ b/app/assets/javascripts/boards/models/list.js.es6 @@ -87,6 +87,17 @@ class List { }); } + newIssue (issue) { + this.addIssue(issue); + this.issuesSize++; + + gl.boardService.newIssue(this.id, issue) + .then((resp) => { + const data = resp.json(); + issue.id = data.iid; + }); + } + createIssues (data) { data.forEach((issueObj) => { this.addIssue(new ListIssue(issueObj)); diff --git a/app/assets/javascripts/boards/services/board_service.js.es6 b/app/assets/javascripts/boards/services/board_service.js.es6 index 9b80fb2e99f..c8b1d4f2b5a 100644 --- a/app/assets/javascripts/boards/services/board_service.js.es6 +++ b/app/assets/javascripts/boards/services/board_service.js.es6 @@ -58,4 +58,12 @@ class BoardService { to_list_id }); } + + newIssue (id, issue) { + return this.issues.save({ id }, { + issue: { + title: issue.title + } + }); + } }; diff --git a/app/controllers/projects/boards/issues_controller.rb b/app/controllers/projects/boards/issues_controller.rb index 4aa7982eab4..41794193784 100644 --- a/app/controllers/projects/boards/issues_controller.rb +++ b/app/controllers/projects/boards/issues_controller.rb @@ -19,6 +19,15 @@ module Projects } end + def create + list = project.board.lists.find(params[:list_id]) + + issue = Issues::CreateService.new(project, current_user, issue_params.merge(request: request)).execute + issue.labels << list.label + + render json: issue.to_json + end + def update service = ::Boards::Issues::MoveService.new(project, current_user, move_params) @@ -54,6 +63,10 @@ module Projects def move_params params.permit(:id, :from_list_id, :to_list_id) end + + def issue_params + params.require(:issue).permit(:title) + end end end end diff --git a/app/views/projects/boards/components/_board.html.haml b/app/views/projects/boards/components/_board.html.haml index 6c95812aebd..bd60e56a340 100644 --- a/app/views/projects/boards/components/_board.html.haml +++ b/app/views/projects/boards/components/_board.html.haml @@ -42,14 +42,16 @@ ":data-board" => "list.id" } - if can? current_user, :create_issue, @project %board-new-issue{ "inline-template" => true, + ":list" => "list", ":show-issue-form.sync" => "showIssueForm", - "v-if" => "list.type !== 'done' && showIssueForm" } - %li.card + "v-show" => "list.type !== 'done' && showIssueForm" } + %li.card.board-new-issue-form %form{ "@submit" => "submit($event)" } %label.label-light Title %input.form-control{ type: "text", - "v-model" => "title" } + "v-model" => "title", + "v-el:input" => true } .clearfix.prepend-top-10 %button.btn.btn-success.pull-left{ type: "submit", ":disabled" => "title === ''" } diff --git a/config/routes/project.rb b/config/routes/project.rb index 224ec7e8324..14d986c9475 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -424,7 +424,7 @@ resources :namespaces, path: '/', constraints: { id: /[a-zA-Z.0-9_\-]+/ }, only: post :generate end - resources :issues, only: [:index] + resources :issues, only: [:index, :create] end end end From 674844d0e98e3e9a730f3e67f17d688ec769f0e5 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Mon, 3 Oct 2016 14:54:42 +0100 Subject: [PATCH 03/16] Fixed issue with backlog list not saving Fixed issue when dragging cards --- app/assets/javascripts/boards/components/board_list.js.es6 | 4 ++-- .../javascripts/boards/components/board_new_issue.js.es6 | 3 ++- app/controllers/projects/boards/issues_controller.rb | 2 +- app/views/projects/boards/components/_card.html.haml | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/boards/components/board_list.js.es6 b/app/assets/javascripts/boards/components/board_list.js.es6 index 208aac504fd..bc74a7b23c9 100644 --- a/app/assets/javascripts/boards/components/board_list.js.es6 +++ b/app/assets/javascripts/boards/components/board_list.js.es6 @@ -78,7 +78,7 @@ disabled: this.disabled, filter: '.board-list-count, .board-new-issue-form', onStart: (e) => { - const card = this.$refs.issue[e.oldIndex]; + const card = this.$refs.issue[e.oldIndex - 1]; Store.moving.issue = card.issue; Store.moving.list = card.list; @@ -89,7 +89,7 @@ gl.issueBoards.BoardsStore.moveIssueToList(Store.moving.list, this.list, Store.moving.issue); }, onRemove: (e) => { - this.$refs.issue[e.oldIndex].$destroy(true); + this.$refs.issue[e.oldIndex - 1].$destroy(true); } }); diff --git a/app/assets/javascripts/boards/components/board_new_issue.js.es6 b/app/assets/javascripts/boards/components/board_new_issue.js.es6 index 60c13afefeb..eb26ed9721d 100644 --- a/app/assets/javascripts/boards/components/board_new_issue.js.es6 +++ b/app/assets/javascripts/boards/components/board_new_issue.js.es6 @@ -19,9 +19,10 @@ methods: { submit(e) { e.preventDefault(); + const labels = this.list.label ? [this.list.label] : []; const issue = new ListIssue({ title: this.title, - labels: [this.list.label] + labels }); this.list.newIssue(issue); diff --git a/app/controllers/projects/boards/issues_controller.rb b/app/controllers/projects/boards/issues_controller.rb index 41794193784..3b1b236a89a 100644 --- a/app/controllers/projects/boards/issues_controller.rb +++ b/app/controllers/projects/boards/issues_controller.rb @@ -23,7 +23,7 @@ module Projects list = project.board.lists.find(params[:list_id]) issue = Issues::CreateService.new(project, current_user, issue_params.merge(request: request)).execute - issue.labels << list.label + issue.labels << list.label if list.label render json: issue.to_json end diff --git a/app/views/projects/boards/components/_card.html.haml b/app/views/projects/boards/components/_card.html.haml index e8b60b54d80..25701b0a99e 100644 --- a/app/views/projects/boards/components/_card.html.haml +++ b/app/views/projects/boards/components/_card.html.haml @@ -15,7 +15,7 @@ ":title" => "issue.title" } {{ issue.title }} .card-footer - %span.card-number + %span.card-number{ "v-if" => "issue.id" } = precede '#' do {{ issue.id }} %button.label.color-label.has-tooltip{ "v-for" => "label in issue.labels", From a9c98be056d1c493670e846771b08cf8985bf7a6 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 4 Oct 2016 08:04:03 +0100 Subject: [PATCH 04/16] Moved form outside of list This was causing issues with moving from the done list --- .../boards/components/board_list.js.es6 | 4 +- app/assets/stylesheets/pages/boards.scss | 10 ++++- .../boards/components/_board.html.haml | 41 ++++++++++--------- 3 files changed, 32 insertions(+), 23 deletions(-) diff --git a/app/assets/javascripts/boards/components/board_list.js.es6 b/app/assets/javascripts/boards/components/board_list.js.es6 index bc74a7b23c9..208aac504fd 100644 --- a/app/assets/javascripts/boards/components/board_list.js.es6 +++ b/app/assets/javascripts/boards/components/board_list.js.es6 @@ -78,7 +78,7 @@ disabled: this.disabled, filter: '.board-list-count, .board-new-issue-form', onStart: (e) => { - const card = this.$refs.issue[e.oldIndex - 1]; + const card = this.$refs.issue[e.oldIndex]; Store.moving.issue = card.issue; Store.moving.list = card.list; @@ -89,7 +89,7 @@ gl.issueBoards.BoardsStore.moveIssueToList(Store.moving.list, this.list, Store.moving.issue); }, onRemove: (e) => { - this.$refs.issue[e.oldIndex - 1].$destroy(true); + this.$refs.issue[e.oldIndex].$destroy(true); } }); diff --git a/app/assets/stylesheets/pages/boards.scss b/app/assets/stylesheets/pages/boards.scss index 46e92b8a187..6e6cb521dbc 100644 --- a/app/assets/stylesheets/pages/boards.scss +++ b/app/assets/stylesheets/pages/boards.scss @@ -162,6 +162,10 @@ lex list-style: none; overflow-y: scroll; overflow-x: hidden; + + &.is-smaller { + height: calc(100% - 185px); + } } .board-list-loading { @@ -234,6 +238,10 @@ lex } } +.board-new-issue-form { + margin: 5px; +} + .board-issue-count-holder { margin-top: -3px; @@ -251,7 +259,7 @@ lex border-radius: $border-radius-base; border-color: $border-color; border-style: solid; - border-width: 1px 1px 1px 1px; + border-width: 1px; &.has-btn { border-top-right-radius: 0; diff --git a/app/views/projects/boards/components/_board.html.haml b/app/views/projects/boards/components/_board.html.haml index bd60e56a340..efec465bbb2 100644 --- a/app/views/projects/boards/components/_board.html.haml +++ b/app/views/projects/boards/components/_board.html.haml @@ -37,28 +37,29 @@ ":issue-link-base" => "issueLinkBase" } .board-list-loading.text-center{ "v-if" => "loading" } = icon("spinner spin") + - if can? current_user, :create_issue, @project + %board-new-issue{ "inline-template" => true, + ":list" => "list", + ":show-issue-form.sync" => "showIssueForm", + "v-show" => "list.type !== 'done' && showIssueForm" } + .card.board-new-issue-form + %form{ "@submit" => "submit($event)" } + %label.label-light + Title + %input.form-control{ type: "text", + "v-model" => "title", + "v-el:input" => true } + .clearfix.prepend-top-10 + %button.btn.btn-success.pull-left{ type: "submit", + ":disabled" => "title === ''" } + Submit issue + %button.btn.btn-default.pull-right{ type: "button", + "@click" => "cancel" } + Cancel %ul.board-list{ "v-el:list" => true, "v-show" => "!loading", - ":data-board" => "list.id" } - - if can? current_user, :create_issue, @project - %board-new-issue{ "inline-template" => true, - ":list" => "list", - ":show-issue-form.sync" => "showIssueForm", - "v-show" => "list.type !== 'done' && showIssueForm" } - %li.card.board-new-issue-form - %form{ "@submit" => "submit($event)" } - %label.label-light - Title - %input.form-control{ type: "text", - "v-model" => "title", - "v-el:input" => true } - .clearfix.prepend-top-10 - %button.btn.btn-success.pull-left{ type: "submit", - ":disabled" => "title === ''" } - Submit issue - %button.btn.btn-default.pull-right{ type: "button", - "@click" => "cancel" } - Cancel + ":data-board" => "list.id", + ":class" => "{ 'is-smaller': showIssueForm }" } = render "projects/boards/components/card" %li.board-list-count.text-center{ "v-if" => "showCount" } = icon("spinner spin", "v-show" => "list.loadingMore" ) From 284af578f37ffb9ec7bcc44ae9d8897be6e68d2f Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 4 Oct 2016 12:53:39 +0100 Subject: [PATCH 05/16] Added tests --- .../boards/components/_board.html.haml | 2 +- spec/features/boards/new_issue_spec.rb | 80 +++++++++++++++++++ 2 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 spec/features/boards/new_issue_spec.rb diff --git a/app/views/projects/boards/components/_board.html.haml b/app/views/projects/boards/components/_board.html.haml index efec465bbb2..069b8bd9e55 100644 --- a/app/views/projects/boards/components/_board.html.haml +++ b/app/views/projects/boards/components/_board.html.haml @@ -19,7 +19,7 @@ - if can? current_user, :create_issue, @project %button.btn.btn-small.btn-default.pull-right{ type: "button", "@click" => "showNewIssueForm", - "v-if" => "list.type !== 'done'" } + "v-if" => "list.type !== 'done' && list.type !== 'blank'" } = icon("plus") - if can?(current_user, :admin_list, @project) %board-delete{ "inline-template" => true, diff --git a/spec/features/boards/new_issue_spec.rb b/spec/features/boards/new_issue_spec.rb new file mode 100644 index 00000000000..c046e6b8d79 --- /dev/null +++ b/spec/features/boards/new_issue_spec.rb @@ -0,0 +1,80 @@ +require 'rails_helper' + +describe 'Issue Boards new issue', feature: true, js: true do + include WaitForAjax + include WaitForVueResource + + let(:project) { create(:project_with_board, :public) } + let(:user) { create(:user) } + + context 'authorized user' do + before do + project.team << [user, :master] + + login_as(user) + + visit namespace_project_board_path(project.namespace, project) + wait_for_vue_resource + + expect(page).to have_selector('.board', count: 3) + end + + it 'displays new issue button' do + expect(page).to have_selector('.board-issue-count-holder .btn', count: 1) + end + + it 'does not display new issue button in done list' do + page.within('.board:nth-child(3)') do + expect(page).not_to have_selector('.board-issue-count-holder .btn') + end + end + + it 'shows form when clicking button' do + page.within(first('.board')) do + find('.board-issue-count-holder .btn').click + + expect(page).to have_selector('.board-new-issue-form') + end + end + + it 'hides form when clicking cancel' do + page.within(first('.board')) do + find('.board-issue-count-holder .btn').click + + expect(page).to have_selector('.board-new-issue-form') + + click_button 'Cancel' + + expect(page).to have_selector('.board-new-issue-form', visible: false) + end + end + + it 'creates new issue' do + page.within(first('.board')) do + find('.board-issue-count-holder .btn').click + end + + page.within(first('.board-new-issue-form')) do + find('.form-control').set('bug') + click_button 'Submit issue' + end + + wait_for_vue_resource + + page.within(first('.board .board-issue-count')) do + expect(page).to have_content('1') + end + end + end + + context 'unauthorized user' do + before do + visit namespace_project_board_path(project.namespace, project) + wait_for_vue_resource + end + + it 'does not display new issue button' do + expect(page).to have_selector('.board-issue-count-holder .btn', count: 0) + end + end +end From da8998f940fde0b193261c6d357af9d701919293 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 4 Oct 2016 14:24:49 +0100 Subject: [PATCH 06/16] Changed how button is hidden when blank list Added for attribute to title label --- .../projects/boards/components/_board.html.haml | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/app/views/projects/boards/components/_board.html.haml b/app/views/projects/boards/components/_board.html.haml index 069b8bd9e55..f2c9ac809c1 100644 --- a/app/views/projects/boards/components/_board.html.haml +++ b/app/views/projects/boards/components/_board.html.haml @@ -12,14 +12,14 @@ %header.board-header{ ":class" => "{ 'has-border': list.label }", ":style" => "{ borderTopColor: (list.label ? list.label.color : null) }" } %h3.board-title.js-board-handle{ ":class" => "{ 'user-can-drag': (!disabled && !list.preset) }" } {{ list.title }} - .board-issue-count-holder.pull-right.clearfix - %span.board-issue-count.pull-left{ "v-if" => "list.type !== 'blank'", - ":class" => "{ 'has-btn': list.type !== 'done' }" } + .board-issue-count-holder.pull-right.clearfix{ "v-if" => "list.type !== 'blank'" } + %span.board-issue-count.pull-left{ ":class" => "{ 'has-btn': list.type !== 'done' }" } {{ list.issuesSize }} - if can? current_user, :create_issue, @project %button.btn.btn-small.btn-default.pull-right{ type: "button", "@click" => "showNewIssueForm", - "v-if" => "list.type !== 'done' && list.type !== 'blank'" } + "v-if" => "list.type !== 'done'", + "aria-label" => "Show new issue form" } = icon("plus") - if can?(current_user, :admin_list, @project) %board-delete{ "inline-template" => true, @@ -44,11 +44,12 @@ "v-show" => "list.type !== 'done' && showIssueForm" } .card.board-new-issue-form %form{ "@submit" => "submit($event)" } - %label.label-light + %label.label-light{ ":for" => "list.id + '-title'" } Title %input.form-control{ type: "text", "v-model" => "title", - "v-el:input" => true } + "v-el:input" => true, + ":id" => "list.id + '-title'" } .clearfix.prepend-top-10 %button.btn.btn-success.pull-left{ type: "submit", ":disabled" => "title === ''" } From 8af7c32526b9b22b62013a682352a0ea1d3345ae Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 4 Oct 2016 14:27:21 +0100 Subject: [PATCH 07/16] Made the CSS a bit more reasonable Changed how the data is passed into Vue resource --- app/assets/javascripts/boards/services/board_service.js.es6 | 4 +--- app/assets/stylesheets/pages/boards.scss | 6 ++---- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/boards/services/board_service.js.es6 b/app/assets/javascripts/boards/services/board_service.js.es6 index c8b1d4f2b5a..2b825c3949f 100644 --- a/app/assets/javascripts/boards/services/board_service.js.es6 +++ b/app/assets/javascripts/boards/services/board_service.js.es6 @@ -61,9 +61,7 @@ class BoardService { newIssue (id, issue) { return this.issues.save({ id }, { - issue: { - title: issue.title - } + issue }); } }; diff --git a/app/assets/stylesheets/pages/boards.scss b/app/assets/stylesheets/pages/boards.scss index 6e6cb521dbc..6e81c12aa55 100644 --- a/app/assets/stylesheets/pages/boards.scss +++ b/app/assets/stylesheets/pages/boards.scss @@ -164,7 +164,7 @@ lex overflow-x: hidden; &.is-smaller { - height: calc(100% - 185px); + height: calc(100% - 185px); } } @@ -257,9 +257,7 @@ lex padding-left: 10px; line-height: 21px; border-radius: $border-radius-base; - border-color: $border-color; - border-style: solid; - border-width: 1px; + border: 1px solid $border-color; &.has-btn { border-top-right-radius: 0; From 3397a8a6b5022c10701b6d9ef9b045ca05caf829 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 4 Oct 2016 14:46:01 +0100 Subject: [PATCH 08/16] Added tooltip to new issue button --- app/views/projects/boards/components/_board.html.haml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/views/projects/boards/components/_board.html.haml b/app/views/projects/boards/components/_board.html.haml index f2c9ac809c1..5cbc4eadcd0 100644 --- a/app/views/projects/boards/components/_board.html.haml +++ b/app/views/projects/boards/components/_board.html.haml @@ -16,10 +16,12 @@ %span.board-issue-count.pull-left{ ":class" => "{ 'has-btn': list.type !== 'done' }" } {{ list.issuesSize }} - if can? current_user, :create_issue, @project - %button.btn.btn-small.btn-default.pull-right{ type: "button", + %button.btn.btn-small.btn-default.pull-right.has-tooltip{ type: "button", "@click" => "showNewIssueForm", "v-if" => "list.type !== 'done'", - "aria-label" => "Show new issue form" } + "aria-label" => "Add an issue", + "title" => "Add an issue", + data: { placement: "top", container: "body" } } = icon("plus") - if can?(current_user, :admin_list, @project) %board-delete{ "inline-template" => true, From 905af8471691fc0bb991aca5276185796dbe28c9 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 4 Oct 2016 16:10:44 +0100 Subject: [PATCH 09/16] Changed new issue button permissions This is because contributors can't create issues with labels --- app/views/projects/boards/components/_board.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/boards/components/_board.html.haml b/app/views/projects/boards/components/_board.html.haml index 5cbc4eadcd0..4d7d8319204 100644 --- a/app/views/projects/boards/components/_board.html.haml +++ b/app/views/projects/boards/components/_board.html.haml @@ -15,7 +15,7 @@ .board-issue-count-holder.pull-right.clearfix{ "v-if" => "list.type !== 'blank'" } %span.board-issue-count.pull-left{ ":class" => "{ 'has-btn': list.type !== 'done' }" } {{ list.issuesSize }} - - if can? current_user, :create_issue, @project + - if can?(current_user, :admin_issue, @project) %button.btn.btn-small.btn-default.pull-right.has-tooltip{ type: "button", "@click" => "showNewIssueForm", "v-if" => "list.type !== 'done'", From e7a4bbb04a86259a569f6ac239ecb35ad36f39b5 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 4 Oct 2016 15:37:13 -0300 Subject: [PATCH 10/16] Add authorization to Projects::Boards::IssuesController#create action --- app/controllers/projects/boards/issues_controller.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/controllers/projects/boards/issues_controller.rb b/app/controllers/projects/boards/issues_controller.rb index 3b1b236a89a..fea7a35232d 100644 --- a/app/controllers/projects/boards/issues_controller.rb +++ b/app/controllers/projects/boards/issues_controller.rb @@ -2,6 +2,7 @@ module Projects module Boards class IssuesController < Boards::ApplicationController before_action :authorize_read_issue!, only: [:index] + before_action :authorize_create_issue!, only: [:create] before_action :authorize_update_issue!, only: [:update] def index @@ -52,6 +53,10 @@ module Projects return render_403 unless can?(current_user, :read_issue, project) end + def authorize_create_issue! + return render_403 unless can?(current_user, :admin_issue, project) + end + def authorize_update_issue! return render_403 unless can?(current_user, :update_issue, issue) end From 97ec0c05f66e40a6de6620efcf5c92b7c2979f95 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 4 Oct 2016 17:41:00 -0300 Subject: [PATCH 11/16] Add service to create a new issue in a board list --- .../projects/boards/issues_controller.rb | 29 +++++++++------- app/services/boards/issues/create_service.rb | 16 +++++++++ .../boards/issues/create_service_spec.rb | 33 +++++++++++++++++++ 3 files changed, 67 insertions(+), 11 deletions(-) create mode 100644 app/services/boards/issues/create_service.rb create mode 100644 spec/services/boards/issues/create_service_spec.rb diff --git a/app/controllers/projects/boards/issues_controller.rb b/app/controllers/projects/boards/issues_controller.rb index fea7a35232d..095af6c35eb 100644 --- a/app/controllers/projects/boards/issues_controller.rb +++ b/app/controllers/projects/boards/issues_controller.rb @@ -10,23 +10,21 @@ module Projects issues = issues.page(params[:page]) render json: { - issues: issues.as_json( - only: [:iid, :title, :confidential], - include: { - assignee: { only: [:id, :name, :username], methods: [:avatar_url] }, - labels: { only: [:id, :title, :description, :color, :priority], methods: [:text_color] } - }), + issues: serialize_as_json(issues), size: issues.total_count } end def create list = project.board.lists.find(params[:list_id]) + service = ::Boards::Issues::CreateService.new(project, current_user, issue_params) + issue = service.execute(list) - issue = Issues::CreateService.new(project, current_user, issue_params.merge(request: request)).execute - issue.labels << list.label if list.label - - render json: issue.to_json + if issue.valid? + render json: serialize_as_json(issue) + else + render json: issue.errors, status: :unprocessable_entity + end end def update @@ -70,7 +68,16 @@ module Projects end def issue_params - params.require(:issue).permit(:title) + params.require(:issue).permit(:title).merge(request: request) + end + + def serialize_as_json(resource) + resource.as_json( + only: [:iid, :title, :confidential], + include: { + assignee: { only: [:id, :name, :username], methods: [:avatar_url] }, + labels: { only: [:id, :title, :description, :color, :priority], methods: [:text_color] } + }) end end end diff --git a/app/services/boards/issues/create_service.rb b/app/services/boards/issues/create_service.rb new file mode 100644 index 00000000000..3701afd441f --- /dev/null +++ b/app/services/boards/issues/create_service.rb @@ -0,0 +1,16 @@ +module Boards + module Issues + class CreateService < Boards::BaseService + def execute(list) + params.merge!(label_ids: [list.label_id]) + create_issue + end + + private + + def create_issue + ::Issues::CreateService.new(project, current_user, params).execute + end + end + end +end diff --git a/spec/services/boards/issues/create_service_spec.rb b/spec/services/boards/issues/create_service_spec.rb new file mode 100644 index 00000000000..33e10e79f6d --- /dev/null +++ b/spec/services/boards/issues/create_service_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe Boards::Issues::CreateService, services: true do + describe '#execute' do + let(:project) { create(:project_with_board) } + let(:board) { project.board } + 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') } + + before do + project.team << [user, :developer] + end + + it 'delegates the create proceedings to Issues::CreateService' do + expect_any_instance_of(Issues::CreateService).to receive(:execute).once + + service.execute(list) + end + + it 'creates a new issue' do + expect { service.execute(list) }.to change(project.issues, :count).by(1) + end + + it 'adds the label of the list to the issue' do + issue = service.execute(list) + + expect(issue.labels).to eq [label] + end + end +end From fe3f1657ab88cbb60281681771c4b9dd870b65c6 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 4 Oct 2016 17:41:24 -0300 Subject: [PATCH 12/16] Add tests to Projects::Boards::IssuesController#create action --- .../projects/boards/issues_controller_spec.rb | 64 +++++++++++++++++-- 1 file changed, 57 insertions(+), 7 deletions(-) diff --git a/spec/controllers/projects/boards/issues_controller_spec.rb b/spec/controllers/projects/boards/issues_controller_spec.rb index 2896636db5a..566658b508d 100644 --- a/spec/controllers/projects/boards/issues_controller_spec.rb +++ b/spec/controllers/projects/boards/issues_controller_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' describe Projects::Boards::IssuesController do let(:project) { create(:project_with_board) } let(:user) { create(:user) } + let(:guest) { create(:user) } let(:planning) { create(:label, project: project, name: 'Planning') } let(:development) { create(:label, project: project, name: 'Development') } @@ -12,6 +13,7 @@ describe Projects::Boards::IssuesController do before do project.team << [user, :master] + project.team << [guest, :guest] end describe 'GET index' do @@ -61,6 +63,60 @@ describe Projects::Boards::IssuesController do 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' + + expect(response).to have_http_status(200) + end + + it 'returns the created issue' do + create_issue user: user, list: list1, title: 'New issue' + + expect(response).to match_response_schema('issue') + end + end + + 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 + + expect(response).to have_http_status(422) + end + end + + context 'when list does not belongs to project board' do + it 'returns a not found 404 response' do + list = create(:list) + + create_issue user: user, list: list, title: 'New issue' + + expect(response).to have_http_status(404) + end + end + end + + context 'with unauthorized user' do + it 'returns a forbidden 403 response' do + create_issue user: guest, list: list1, title: 'New issue' + + expect(response).to have_http_status(403) + end + end + + def create_issue(user:, list:, title:) + sign_in(user) + + post :create, namespace_id: project.namespace.to_param, + project_id: project.to_param, + list_id: list.to_param, + issue: { title: title }, + format: :json + end + end + describe 'PATCH update' do let(:issue) { create(:labeled_issue, project: project, labels: [planning]) } @@ -93,13 +149,7 @@ describe Projects::Boards::IssuesController do end context 'with unauthorized user' do - let(:guest) { create(:user) } - - before do - project.team << [guest, :guest] - end - - it 'returns a successful 403 response' do + it 'returns a forbidden 403 response' do move user: guest, issue: issue, from_list_id: list1.id, to_list_id: list2.id expect(response).to have_http_status(403) From b9ede4f1a5aa213f0342daff5e7281758e989a8e Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 4 Oct 2016 17:43:05 -0300 Subject: [PATCH 13/16] Update CHANGELOG --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 649215db31c..37dbf56c3e9 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -19,6 +19,7 @@ v 8.13.0 (unreleased) - Fix VueJS template tags being rendered in code comments - Fix issue with page scrolling to top when closing or pinning sidebar (lukehowell) - Allow the Koding integration to be configured through the API + - Add new issue button to each list on Issues Board - Added soft wrap button to repository file/blob editor - Add word-wrap to issue title on issue and milestone boards (ClemMakesApps) - Fix todos page mobile viewport layout (ClemMakesApps) From a68f1fdd2975a9e7cab39ab7d40e3eae2cc676db Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 5 Oct 2016 14:27:29 +0100 Subject: [PATCH 14/16] Fix form not re-enabling thanks to jQuery Stop issue being dragged if it doesn't have an ID --- app/assets/javascripts/boards/components/board_list.js.es6 | 2 +- .../javascripts/boards/components/board_new_issue.js.es6 | 6 +++++- app/assets/javascripts/boards/models/list.js.es6 | 2 +- app/views/projects/boards/components/_board.html.haml | 3 ++- app/views/projects/boards/components/_card.html.haml | 2 +- 5 files changed, 10 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/boards/components/board_list.js.es6 b/app/assets/javascripts/boards/components/board_list.js.es6 index 208aac504fd..7022a29e818 100644 --- a/app/assets/javascripts/boards/components/board_list.js.es6 +++ b/app/assets/javascripts/boards/components/board_list.js.es6 @@ -76,7 +76,7 @@ group: 'issues', sort: false, disabled: this.disabled, - filter: '.board-list-count, .board-new-issue-form', + filter: '.board-list-count, .is-disabled', onStart: (e) => { const card = this.$refs.issue[e.oldIndex]; diff --git a/app/assets/javascripts/boards/components/board_new_issue.js.es6 b/app/assets/javascripts/boards/components/board_new_issue.js.es6 index eb26ed9721d..315c16ee242 100644 --- a/app/assets/javascripts/boards/components/board_new_issue.js.es6 +++ b/app/assets/javascripts/boards/components/board_new_issue.js.es6 @@ -25,7 +25,11 @@ labels }); - this.list.newIssue(issue); + this.list.newIssue(issue) + .then(() => { + // Need this because our jQuery very kindly disables buttons on ALL form submissions + $(this.$els.submitButton).enable(); + }); this.cancel(); }, diff --git a/app/assets/javascripts/boards/models/list.js.es6 b/app/assets/javascripts/boards/models/list.js.es6 index 14e42db4cd2..5d0a561cdba 100644 --- a/app/assets/javascripts/boards/models/list.js.es6 +++ b/app/assets/javascripts/boards/models/list.js.es6 @@ -91,7 +91,7 @@ class List { this.addIssue(issue); this.issuesSize++; - gl.boardService.newIssue(this.id, issue) + return gl.boardService.newIssue(this.id, issue) .then((resp) => { const data = resp.json(); issue.id = data.iid; diff --git a/app/views/projects/boards/components/_board.html.haml b/app/views/projects/boards/components/_board.html.haml index 4d7d8319204..26b2236b581 100644 --- a/app/views/projects/boards/components/_board.html.haml +++ b/app/views/projects/boards/components/_board.html.haml @@ -54,7 +54,8 @@ ":id" => "list.id + '-title'" } .clearfix.prepend-top-10 %button.btn.btn-success.pull-left{ type: "submit", - ":disabled" => "title === ''" } + ":disabled" => "title === ''", + "v-el:submit-button" => true } Submit issue %button.btn.btn-default.pull-right{ type: "button", "@click" => "cancel" } diff --git a/app/views/projects/boards/components/_card.html.haml b/app/views/projects/boards/components/_card.html.haml index 25701b0a99e..f15c87c8185 100644 --- a/app/views/projects/boards/components/_card.html.haml +++ b/app/views/projects/boards/components/_card.html.haml @@ -7,7 +7,7 @@ ":issue-link-base" => "issueLinkBase", ":disabled" => "disabled", "track-by" => "id" } - %li.card{ ":class" => "{ 'user-can-drag': !disabled }", + %li.card{ ":class" => "{ 'user-can-drag': !disabled && issue.id, 'is-disabled': disabled || !issue.id }", ":index" => "index" } %h4.card-title = icon("eye-slash", class: "confidential-icon", "v-if" => "issue.confidential") From 2f53a8d08f3b1595dfa48c308e408083b89651f4 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 5 Oct 2016 14:58:29 +0100 Subject: [PATCH 15/16] Shows error if response returns an error Added validation so the user shouldnt be able to submit the form without the title present --- .../boards/components/board_new_issue.js.es6 | 21 +++++++++++++++++-- .../boards/components/_board.html.haml | 3 +++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/boards/components/board_new_issue.js.es6 b/app/assets/javascripts/boards/components/board_new_issue.js.es6 index 315c16ee242..58cc30b05af 100644 --- a/app/assets/javascripts/boards/components/board_new_issue.js.es6 +++ b/app/assets/javascripts/boards/components/board_new_issue.js.es6 @@ -8,7 +8,8 @@ }, data() { return { - title: '' + title: '', + error: false }; }, watch: { @@ -19,6 +20,10 @@ methods: { submit(e) { e.preventDefault(); + if (this.title.trim() === '') return; + + this.error = false; + const labels = this.list.label ? [this.list.label] : []; const issue = new ListIssue({ title: this.title, @@ -26,9 +31,21 @@ }); this.list.newIssue(issue) - .then(() => { + .then((data) => { // Need this because our jQuery very kindly disables buttons on ALL form submissions $(this.$els.submitButton).enable(); + }) + .catch(() => { + // Need this because our jQuery very kindly disables buttons on ALL form submissions + $(this.$els.submitButton).enable(); + + // Remove issue with no ID + const issue = this.list.findIssue(undefined); + this.list.removeIssue(issue); + + // Show error message + this.error = true; + this.showIssueForm = true; }); this.cancel(); diff --git a/app/views/projects/boards/components/_board.html.haml b/app/views/projects/boards/components/_board.html.haml index 26b2236b581..ba1502c97b6 100644 --- a/app/views/projects/boards/components/_board.html.haml +++ b/app/views/projects/boards/components/_board.html.haml @@ -46,6 +46,9 @@ "v-show" => "list.type !== 'done' && showIssueForm" } .card.board-new-issue-form %form{ "@submit" => "submit($event)" } + .flash-container{ "v-if" => "error" } + .flash-alert + An error occured. Please try again. %label.label-light{ ":for" => "list.id + '-title'" } Title %input.form-control{ type: "text", From 8ad923695e54c7ac6e9c47c37c505a7ac6b34017 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 6 Oct 2016 14:52:36 +0100 Subject: [PATCH 16/16] Remove the code finding an issue by undefined --- .../javascripts/boards/components/board_new_issue.js.es6 | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/assets/javascripts/boards/components/board_new_issue.js.es6 b/app/assets/javascripts/boards/components/board_new_issue.js.es6 index 58cc30b05af..a4fad422eca 100644 --- a/app/assets/javascripts/boards/components/board_new_issue.js.es6 +++ b/app/assets/javascripts/boards/components/board_new_issue.js.es6 @@ -39,8 +39,7 @@ // Need this because our jQuery very kindly disables buttons on ALL form submissions $(this.$els.submitButton).enable(); - // Remove issue with no ID - const issue = this.list.findIssue(undefined); + // Remove the issue this.list.removeIssue(issue); // Show error message