From c3b3e3ea0449a31b55ab00329c7383a73e07809b Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Sun, 4 Apr 2010 18:20:07 +0200 Subject: [PATCH 01/16] Manual apply fix from 1.7_branch. Preserve database integrity for recurring todos when deleting project or context. Fixes #880. Fixes #895 --- app/controllers/projects_controller.rb | 1 + app/models/context.rb | 1 + app/models/project.rb | 1 + app/models/recurring_todo.rb | 5 +++++ 4 files changed, 8 insertions(+) diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 2baffa1a..2364ea97 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -163,6 +163,7 @@ class ProjectsController < ApplicationController end def destroy + @project.recurring_todos.each {|rt| rt.remove_from_project!} @project.destroy @active_projects_count = current_user.projects.active.count @hidden_projects_count = current_user.projects.hidden.count diff --git a/app/models/context.rb b/app/models/context.rb index 41d27a53..a7d79f72 100644 --- a/app/models/context.rb +++ b/app/models/context.rb @@ -1,6 +1,7 @@ class Context < ActiveRecord::Base has_many :todos, :dependent => :delete_all, :include => :project, :order => "todos.completed_at DESC" + has_many :recurring_todos, :dependent => :delete_all belongs_to :user named_scope :active, :conditions => { :hide => false } diff --git a/app/models/project.rb b/app/models/project.rb index 88053a1c..20a75f97 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -27,6 +27,7 @@ class Project < ActiveRecord::Base :order => "show_from" has_many :notes, :dependent => :delete_all, :order => "created_at DESC" + has_many :recurring_todos belongs_to :default_context, :class_name => "Context", :foreign_key => "default_context_id" belongs_to :user diff --git a/app/models/recurring_todo.rb b/app/models/recurring_todo.rb index 353ce0ca..23541dbc 100644 --- a/app/models/recurring_todo.rb +++ b/app/models/recurring_todo.rb @@ -639,6 +639,11 @@ class RecurringTodo < ActiveRecord::Base starred? end + def remove_from_project! + self.project = nil + self.save + end + def inc_occurences self.occurences_count += 1 self.save From fdba48c76991aa6efcf6963cabafedf5ea41bb8a Mon Sep 17 00:00:00 2001 From: Eric Allen Date: Sun, 4 Apr 2010 22:30:04 -0400 Subject: [PATCH 02/16] AJAX loading of context names To make askIfNewContextProvided work again, allowing people to add actions again --- public/javascripts/application.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/public/javascripts/application.js b/public/javascripts/application.js index 522081d8..7aab4d4c 100644 --- a/public/javascripts/application.js +++ b/public/javascripts/application.js @@ -141,6 +141,17 @@ function setup_container_toggles(){ function askIfNewContextProvided() { var givenContextName = $('#todo_context_name').val(); + var contextNames = []; + var contextNamesRequest = $.ajax({url: relative_to_root('contexts.autocomplete'), + async: false, + dataType: "text", + data: "q="+givenContextName, + success: function(result){ + lines = result.split("\n"); + for(var i = 0; i < lines.length; i++){ + contextNames.push(lines[i].split("|")[0]); + } + }}); if (givenContextName.length == 0) return true; // do nothing and depend on rails validation error for (var i = 0; i < contextNames.length; ++i) { if (contextNames[i] == givenContextName) return true; From 68701adaca7648dff8cbcc756adffa3dafa5a835 Mon Sep 17 00:00:00 2001 From: Eric Allen Date: Wed, 7 Apr 2010 10:06:46 -0400 Subject: [PATCH 03/16] Sanitize output well, but entity-ize < and > in notes Coming from a rich message or API call, notes can contain HTML and it will render to the browser. Coming from a normal todo creation, though, all < and > characters will be replaced with the corresponding entities. This preserves HTML emails, but prevents users from breaking the layout by entering broken HTML for todo notes. Closes #765 --- app/helpers/application_helper.rb | 8 ++++++-- app/models/todo.rb | 12 ++++++++++-- config/environment.rb | 1 + 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 76622c50..b4561f80 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -202,11 +202,15 @@ module ApplicationHelper end def format_note(note) - note.gsub!(//, '>') note = markdown(note) note = auto_link_message(note) note = auto_link(note) + + # add onenote and message protocols + Sanitize::Config::RELAXED[:protocols]['a']['href'] << 'onenote' + Sanitize::Config::RELAXED[:protocols]['a']['href'] << 'message' + + note = Sanitize.clean(note, Sanitize::Config::RELAXED) return note end end diff --git a/app/models/todo.rb b/app/models/todo.rb index 82c19cb2..38580d3f 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -286,7 +286,15 @@ class Todo < ActiveRecord::Base def active_to_block return successors.find_all {|t| t.active? or t.deferred?} end - + + def notes=(value) + super(value.gsub(//, '>')) + end + + def raw_notes=(value) + self[:notes] = value + end + # Rich Todo API def self.from_rich_message(user, default_context_id, description, notes) @@ -324,7 +332,7 @@ class Todo < ActiveRecord::Base todo = user.todos.build todo.description = description - todo.notes = notes + todo.raw_notes = notes todo.context_id = context_id todo.project_id = project_id unless project_id.nil? return todo diff --git a/config/environment.rb b/config/environment.rb index 0b26e5eb..981ebeb2 100644 --- a/config/environment.rb +++ b/config/environment.rb @@ -23,6 +23,7 @@ Rails::Initializer.run do |config| config.gem "RedCloth" config.gem "soap4r", :lib => false config.gem 'datanoise-actionwebservice', :lib => 'actionwebservice' + config.gem 'sanitize' config.action_controller.use_accept_header = true From a022f449c16c822446e5c2eacbf23667fcf07f8e Mon Sep 17 00:00:00 2001 From: Eric Allen Date: Fri, 9 Apr 2010 09:34:57 -0400 Subject: [PATCH 04/16] Fixed failing tests - Some fixtures added by Erik Ordway broke tests - message:// links had an edge case I broke with 68701ada - One test had a hard-coded id that changed Closes #1019 --- app/helpers/application_helper.rb | 6 +++--- app/models/todo.rb | 2 +- spec/models/message_gateway_spec.rb | 2 +- test/fixtures/contexts.yml | 18 ------------------ test/fixtures/preferences.yml | 18 ------------------ test/functional/todos_controller_test.rb | 3 +-- test/functional/users_controller_test.rb | 5 +++-- 7 files changed, 9 insertions(+), 45 deletions(-) diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index b4561f80..a6b51172 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -196,15 +196,15 @@ module ApplicationHelper # do not change string; URL is alreay linked href else - content_tag(:a, h(href), :href => h(href)) + content = content_tag(:a, h(href), :href => h(href)) end end end def format_note(note) - note = markdown(note) note = auto_link_message(note) - note = auto_link(note) + note = markdown(note) + note = auto_link(note, :link => :urls) # add onenote and message protocols Sanitize::Config::RELAXED[:protocols]['a']['href'] << 'onenote' diff --git a/app/models/todo.rb b/app/models/todo.rb index 38580d3f..7fd64628 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -288,7 +288,7 @@ class Todo < ActiveRecord::Base end def notes=(value) - super(value.gsub(//, '>')) + super(value.try(:gsub, //, '>')) end def raw_notes=(value) diff --git a/spec/models/message_gateway_spec.rb b/spec/models/message_gateway_spec.rb index 2f0f5efc..188093ee 100644 --- a/spec/models/message_gateway_spec.rb +++ b/spec/models/message_gateway_spec.rb @@ -2,7 +2,7 @@ require File.dirname(__FILE__) + '/../spec_helper' describe MessageGateway do before :each do - todo = mock_model(Todo, :description= => nil, :notes= => nil, :context_id= => nil, :save! => nil) + todo = mock_model(Todo, :description= => nil, :raw_notes= => nil, :context_id= => nil, :save! => nil) @user = mock_model(User, :prefs => mock_model(Preference, :sms_context => mock_model(Context)), diff --git a/test/fixtures/contexts.yml b/test/fixtures/contexts.yml index 87ea5dba..49ce9d2f 100644 --- a/test/fixtures/contexts.yml +++ b/test/fixtures/contexts.yml @@ -131,21 +131,3 @@ anothercontext: user_id: 4 created_at: <%= today %> updated_at: <%= today %> - -inbox: - id: 15 - name: Inbox - position: 1 - hide: false - user_id: 5 - created_at: <%= today %> - updated_at: <%= today %> - -anothercontext: - id: 16 - name: anothercontext - position: 2 - hide: false - user_id: 5 - created_at: <%= today %> - updated_at: <%= today %> \ No newline at end of file diff --git a/test/fixtures/preferences.yml b/test/fixtures/preferences.yml index 77129787..b047f8d6 100644 --- a/test/fixtures/preferences.yml +++ b/test/fixtures/preferences.yml @@ -54,21 +54,3 @@ sms_user_prefs: show_project_on_todo_done: true sms_email: 5555555555@tmomail.net sms_context_id: 13 - -other_user_prefs: - id: 4 - user_id: 5 - staleness_starts: 7 - date_format: "%d/%m/%Y" - title_date_format: "%A, %d %B %Y" - show_number_completed: 5 - show_completed_projects_in_sidebar: true - show_hidden_contexts_in_sidebar: true - show_hidden_projects_in_sidebar: true - admin_email: butshesagirl@rousette.org.uk - week_starts: 1 - due_style: 0 - refresh: 0 - time_zone: "Los_Angeles" - verbose_action_descriptors: false - show_project_on_todo_done: true diff --git a/test/functional/todos_controller_test.rb b/test/functional/todos_controller_test.rb index 855b8397..0035f291 100644 --- a/test/functional/todos_controller_test.rb +++ b/test/functional/todos_controller_test.rb @@ -542,10 +542,9 @@ class TodosControllerTest < ActionController::TestCase def test_format_note_link_message login_as(:admin_user) todo = users(:admin_user).todos.first - todo.notes = "A Mail.app message:// link" + todo.raw_notes = "A Mail.app message:// link" todo.save! get :index - # puts css_select("div#notes_todo_#{todo.id}") assert_select("div#notes_todo_#{todo.id}", 'A Mail.app message://<ABCDEF-GHADB-123455-FOO-BAR@example.com> link') assert_select("div#notes_todo_#{todo.id} a", 'message://<ABCDEF-GHADB-123455-FOO-BAR@example.com>') assert_select("div#notes_todo_#{todo.id} a[href=message://<ABCDEF-GHADB-123455-FOO-BAR@example.com>]", 'message://<ABCDEF-GHADB-123455-FOO-BAR@example.com>') diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb index 9b03043c..e53942cc 100644 --- a/test/functional/users_controller_test.rb +++ b/test/functional/users_controller_test.rb @@ -52,8 +52,9 @@ class UsersControllerTest < ActionController::TestCase def test_destroy_user login_as :admin_user @no_users_before = User.find(:all).size - xhr :post, :destroy, :id => users(:ldap_user).id.to_param - assert_rjs :page, "user-3", :remove + user_id = users(:ldap_user).id + xhr :post, :destroy, :id => user_id.to_param + assert_rjs :page, "user-#{user_id}", :remove assert_equal @no_users_before-1, User.find(:all).size end From 6e65435895a253f044a0b49560c0a64a66236789 Mon Sep 17 00:00:00 2001 From: Eric Allen Date: Sat, 10 Apr 2010 15:11:59 -0400 Subject: [PATCH 05/16] Fix failing tests of context deletion The notice causes the test to think the context name is still displayed. Fixed by looking for the name plus an opening parenthesis, which doesn't occur in the notice. --- app/views/contexts/_context_listing.rhtml | 3 ++- features/step_definitions/context_steps.rb | 8 +++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/app/views/contexts/_context_listing.rhtml b/app/views/contexts/_context_listing.rhtml index f8c80b48..df90a138 100644 --- a/app/views/contexts/_context_listing.rhtml +++ b/app/views/contexts/_context_listing.rhtml @@ -35,7 +35,8 @@ :method => 'get', :with => "'_source_view=#{@source_view}'", :before => "$('#{dom_id(context)}').block({message:null});", - :complete => "$('#{dom_id(context)}').unblock();" + :complete => "$('#{dom_id(context)}').unblock();", + :html => {:id => "edit_context_#{context.id}_link"} ) %> diff --git a/features/step_definitions/context_steps.rb b/features/step_definitions/context_steps.rb index 58813a63..9bab68a5 100644 --- a/features/step_definitions/context_steps.rb +++ b/features/step_definitions/context_steps.rb @@ -23,7 +23,7 @@ Then /^he should see that a context named "([^\"]*)" is present$/ do |context_na end Then /^he should see that a context named "([^\"]*)" is not present$/ do |context_name| - Then "I should not see \"#{context_name}\"" + Then "I should not see \"#{context_name} (\"" end Given /^I have a context "([^\"]*)" with (.*) actions$/ do |context_name, number_of_actions| @@ -38,10 +38,16 @@ When /^I delete the context "([^\"]*)"$/ do |context_name| context.should_not be_nil click_link "delete_context_#{context.id}" selenium.get_confirmation.should == "Are you sure that you want to delete the context '#{context_name}'?" + wait_for do + !selenium.is_element_present("delete_context_#{context.id}") + end end When /^I edit the context to rename it to "([^\"]*)"$/ do |new_name| click_link "edit_context_#{@context.id}" fill_in "context_name", :with => new_name click_button "submit_context_#{@context.id}" + wait_for do + selenium.is_visible("flash") + end end From 10fecaca38e917b4bd6701e69ed895e3da25b01f Mon Sep 17 00:00:00 2001 From: Eric Allen Date: Sat, 10 Apr 2010 15:22:22 -0400 Subject: [PATCH 06/16] Fix failing note deletion tests Text will remain present because the raw source isn't generated from the modified DOM. Also, there were some hard-coded id's in there that have changed. All Cucumber Selenium tests are now passing. --- features/step_definitions/note_steps.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/features/step_definitions/note_steps.rb b/features/step_definitions/note_steps.rb index 93fab0f6..630f974d 100644 --- a/features/step_definitions/note_steps.rb +++ b/features/step_definitions/note_steps.rb @@ -26,8 +26,10 @@ When /^I add note "([^\"]*)" from the "([^\"]*)" project page$/ do |note, projec end When /^I delete the first note$/ do + title = selenium.get_text("css=div.container h2") + id = title.split(' ').last click_link "delete note" - selenium.get_confirmation.should == "Are you sure that you want to delete the note '1'?" + selenium.get_confirmation.should == "Are you sure that you want to delete the note '#{id}'?" end When /^I click the icon next to the note$/ do @@ -53,8 +55,11 @@ Then /^I should see note "([^\"]*)" on the notes page$/ do |note| end Then /^the first note should disappear$/ do - # the first note contains "A note 1", generated by the Given def above - Then "I should not see \"A note 1\"" + title = selenium.get_text("css=div.container h2") + id = title.split(' ').last + wait_for :timeout => 15 do + !selenium.is_visible("note_#{id}") + end end From 1e4d250a15ba6c74ae7bd1323c448a5828dafa38 Mon Sep 17 00:00:00 2001 From: Eric Allen Date: Sat, 10 Apr 2010 15:23:38 -0400 Subject: [PATCH 07/16] Remove debugging print statements --- features/step_definitions/project_steps.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/features/step_definitions/project_steps.rb b/features/step_definitions/project_steps.rb index f08d349d..801104c9 100644 --- a/features/step_definitions/project_steps.rb +++ b/features/step_definitions/project_steps.rb @@ -33,7 +33,6 @@ Then /^I should see the bold text "([^\"]*)" in the project description$/ do |bo response.should have_xpath(xpath) bold_text = response.selenium.get_text("xpath=#{xpath}") - puts "bt=#{bold_text}" bold_text.should =~ /#{bold}/ end @@ -43,6 +42,5 @@ Then /^I should see the italic text "([^\"]*)" in the project description$/ do | response.should have_xpath(xpath) italic_text = response.selenium.get_text("xpath=#{xpath}") - puts "it=#{italic_text}" italic_text.should =~ /#{italic}/ -end \ No newline at end of file +end From 05cbea50aa7007295bd59ab0ba5f8b64a0ff8ddb Mon Sep 17 00:00:00 2001 From: Eric Allen Date: Sat, 10 Apr 2010 15:31:28 -0400 Subject: [PATCH 08/16] Using Rails.env is not the right approach here All Cucumber stories now passing --- features/step_definitions/generic_steps.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/step_definitions/generic_steps.rb b/features/step_definitions/generic_steps.rb index 5e1b684f..506bb493 100644 --- a/features/step_definitions/generic_steps.rb +++ b/features/step_definitions/generic_steps.rb @@ -3,7 +3,7 @@ Then /the badge should show (.*)/ do |number| badge = -1 xpath= "//span[@id='badge_count']" - if Rails.env == 'selenium' + if response.respond_to? :selenium response.should have_xpath(xpath) badge = response.selenium.get_text("xpath=#{xpath}").to_i else From 67df223488e99d467f00eda921321dfefcb0e42a Mon Sep 17 00:00:00 2001 From: Eric Allen Date: Sat, 10 Apr 2010 16:08:09 -0400 Subject: [PATCH 09/16] Drag todos between contexts Closes #1020 --- app/helpers/todos_helper.rb | 2 +- app/views/contexts/_context.rhtml | 1 + app/views/todos/update.js.rjs | 2 ++ public/javascripts/application.js | 31 ++++++++++++++++++++++++++++--- public/stylesheets/standard.css | 14 +++++++++++++- 5 files changed, 45 insertions(+), 5 deletions(-) diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb index 6c8703c6..994fb305 100644 --- a/app/helpers/todos_helper.rb +++ b/app/helpers/todos_helper.rb @@ -122,7 +122,7 @@ module TodosHelper :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') + :title => "Drop an action to make it depend on this action", :class => 'successor_target drop_target') end end diff --git a/app/views/contexts/_context.rhtml b/app/views/contexts/_context.rhtml index c95c93b9..7b33ee9a 100644 --- a/app/views/contexts/_context.rhtml +++ b/app/views/contexts/_context.rhtml @@ -16,4 +16,5 @@ <%= render :partial => "todos/todo", :collection => @not_done, :locals => { :parent_container_type => "context" } %> +
diff --git a/app/views/todos/update.js.rjs b/app/views/todos/update.js.rjs index 6cdf7367..d15595f5 100644 --- a/app/views/todos/update.js.rjs +++ b/app/views/todos/update.js.rjs @@ -153,3 +153,5 @@ else page.show 'error_status' page.replace_html 'error_status', "#{error_messages_for('todo')}" end + +page << "enable_rich_interaction();" diff --git a/public/javascripts/application.js b/public/javascripts/application.js index 7aab4d4c..fc3122a9 100644 --- a/public/javascripts/application.js +++ b/public/javascripts/application.js @@ -232,21 +232,46 @@ function enable_rich_interaction(){ function drop_todo(evt, ui) { dragged_todo = ui.draggable[0].id.split('_')[2]; dropped_todo = $(this).parents('.item-show').get(0).id.split('_')[2]; - ui.draggable.hide(); + ui.draggable.remove(); $(this).block({message: null}); $.post(relative_to_root('todos/add_predecessor'), {successor: dragged_todo, predecessor: dropped_todo}, null, 'script'); } + function drag_todo(){ + $('.drop_target').show(); + $(this).parents(".container").find(".context_target").hide(); + } + $('.item-show').draggable({handle: '.grip', revert: 'invalid', - start: function() {$('.successor_target').show();}, - stop: function() {$('.successor_target').hide();}}); + start: drag_todo, + stop: function() {$('.drop_target').hide();}}); $('.successor_target').droppable({drop: drop_todo, tolerance: 'pointer', hoverClass: 'hover'}); + + /* Drag & drop for changing contexts */ + function drop_todo_on_context(evt, ui) { + target = $(this); + dragged_todo = ui.draggable[0].id.split('_')[2]; + context_id = this.id.split('_')[1]; + ui.draggable.remove(); + target.block({message: null}); + setTimeout(function() {target.show()}, 0); + $.post(relative_to_root('todos/update'), + {id: dragged_todo, + "todo[id]": dragged_todo, + "todo[context_id]": context_id}, + function(){target.unblock(); target.hide();}, 'script'); + } + + $('.context_target').droppable({ + drop: drop_todo_on_context, + tolerance: 'pointer', + hoverClass: 'hover'}); /* Reset auto updater */ field_touched = false; diff --git a/public/stylesheets/standard.css b/public/stylesheets/standard.css index b326652d..d214f861 100644 --- a/public/stylesheets/standard.css +++ b/public/stylesheets/standard.css @@ -939,11 +939,14 @@ div.message { cursor: move; } +.drop_target { + display:none; +} + .successor_target { background-image:url("../images/add_successor_off.png"); background-repeat: no-repeat; background-position: center right; - display: none; } .successor_target.hover { @@ -952,6 +955,15 @@ div.message { background-position: center right; } +.context_target { + height: 15px; + margin: 4px; + border: thick dotted #CCC; +} + +.context_target.hover { +} + /* Error message styles */ .fieldWithErrors { padding: 2px; From e15425d54662d4e12182a8cd681a5d109a143fbc Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Wed, 14 Apr 2010 09:15:02 +0200 Subject: [PATCH 10/16] if every_other1 is not filled in for recurring patterns, things start to fall apart. Added basic checks. --- app/models/recurring_todo.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/models/recurring_todo.rb b/app/models/recurring_todo.rb index 23541dbc..ce5ffeae 100644 --- a/app/models/recurring_todo.rb +++ b/app/models/recurring_todo.rb @@ -21,6 +21,7 @@ class RecurringTodo < ActiveRecord::Base validates_length_of :notes, :maximum => 60000, :allow_nil => true validates_presence_of :context + validates_presence_of :every_other1, :message => ": every other nth day/month must be filled in" named_scope :active, :conditions => { :state => 'active'} named_scope :completed, :conditions => { :state => 'completed'} @@ -318,6 +319,7 @@ class RecurringTodo < ActiveRecord::Base def recurrence_pattern case recurring_period when 'daily' + return "invalid repeat pattern" if every_other1.nil? if only_work_days return "on work days" else @@ -328,18 +330,21 @@ class RecurringTodo < ActiveRecord::Base end end when 'weekly' + return "invalid repeat pattern" if every_other1.nil? if every_other1 > 1 return "every #{every_other1} weeks" else return 'weekly' end when 'monthly' + return "invalid repeat pattern" if every_other1.nil? if self.recurrence_selector == 0 return "every #{self.every_other2} month#{self.every_other2>1?'s':''} on day #{self.every_other1}" else return "every #{self.xth} #{self.day_of_week} of every #{self.every_other2} month#{self.every_other2>1?'s':''}" end when 'yearly' + return "invalid repeat pattern" if every_other1.nil? if self.recurrence_selector == 0 return "every year on #{self.month_of_year} #{self.every_other1}" else From b9f1f57740fa87a2a5cb355272504b7af84070f8 Mon Sep 17 00:00:00 2001 From: rverchere Date: Fri, 16 Apr 2010 04:18:15 +0800 Subject: [PATCH 11/16] Closes #1023. Display login form when ldap is the only auth system --- app/views/login/login.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/login/login.html.erb b/app/views/login/login.html.erb index b4a1dda5..c651165a 100644 --- a/app/views/login/login.html.erb +++ b/app/views/login/login.html.erb @@ -1,5 +1,5 @@ <% auth_schemes = Tracks::Config.auth_schemes - show_database_form = auth_schemes.include?('database') + show_database_form = auth_schemes.include?('database') || auth_schemes.include?('ldap') show_openid_form = auth_schemes.include?('open_id') show_cas_form = auth_schemes.include?('cas') -%> From c769b2a7eb20c9bb10ac989b656ded47ffd4a7f1 Mon Sep 17 00:00:00 2001 From: rverchere Date: Fri, 16 Apr 2010 04:23:14 +0800 Subject: [PATCH 12/16] Closes #1024. Do not create user with ldap auth if wrong password --- app/controllers/users_controller.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 5aa985ba..387c7689 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -71,9 +71,17 @@ class UsersController < ApplicationController render :action => "nosignup", :layout => "login" return end - + user = User.new(params['user']) + if Tracks::Config.auth_schemes.include?('ldap') && + user.auth_type == 'ldap' && + !SimpleLdapAuthenticator.valid?(user.login, params['user']['password']) + notify :warning, "Incorrect password" + redirect_to :action => 'new' + return + end + if Tracks::Config.auth_schemes.include?('cas') if user.auth_type.eql? "cas" user.crypted_password = "cas" From 33af53c313ad5117020202f852493458e93a9f86 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Mon, 19 Apr 2010 14:05:02 +0200 Subject: [PATCH 13/16] make sure tracks does not crash on missing every_other2 for recurring todos --- app/models/recurring_todo.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/recurring_todo.rb b/app/models/recurring_todo.rb index ce5ffeae..a04b203b 100644 --- a/app/models/recurring_todo.rb +++ b/app/models/recurring_todo.rb @@ -337,7 +337,7 @@ class RecurringTodo < ActiveRecord::Base return 'weekly' end when 'monthly' - return "invalid repeat pattern" if every_other1.nil? + return "invalid repeat pattern" if every_other1.nil? || every_other2.nil? if self.recurrence_selector == 0 return "every #{self.every_other2} month#{self.every_other2>1?'s':''} on day #{self.every_other1}" else From db5fca6b49ea989526fd13bf869e6d3e0629c8d5 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Mon, 19 Apr 2010 17:11:31 +0200 Subject: [PATCH 14/16] change header for basic authentication using the patch by zcrar70. Fixes #1021 --- lib/login_system.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/login_system.rb b/lib/login_system.rb index 4ad9312c..e3c5a779 100644 --- a/lib/login_system.rb +++ b/lib/login_system.rb @@ -189,7 +189,7 @@ module LoginSystem end def basic_auth_denied - response.headers["Status"] = "Unauthorized" + response.headers["Status"] = "401 Unauthorized" response.headers["WWW-Authenticate"] = "Basic realm=\"'Tracks Login Required'\"" render :text => "401 Unauthorized: You are not authorized to interact with Tracks.", :status => 401 end From 38f0cf1b7a7c033b84fd18fa5fd6af40b3f114a3 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Fri, 23 Apr 2010 16:51:35 +0200 Subject: [PATCH 15/16] adds extensive validation to recurring todos. Fixes #967 --- app/models/recurring_todo.rb | 138 ++++++++++++++++++++++++++--------- 1 file changed, 105 insertions(+), 33 deletions(-) diff --git a/app/models/recurring_todo.rb b/app/models/recurring_todo.rb index a04b203b..ad5c09d2 100644 --- a/app/models/recurring_todo.rb +++ b/app/models/recurring_todo.rb @@ -6,6 +6,9 @@ class RecurringTodo < ActiveRecord::Base has_many :todos + named_scope :active, :conditions => { :state => 'active'} + named_scope :completed, :conditions => { :state => 'completed'} + attr_protected :user acts_as_state_machine :initial => :active, :column => 'state' @@ -15,25 +18,94 @@ class RecurringTodo < ActiveRecord::Base t.occurences_count = 0 } state :completed, :enter => Proc.new { |t| t.completed_at = Time.zone.now }, :exit => Proc.new { |t| t.completed_at = nil } - - validates_presence_of :description - validates_length_of :description, :maximum => 100 - validates_length_of :notes, :maximum => 60000, :allow_nil => true - - validates_presence_of :context - validates_presence_of :every_other1, :message => ": every other nth day/month must be filled in" - - named_scope :active, :conditions => { :state => 'active'} - named_scope :completed, :conditions => { :state => 'completed'} event :complete do transitions :to => :completed, :from => [:active] end - + event :activate do transitions :to => :active, :from => [:completed] end + validates_presence_of :description + validates_presence_of :recurring_period + validates_length_of :description, :maximum => 100 + validates_length_of :notes, :maximum => 60000, :allow_nil => true + + validates_presence_of :context + + validate :period_specific_validations + validate :starts_and_ends_on_validations + + def period_specific_validations + periods = %W[daily weekly monthly yearly] + if periods.include?(recurring_period) + self.send("validate_#{recurring_period}") + else + errors.add(:recurring_period, "is an unknown recurrence pattern: '#{self.recurring_period}'") + end + end + + def validate_daily + errors.add_to_base("Please choose a recurrence setting") if daily_selector.nil? || daily_selector.blank? + if (daily_selector == "daily_every_x_day") && (daily_every_x_days.nil? || daily_every_x_days.blank?) + errors.add_to_base("Every other nth day may not be empty for recurrence setting") + end + end + + def validate_weekly + if weekly_every_x_week.nil? || weekly_every_x_week.blank? + errors.add_to_base("Every other nth week may not be empty for recurrence setting") + end + something_set = false + %w{sunday monday tuesday wednesday thursday friday}.each do |day| + something_set ||= self.send("on_#{day}") + + end + errors.add_to_base("You must specify at least one day on which the todo recurs") if !something_set + end + + def validate_monthly + case recurrence_selector + when 0 # 'monthly_every_x_day' + errors.add_to_base("The day of the month may not be empty for recurrence setting") if monthly_every_x_day.nil? || monthly_every_x_day.blank? + errors.add_to_base("Every other nth month may not be empty for recurrence setting") if monthly_every_x_month.nil? || monthly_every_x_month.blank? + when 1 # 'monthly_every_xth_day' + errors.add_to_base("Every other nth month may not be empty for recurrence setting") if monthly_every_x_month2.nil? || monthly_every_x_month2.blank? + errors.add_to_base("The nth day of the month may not be empty for recurrence setting") if monthly_every_xth_day.nil? || monthly_every_xth_day.blank? + errors.add_to_base("The day of the month may not be empty for recurrence setting") if monthly_day_of_week.nil? || monthly_day_of_week.blank? + else + raise Exception.new, "unexpected value of recurrence selector '#{self.recurrence_selector}'" + end + end + + def validate_yearly + case recurrence_selector + when 0 # 'yearly_every_x_day' + errors.add_to_base("The month of the year may not be empty for recurrence setting") if yearly_month_of_year.nil? || yearly_month_of_year.blank? + errors.add_to_base("The day of the month may not be empty for recurrence setting") if yearly_every_x_day.nil? || yearly_every_x_day.blank? + when 1 # 'yearly_every_xth_day' + errors.add_to_base("The month of the year may not be empty for recurrence setting") if yearly_month_of_year2.nil? || yearly_month_of_year2.blank? + errors.add_to_base("The nth day of the month may not be empty for recurrence setting") if yearly_every_xth_day.nil? || yearly_every_xth_day.blank? + errors.add_to_base("The day of the week may not be empty for recurrence setting") if yearly_day_of_week.nil? || yearly_day_of_week.blank? + else + raise Exception.new, "unexpected value of recurrence selector '#{self.recurrence_selector}'" + end + end + + + def starts_and_ends_on_validations + errors.add_to_base("The start date needs to be filled in") if start_from.nil? || start_from.blank? + case self.ends_on + when 'ends_on_number_of_times' + errors.add_to_base("The number of recurrences needs to be filled in for 'Ends on'") if number_of_occurences.nil? || number_of_occurences.blank? + when "ends_on_end_date" + errors.add_to_base("The end date needs to be filled in for 'Ends on'") if end_date.nil? || end_date.blank? + else + errors.add_to_base("The end of the recurrence is not selected") unless ends_on == "no_end_date" + end + end + # the following recurrence patterns can be stored: # # daily todos - recurrence_period = 'daily' @@ -65,7 +137,7 @@ class RecurringTodo < ActiveRecord::Base when 'daily_every_work_day' self.only_work_days = true else - raise Exception.new, "unknown daily recurrence pattern: '#{selector}'" + raise Exception.new, "unknown daily recurrence pattern: '#{selector}'" end end @@ -200,7 +272,7 @@ class RecurringTodo < ActiveRecord::Base end end - def monthly_every_x_month2=(x) + def monthly_every_x_month2=(x) self.every_other2 = x if recurring_period=='monthly' && recurrence_selector == 1 end @@ -215,7 +287,7 @@ class RecurringTodo < ActiveRecord::Base end def monthly_every_xth_day=(x) - self.every_other3 = x if recurring_period=='monthly' + self.every_other3 = x if recurring_period=='monthly' end def monthly_every_xth_day(default=nil) @@ -353,7 +425,7 @@ class RecurringTodo < ActiveRecord::Base else return 'unknown recurrence pattern: period unknown' end - end + end def xth xth_day = ['first','second','third','fourth','last'] @@ -387,9 +459,9 @@ class RecurringTodo < ActiveRecord::Base end def get_show_from_date(previous) - case self.target + case self.target when 'due_date' - # so set show from date relative to due date unless show_always is true or show_from_delta is nil + # so set show from date relative to due date unless show_always is true or show_from_delta is nil if self.show_always? or self.show_from_delta.nil? nil else @@ -405,17 +477,17 @@ class RecurringTodo < ActiveRecord::Base def get_next_date(previous) case self.recurring_period - when 'daily' + when 'daily' return get_daily_date(previous) - when 'weekly' + when 'weekly' return get_weekly_date(previous) - when 'monthly' + when 'monthly' return get_monthly_date(previous) when 'yearly' return get_yearly_date(previous) else raise Exception.new, "unknown recurrence pattern: '#{self.recurring_period}'" - end + end end def get_daily_date(previous) @@ -435,7 +507,7 @@ class RecurringTodo < ActiveRecord::Base unless self.start_from.nil? # check if the start_from date is later than previous. If so, use # start_from as start to search for next date - start = self.start_from if self.start_from > previous + start = self.start_from if self.start_from > previous end end @@ -462,7 +534,7 @@ class RecurringTodo < ActiveRecord::Base start = self.start_from.nil? ? Time.zone.now : self.start_from else start = previous + 1.day - if start.wday() == 0 + if start.wday() == 0 # we went to a new week , go to the nth next week and find first match # that week start += self.every_other1.week @@ -470,7 +542,7 @@ class RecurringTodo < ActiveRecord::Base unless self.start_from.nil? # check if the start_from date is later than previous. If so, use # start_from as start to search for next date - start = self.start_from if self.start_from > previous + start = self.start_from if self.start_from > previous end end @@ -488,7 +560,7 @@ class RecurringTodo < ActiveRecord::Base return start + (i-start.wday()).days unless self.every_day[i,1] == ' ' end - raise Exception.new, "unable to find next weekly date (#{self.every_day})" + raise Exception.new, "unable to find next weekly date (#{self.every_day})" end def get_monthly_date(previous) @@ -536,7 +608,7 @@ class RecurringTodo < ActiveRecord::Base end return the_next else - raise Exception.new, "unknown monthly recurrence selection (#{self.recurrence_selector})" + raise Exception.new, "unknown monthly recurrence selection (#{self.recurrence_selector})" end return nil end @@ -562,7 +634,7 @@ class RecurringTodo < ActiveRecord::Base start+= 1.day end n -= 1 - start += 1.day unless n==0 + start += 1.day unless n==0 end # convert back to local timezone return Time.zone.local(start.year, start.month, start.day) @@ -579,10 +651,10 @@ class RecurringTodo < ActiveRecord::Base if start.month > month || (start.month == month && start.day >= day) # if there is no next month n and day m in this year, search in next # year - start = Time.zone.local(start.year+1, month, 1) + start = Time.zone.local(start.year+1, month, 1) else # if there is a next month n, stay in this year - start = Time.zone.local(start.year, month, 1) + start = Time.zone.local(start.year, month, 1) end return Time.zone.local(start.year, month, day) @@ -599,7 +671,7 @@ class RecurringTodo < ActiveRecord::Base return the_next else - raise Exception.new, "unknown monthly recurrence selection (#{self.recurrence_selector})" + raise Exception.new, "unknown monthly recurrence selection (#{self.recurrence_selector})" end return nil end @@ -640,8 +712,8 @@ class RecurringTodo < ActiveRecord::Base else _add_tags(Todo::STARRED_TAG_NAME) tags.reload - end - starred? + end + starred? end def remove_from_project! @@ -669,7 +741,7 @@ class RecurringTodo < ActiveRecord::Base unless self.start_from.nil? # check if the start_from date is later than previous. If so, use # start_from as start to search for next date - start = self.start_from if self.start_from > previous + start = self.start_from if self.start_from > previous end end return start From 06ac3067d3996a47dd2347f3e30e5c7dc1f4eab2 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Fri, 23 Apr 2010 17:13:03 +0200 Subject: [PATCH 16/16] fix validations for daily pattern and fix disappearing target fields for recurring todos --- app/models/recurring_todo.rb | 3 +-- public/javascripts/application.js | 22 ++++++++-------------- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/app/models/recurring_todo.rb b/app/models/recurring_todo.rb index ad5c09d2..86c549f1 100644 --- a/app/models/recurring_todo.rb +++ b/app/models/recurring_todo.rb @@ -47,8 +47,7 @@ class RecurringTodo < ActiveRecord::Base end def validate_daily - errors.add_to_base("Please choose a recurrence setting") if daily_selector.nil? || daily_selector.blank? - if (daily_selector == "daily_every_x_day") && (daily_every_x_days.nil? || daily_every_x_days.blank?) + if (!only_work_days) && (daily_every_x_days.nil? || daily_every_x_days.blank?) errors.add_to_base("Every other nth day may not be empty for recurrence setting") end end diff --git a/public/javascripts/application.js b/public/javascripts/application.js index fc3122a9..cfe11d2e 100644 --- a/public/javascripts/application.js +++ b/public/javascripts/application.js @@ -14,16 +14,14 @@ var TracksForm = { toggleDiv.toggleClass('hide_form'); }, hide_all_recurring: function () { - $('#recurring_daily').hide(); - $('#recurring_weekly').hide(); - $('#recurring_monthly').hide(); - $('#recurring_yearly').hide(); + $.each(['daily', 'weekly', 'monthly', 'yearly'], function(){ + $('#recurring_'+this).hide(); + }); }, hide_all_edit_recurring: function () { - $('#recurring_edit_daily').hide(); - $('#recurring_edit_weekly').hide(); - $('#recurring_edit_monthly').hide(); - $('#recurring_edit_yearly').hide(); + $.each(['daily', 'weekly', 'monthly', 'yearly'], function(){ + $('#recurring_edit_'+this).hide(); + }); }, toggle_overlay: function () { el = document.getElementById("overlay"); @@ -435,16 +433,12 @@ $(document).ready(function() { TracksForm.toggle_overlay(); }); $("#recurring_edit_period input").live('click', function(){ - $.each(['daily', 'weekly', 'monthly', 'yearly'], function(){ - $('#recurring_edit_'+this).hide(); - }); + TracksForm.hide_all_edit_recurring(); $('#recurring_edit_'+this.id.split('_')[5]).show(); }); $("#recurring_period input").live('click', function(){ - $.each(['daily', 'weekly', 'monthly', 'yearly', 'target'], function(){ - $('#recurring_'+this).hide(); - }); + TracksForm.hide_all_recurring(); $('#recurring_'+this.id.split('_')[4]).show(); });