From 82ac4e22b0d5b362baf11286b883ab88068da41f Mon Sep 17 00:00:00 2001 From: Carsten Otto Date: Thu, 14 May 2015 13:07:20 +0200 Subject: [PATCH 1/9] fix syntax --- app/controllers/todos_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/todos_controller.rb b/app/controllers/todos_controller.rb index f54669f6..b239dec9 100644 --- a/app/controllers/todos_controller.rb +++ b/app/controllers/todos_controller.rb @@ -1137,7 +1137,7 @@ end end def update_project - @project_changed = false; + @project_changed = false if params['todo']['project_id'].blank? && !params['project_name'].nil? if params['project_name'] == 'None' project = Project.null_object From 73bb53e1e5b8a7757a8f4d4a19e79e4afbf1a587 Mon Sep 17 00:00:00 2001 From: Carsten Otto Date: Sun, 12 Apr 2015 19:28:40 +0200 Subject: [PATCH 2/9] Still show action if it could not be added as predecessor If it is added as a predecessor, other code takes care to hide the action. For a very short time, barely noticeable, the action is shown moving back to its original position. See pull request #1777 --- app/assets/javascripts/tracks.js.erb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/assets/javascripts/tracks.js.erb b/app/assets/javascripts/tracks.js.erb index 1974af86..0163b8ce 100644 --- a/app/assets/javascripts/tracks.js.erb +++ b/app/assets/javascripts/tracks.js.erb @@ -301,7 +301,6 @@ var TodoItems = { /* Drag & Drop for successor/predecessor */ var dragged_todo = ui.draggable[0].id.split('_')[2]; var dropped_todo = this.id.split('_')[2]; - ui.draggable.remove(); $('.drop_target').hide(); // IE8 doesn't call stop() in this situation ajax_options = default_ajax_options_for_scripts('POST', relative_to_root('todos/add_predecessor'), $(this)); @@ -324,7 +323,7 @@ var TodoItems = { setup_drag_and_drop: function() { $('.item-show').draggable({ handle: '.grip', - revert: 'invalid', + revert: true, start: TodoItems.drag_todo, stop: function() { $('.drop_target').hide(); From fd8f4e2b32e5c238809768ad48705d5b15082cff Mon Sep 17 00:00:00 2001 From: Carsten Otto Date: Sun, 12 Apr 2015 19:56:58 +0200 Subject: [PATCH 3/9] Fix syntax --- app/controllers/todos_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/todos_controller.rb b/app/controllers/todos_controller.rb index f54669f6..85839bcc 100644 --- a/app/controllers/todos_controller.rb +++ b/app/controllers/todos_controller.rb @@ -85,7 +85,7 @@ class TodosController < ApplicationController create_multiple else p = Todos::TodoCreateParamsHelper.new(params, current_user) - p.parse_dates() unless mobile? + p.parse_dates unless mobile? tag_list = p.tag_list @todo = current_user.todos.build @@ -1137,7 +1137,7 @@ end end def update_project - @project_changed = false; + @project_changed = false if params['todo']['project_id'].blank? && !params['project_name'].nil? if params['project_name'] == 'None' project = Project.null_object From bba13194fd7886c2cade971ccabd2cb6e5060dfc Mon Sep 17 00:00:00 2001 From: Carsten Otto Date: Mon, 13 Apr 2015 21:45:39 +0200 Subject: [PATCH 4/9] show dependency validation errors in action edit form --- app/controllers/todos_controller.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/app/controllers/todos_controller.rb b/app/controllers/todos_controller.rb index 85839bcc..fcc7805c 100644 --- a/app/controllers/todos_controller.rb +++ b/app/controllers/todos_controller.rb @@ -423,7 +423,16 @@ class TodosController < ApplicationController update_dependencies update_attributes_of_todo - @saved = @todo.save + begin + @saved = @todo.save! + rescue ActiveRecord::RecordInvalid => exception + record = exception.record + if record.is_a?(Dependency) + record.errors.each { |key,value| @todo.errors[key] << value } + end + @saved = false + end + # this is set after save and cleared after reload, so save it here @removed_predecessors = @todo.removed_predecessors From 131053fc1f12c4159ac4e8b97cdcf5675e6fcfc2 Mon Sep 17 00:00:00 2001 From: Carsten Otto Date: Fri, 22 May 2015 22:37:05 +0200 Subject: [PATCH 5/9] cleanup test --- features/edit_a_todo.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/edit_a_todo.feature b/features/edit_a_todo.feature index c1cf9391..a7c90231 100644 --- a/features/edit_a_todo.feature +++ b/features/edit_a_todo.feature @@ -326,7 +326,7 @@ Feature: Edit a next action from every page Given I have a deferred todo "moving" in context "@pc" with tags "tag" When I go to the tickler page And I edit the context of "moving" to "@new" - And I should see the container for context "@new" + Then I should see the container for context "@new" @javascript Scenario: Making an error when editing a todo will show error message From e7495e32b766dd9e251d083f27eb48115d41be62 Mon Sep 17 00:00:00 2001 From: Carsten Otto Date: Fri, 22 May 2015 22:55:58 +0200 Subject: [PATCH 6/9] remove redundant code --- app/views/todos/destroy.js.erb | 2 +- app/views/todos/toggle_check.js.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/todos/destroy.js.erb b/app/views/todos/destroy.js.erb index 5848f716..fa895ef5 100644 --- a/app/views/todos/destroy.js.erb +++ b/app/views/todos/destroy.js.erb @@ -28,7 +28,7 @@ function show_empty_messages() { } function remove_todo_from_page() { - <% if (@remaining_in_context == 0) && update_needs_to_hide_container + <% if update_needs_to_hide_container # remove context with deleted todo -%> $('#<%=item_container_id(@todo)%>').fadeOut(400, function() { diff --git a/app/views/todos/toggle_check.js.erb b/app/views/todos/toggle_check.js.erb index 74aa9167..6729a805 100644 --- a/app/views/todos/toggle_check.js.erb +++ b/app/views/todos/toggle_check.js.erb @@ -34,7 +34,7 @@ var <%= object_name %> = { redirect_to(path); }, remove_todo: function(next_steps) { - <% if (@remaining_in_context == 0) && update_needs_to_hide_container + <% if update_needs_to_hide_container # remove context with deleted todo -%> $('#<%= item_container_id(@original_item)%>').slideUp(400, function() { From 4ce7c6bcd4adc4d48fe1b2765945d1dd563d0db3 Mon Sep 17 00:00:00 2001 From: Carsten Otto Date: Fri, 22 May 2015 23:02:41 +0200 Subject: [PATCH 7/9] Provide data needed for partial views The partials for context and project need to compute the context's/project's id, which is done based on @context/@project. Provide this data. Fixes #1836 --- app/controllers/todos_controller.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/app/controllers/todos_controller.rb b/app/controllers/todos_controller.rb index b239dec9..b272ece8 100644 --- a/app/controllers/todos_controller.rb +++ b/app/controllers/todos_controller.rb @@ -425,6 +425,8 @@ class TodosController < ApplicationController @saved = @todo.save + provide_project_or_context_for_view + # this is set after save and cleared after reload, so save it here @removed_predecessors = @todo.removed_predecessors @@ -457,6 +459,15 @@ class TodosController < ApplicationController end end + def provide_project_or_context_for_view + # see application_helper:source_view_key, used in shown partials + if source_view_is :project + @project = @todo.project + elsif source_view_is :context + @context = @todo.context + end + end + def destroy @source_view = params['_source_view'] || 'todo' @todo = current_user.todos.find(params['id']) From 0adad478578e4a9b3c8a0740895a873c88837227 Mon Sep 17 00:00:00 2001 From: Carsten Otto Date: Fri, 22 May 2015 22:54:12 +0200 Subject: [PATCH 8/9] re-add action in correct container if context is changed fixes #1804 --- app/helpers/todos_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb index ccae4c0d..7c513715 100644 --- a/app/helpers/todos_helper.rb +++ b/app/helpers/todos_helper.rb @@ -513,7 +513,7 @@ module TodosHelper def update_needs_to_remove_todo_from_container source_view do |page| page.context { return @context_changed || @todo_deferred_state_changed || @todo_pending_state_changed || @todo_should_be_hidden } - page.project { return @todo_deferred_state_changed || @todo_pending_state_changed || @project_changed} + page.project { return @context_changed || @todo_deferred_state_changed || @todo_pending_state_changed || @project_changed} page.deferred { return todo_moved_out_of_container || !(@todo.deferred? || @todo.pending?) } page.calendar { return @due_date_changed || !@todo.due } page.stats { return @todo.completed? } @@ -547,7 +547,7 @@ module TodosHelper def append_updated_todo source_view do |page| page.context { return @todo_deferred_state_changed || @todo_pending_state_changed } - page.project { return @todo_deferred_state_changed || @todo_pending_state_changed } + page.project { return @context_changed || @todo_deferred_state_changed || @todo_pending_state_changed } page.deferred { return todo_moved_out_of_container && (@todo.deferred? || @todo.pending?) } page.calendar { return @due_date_changed && @todo.due } page.stats { return false } From 04fdf8c6201e9fed1b83496ffb63f72a44a64855 Mon Sep 17 00:00:00 2001 From: Carsten Otto Date: Fri, 22 May 2015 23:03:12 +0200 Subject: [PATCH 9/9] add test for #1804 and #1836 --- features/edit_a_todo.feature | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/features/edit_a_todo.feature b/features/edit_a_todo.feature index a7c90231..54b087c4 100644 --- a/features/edit_a_todo.feature +++ b/features/edit_a_todo.feature @@ -328,6 +328,13 @@ Feature: Edit a next action from every page And I edit the context of "moving" to "@new" Then I should see the container for context "@new" + @javascript + Scenario: Editing the context of a todo in the project view to a new context will show new context + Given I have a todo "something" in the context "@pc" in the project "project" + When I go to the "project" project page + And I edit the context of "something" to "@new" + Then I should see the container for context "@new" + @javascript Scenario: Making an error when editing a todo will show error message Given I have a todo "test"