diff --git a/app/controllers/todos_controller.rb b/app/controllers/todos_controller.rb index 405a44c6..efcba0f3 100644 --- a/app/controllers/todos_controller.rb +++ b/app/controllers/todos_controller.rb @@ -90,7 +90,7 @@ class TodosController < ApplicationController end if @todo.errors.empty? - @todo.starred= (params[:new_todo_starred]||"").include? "true" + @todo.starred= (params[:new_todo_starred]||"").include? "true" if params[:new_todo_starred] @todo.add_predecessor_list(predecessor_list) @@ -1516,6 +1516,13 @@ class TodosController < ApplicationController @params = params['request'] || params @prefs = prefs @attributes = params['request'] && params['request']['todo'] || params['todo'] + + if @attributes[:tags] + # the REST api may use which will collide with tags association, so rename tags to add_tags + add_tags = @attributes[:tags] + @attributes.delete :tags + @attributes[:add_tags] = add_tags + end end def attributes diff --git a/app/models/todo.rb b/app/models/todo.rb index dd309d94..cbe6d162 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -289,7 +289,7 @@ class Todo < ActiveRecord::Base def add_predecessor(t) return if t.nil? - + @predecessor_array = predecessors @predecessor_array << t end @@ -311,11 +311,11 @@ class Todo < ActiveRecord::Base def raw_notes=(value) self[:notes] = value end - + # XML API fixups def predecessor_dependencies=(params) value = params[:predecessor] - + if !value.nil? if value.class == Array value.each do |ele| @@ -325,7 +325,7 @@ class Todo < ActiveRecord::Base predecessor_dependencies.build(value) end end - else + else if ele.is_a? String add_predecessor(self.user.todos.find_by_id(ele.to_i)) unless ele.blank? else @@ -334,7 +334,7 @@ class Todo < ActiveRecord::Base end end end - + alias_method :original_context=, :context= def context=(value) if value.is_a? Context @@ -343,7 +343,7 @@ class Todo < ActiveRecord::Base self.original_context=(Context.create(value)) end end - + alias_method :original_project=, :project= def project=(value) if value.is_a? Project @@ -352,23 +352,13 @@ class Todo < ActiveRecord::Base self.original_project=(Project.create(value)) end end - - def tags=(params) - value = params[:tag] - - if !value.nil? - if value.class == Array - value.each do |attrs| - tags.build(attrs) - end - else - tags.build(value) - end - end - end - - # Rich Todo API + # used by the REST API. will also work, this is renamed to add_tags in TodosController::TodoCreateParamsHelper::initialize + def add_tags=(params) + tag_with params[:tag].inject([]) { |list, value| list << value[:name] } unless params[:tag].nil? + end + + # Rich Todo API def self.from_rich_message(user, default_context_id, description, notes) fields = description.match(/([^>@]*)@?([^>]*)>?(.*)/) description = fields[1].strip diff --git a/lib/tagging_extensions.rb b/lib/tagging_extensions.rb index 3a992efa..2808f42a 100644 --- a/lib/tagging_extensions.rb +++ b/lib/tagging_extensions.rb @@ -2,7 +2,7 @@ class ActiveRecord::Base #:nodoc: # These extensions make models taggable. This file is automatically generated and required by your app if you run the tagging generator included with has_many_polymorphs. module TaggingExtensions - + # Add tags to self. Accepts a string of tagnames, an array of tagnames, an array of ids, or an array of Tags. # # We need to avoid name conflicts with the built-in ActiveRecord association methods, thus the underscores. @@ -21,8 +21,8 @@ class ActiveRecord::Base #:nodoc: end end end - - # Removes tags from self. Accepts a string of tagnames, an array of tagnames, an array of ids, or an array of Tags. + + # Removes tags from self. Accepts a string of tagnames, an array of tagnames, an array of ids, or an array of Tags. def _remove_tags outgoing taggable?(true) outgoing = tag_cast_to_string(outgoing) @@ -35,20 +35,20 @@ class ActiveRecord::Base #:nodoc: def tag_list # Redefined later to avoid an RDoc parse error. end - + # Replace the existing tags on self. Accepts a string of tagnames, an array of tagnames, an array of ids, or an array of Tags. - def tag_with list + def tag_with list #:stopdoc: taggable?(true) list = tag_cast_to_string(list) - + # Transactions may not be ideal for you here; be aware. - Tag.transaction do + Tag.transaction do current = tags.map(&:name) _add_tags(list - current) _remove_tags(current - list) end - + self #:startdoc: end @@ -64,10 +64,10 @@ class ActiveRecord::Base #:nodoc: def tag_list=(value) tag_with(value) - end + end + + private - private - def tag_cast_to_string obj #:nodoc: case obj when Array @@ -88,9 +88,9 @@ class ActiveRecord::Base #:nodoc: else raise "Invalid object of class #{obj.class} as tagging method parameter" end.flatten.compact.map(&:downcase).uniq - end - - # Check if a model is in the :taggables target list. The alternative to this check is to explicitly include a TaggingMethods module (which you would create) in each target model. + end + + # Check if a model is in the :taggables target list. The alternative to this check is to explicitly include a TaggingMethods module (which you would create) in each target model. def taggable?(should_raise = false) #:nodoc: unless flag = respond_to?(:tags) raise "#{self.class} is not a taggable model" if should_raise @@ -99,69 +99,69 @@ class ActiveRecord::Base #:nodoc: end end - + module TaggingFinders # Find all the objects tagged with the supplied list of tags - # + # # Usage : Model.tagged_with("ruby") # Model.tagged_with("hello", "world") # Model.tagged_with("hello", "world", :limit => 10) # # XXX This query strategy is not performant, and needs to be rewritten as an inverted join or a series of unions - # + # def tagged_with(*tag_list) options = tag_list.last.is_a?(Hash) ? tag_list.pop : {} tag_list = parse_tags(tag_list) - + scope = scope(:find) options[:select] ||= "#{table_name}.*" options[:from] ||= "#{table_name}, tags, taggings" - + sql = "SELECT #{(scope && scope[:select]) || options[:select]} " sql << "FROM #{(scope && scope[:from]) || options[:from]} " add_joins!(sql, options[:joins], scope) - + sql << "WHERE #{table_name}.#{primary_key} = taggings.taggable_id " sql << "AND taggings.taggable_type = '#{ActiveRecord::Base.send(:class_name_of_active_record_descendant, self).to_s}' " sql << "AND taggings.tag_id = tags.id " - + tag_list_condition = tag_list.map {|name| "'#{name}'"}.join(", ") - + sql << "AND (tags.name IN (#{sanitize_sql(tag_list_condition)})) " sql << "AND #{sanitize_sql(options[:conditions])} " if options[:conditions] - - columns = column_names.map do |column| + + columns = column_names.map do |column| "#{table_name}.#{column}" end.join(", ") - + sql << "GROUP BY #{columns} " sql << "HAVING COUNT(taggings.tag_id) = #{tag_list.size}" - + add_order!(sql, options[:order], scope) add_limit!(sql, options, scope) add_lock!(sql, options, scope) - + find_by_sql(sql) end - + def self.tagged_with_any(*tag_list) options = tag_list.last.is_a?(Hash) ? tag_list.pop : {} tag_list = parse_tags(tag_list) - + scope = scope(:find) options[:select] ||= "#{table_name}.*" options[:from] ||= "#{table_name}, meta_tags, taggings" - + sql = "SELECT #{(scope && scope[:select]) || options[:select]} " sql << "FROM #{(scope && scope[:from]) || options[:from]} " add_joins!(sql, options, scope) - + sql << "WHERE #{table_name}.#{primary_key} = taggings.taggable_id " sql << "AND taggings.taggable_type = '#{ActiveRecord::Base.send(:class_name_of_active_record_descendant, self).to_s}' " sql << "AND taggings.meta_tag_id = meta_tags.id " - + sql << "AND (" or_options = [] tag_list.each do |name| @@ -169,30 +169,30 @@ class ActiveRecord::Base #:nodoc: end or_options_joined = or_options.join(" OR ") sql << "#{or_options_joined}) " - - + + sql << "AND #{sanitize_sql(options[:conditions])} " if options[:conditions] - + columns = column_names.map do |column| "#{table_name}.#{column}" end.join(", ") - + sql << "GROUP BY #{columns} " - + add_order!(sql, options[:order], scope) add_limit!(sql, options, scope) add_lock!(sql, options, scope) - + find_by_sql(sql) end - + def parse_tags(tags) return [] if tags.blank? tags = Array(tags).first tags = tags.respond_to?(:flatten) ? tags.flatten : tags.split(Tag::DELIMITER) tags.map { |tag| tag.strip.squeeze(" ") }.flatten.compact.map(&:downcase).uniq end - + end include TaggingExtensions diff --git a/test/integration/todo_xml_api_test.rb b/test/integration/todo_xml_api_test.rb index 6b080a41..2a5ea739 100644 --- a/test/integration/todo_xml_api_test.rb +++ b/test/integration/todo_xml_api_test.rb @@ -35,7 +35,7 @@ class TodoXmlApiTest < ActionController::IntegrationTest authenticated_get_xml "/tickler", @user.login, @password, {} assert_no_tag :tag => "user_id" end - + def test_create_todo_with_show_from old_count = @user.todos.count authenticated_post_xml_to_todo_create " @@ -49,7 +49,7 @@ class TodoXmlApiTest < ActionController::IntegrationTest assert_response :success assert_equal @user.todos.count, old_count + 1 end - + def test_post_create_todo_with_dependencies authenticated_post_xml_to_todo_create " @@ -67,7 +67,7 @@ class TodoXmlApiTest < ActionController::IntegrationTest assert_not_nil todo assert !todo.uncompleted_predecessors.empty? end - + def test_post_create_todo_with_tags authenticated_post_xml_to_todo_create " @@ -76,6 +76,7 @@ class TodoXmlApiTest < ActionController::IntegrationTest #{projects(:timemachine).id} starred + starred1 starred2 " @@ -83,9 +84,10 @@ class TodoXmlApiTest < ActionController::IntegrationTest assert_response :success todo = @user.todos.find_by_description("this will succeed 3") assert_not_nil todo - assert !todo.starred? + assert_equal "starred, starred1, starred2", todo.tag_list + assert todo.starred? end - + def test_post_create_todo_with_new_context authenticated_post_xml_to_todo_create " @@ -95,14 +97,14 @@ class TodoXmlApiTest < ActionController::IntegrationTest @SomeNewContext " - + assert_response :success todo = @user.todos.find_by_description("this will succeed 4") assert_not_nil todo assert_not_nil todo.context assert_equal todo.context.name, "@SomeNewContext" end - + def test_post_create_todo_with_new_project authenticated_post_xml_to_todo_create " @@ -132,8 +134,8 @@ class TodoXmlApiTest < ActionController::IntegrationTest assert_select 'error', 2 end end - - def test_fails_with_401_if_not_authorized_user + + def test_fails_with_401_if_not_authorized_user authenticated_post_xml_to_todo_create '', 'nobody', 'nohow' assert_response 401 end