From 960326efb43a72a9bfab984f577d6409f234332e Mon Sep 17 00:00:00 2001
From: Reinier Balt <lrbalt@gmail.com>
Date: Sun, 20 Nov 2011 14:48:49 +0100
Subject: [PATCH] Fix #1223 for the single tag and single dependency case in
 the REST API

---
 app/controllers/todos_controller.rb   |  6 ++--
 app/models/todo.rb                    | 23 ++++-----------
 test/integration/todo_xml_api_test.rb | 42 ++++++++++++++++++++++++---
 3 files changed, 48 insertions(+), 23 deletions(-)

diff --git a/app/controllers/todos_controller.rb b/app/controllers/todos_controller.rb
index b01d1dc5..1661f22f 100644
--- a/app/controllers/todos_controller.rb
+++ b/app/controllers/todos_controller.rb
@@ -1518,10 +1518,12 @@ class TodosController < ApplicationController
       @attributes = params['request'] && params['request']['todo']  || params['todo']
 
       if @attributes && @attributes[:tags]
+        # for single tags, @attributed[:tags] returns a hash. For multiple tags,
+        # it with return an array of hashes. Make sure it is always an array of hashes
+        @attributes[:tags][:tag] = [@attributes[:tags][:tag]] unless @attributes[:tags][:tag].class == Array
         # the REST api may use <tags> which will collide with tags association, so rename tags to add_tags
-        add_tags = @attributes[:tags]
+        @attributes[:add_tags] = @attributes[:tags]
         @attributes.delete :tags
-        @attributes[:add_tags] = add_tags
       end
     end
 
diff --git a/app/models/todo.rb b/app/models/todo.rb
index 67c6db17..4f34f9b8 100644
--- a/app/models/todo.rb
+++ b/app/models/todo.rb
@@ -299,24 +299,13 @@ class Todo < ActiveRecord::Base
   # XML API fixups
   def predecessor_dependencies=(params)
     value = params[:predecessor]
+    return if value.nil?
 
-    if !value.nil?
-      if value.class == Array
-        value.each do |ele|
-          if ele.is_a? String
-            add_predecessor(self.user.todos.find_by_id(ele.to_i)) unless ele.blank?
-          else
-            predecessor_dependencies.build(value)
-          end
-        end
-      else
-        if ele.is_a? String
-          add_predecessor(self.user.todos.find_by_id(ele.to_i)) unless ele.blank?
-        else
-          predecessor_dependencies.build(value)
-        end
-      end
-    end
+    # for multiple dependencies, value will be an array of id's, but for a single dependency,
+    # value will be a string. In that case convert to array
+    value = [value] unless value.class == Array
+
+    value.each { |ele| add_predecessor(self.user.todos.find_by_id(ele.to_i)) unless ele.blank? }
   end
 
   alias_method :original_context=, :context=
diff --git a/test/integration/todo_xml_api_test.rb b/test/integration/todo_xml_api_test.rb
index 2a5ea739..d1a41a16 100644
--- a/test/integration/todo_xml_api_test.rb
+++ b/test/integration/todo_xml_api_test.rb
@@ -50,10 +50,10 @@ class TodoXmlApiTest < ActionController::IntegrationTest
     assert_equal @user.todos.count, old_count + 1
   end
 
-  def test_post_create_todo_with_dependencies
+  def test_post_create_todo_with_multiple_dependencies
     authenticated_post_xml_to_todo_create "
 <todo>
-  <description>this will succeed 2</description>
+  <description>this will succeed 2.0</description>
   <context_id>#{contexts(:office).id}</context_id>
   <project_id>#{projects(:timemachine).id}</project_id>
   <predecessor_dependencies>
@@ -63,12 +63,29 @@ class TodoXmlApiTest < ActionController::IntegrationTest
 </todo>"
 
     assert_response :success
-    todo = @user.todos.find_by_description("this will succeed 2")
+    todo = @user.todos.find_by_description("this will succeed 2.0")
     assert_not_nil todo
     assert !todo.uncompleted_predecessors.empty?
   end
 
-  def test_post_create_todo_with_tags
+  def test_post_create_todo_with_single_dependency
+    authenticated_post_xml_to_todo_create "
+<todo>
+  <description>this will succeed 2.1</description>
+  <context_id>#{contexts(:office).id}</context_id>
+  <project_id>#{projects(:timemachine).id}</project_id>
+  <predecessor_dependencies>
+    <predecessor>6</predecessor>
+  </predecessor_dependencies>
+</todo>"
+
+    assert_response :success
+    todo = @user.todos.find_by_description("this will succeed 2.1")
+    assert_not_nil todo
+    assert !todo.uncompleted_predecessors.empty?
+  end
+
+  def test_post_create_todo_with_multiple_tags
     authenticated_post_xml_to_todo_create "
 <todo>
   <description>this will succeed 3</description>
@@ -88,6 +105,23 @@ class TodoXmlApiTest < ActionController::IntegrationTest
     assert todo.starred?
   end
 
+  def test_post_create_todo_with_single_tag
+    authenticated_post_xml_to_todo_create "
+<todo>
+  <description>this will succeed 3.1</description>
+  <context_id>#{contexts(:office).id}</context_id>
+  <project_id>#{projects(:timemachine).id}</project_id>
+  <tags>
+    <tag><name>tracks</name></tag>
+  </tags>
+</todo>"
+
+    assert_response :success
+    todo = @user.todos.find_by_description("this will succeed 3.1")
+    assert_not_nil todo
+    assert_equal "tracks", todo.tag_list
+  end
+
   def test_post_create_todo_with_new_context
     authenticated_post_xml_to_todo_create "
 <todo>