Refactored TodosController#create method by introducing class to make it easier to work with the creation params.

git-svn-id: http://www.rousette.org.uk/svn/tracks-repos/trunk@628 a4c988fc-2ded-0310-b66e-134b36920a42

tracks/app/controllers/application.rb

@@ -178,8 +178,7 @@ class ApplicationController < ActionController::Base
   private
         
   def parse_date_per_user_prefs( s )
-    return nil if s.blank?
-    Date.strptime(s, prefs.date_format)
+    prefs.parse_date(s)
   end
     
   def init_data_for_sidebar

tracks/app/controllers/todos_controller.rb

@@ -35,48 +35,28 @@ class TodosController < ApplicationController
   end
   
   def create
-    @todo = current_user.todos.build
-    p = params['request'] || params
+    p = TodoCreateParamsHelper.new(params, prefs)        
+    p.parse_dates() unless mobile?
     
-    if p['todo']['show_from'] && !mobile?
-      p['todo']['show_from'] = parse_date_per_user_prefs(p['todo']['show_from'])
-    end
+    @todo = current_user.todos.build(p.attributes)
     
-    @todo.attributes = p['todo']
-    
-    if p['todo']['project_id'].blank? && !p['project_name'].blank? && p['project_name'] != 'None'
-      project = current_user.projects.find_by_name(p['project_name'].strip)
-      unless project
-          project = current_user.projects.build
-          project.name = p['project_name'].strip
-          project.save
-          @new_project_created = true
-      end
+    if p.project_specified_by_name?
+      project = current_user.projects.find_or_create_by_name(p.project_name)
+      @new_project_created = project.new_record_before_save?
       @todo.project_id = project.id
     end
     
-    if p['todo']['context_id'].blank? && !p['context_name'].blank?
-      context = current_user.contexts.find_by_name(p['context_name'].strip)
-      unless context
-          context = current_user.contexts.build
-          context.name = p['context_name'].strip
-          context.save
-          @new_context_created = true
-          @not_done_todos = [@todo]
-      end
+    if p.context_specified_by_name?
+      context = current_user.contexts.find_or_create_by_name(p.context_name)
+      @new_context_created = context.new_record_before_save?
+      @not_done_todos = [@todo] if @new_context_created
       @todo.context_id = context.id
     end
 
-    if @todo.due?
-      @todo.due = parse_date_per_user_prefs(p['todo']['due']) unless mobile?
-    else
-      @todo.due = ""
-    end
-    
     @saved = @todo.save
-    if @saved
-      @todo.tag_with(params[:tag_list], current_user) if params[:tag_list] 
-      @todo.reload
+    unless (@saved == false) || p.tag_list.blank?
+      @todo.tag_with(p.tag_list, current_user)
+      @todo.tags.reload
     end
     
     respond_to do |format|
@@ -303,7 +283,6 @@ class TodosController < ApplicationController
   end
   
   # /todos/tag/[tag_name] shows all the actions tagged with tag_name
-  #
   def tag
     
     @tag_name = params[:name]
@@ -610,4 +589,56 @@ class TodosController < ApplicationController
       end
     end
 
+    class TodoCreateParamsHelper
+
+      def initialize(params, prefs)
+        @params = params['request'] || params
+        @prefs = prefs
+        @attributes = params['request'] && params['request']['todo']  || params['todo']
+      end
+      
+      def attributes
+        @attributes
+      end
+      
+      def show_from
+        @attributes['show_from']
+      end
+      
+      def due
+        @attributes['due']
+      end
+      
+      def project_name
+        @params['project_name'].strip unless @params['project_name'].nil?
+      end
+      
+      def context_name
+        @params['context_name'].strip unless @params['context_name'].nil?
+      end
+      
+      def tag_list
+        @params['tag_list']
+      end
+      
+      def parse_dates()
+        @attributes['show_from'] = @prefs.parse_date(show_from)
+        @attributes['due'] = @prefs.parse_date(due)
+        @attributes['due'] ||= ''
+      end
+      
+      def project_specified_by_name?
+        return false unless @attributes['project_id'].blank?
+        return false if project_name.blank?
+        return false if project_name == 'None'
+        true
+      end
+      
+      def context_specified_by_name?
+        return false unless @attributes['context_id'].blank?
+        return false if context_name.blank?
+        true
+      end
+      
+    end
 end

tracks/app/models/context.rb

@@ -38,6 +38,10 @@ class Context < ActiveRecord::Base
     s += "Context is #{hidden? ? 'Hidden' : 'Active'}."
     s += "</p>"
     s
+  end
+  
+  def new_record_before_save?
+    @new_record_before_save
   end  
 
 end

tracks/app/models/preference.rb

@@ -22,4 +22,9 @@ class Preference < ActiveRecord::Base
     return show_number_completed == 0
   end
   
+  def parse_date(s)
+    return nil if s.blank?
+    Date.strptime(s, date_format)
+  end
+  
 end

tracks/app/models/project.rb

@@ -90,6 +90,11 @@ class Project < ActiveRecord::Base
   def name=(value)
     self[:name] = value.gsub(/\s{2,}/, " ").strip
   end
+  
+  def new_record_before_save?
+    @new_record_before_save
+  end  
+  
 end
 
 class NullProject

tracks/test/unit/preference_test.rb

@@ -19,5 +19,17 @@ class PreferenceTest < Test::Rails::TestCase
     assert @other_user.preference.show_project_on_todo_done
     assert !@admin_user.preference.show_project_on_todo_done
   end
+  
+  def test_parse_date
+    assert_equal Date.new(2007, 5, 20).to_s, @admin_user.preference.parse_date('20/5/2007').to_s
+  end
+  
+  def test_parse_date_returns_nil_if_string_is_empty
+    assert_nil @admin_user.preference.parse_date('')
+  end
+
+  def test_parse_date_returns_nil_if_string_is_nil
+    assert_nil @admin_user.preference.parse_date(nil)
+  end
 
 end

tracks/test/unit/todo_create_params_helper_test.rb

@@ -0,0 +1,136 @@
+require File.dirname(__FILE__) + '/../test_helper'
+require 'todos_controller'
+
+class TodoCreateParamsHelperTest < Test::Rails::TestCase
+
+  def test_works_with_request_as_root_hash_entry
+    params = {'request' => { 'todo' => { 'description' => 'foo'}}}
+    prefs = flexmock()
+    params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs)
+    assert_equal({'description' => 'foo'}, params_helper.attributes)
+  end
+
+  def test_works_with_todo_as_root_hash_entry
+    params = { 'todo' => { 'description' => 'foo'}}
+    prefs = flexmock()
+    params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs)
+    assert_equal({'description' => 'foo'}, params_helper.attributes)
+  end
+  
+  def test_show_from_accessor
+    expected_date = Time.now.to_date
+    params = { 'todo' => { 'show_from' => expected_date}}
+    prefs = flexmock()
+    params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs)
+    assert_equal(expected_date, params_helper.show_from)
+  end
+  
+  def test_due_accessor
+    expected_date = Time.now.to_date
+    params = { 'todo' => { 'due' => expected_date}}
+    prefs = flexmock()
+    params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs)
+    assert_equal(expected_date, params_helper.due)
+  end
+  
+  def test_tag_list_accessor
+    params = { 'todo' => { }, 'tag_list' => 'foo, bar'}
+    prefs = flexmock()
+    params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs)
+    assert_equal('foo, bar', params_helper.tag_list)
+  end
+  
+  def test_parse_dates_parses_show_from_date_based_on_prefs
+    params = { 'todo' => { 'show_from' => '5/20/07', 'due' => '5/23/07'}}
+    prefs = flexmock()
+    prefs.should_receive(:parse_date).with('5/20/07').and_return(Date.new(2007, 5, 20))
+    prefs.should_receive(:parse_date).with('5/23/07').and_return(Date.new(2007, 5, 23))
+    params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs)
+    params_helper.parse_dates()
+    assert_equal Date.new(2007, 5, 20), params_helper.show_from
+  end
+
+  def test_parse_dates_parses_due_date_based_on_prefs
+    params = { 'todo' => { 'show_from' => '5/20/07', 'due' => '5/23/07'}}
+    prefs = flexmock()
+    prefs.should_receive(:parse_date).with('5/20/07').and_return(Date.new(2007, 5, 20))
+    prefs.should_receive(:parse_date).with('5/23/07').and_return(Date.new(2007, 5, 23))
+    params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs)
+    params_helper.parse_dates()
+    assert_equal Date.new(2007, 5, 23), params_helper.due
+  end
+
+  def test_parse_dates_sets_due_to_empty_string_if_nil
+    params = { 'todo' => { 'show_from' => '5/20/07', 'due' => nil}}
+    prefs = flexmock()
+    prefs.should_receive(:parse_date).with('5/20/07').and_return(Date.new(2007, 5, 20))
+    prefs.should_receive(:parse_date).with(nil).and_return(nil)
+    params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs)
+    params_helper.parse_dates()
+    assert_equal '', params_helper.due
+  end
+  
+  def test_project_name_is_stripped_of_leading_and_trailing_whitespace
+    params = { 'project_name' => ' Visit New Orleans  ' }
+    prefs = flexmock()
+    params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs)
+    assert_equal 'Visit New Orleans', params_helper.project_name
+  end
+    
+  def test_project_name_is_nil_when_unspecified
+    params = { }
+    prefs = flexmock()
+    params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs)
+    assert_nil params_helper.project_name
+  end
+    
+  def test_context_name_is_stripped_of_leading_and_trailing_whitespace
+    params = { 'context_name' => ' mobile phone  ' }
+    prefs = flexmock()
+    params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs)
+    assert_equal 'mobile phone', params_helper.context_name
+  end
+  
+  def test_context_name_is_nil_when_unspecified
+    params = { }
+    prefs = flexmock()
+    params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs)
+    assert_nil params_helper.context_name
+  end
+  
+  def test_project_specified_by_name_is_false_when_project_id_is_specified
+    params = { 'todo' => { 'project_id' => 2 } }
+    prefs = flexmock()
+    params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs)
+    assert_equal false, params_helper.project_specified_by_name?
+  end
+  
+  def test_project_specified_by_name_is_false_when_project_name_is_blank
+    params = { 'project_name' => nil, 'todo' => {} }
+    prefs = flexmock()
+    params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs)
+    assert_equal false, params_helper.project_specified_by_name?
+  end
+  
+  def test_project_specified_by_name_is_false_when_project_name_is_none
+    params = { 'project_name' => 'None', 'todo' => {} }
+    prefs = flexmock()
+    params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs)
+    assert_equal false, params_helper.project_specified_by_name?
+  end
+  
+  def test_context_specified_by_name_is_false_when_context_id_is_specified
+    params = { 'todo' => { 'context_id' => 3 } }
+    prefs = flexmock()
+    params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs)
+    assert_equal false, params_helper.context_specified_by_name?
+  end
+    
+  def test_context_specified_by_name_is_false_when_context_name_is_blank
+    params = { 'context_name' => nil, 'todo' => {} }
+    prefs = flexmock()
+    params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs)
+    assert_equal false, params_helper.context_specified_by_name?
+  end
+  
+end