From 6e97541ab317da53bdba95dd774b30c8f6044006 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Tue, 21 Jun 2011 11:03:23 +0200 Subject: [PATCH] make sure toggle_check and deleting of todos, recurring_todos and projects work in the new done views --- app/controllers/recurring_todos_controller.rb | 5 ++-- app/controllers/todos_controller.rb | 22 +++++---------- app/models/recurring_todo.rb | 8 +----- app/models/todo.rb | 22 +++++++-------- app/views/recurring_todos/destroy.js.erb | 2 +- app/views/todos/toggle_check.js.erb | 4 +-- config/locales/en.yml | 2 ++ ...82432_make_old_recurring_todos_validate.rb | 15 ++++++++++ features/step_definitions/container_steps.rb | 21 +++++++++++++- features/view_done.feature | 28 +++++++++++++++++++ 10 files changed, 88 insertions(+), 41 deletions(-) create mode 100644 db/migrate/20110621082432_make_old_recurring_todos_validate.rb diff --git a/app/controllers/recurring_todos_controller.rb b/app/controllers/recurring_todos_controller.rb index c1dbc741..dd34bb1b 100644 --- a/app/controllers/recurring_todos_controller.rb +++ b/app/controllers/recurring_todos_controller.rb @@ -7,7 +7,7 @@ class RecurringTodosController < ApplicationController def index @page_title = t('todos.recurring_actions_title') - + @source_view = params['_source_view'] || 'recurring_todo' find_and_inactivate @recurring_todos = current_user.recurring_todos.active @completed_recurring_todos = current_user.recurring_todos.completed.find(:all, :limit => 10) @@ -27,6 +27,7 @@ class RecurringTodosController < ApplicationController def done @page_title = t('todos.completed_recurring_actions_title') + @source_view = params['_source_view'] || 'recurring_todo' items_per_page = 20 page = params[:page] || 1 @completed_recurring_todos = current_user.recurring_todos.completed.paginate :page => params[:page], :per_page => items_per_page @@ -141,7 +142,6 @@ class RecurringTodosController < ApplicationController end def destroy - # remove all references to this recurring todo @todos = @recurring_todo.todos @number_of_todos = @todos.size @@ -186,7 +186,6 @@ class RecurringTodosController < ApplicationController @completed_remaining = current_user.recurring_todos.completed.count # from completed back to active -> check if there is an active todo - # current_user.todos.count(:all, {:conditions => ["state = ? AND recurring_todo_id = ?", 'active',params[:id]]}) @active_todos = @recurring_todo.todos.active.count # create todo if there is no active todo belonging to the activated # recurring_todo diff --git a/app/controllers/todos_controller.rb b/app/controllers/todos_controller.rb index 887acba6..72179169 100644 --- a/app/controllers/todos_controller.rb +++ b/app/controllers/todos_controller.rb @@ -282,15 +282,11 @@ class TodosController < ApplicationController # check if this todo has a related recurring_todo. If so, create next todo @new_recurring_todo = check_for_next_todo(@todo) if @saved - if @todo.completed? - @pending_to_activate = @todo.pending_to_activate - @pending_to_activate.each do |t| - t.activate! - end - else - @active_to_block = @todo.active_to_block - @active_to_block.each do |t| - t.block! + if @saved + if @todo.completed? + @pending_to_activate = @todo.activate_pending_todos + else + @active_to_block = @todo.block_successors end end @@ -1214,17 +1210,13 @@ class TodosController < ApplicationController def update_completed_state if params['done'] == '1' && !@todo.completed? @todo.complete! - @todo.pending_to_activate.each do |t| - t.activate! - end + @todo.activate_pending_todos end # strange. if checkbox is not checked, there is no 'done' in params. # Therefore I've used the negation if !(params['done'] == '1') && @todo.completed? @todo.activate! - @todo.active_to_block.each do |t| - t.block! - end + @todo.block_successors end end diff --git a/app/models/recurring_todo.rb b/app/models/recurring_todo.rb index e46ad36e..e1ff6003 100644 --- a/app/models/recurring_todo.rb +++ b/app/models/recurring_todo.rb @@ -702,13 +702,7 @@ class RecurringTodo < ActiveRecord::Base end def toggle_completion! - saved = false - if completed? - saved = activate! - else - saved = complete! - end - return saved + return completed? ? activate! : complete! end def toggle_star! diff --git a/app/models/todo.rb b/app/models/todo.rb index 49ecbe2a..382d8588 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -210,13 +210,7 @@ class Todo < ActiveRecord::Base end def toggle_completion! - saved = false - if completed? - saved = activate! - else - saved = complete! - end - return saved + return completed? ? activate! : complete! end def show_from @@ -287,14 +281,18 @@ class Todo < ActiveRecord::Base @predecessor_array << t end - # Return todos that should be activated if the current todo is completed - def pending_to_activate - return successors.find_all {|t| t.uncompleted_predecessors.empty?} + # activate todos that should be activated if the current todo is completed + def activate_pending_todos + pending_todos = successors.find_all {|t| t.uncompleted_predecessors.empty?} + pending_todos.each {|t| t.activate! } + return pending_todos end # Return todos that should be blocked if the current todo is undone - def active_to_block - return successors.find_all {|t| t.active? or t.deferred?} + def block_successors + active_successors = successors.find_all {|t| t.active? or t.deferred?} + active_successors.each {|t| t.block!} + return active_successors end def raw_notes=(value) diff --git a/app/views/recurring_todos/destroy.js.erb b/app/views/recurring_todos/destroy.js.erb index b6462ed9..4a692eb5 100644 --- a/app/views/recurring_todos/destroy.js.erb +++ b/app/views/recurring_todos/destroy.js.erb @@ -1,6 +1,6 @@ <%- if @saved -%> show_empty_messages(); - TracksPages.page_notify('notice', '<%= escape_javascript(t('todos.recurring_deleted_success') + t(:todo_removed, :count => @number_of_todos)) %>', 5); + TracksPages.page_notify('notice', '<%= escape_javascript(t('todos.recurring_deleted_success') + t('todos.recurring_pattern_removed', :count => pluralize(@number_of_todos,t('common.todo')))) %>', 5); remove_recurring_todo_from_page(); <%- else -%> TracksPages.page_notify('error', '<%= t('todos.error_deleting_recurring', :description => @recurring_todo.description) %>', 8); diff --git a/app/views/todos/toggle_check.js.erb b/app/views/todos/toggle_check.js.erb index e793cf84..6a170544 100644 --- a/app/views/todos/toggle_check.js.erb +++ b/app/views/todos/toggle_check.js.erb @@ -12,7 +12,7 @@ animation << "activate_pending_todos" animation << "remove_source_container" else - animation << "add_todo_to_context" + animation << "add_todo_to_context" unless source_view_is(:done) animation << "block_predecessors" end animation << "update_empty_container" if source_view_is_one_of(:tag, :todo) -%> @@ -161,7 +161,7 @@ function remove_source_container(next_steps) { } function html_for_todo() { - return "<%= @saved ? escape_javascript(render(:partial => @todo, :locals => { + return "<%= @saved && !source_view_is(:done) ? escape_javascript(render(:partial => @todo, :locals => { :parent_container_type => parent_container_type, :suppress_project => source_view_is(:project), :suppress_context => source_view_is(:context) diff --git a/config/locales/en.yml b/config/locales/en.yml index b83c08c6..e1423a24 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -70,6 +70,7 @@ en: context: "Context" action: "Action" actions: "Actions" + todo: 'todo' server_error: "An error occurred on the server." contexts: "Contexts" numbered_step: "Step %{number}" @@ -485,6 +486,7 @@ en: no_completed_recurring: "Currently there are no completed recurring todos" add_new_recurring: "Add a new recurring action" recurring_deleted_success: "The recurring action was deleted succesfully." + recurring_pattern_removed: "The recurrence pattern is removed from %{count}" deleted_success: "The action was deleted succesfully." error_deleting_recurring: "There was an error deleting the recurring todo \'%{description}\'" error_saving_recurring: "There was an error saving the recurring todo \'%{description}\'" diff --git a/db/migrate/20110621082432_make_old_recurring_todos_validate.rb b/db/migrate/20110621082432_make_old_recurring_todos_validate.rb new file mode 100644 index 00000000..51e06d05 --- /dev/null +++ b/db/migrate/20110621082432_make_old_recurring_todos_validate.rb @@ -0,0 +1,15 @@ +class MakeOldRecurringTodosValidate < ActiveRecord::Migration + def self.up + RecurringTodo.find(:all).each do |rt| + # show_always may not be nil + rt.show_always = false if rt.show_always.nil? + # start date should be filled + rt.start_from = rt.created_at if rt.start_from.nil? || rt.start_from.blank? + rt.save! + end + end + + def self.down + # no down: leave them validatable + end +end diff --git a/features/step_definitions/container_steps.rb b/features/step_definitions/container_steps.rb index a740c08a..b3228384 100644 --- a/features/step_definitions/container_steps.rb +++ b/features/step_definitions/container_steps.rb @@ -71,4 +71,23 @@ Then /^I should see "([^"]*)" in project container for "([^"]*)"$/ do |todo_desc selenium.wait_for_element("xpath=#{xpath}", :timeout_in_seconds => 5) selenium.is_visible(xpath).should be_true -end \ No newline at end of file +end + +Then /^I should see "([^"]*)" in the active recurring todos container$/ do |repeat_pattern| + repeat = @current_user.recurring_todos.find_by_description(repeat_pattern) + repeat.should_not be_nil + + xpath = "//div[@id='active_recurring_todos_container']//div[@id='recurring_todo_#{repeat.id}']" + selenium.wait_for_element("xpath=#{xpath}", :timeout_in_seconds => 5) + selenium.is_visible(xpath).should be_true +end + +Then /^I should not see "([^"]*)" in the completed recurring todos container$/ do |repeat_pattern| + repeat = @current_user.recurring_todos.find_by_description(repeat_pattern) + repeat.should_not be_nil + + xpath = "//div[@id='completed_recurring_todos_container']//div[@id='recurring_todo_#{repeat.id}']" + selenium.wait_for_element("xpath=#{xpath}", :timeout_in_seconds => 5) + selenium.is_visible(xpath).should be_true +end + diff --git a/features/view_done.feature b/features/view_done.feature index 9642a9d6..35fc4a44 100644 --- a/features/view_done.feature +++ b/features/view_done.feature @@ -110,3 +110,31 @@ Feature: Show done When I follow "2" Then I should be on the done recurring todos page And the page should be "2" + + @selenium + Scenario: I can toggle a done recurring todo active from done page + Given I have a completed repeat pattern "test" + When I go to the done recurring todos page + Then I should see "test" + When I mark the pattern "test" as active + Then I should not see "test" in the completed recurring todos container + When I go to the recurring todos page + Then I should see "test" in the active recurring todos container + + Scenario: I can delete a recurring todo from the done page + Given this scenario is pending + + Scenario: I can toggle a todo active from the done page + Given this scenario is pending + + Scenario: I can toggle a todo active from the all done page + Given this scenario is pending + + Scenario: I can toggle a todo active from the project done page + Given this scenario is pending + + Scenario: I can toggle a todo active from the context done page + Given this scenario is pending + + Scenario: I can edit a project to active from the project done page + Given this scenario is pending