From fa98c0865e638d28ea69ef36fd859ffb41ac66df Mon Sep 17 00:00:00 2001 From: Eric Allen Date: Sat, 8 May 2010 19:30:58 -0400 Subject: [PATCH 1/4] Override AASM's initial state if specified Fixes #977 --- app/controllers/todos_controller.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/app/controllers/todos_controller.rb b/app/controllers/todos_controller.rb index 7d203e59..480744fb 100644 --- a/app/controllers/todos_controller.rb +++ b/app/controllers/todos_controller.rb @@ -55,7 +55,7 @@ class TodosController < ApplicationController predecessor_list = p.predecessor_list @todo = current_user.todos.build(p.attributes) - + if p.project_specified_by_name? project = current_user.projects.find_or_create_by_name(p.project_name) @new_project_created = project.new_record_before_save? @@ -70,8 +70,16 @@ class TodosController < ApplicationController end @todo.add_predecessor_list(predecessor_list) + + # Fix for #977 because AASM overrides @state on creation + specified_state = @todo.state + @todo.update_state_from_project @saved = @todo.save + + # Fix for #977 because AASM overrides @state on creation + @todo.update_attribute('state', specified_state) unless specified_state == "immediate" + unless (@saved == false) || tag_list.blank? @todo.tag_with(tag_list) @todo.tags.reload From 86d7724b75be46a0ada3301c252960002372ff31 Mon Sep 17 00:00:00 2001 From: Eric Allen Date: Sun, 9 May 2010 18:57:56 -0400 Subject: [PATCH 2/4] Revert "Move dependency drop target into image" We decided that the small drop target was harder to hit, and the justifcation for the change wasn't worth it. This reverts commit ec68e04f2723682c5d4ab1aa08dbeb49cacf15ad. Conflicts: app/helpers/todos_helper.rb public/javascripts/application.js public/stylesheets/standard.css --- app/helpers/todos_helper.rb | 6 +- artwork/add_successor_on.svg | 179 ---------------------------- artwork/predecessor.svg | 175 --------------------------- public/images/add_successor_off.png | Bin 801 -> 0 bytes public/images/add_successor_on.png | Bin 794 -> 0 bytes public/javascripts/application.js | 4 +- public/stylesheets/standard.css | 13 +- 7 files changed, 7 insertions(+), 370 deletions(-) delete mode 100644 artwork/add_successor_on.svg delete mode 100644 artwork/predecessor.svg delete mode 100644 public/images/add_successor_off.png delete mode 100644 public/images/add_successor_on.png diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb index 994fb305..b8ea341e 100644 --- a/app/helpers/todos_helper.rb +++ b/app/helpers/todos_helper.rb @@ -119,10 +119,8 @@ module TodosHelper def grip_span unless @todo.completed? image_tag('grip.png', :width => '7', :height => '16', :border => '0', - :title => 'Drag onto another action to make it depend on that action', - :class => 'grip') + - image_tag('blank.png', :width => 16, :height => 16, :border => 0, - :title => "Drop an action to make it depend on this action", :class => 'successor_target drop_target') + :title => 'Drag onto another action to make it depend on that action', + :class => 'grip') end end diff --git a/artwork/add_successor_on.svg b/artwork/add_successor_on.svg deleted file mode 100644 index b7b3165c..00000000 --- a/artwork/add_successor_on.svg +++ /dev/null @@ -1,179 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - image/svg+xml - - - - - - - - - - - diff --git a/artwork/predecessor.svg b/artwork/predecessor.svg deleted file mode 100644 index 75e1ea1a..00000000 --- a/artwork/predecessor.svg +++ /dev/null @@ -1,175 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - image/svg+xml - - - - - - - - - - - diff --git a/public/images/add_successor_off.png b/public/images/add_successor_off.png deleted file mode 100644 index 183f1326da59fb9ce8dfc4de71f365f9c1e3dad6..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 801 zcmV++1K#|JP)A_TU&dEb1w4v{O4RQ_k3<{?vtzz{)F`O^mzOG z`)>>k4BU>#BFA>^5qk^um1XlS?|jYcmjiXs;m7n5siYcEaH z+*VcfLM#?bjE#*&L!nSO6bfCQo}PXKU=skSq9|t_$Eg!SuuLZNdVYTXk!4vK0GOSf zeZRG}b-%T>H5v|w&qX4U#uKqzF4q}`0o%5-rfGgH6biZOAE{I-RbOBK&Fl3>6-5bh z&RYl}@1&GAWsDJB*HJ7MA%qYB>c+>%uc@kf#qD+n7-KC52L~0-xt}pMGdVeVX<}kR ztFbH#x~?Oe%}UcWT~f-NX__AcfqW83NQ=< zLI~Kl4MK=ZO1V8aIC!^MEYjxY=Bs|cADnZf)9H=f-Q9-^3kz>4V+^{kqg*cI_|jw6 z*Vi|PhKBA4A!uV`Zzqt3ASzjS`MtPt|k+S#Jx-=V-iBX zD2no8e}A6?K#gTtFbo5^Tn?Ob{K@k2@`uC2!$`b`(W1 f(&@DPuLa--f+vKa3jVpwx}gdBNaMgZL5u;Go3lJoHJ+6nX`N@YQ)ln-}Uo* zlPB*(gb)CL#oG?Hnx4wNrl)#|&Al#GQVmy8e&sb&mtS1J_!8yd56Gsrw#xY2b$jIC z#=~-O^#apd1=s)>;3{TO_2t=U&1ld0Dd~6TdhbUQWs!etj2aQgm$ngD>N-N&E% zl7lDosS88J$~iNN)uH;bNU3djBs~jvek>zt$q$iOAzSX@)?&l{ne98GSt^H+X90?Msh1*A=)5X%MFb=4d}3 z-1qe1o7L+!A7PfxHGe~xl!}9XW#r84SkJAqLYSOc1#m45I3}oG_;nn3{`lU&lJy%g1;6Xg+aZ$A+aRpFFvd`MDfgQo)1+ z2pi1S@h3tE8xte^4bDm`h-E+sXX3!ZmcQi5p#PPc1TIlJ?g)EW5HU!FBYz#L5dYSI Y?{CsPln#@-BLDyZ07*qoM6N<$f&*G*_W%F@ diff --git a/public/javascripts/application.js b/public/javascripts/application.js index cfe11d2e..4b025f51 100644 --- a/public/javascripts/application.js +++ b/public/javascripts/application.js @@ -229,7 +229,7 @@ function enable_rich_interaction(){ /* Drag & Drop for successor/predecessor */ function drop_todo(evt, ui) { dragged_todo = ui.draggable[0].id.split('_')[2]; - dropped_todo = $(this).parents('.item-show').get(0).id.split('_')[2]; + dropped_todo = this.id.split('_')[2]; ui.draggable.remove(); $(this).block({message: null}); $.post(relative_to_root('todos/add_predecessor'), @@ -247,7 +247,7 @@ function enable_rich_interaction(){ start: drag_todo, stop: function() {$('.drop_target').hide();}}); - $('.successor_target').droppable({drop: drop_todo, + $('.item-show').droppable({drop: drop_todo, tolerance: 'pointer', hoverClass: 'hover'}); diff --git a/public/stylesheets/standard.css b/public/stylesheets/standard.css index d214f861..7d2963cc 100644 --- a/public/stylesheets/standard.css +++ b/public/stylesheets/standard.css @@ -943,16 +943,9 @@ div.message { display:none; } -.successor_target { - background-image:url("../images/add_successor_off.png"); - background-repeat: no-repeat; - background-position: center right; -} - -.successor_target.hover { - background-image:url("../images/add_successor_on.png"); - background-repeat: no-repeat; - background-position: center right; +.hover { + background: #EAEAEA; + font-weight: bold; } .context_target { From 92bb54bbf572983aef2a57272fba688fbcc5542c Mon Sep 17 00:00:00 2001 From: Eric Allen Date: Sun, 9 May 2010 18:59:02 -0400 Subject: [PATCH 3/4] Revert "fix cucumber story for drag and drop for dependencies. Was broken since last change of drop target to a hidden img that appears when dragging starts" This reverts commit 8dbf790810e8f2b91e6f20d3897d9f32fb801ef7. Conflicts: app/helpers/todos_helper.rb features/step_definitions/todo_steps.rb --- features/step_definitions/todo_steps.rb | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/features/step_definitions/todo_steps.rb b/features/step_definitions/todo_steps.rb index b2bd9d24..54fec93c 100644 --- a/features/step_definitions/todo_steps.rb +++ b/features/step_definitions/todo_steps.rb @@ -37,15 +37,8 @@ When /^I drag "(.*)" to "(.*)"$/ do |dragged, target| drag_id = Todo.find_by_description(dragged).id drop_id = Todo.find_by_description(target).id drag_name = "xpath=//div[@id='line_todo_#{drag_id}']//img[@class='grip']" - # xpath does not seem to work here... reverting to css - # xpath=//div[@id='line_todo_#{drop_id}']//img[@class='successor_target'] - drop_name = "css=div#line_todo_#{drop_id} img.successor_target" - - # HACK: the target img is hidden until drag starts. We need to show the img or the - # xpath will not find it - js="$('div#line_todo_#{drop_id} img.successor_target').show();" - selenium.get_eval "(function() {with(this) {#{js}}}).call(selenium.browserbot.getCurrentWindow());" - + drop_name = "xpath=//div[@id='line_todo_#{drop_id}']//div[@class='description']" + selenium.drag_and_drop_to_object(drag_name, drop_name) arrow = "xpath=//div[@id='line_todo_#{drop_id}']/div/a[@class='show_successors']/img" From cfc6d117b815c43eccdc65168e0370c23a658adc Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Thu, 13 May 2010 18:24:26 +0200 Subject: [PATCH 4/4] fix #1027. Several tests were broken because of the more strict validations on the recurring_todo model --- app/models/recurring_todo.rb | 1 - test/fixtures/recurring_todos.yml | 14 +++++++++----- test/functional/login_controller_test.rb | 2 +- test/functional/recurring_todos_controller_test.rb | 5 +++++ test/functional/todos_controller_test.rb | 8 ++++++-- test/functional/users_controller_test.rb | 8 ++++---- 6 files changed, 25 insertions(+), 13 deletions(-) diff --git a/app/models/recurring_todo.rb b/app/models/recurring_todo.rb index 2f6d623b..4a916c6d 100644 --- a/app/models/recurring_todo.rb +++ b/app/models/recurring_todo.rb @@ -30,7 +30,6 @@ class RecurringTodo < ActiveRecord::Base validates_presence_of :description validates_presence_of :recurring_period validates_presence_of :target - validates_presence_of :recurring_period validates_presence_of :ends_on validates_presence_of :context diff --git a/test/fixtures/recurring_todos.yml b/test/fixtures/recurring_todos.yml index db630d68..9597d674 100644 --- a/test/fixtures/recurring_todos.yml +++ b/test/fixtures/recurring_todos.yml @@ -1,4 +1,3 @@ -<% def today Time.zone.now.beginning_of_day.to_s(:db) @@ -30,7 +29,7 @@ call_bill_gates_every_day: description: Call Bill Gates every day notes: ~ state: active - start_from: ~ + start_from: <%= last_week %> ends_on: no_end_date end_date: ~ number_of_occurences: ~ @@ -38,6 +37,7 @@ call_bill_gates_every_day: show_from_delta: ~ recurring_period: daily recurrence_selector: ~ + show_always: 0 every_other1: 1 every_other2: ~ every_other3: ~ @@ -62,6 +62,7 @@ call_bill_gates_every_workday: number_of_occurences: ~ target: due_date show_from_delta: ~ + show_always: 0 recurring_period: daily recurrence_selector: ~ every_other1: 1 @@ -82,7 +83,7 @@ call_bill_gates_every_week: description: Call Bill Gates every week notes: ~ state: active - start_from: ~ + start_from: <%= today %> ends_on: no_end_date end_date: ~ number_of_occurences: ~ @@ -90,6 +91,7 @@ call_bill_gates_every_week: show_from_delta: ~ recurring_period: weekly recurrence_selector: ~ + show_always: 0 every_other1: 2 every_other2: ~ every_other3: ~ @@ -108,7 +110,7 @@ check_with_bill_every_last_friday_of_month: description: Check with Bill every last friday of the month notes: ~ state: active - start_from: ~ + start_from: <%= today %> ends_on: no_end_date end_date: ~ number_of_occurences: ~ @@ -116,6 +118,7 @@ check_with_bill_every_last_friday_of_month: show_from_delta: 5 recurring_period: monthly recurrence_selector: 1 + show_always: 0 every_other1: 1 every_other2: 2 every_other3: 5 @@ -134,12 +137,13 @@ birthday_reinier: description: Congratulate Reinier on his birthday notes: ~ state: active - start_from: ~ + start_from: <%= today %> ends_on: no_end_date end_date: ~ number_of_occurences: ~ target: due_date show_from_delta: 5 + show_always: 0 recurring_period: yearly recurrence_selector: 0 every_other1: 8 diff --git a/test/functional/login_controller_test.rb b/test/functional/login_controller_test.rb index 50da6ae8..051bb744 100644 --- a/test/functional/login_controller_test.rb +++ b/test/functional/login_controller_test.rb @@ -51,7 +51,7 @@ class LoginControllerTest < ActionController::TestCase def test_login_with_no_users_redirects_to_signup User.delete_all get :login - assert_redirected_to :controller => 'users', :action => 'new' + assert_redirected_to signup_url end def test_logout diff --git a/test/functional/recurring_todos_controller_test.rb b/test/functional/recurring_todos_controller_test.rb index 394e72d7..ebe64ed0 100644 --- a/test/functional/recurring_todos_controller_test.rb +++ b/test/functional/recurring_todos_controller_test.rb @@ -50,6 +50,7 @@ class RecurringTodosControllerTest < ActionController::TestCase "recurring_period"=>"yearly", "recurring_show_days_before"=>"10", "recurring_target"=>"due_date", + "recurring_show_always" => "1", "start_from"=>"18/08/2008", "weekly_every_x_week"=>"1", "weekly_return_monday"=>"m", @@ -110,6 +111,9 @@ class RecurringTodosControllerTest < ActionController::TestCase @yearly.every_other1 = target_date.day @yearly.every_other2 = target_date.month @yearly.show_from_delta = 10 +# unless @yearly.valid? +# @yearly.errors.each {|obj, error| puts error} +# end assert @yearly.save # toggle twice to force generation of new todo @@ -155,6 +159,7 @@ class RecurringTodosControllerTest < ActionController::TestCase "recurring_period"=>"yearly", "recurring_show_days_before"=>"0", "recurring_target"=>"due_date", + "recurring_show_always" => "1", "start_from"=>"1/10/2012", # adjust after 2012 "weekly_every_x_week"=>"1", "weekly_return_monday"=>"w", diff --git a/test/functional/todos_controller_test.rb b/test/functional/todos_controller_test.rb index 0035f291..ec93e728 100644 --- a/test/functional/todos_controller_test.rb +++ b/test/functional/todos_controller_test.rb @@ -376,12 +376,16 @@ class TodosControllerTest < ActionController::TestCase # change recurrence pattern to monthly and set show_from 2 days before due # date this forces the next todo to be put in the tickler recurring_todo_1.show_from_delta = 2 + recurring_todo_1.show_always = 0 + recurring_todo_1.target = 'due_date' recurring_todo_1.recurring_period = 'monthly' recurring_todo_1.recurrence_selector = 0 recurring_todo_1.every_other1 = 1 recurring_todo_1.every_other2 = 2 recurring_todo_1.every_other3 = 5 - recurring_todo_1.save + # use assert to catch validation errors if present. we need to replace + # this with a good factory implementation + assert recurring_todo_1.save # mark next_todo as complete by toggle_check xhr :post, :toggle_check, :id => next_todo.id, :_source_view => 'todo' @@ -416,7 +420,7 @@ class TodosControllerTest < ActionController::TestCase recurring_todo_1.recurrence_selector = 0 recurring_todo_1.every_other1 = today.day recurring_todo_1.every_other2 = 1 - recurring_todo_1.save + assert recurring_todo_1.save # mark todo_1 as complete by toggle_check, this gets rid of todo_1 that was # not correctly created from the adjusted recurring pattern we defined diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb index e53942cc..e20d9095 100644 --- a/test/functional/users_controller_test.rb +++ b/test/functional/users_controller_test.rb @@ -154,19 +154,19 @@ class UsersControllerTest < ActionController::TestCase def test_create_with_invalid_password_redirects_to_new_user_page login_as :admin_user post :create, :user => {:login => 'newbie', :password => '', :password_confirmation => ''} - assert_redirected_to :controller => 'users', :action => 'new' + assert_redirected_to signup_path end def test_create_with_invalid_login_does_not_add_a_new_user login_as :admin_user post :create, :user => {:login => 'n', :password => 'newbiepass', :password_confirmation => 'newbiepass'} - assert_redirected_to :controller => 'users', :action => 'new' + assert_redirected_to signup_path end def test_create_with_invalid_login_redirects_to_new_user_page login_as :admin_user post :create, :user => {:login => 'n', :password => 'newbiepass', :password_confirmation => 'newbiepass'} - assert_redirected_to :controller => 'users', :action => 'new' + assert_redirected_to signup_path end def test_create_with_duplicate_login_does_not_add_a_new_user @@ -179,7 +179,7 @@ class UsersControllerTest < ActionController::TestCase def test_create_with_duplicate_login_redirects_to_new_user_page login_as :admin_user post :create, :user => {:login => 'jane', :password => 'newbiepass', :password_confirmation => 'newbiepass'} - assert_redirected_to :controller => 'users', :action => 'new' + assert_redirected_to signup_path end end