fix failing test and make sure that you can supply 'starred' tag. Refactor todo model

This commit is contained in:
Reinier Balt 2011-11-16 16:37:04 +01:00
parent 72edf10ad3
commit 906ff11633
4 changed files with 71 additions and 72 deletions

View file

@ -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 <tags> 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

View file

@ -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. <tags> 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

View file

@ -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 <tt>self</tt>. 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 <tt>self</tt>. Accepts a string of tagnames, an array of tagnames, an array of ids, or an array of Tags.
# Removes tags from <tt>self</tt>. 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 <tt>self</tt>. 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

View file

@ -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 "
<todo>
@ -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 "
<todo>
@ -76,6 +76,7 @@ class TodoXmlApiTest < ActionController::IntegrationTest
<project_id>#{projects(:timemachine).id}</project_id>
<tags>
<tag><name>starred</name></tag>
<tag><name>starred1</name></tag>
<tag><name>starred2</name></tag>
</tags>
</todo>"
@ -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 "
<todo>
@ -95,14 +97,14 @@ class TodoXmlApiTest < ActionController::IntegrationTest
<name>@SomeNewContext</name>
</context>
</todo>"
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 "
<todo>
@ -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