From fde151123c8189b2832296123913b975147492ca Mon Sep 17 00:00:00 2001
From: Reinier Balt <lrbalt@gmail.com>
Date: Fri, 30 Sep 2011 12:06:43 +0200
Subject: [PATCH] refactor tag helpers

---
 app/helpers/todos_helper.rb | 22 +++++++++++-----------
 app/models/tag.rb           | 23 +++++++++++++----------
 app/models/todo.rb          |  3 ++-
 config/routes.rb            |  2 +-
 4 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb
index f1873b34..62c39023 100644
--- a/app/helpers/todos_helper.rb
+++ b/app/helpers/todos_helper.rb
@@ -122,20 +122,20 @@ module TodosHelper
     todo.tags.collect{|t| t.name}.join(', ')
   end
 
-  def tag_list(todo=@todo)
-    tags_except_starred = todo.tags.reject{|t| t.name == Todo::STARRED_TAG_NAME}
-    tag_list = tags_except_starred.collect{|t| "<span class=\"tag #{t.name.gsub(' ','-')}\">" + link_to(t.name, :controller => "todos", :action => "tag", :id => t.name) + "</span>"}.join('')
-    "<span class='tags'>#{tag_list}</span>"
+  def tag_span (tag, mobile=false)
+    content_tag(:span, :class => "tag #{tag.name.gsub(' ','-')}") { link_to (tag.name, mobile ? mobile_tag_path(tag.name) : tag_path(tag.name)) }
+  end
+
+  def tag_list(todo=@todo, mobile=false)
+    content_tag(:span, :class => 'tags') { todo.tags.all_except_starred.collect{|tag| tag_span(tag, mobile)}.join('') }
   end
 
   def tag_list_mobile(todo=@todo)
-    tags_except_starred = todo.tags.reject{|t| t.name == Todo::STARRED_TAG_NAME}
-    # removed the link. TODO: add link to mobile view of tagged actions
-    tag_list = tags_except_starred.collect{|t|
-      "<span class=\"tag\">" +
-        link_to(t.name, {:action => "tag", :controller => "todos", :id => t.name+".m"}) +
-        "</span>"}.join('')
-    if tag_list.empty? then "" else "<span class=\"tags\">#{tag_list}</span>" end
+    unless todo.tags.all_except_starred.empty?
+      return tag_list(todo, true)
+    else
+      return ""
+    end
   end
 
   def deferred_due_date(todo=@todo)
diff --git a/app/models/tag.rb b/app/models/tag.rb
index 28f72662..0d6d24e5 100644
--- a/app/models/tag.rb
+++ b/app/models/tag.rb
@@ -11,38 +11,41 @@ class Tag < ActiveRecord::Base
   # rescue the ActiveRecord database constraint errors instead.
   validates_presence_of :name
   validates_uniqueness_of :name, :case_sensitive => false
-  
+
   # Change this validation if you need more complex tag names.
   # validates_format_of :name, :with => /^[a-zA-Z0-9\_\-]+$/, :message => "can not contain special characters"
-  
+
   # Set up the polymorphic relationship.
-  has_many_polymorphs :taggables, 
-    :from => [:todos, :recurring_todos], 
-    :through => :taggings, 
+  has_many_polymorphs :taggables,
+    :from => [:todos, :recurring_todos],
+    :through => :taggings,
     :dependent => :destroy,
-    :skip_duplicates => false, 
+    :skip_duplicates => false,
     :parent_extend => proc {
     # Defined on the taggable models, not on Tag itself. Return the tagnames
     # associated with this record as a string.
     def to_s
       self.map(&:name).sort.join(Tag::JOIN_DELIMITER)
     end
+    def all_except_starred
+      self.reject{|tag| tag.name == Todo::STARRED_TAG_NAME}
+    end
   }
-    
+
   # Callback to strip extra spaces from the tagname before saving it. If you
   # allow tags to be renamed later, you might want to use the
   # <tt>before_save</tt> callback instead.
-  def before_create 
+  def before_create
     self.name = name.downcase.strip.squeeze(" ")
   end
 
   def on(taggable, user)
     taggings.create :taggable => taggable, :user => user
   end
-  
+
   # Tag::Error class. Raised by ActiveRecord::Base::TaggingExtensions if
   # something goes wrong.
   class Error < StandardError
   end
-    
+
 end
diff --git a/app/models/todo.rb b/app/models/todo.rb
index b93ca342..ea6e17c9 100644
--- a/app/models/todo.rb
+++ b/app/models/todo.rb
@@ -46,13 +46,14 @@ class Todo < ActiveRecord::Base
   STARRED_TAG_NAME = "starred"
   DEFAULT_INCLUDES = [ :project, :context, :tags, :taggings, :pending_successors, :uncompleted_predecessors, :recurring_todo ]
 
-  # regular expressions for dependencies
+  # regular expressions for dependencies. TODO: are these still used?
   RE_TODO = /[^']+/
   RE_CONTEXT = /[^']+/
   RE_PROJECT = /[^']+/
   RE_PARTS = /'(#{RE_TODO})'\s<'(#{RE_CONTEXT})';\s'(#{RE_PROJECT})'>/ # results in array
   RE_SPEC = /'#{RE_TODO}'\s<'#{RE_CONTEXT}';\s'#{RE_PROJECT}'>/ # results in string
 
+  # state machine
   include AASM
   aasm_column :state
   aasm_initial_state Proc.new { |t| (t.show_from && t.user && (t.show_from > t.user.date)) ? :deferred : :active}
diff --git a/config/routes.rb b/config/routes.rb
index 959b7491..b3897e5d 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -39,7 +39,7 @@ ActionController::Routing::Routes.draw do |map|
     # so /todos/tag/version1.5.xml will result in :name => 'version1.5.xml'
     # UPDATE: added support for mobile view. All tags ending on .m will be
     # routed to mobile view of tags.
-    todos.tag 'todos/tag/:name.m', :action => "tag", :format => 'm'
+    todos.mobile_tag 'todos/tag/:name.m', :action => "tag", :format => 'm'
     todos.tag 'todos/tag/:name', :action => "tag", :name => /.*/
     todos.done_tag 'todos/done/tag/:name', :action => "done_tag"
     todos.all_done_tag 'todos/all_done/tag/:name', :action => "all_done_tag"