From 914f97310815365f083a38b02d8dbf6c99b63b5f Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 17 Jun 2016 18:31:37 +0100 Subject: Removed update method Re-structured controller spec Renamed issuable param to issuable_id --- app/assets/javascripts/right_sidebar.js.coffee | 2 +- app/views/shared/issuable/_sidebar.html.haml | 2 +- config/routes.rb | 2 +- spec/controllers/projects/todo_controller_spec.rb | 166 +++++++++++----------- 4 files changed, 83 insertions(+), 89 deletions(-) diff --git a/app/assets/javascripts/right_sidebar.js.coffee b/app/assets/javascripts/right_sidebar.js.coffee index be4e0dcaaf7..12340bbce54 100644 --- a/app/assets/javascripts/right_sidebar.js.coffee +++ b/app/assets/javascripts/right_sidebar.js.coffee @@ -63,7 +63,7 @@ class @Sidebar type: ajaxType dataType: 'json' data: - issuable_id: $this.data('issuable') + issuable_id: $this.data('issuable-id') issuable_type: $this.data('issuable-type') beforeSend: => @beforeTodoSend($this, $todoLoading) diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index cd29fff581e..adfab1af53e 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -9,7 +9,7 @@ %a.gutter-toggle.pull-right.js-sidebar-toggle{ role: "button", href: "#", aria: { label: "Toggle sidebar" } } = sidebar_gutter_toggle_icon - if current_user - %button.btn.btn-default.issuable-header-btn.pull-right.js-issuable-todo{ type: "button", aria: { label: (todo.nil? ? "Add Todo" : "Mark Done") }, data: { todo_text: "Add Todo", mark_text: "Mark Done", issuable: issuable.id, issuable_type: issuable.class.name.underscore, url: namespace_project_todos_path(@project.namespace, @project), delete_path: (dashboard_todo_path(todo) if todo) } } + %button.btn.btn-default.issuable-header-btn.pull-right.js-issuable-todo{ type: "button", aria: { label: (todo.nil? ? "Add Todo" : "Mark Done") }, data: { todo_text: "Add Todo", mark_text: "Mark Done", issuable_id: issuable.id, issuable_type: issuable.class.name.underscore, url: namespace_project_todos_path(@project.namespace, @project), delete_path: (dashboard_todo_path(todo) if todo) } } %span.js-issuable-todo-text - if todo Mark Done diff --git a/config/routes.rb b/config/routes.rb index a908513cc7a..026b2691323 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -797,7 +797,7 @@ Rails.application.routes.draw do end end - resources :todos, only: [:create, :update] + resources :todos, only: [:create] resources :uploads, only: [:create] do collection do diff --git a/spec/controllers/projects/todo_controller_spec.rb b/spec/controllers/projects/todo_controller_spec.rb index 2160f0b802c..40a3403b660 100644 --- a/spec/controllers/projects/todo_controller_spec.rb +++ b/spec/controllers/projects/todo_controller_spec.rb @@ -7,101 +7,95 @@ describe Projects::TodosController do let(:merge_request) { create(:merge_request, source_project: project) } context 'Issues' do - describe 'POST #create' do - before do - sign_in(user) - project.team << [user, :developer] + describe 'POST create' do + context 'when authorized' do + before do + sign_in(user) + project.team << [user, :developer] + end + + it 'should create todo for issue' do + expect do + post(:create, namespace_id: project.namespace.path, + project_id: project.path, + issuable_id: issue.id, + issuable_type: 'issue') + end.to change { user.todos.count }.by(1) + + expect(response.status).to eq(200) + end end - it 'should create todo for issue' do - expect do - post(:create, namespace_id: project.namespace.path, - project_id: project.path, - issuable_id: issue.id, - issuable_type: "issue") - end.to change { user.todos.count }.by(1) - - expect(response.status).to eq(200) - end - end - - describe 'POST #create when not authorized' do - before do - sign_in(user) - end - - it 'should create todo for issue' do - expect do - post(:create, namespace_id: project.namespace.path, - project_id: project.path, - issuable_id: issue.id, - issuable_type: "issue") - end.to change { user.todos.count }.by(0) - - expect(response.status).to eq(404) - end - end - - describe 'POST #create when not logged in' do - it 'should create todo for issue' do - expect do - post(:create, namespace_id: project.namespace.path, - project_id: project.path, - issuable_id: issue.id, - issuable_type: "issue") - end.to change { user.todos.count }.by(0) - - expect(response.status).to eq(302) + context 'when not authorized' do + it 'should not create todo for issue that user has no access to' do + sign_in(user) + expect do + post(:create, namespace_id: project.namespace.path, + project_id: project.path, + issuable_id: issue.id, + issuable_type: 'issue') + end.to change { user.todos.count }.by(0) + + expect(response.status).to eq(404) + end + + it 'should not create todo for issue when user not logged in' do + expect do + post(:create, namespace_id: project.namespace.path, + project_id: project.path, + issuable_id: issue.id, + issuable_type: 'issue') + end.to change { user.todos.count }.by(0) + + expect(response.status).to eq(302) + end end end end context 'Merge Requests' do - describe 'POST #create' do - before do - sign_in(user) - project.team << [user, :developer] + describe 'POST create' do + context 'when authorized' do + before do + sign_in(user) + project.team << [user, :developer] + end + + it 'should create todo for merge request' do + expect do + post(:create, namespace_id: project.namespace.path, + project_id: project.path, + issuable_id: merge_request.id, + issuable_type: 'merge_request') + end.to change { user.todos.count }.by(1) + + expect(response.status).to eq(200) + end end - it 'should create todo for issue' do - expect do - post(:create, namespace_id: project.namespace.path, - project_id: project.path, - issuable_id: merge_request.id, - issuable_type: "merge_request") - end.to change { user.todos.count }.by(1) - - expect(response.status).to eq(200) - end - end - - describe 'POST #create when not authorized' do - before do - sign_in(user) - end - - it 'should create todo for issue' do - expect do - post(:create, namespace_id: project.namespace.path, - project_id: project.path, - issuable_id: merge_request.id, - issuable_type: "merge_request") - end.to change { user.todos.count }.by(0) - - expect(response.status).to eq(404) - end - end - - describe 'POST #create when not logged in' do - it 'should create todo for issue' do - expect do - post(:create, namespace_id: project.namespace.path, - project_id: project.path, - issuable_id: merge_request.id, - issuable_type: "merge_request") - end.to change { user.todos.count }.by(0) - - expect(response.status).to eq(302) + context 'when not authorized' do + it 'should not create todo for merge request user has no access to' do + sign_in(user) + expect do + post(:create, namespace_id: project.namespace.path, + project_id: project.path, + issuable_id: merge_request.id, + issuable_type: 'merge_request') + end.to change { user.todos.count }.by(0) + + expect(response.status).to eq(404) + end + + it 'should not create todo for merge request user has no access to' do + expect do + post(:create, namespace_id: project.namespace.path, + project_id: project.path, + issuable_id: merge_request.id, + issuable_type: 'merge_request') + end.to change { user.todos.count }.by(0) + + expect(response.status).to eq(302) + end end end end -- cgit v1.2.1