fix #1429 by correctly handling tags with dots

This was a very old regression. This used to work before the rails 3.2 upgrade... Added tests
to prevent future unnoticed breakage

app/controllers/todos_controller.rb

@@ -594,13 +594,8 @@ class TodosController < ApplicationController
     get_params_for_tag_view
     @page_title = t('todos.tagged_page_title', :tag_name => @tag_title)
     @source_view = params['_source_view'] || 'tag'
-
-    if mobile?
-      # mobile tags are routed with :name ending on .m. So we need to chomp it
-      @tag_name = @tag_name.chomp('.m')
-    else
-      init_data_for_sidebar
-    end
+    
+    init_data_for_sidebar unless mobile?
 
     todos_with_tag_ids = find_todos_with_tag_expr(@tag_expr)
 
@@ -833,8 +828,9 @@ class TodosController < ApplicationController
   end
 
   def get_params_for_tag_view
-    # use sanitize to prevent XSS attacks
+    filter_format_for_tag_view
 
+    # use sanitize to prevent XSS attacks
     @tag_expr = []
     @tag_expr << sanitize(params[:name]).split(',')
     @tag_expr << sanitize(params[:and]).split(',') if params[:and]
@@ -850,6 +846,27 @@ class TodosController < ApplicationController
     @tag_title = @single_tag ? @tag_name : tag_title(@tag_expr)
   end
 
+  def filter_format_for_tag_view
+    # routes for tag view do not set :format
+    if params[:name] =~ /.*\.m$/
+      set_format_for_tag_view(:m)
+    elsif params[:name] =~ /.*\.txt$/
+      set_format_for_tag_view(:txt)
+      # set content-type to text/plain or it remains text/html
+      response.headers["Content-Type"] = 'text/plain'
+    elsif params[:format].nil?
+      # if no format is given, default to html
+      # note that if url has ?format=m, we should not overwrite it here
+      request.format, params[:format] = :html, :html
+    end
+  end
+
+  def set_format_for_tag_view(format)
+    # tag name ends with .m, set format to :m en remove .m from name
+    request.format, params[:format] = format, format
+    params[:name] = params[:name].chomp(".#{format.to_s}")
+end
+
   def get_ids_from_tag_expr(tag_expr)
     ids = []
     tag_expr.each do |tag_list|

config/routes.rb

@@ -101,7 +101,12 @@ Tracksapp::Application.routes.draw do
       post 'add_predecessor'
     end
   end
-  match 'todos/tag/:name' => 'todos#tag', :as => :tag
+
+  # match /todos/tag and put everything in :name, including extensions like .m and .txt. 
+  # This means the controller action needs to parse the extension and set format/content type
+  # Needed for /todos/tag/first.last.m to work
+  match 'todos/tag/:name'     => 'todos#tag', :as => :tag, :format => false, :name => /.*/
+
   match 'tags.autocomplete' => "todos#tags", :format => 'autocomplete'
 
   match 'todos/done/tag/:name' => "todos#done_tag", :as => :done_tag

test/functional/todos_controller_test.rb

@@ -120,6 +120,28 @@ class TodosControllerTest < ActionController::TestCase
     assert_response :success
     assert_equal 3, @tagged
   end
+  
+  def test_find_tagged_with_terms_separated_with_dot
+    login_as :admin_user
+    create_todo(description: "test dotted tag", tag_list: "first.last, second")
+    t = assigns['todo']
+    assert_equal "first.last, second", t.tag_list
+
+    get :tag, name: 'first.last.m'
+    assert_equal "text/html", request.format, "controller should set right content type"
+    assert_equal "text/html", @response.content_type
+    assert_equal "first.last", assigns['tag_name'], ".m should be chomped"
+
+    get :tag, name: 'first.last.txt'
+    assert_equal "text/plain", request.format, "controller should set right content type"
+    assert_equal "text/plain", @response.content_type
+    assert_equal "first.last", assigns['tag_name'], ".txt should be chomped"
+
+    get :tag, name: 'first.last'
+    assert_equal "text/html", request.format, "controller should set right content type"
+    assert_equal "text/html", @response.content_type
+    assert_equal "first.last", assigns['tag_name'], ":name should be correct"
+  end
 
   def test_get_boolean_expression_from_parameters_of_tag_view_single_tag
     login_as(:admin_user)
@@ -920,4 +942,17 @@ class TodosControllerTest < ActionController::TestCase
     assert t4.pending?, "t4 should remain pending"
     assert t4.predecessors.map(&:id).include?(t3.id)
   end
-end
+
+  private
+
+  def create_todo(params={})
+    defaults = { source_view: 'todo', 
+      context_name: "library", project_name: "Build a working time machine", 
+      notes: "note", description: "a new todo", due: nil, tag_list: "a,b,c"}
+
+    params=params.reverse_merge(defaults)
+
+    put :create, _source_view: params[:_source_view], 
+      context_name: params[:context_name], project_name: params[:project_name], tag_list: params[:tag_list],
+      todo: {notes: params[:notes], description: params[:description], due: params[:due]}
+  end

test/unit/tagging_test.rb

@@ -11,6 +11,6 @@ class TaggingTest < ActiveSupport::TestCase
     
     tagging.destroy
     
-    assert_nil Tag.where(:name => "hello").first
+    assert_nil Tag.where(:name => "hello").first, "Tag should be destroyed when last use in tagging was removed"
   end
 end