diff --git a/app/models/recurring_todos/abstract_recurring_todos_builder.rb b/app/models/recurring_todos/abstract_recurring_todos_builder.rb index fd61bcc4..d3a4ecaa 100644 --- a/app/models/recurring_todos/abstract_recurring_todos_builder.rb +++ b/app/models/recurring_todos/abstract_recurring_todos_builder.rb @@ -21,31 +21,16 @@ module RecurringTodos # build does not add tags. For tags, the recurring todos needs to be saved def build @recurring_todo = @pattern.build_recurring_todo(@mapped_attributes) - end def update(recurring_todo) @recurring_todo = @pattern.update_recurring_todo(recurring_todo, @mapped_attributes) - @saved = @recurring_todo.save - @recurring_todo.tag_with(@filterred_attributes.get(:tag_list)) if @saved && @filterred_attributes.get(:tag_list).present? - @recurring_todo.reload - - return @saved + save_recurring_todo end def save build - @saved = @recurring_todo.save - @recurring_todo.tag_with(@filterred_attributes.get(:tag_list)) if @saved && @filterred_attributes.get(:tag_list).present? - return @saved - end - - def save_collection(collection, collection_id) - # save object (project or context) and add its id to @mapped_attributes and remove the object from the attributes - object = @mapped_attributes.get(collection) - object.save - @mapped_attributes.set(collection_id, object.id) - @mapped_attributes.except(collection) + save_recurring_todo end def save_project @@ -71,29 +56,31 @@ module RecurringTodos end def filter_attributes(attributes) + # get pattern independend attributes filterred_attributes = filter_generic_attributes(attributes) - attributes_to_filter.each{|key| filterred_attributes.set(key, attributes.get(key)) if attributes.key?(key)} + # append pattern specific attributes + attributes_to_filter.each{|key| filterred_attributes[key]= attributes[key] if attributes.key?(key)} + filterred_attributes end def filter_generic_attributes(attributes) return Tracks::AttributeHandler.new(@user, { - recurring_period: attributes.get(:recurring_period), - description: attributes.get(:description), - notes: attributes.get(:notes), + recurring_period: attributes[:recurring_period], + description: attributes[:description], + notes: attributes[:notes], tag_list: tag_list_or_empty_string(attributes), - start_from: attributes.get(:start_from), - end_date: attributes.get(:end_date), - ends_on: attributes.get(:ends_on), - show_always: attributes.get(:show_always), - target: attributes.get(:target), - project: attributes.get(:project), - context: attributes.get(:context), - project_id: attributes.get(:project_id), - context_id: attributes.get(:context_id), - target: attributes.get(:recurring_target), - show_from_delta: attributes.get(:recurring_show_days_before), - show_always: attributes.get(:recurring_show_always) + start_from: attributes[:start_from], + end_date: attributes[:end_date], + ends_on: attributes[:ends_on], + target: attributes[:target], + project: attributes[:project], + context: attributes[:context], + project_id: attributes[:project_id], + context_id: attributes[:context_id], + target: attributes[:recurring_target], + show_from_delta: attributes[:recurring_show_days_before], + show_always: attributes[:recurring_show_always] }) end @@ -103,8 +90,9 @@ module RecurringTodos end # helper method to be used in mapped_attributes in subclasses + # changes name of key from source_key to key def map(mapping, key, source_key) - mapping.set(key, mapping.get(source_key)) + mapping[key] = mapping[source_key] mapping.except(source_key) end @@ -117,7 +105,7 @@ module RecurringTodos return nil if key.nil? raise Exception.new, "recurrence selector pattern (#{key}) not given" unless @attributes.selector_key_present?(key) - selector = @attributes.get(key) + selector = @attributes[key] raise Exception.new, "unknown recurrence selector pattern: '#{selector}'" unless valid_selector?(selector) @@ -131,9 +119,28 @@ module RecurringTodos private + def save_recurring_todo + @saved = @recurring_todo.save + save_tags if @saved + return @saved + end + + def save_tags + @recurring_todo.tag_with(@filterred_attributes[:tag_list]) if @filterred_attributes[:tag_list].present? + @recurring_todo.reload + end + + def save_collection(collection, collection_id) + # save object (project or context) and add its id to @mapped_attributes and remove the object from the attributes + object = @mapped_attributes[collection] + object.save + @mapped_attributes[collection_id] = object.id + @mapped_attributes.except(collection) + end + def tag_list_or_empty_string(attributes) # avoid nil - attributes.get(:tag_list).blank? ? "" : attributes.get(:tag_list).strip + attributes[:tag_list].blank? ? "" : attributes[:tag_list].strip end end diff --git a/app/models/recurring_todos/abstract_repeat_pattern.rb b/app/models/recurring_todos/abstract_repeat_pattern.rb index 93bc3fef..0c7ad29a 100644 --- a/app/models/recurring_todos/abstract_repeat_pattern.rb +++ b/app/models/recurring_todos/abstract_repeat_pattern.rb @@ -46,6 +46,10 @@ module RecurringTodos @attributes = Tracks::AttributeHandler.new(@user, recurring_todo.attributes) end + def valid? + @recurring_todo.valid? + end + def validate_not_blank(object, msg) errors[:base] << msg if object.blank? end @@ -80,7 +84,7 @@ module RecurringTodos validate_not_nil(show_always, "Please select when to show the action") validate_not_blank(show_from_delta, "Please fill in the number of days to show the todo before the due date") unless show_always else - raise Exception.new, "unexpected value of recurrence target selector '#{target}'" + errors[:base] << "Unexpected value of recurrence target selector '#{target}'" end end @@ -89,7 +93,7 @@ module RecurringTodos end def get(attribute) - @attributes.get attribute + @attributes[attribute] end end diff --git a/app/models/recurring_todos/recurring_todos_builder.rb b/app/models/recurring_todos/recurring_todos_builder.rb index dd26e637..ed8dd9cf 100644 --- a/app/models/recurring_todos/recurring_todos_builder.rb +++ b/app/models/recurring_todos/recurring_todos_builder.rb @@ -12,15 +12,12 @@ module RecurringTodos parse_project parse_context - @builder = create_builder(@attributes.get(:recurring_period)) + @builder = create_builder(@attributes[:recurring_period]) end def create_builder(selector) - if %w{daily weekly monthly yearly}.include?(selector) - return eval("RecurringTodos::#{selector.capitalize}RecurringTodosBuilder.new(@user, @attributes)") - else - raise Exception.new("Unknown recurrence selector in :recurring_period (#{selector})") - end + raise "Unknown recurrence selector in :recurring_period (#{selector})" unless valid_selector? selector + eval("RecurringTodos::#{selector.capitalize}RecurringTodosBuilder.new(@user, @attributes)") end def build @@ -50,18 +47,26 @@ module RecurringTodos @builder.attributes end + def pattern + @builder.pattern + end + private + def valid_selector?(selector) + %w{daily weekly monthly yearly}.include?(selector) + end + def parse_dates %w{end_date start_from}.each {|date| @attributes.parse_date date } end def parse_project - @project, @new_project_created = @attributes.parse_collection(:project, @user.projects, @attributes.project_name) + @project, @new_project_created = @attributes.parse_collection(:project, @user.projects) end def parse_context - @context, @new_context_created = @attributes.parse_collection(:context, @user.contexts, @attributes.context_name) + @context, @new_context_created = @attributes.parse_collection(:context, @user.contexts) end end diff --git a/lib/tracks/attribute_handler.rb b/lib/tracks/attribute_handler.rb index c944b001..add42787 100644 --- a/lib/tracks/attribute_handler.rb +++ b/lib/tracks/attribute_handler.rb @@ -13,6 +13,10 @@ module Tracks @attributes[attribute.to_sym] end + def [](attribute) + get attribute + end + def set(key, value) @attributes[key.to_sym] = value end @@ -21,6 +25,10 @@ module Tracks @attributes[key.to_sym] ||= value end + def []=(attribute, value) + set attribute, value + end + def except(key) AttributeHandler.new(@user, @attributes.except(key.to_sym)) end @@ -30,19 +38,19 @@ module Tracks end def selector_key_present?(key) - @attributes.key?(key.to_sym) + key?(key) end def parse_date(date) set(date, @user.prefs.parse_date(get(date))) end - def parse_collection(object_type, relation, name) + def parse_collection(object_type, relation) object = nil new_object_created = false if specified_by_name?(object_type) - object, new_object_created = find_or_create_by_name(relation, name) + object, new_object_created = find_or_create_by_name(relation, object_name(object_type)) # put id of object in @attributes, i.e. set :project_id to project.id @attributes[object_type.to_s + "_id"] = object.id unless new_object_created else @@ -53,6 +61,10 @@ module Tracks return object, new_object_created end + def object_name(object_type) + send("#{object_type}_name") + end + def attribute_with_id_of(object_type) map = { project: 'project_id', context: 'context_id' } get map[object_type] diff --git a/test/models/attribute_handler_test.rb b/test/models/attribute_handler_test.rb new file mode 100644 index 00000000..54d6bddd --- /dev/null +++ b/test/models/attribute_handler_test.rb @@ -0,0 +1,97 @@ +require_relative '../test_helper' + +class AttributeHandlerTest < ActiveSupport::TestCase + fixtures :users + + def test_setting_attributes + h = Tracks::AttributeHandler.new(nil, {}) + + h.set('test', '123') + h['other']='one' + assert_equal '123', h.attributes[:test], ":test should be added" + assert_nil h.attributes['test'], "string should be converted to symbol" + assert_equal 'one', h[:other], ":other should be added as symbol using []=" + + assert_nil h.attributes[:new] + h.set_if_nil(:new, 'value') + assert_equal 'value', h.attributes[:new], "value should be set for new key" + h.set_if_nil(:new, 'other') + assert_equal 'value', h.attributes[:new], "value should not be set for existing key" + + h.attributes[:empty] = nil + h.set_if_nil(:empty, "test") + assert_equal "test", h.attributes[:empty], "nil value should be overwritten" + end + + def test_getting_attributes + h = Tracks::AttributeHandler.new(nil, { :get => "me"} ) + assert h.key?(:get), "attributehandler should have key :get" + assert h.key?('get'), "attributehandler should have key :get" + assert_equal "me", h.attributes[:get], "attributehandler should have key :get" + assert_equal "me", h.get('get'), "key should be converted to symbol" + assert_equal "me", h[:get], "AttributeHandler should act like hash" + end + + def test_removing_attributes + h = Tracks::AttributeHandler.new(nil, { :i_am => "here"} ) + assert h.key?(:i_am) + + h.except(:i_am) + assert h.key?(:i_am), "AttributeHandler should be immutable" + + h2 = h.except("i_am") + assert !h2.key?(:i_am), "key as symbol should be removed" + end + + def test_project_specified_by_name + h = Tracks::AttributeHandler.new(nil, { } ) + + assert !h.project_specified_by_name?, "project is not specified by id or by name" + + h[:project_id]=4 + assert !h.project_specified_by_name?, "project is specified by id, not by name" + + h = h.except(:project_id) + h[:project_name] = "A project" + assert h.project_specified_by_name?, "project is specified by name" + + h[:project_name] = "None" + assert !h.project_specified_by_name?, "None is special token to specify nil-project" + end + + def test_context_specified_by_name + h = Tracks::AttributeHandler.new(nil, { } ) + assert !h.context_specified_by_name?, "context is not specified by id or by name" + + h["context_id"] = 4 + assert !h.context_specified_by_name?, "context is specified by id, not by name" + + h = h.except(:context_id) + h[:context_name] = "A context" + assert h.context_specified_by_name?, "context is specified by name" + end + + def test_parse_collection + admin = users(:admin_user) + project = admin.projects.first + h = Tracks::AttributeHandler.new(admin, { "project_id" => project.id } ) + + parsed_project, new_project_created = h.parse_collection(:project, admin.projects) + assert !new_project_created, "should find existing project" + assert_equal project.id, parsed_project.id, "it should find the project" + + h = Tracks::AttributeHandler.new(admin, { "project_name" => project.name } ) + + parsed_project, new_project_created = h.parse_collection(:project, admin.projects) + assert !new_project_created, "should find existing project" + assert_equal project.id, parsed_project.id, "it should find the project" + + h = Tracks::AttributeHandler.new(admin, { "project_name" => "new project" } ) + + parsed_project, new_project_created = h.parse_collection(:project, admin.projects) + assert new_project_created, "should detect that no project exist with that name" + assert_equal "new project", parsed_project.name, "it should return a new project" + assert !parsed_project.persisted?, "new project should not be persisted (yet)" + end + +end \ No newline at end of file diff --git a/test/models/recurring_todos/abstract_recurring_todos_builder_test.rb b/test/models/recurring_todos/abstract_recurring_todos_builder_test.rb index c2ac5575..5c051eb5 100644 --- a/test/models/recurring_todos/abstract_recurring_todos_builder_test.rb +++ b/test/models/recurring_todos/abstract_recurring_todos_builder_test.rb @@ -46,7 +46,7 @@ module RecurringTodos } builder = RecurringTodosBuilder.new(@admin, attributes) - assert_equal "tag, this, that", builder.attributes.get(:tag_list) + assert_equal "tag, this, that", builder.attributes[:tag_list] # given attributes without tag_list attributes = { @@ -55,7 +55,7 @@ module RecurringTodos } builder = RecurringTodosBuilder.new(@admin, attributes) - assert_equal "", builder.attributes.get(:tag_list) + assert_equal "", builder.attributes[:tag_list] # given attributes with nil tag_list attributes = { @@ -65,7 +65,7 @@ module RecurringTodos } builder = RecurringTodosBuilder.new(@admin, attributes) - assert_equal "", builder.attributes.get(:tag_list) + assert_equal "", builder.attributes[:tag_list] # given attributes with empty tag_list ==> should be stripped attributes = { @@ -75,7 +75,7 @@ module RecurringTodos } builder = RecurringTodosBuilder.new(@admin, attributes) - assert_equal "", builder.attributes.get(:tag_list) + assert_equal "", builder.attributes[:tag_list] end def test_tags_should_be_saved_on_create_and_update @@ -137,27 +137,27 @@ module RecurringTodos def test_map_removes_mapped_key attributes = Tracks::AttributeHandler.new(@admin, { :source => "value"}) - arp = WeeklyRecurringTodosBuilder.new(@admin, attributes) - attributes = arp.map(attributes, :target, :source) + a_builder = WeeklyRecurringTodosBuilder.new(@admin, attributes) + attributes = a_builder.map(attributes, :target, :source) - assert_equal "value", attributes.get(:target) - assert_nil attributes.get(:source) + assert_equal "value", attributes[:target] + assert_nil attributes[:source] assert !attributes.key?(:source) end def test_get_selector_removes_selector_from_hash attributes = Tracks::AttributeHandler.new(@admin, { :selector => "weekly" }) - arp = WeeklyRecurringTodosBuilder.new(@admin, attributes) + a_builder = WeeklyRecurringTodosBuilder.new(@admin, attributes) - assert "weekly", arp.get_selector(:selector) - assert !arp.attributes.key?(:selector) + assert "weekly", a_builder.get_selector(:selector) + assert !a_builder.attributes.key?(:selector) end def test_get_selector_raises_exception_when_missing_selector attributes = Tracks::AttributeHandler.new(@admin, { }) - arp = WeeklyRecurringTodosBuilder.new(@admin, attributes) + a_builder = WeeklyRecurringTodosBuilder.new(@admin, attributes) - assert_raise(Exception, "should raise exception when recurrence selector is missing"){ arp.get_selector(:selector) } + assert_raise(Exception, "should raise exception when recurrence selector is missing"){ a_builder.get_selector(:selector) } end end diff --git a/test/models/recurring_todos/abstract_repeat_pattern_test.rb b/test/models/recurring_todos/abstract_repeat_pattern_test.rb index feee55d4..ce3792c8 100644 --- a/test/models/recurring_todos/abstract_repeat_pattern_test.rb +++ b/test/models/recurring_todos/abstract_repeat_pattern_test.rb @@ -9,6 +9,93 @@ module RecurringTodos @admin = users(:admin_user) end + def test_pattern_builds_from_existing_recurring_todo + rt = @admin.recurring_todos.first + + pattern = rt.pattern + assert pattern.is_a?(DailyRepeatPattern), "recurring todo should have daily pattern" + end + + def test_validation_on_due_date + attributes = { + 'recurring_period' => 'weekly', + 'recurring_target' => 'due_date', + 'description' => 'a repeating todo', # generic + 'weekly_return_monday' => 'm', # weekly specific + 'ends_on' => 'ends_on_end_date', + 'end_date' => Time.zone.now + 1.week, + 'context_id' => @admin.contexts.first.id, + 'start_from' => Time.zone.now - 1.week, + 'weekly_every_x_week' => 1, + } + + pattern = create_pattern(attributes) + assert !pattern.valid?, "should fail because show_always and show_from_delta are not there" + + attributes['recurring_show_always'] = false + pattern = create_pattern(attributes) + assert !pattern.valid?, "should fail because show_from_delta is not there" + + attributes[:recurring_show_days_before] = 5 + pattern = create_pattern(attributes) + assert pattern.valid?, "should be valid:" + pattern.errors.full_messages.to_s + end + + def test_validation_on_start_date + attributes = { + 'recurring_period' => 'weekly', + 'recurring_target' => 'due_date', + 'description' => 'a repeating todo', # generic + 'weekly_return_monday' => 'm', # weekly specific + 'ends_on' => 'ends_on_end_date', + 'context_id' => @admin.contexts.first.id, + 'end_date' => Time.zone.now + 1.week, + 'weekly_every_x_week' => 1, + 'recurring_show_always' => false, + 'recurring_show_days_before' => 5, + } + pattern = create_pattern(attributes) + assert !pattern.valid?, "should be not valid because start_from is empty" + + attributes['start_from'] = Time.zone.now - 1.week + pattern = create_pattern(attributes) + assert pattern.valid?, "should be valid: " + pattern.errors.full_messages.to_s + end + + def test_validation_on_end_date + attributes = { + 'recurring_period' => 'weekly', + 'recurring_target' => 'due_date', + 'description' => 'a repeating todo', # generic + 'weekly_return_monday' => 'm', # weekly specific + 'ends_on' => 'invalid_value', + 'context_id' => @admin.contexts.first.id, + 'start_from' => Time.zone.now - 1.week, + 'weekly_every_x_week' => 1, + 'recurring_show_always' => false, + 'recurring_show_days_before' => 5, + } + + pattern = create_pattern(attributes) + assert !pattern.valid? + + attributes['ends_on']='ends_on_end_date' + pattern = create_pattern(attributes) + assert !pattern.valid?, "should not be valid, because end_date is not supplied" + + attributes['end_date']= Time.zone.now + 1.week + pattern = create_pattern(attributes) + assert pattern.valid?, "should be valid" + end + + private + + def create_pattern(attributes) + builder = RecurringTodosBuilder.new(@admin, attributes) + builder.build + builder.pattern + end + end end \ No newline at end of file diff --git a/test/models/recurring_todos/recurring_todos_builder_test.rb b/test/models/recurring_todos/recurring_todos_builder_test.rb index e72e4633..a5771f86 100644 --- a/test/models/recurring_todos/recurring_todos_builder_test.rb +++ b/test/models/recurring_todos/recurring_todos_builder_test.rb @@ -10,7 +10,11 @@ module RecurringTodos end def test_create_builder_needs_selector - assert_raise(Exception){ builder = RecurringTodosBuilder.new(@admin, {}) } + assert_raise(RuntimeError){ builder = RecurringTodosBuilder.new(@admin, {}) } + end + + def test_create_builder_needs_valid_selector + assert_raise(RuntimeError){ builder = RecurringTodosBuilder.new(@admin, { 'recurring_period' => 'wrong_value'}) } end def test_create_builder_uses_selector @@ -35,8 +39,8 @@ module RecurringTodos 'end_date' => '05/05/05' }) - assert builder.attributes.get(:start_from).is_a?(ActiveSupport::TimeWithZone), "Dates should be parsed to ActiveSupport::TimeWithZone class" - assert builder.attributes.get(:end_date).is_a?(ActiveSupport::TimeWithZone), "Dates should be parsed to ActiveSupport::TimeWithZone class" + assert builder.attributes[:start_from].is_a?(ActiveSupport::TimeWithZone), "Dates should be parsed to ActiveSupport::TimeWithZone class" + assert builder.attributes[:end_date ].is_a?(ActiveSupport::TimeWithZone), "Dates should be parsed to ActiveSupport::TimeWithZone class" end def test_exisisting_project_is_used