diff --git a/app/controllers/todos/todo_create_params_helper.rb b/app/controllers/todos/todo_create_params_helper.rb new file mode 100644 index 00000000..fc3c6f10 --- /dev/null +++ b/app/controllers/todos/todo_create_params_helper.rb @@ -0,0 +1,123 @@ +module Todos + class TodoCreateParamsHelper + + attr_reader :new_project_created, :new_context_created + + def initialize(params, user) + @params = params['request'] || params + @attributes = params['request'] && params['request']['todo'] || params['todo'] + @attributes = {} if @attributes.nil? # make sure there is at least an empty hash + @user = user + @errors = [] + + if @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 which will collide with tags association, so rename tags to add_tags + @attributes[:add_tags] = @attributes[:tags] + @attributes.delete :tags + end + + @new_project_created = find_or_create_group(:project, user.projects, project_name) + @new_context_created = find_or_create_group(:context, user.contexts, context_name) + @attributes["starred"] = (params[:new_todo_starred]||"").include? "true" if params[:new_todo_starred] + 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 project_id + @attributes['project_id'] + end + + def context_name + @params['context_name'].strip unless @params['context_name'].nil? + end + + def context_id + @attributes['context_id'] + end + + def tag_list + @params['tag_list'] + end + + def predecessor_list + @params['predecessor_list'] + end + + def parse_dates() + @attributes['show_from'] = @user.prefs.parse_date(show_from) + @attributes['due'] = @user.prefs.parse_date(due) + @attributes['due'] ||= '' + end + + def sequential? + return !@params[:todos_sequential].blank? && @params[:todos_sequential]=='true' + end + + def specified_by_name?(group_type) + return send("#{group_type}_specified_by_name?") + end + + def specified_by_id?(group_type) + group_id = send("#{group_type}_id") + return !group_id.blank? + 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 + + def add_errors(model) + @errors.each {|e| model.errors[ e[:attribute] ] = e[:message] } + end + + private + + def find_or_create_group(group_type, set, name) + return set_id_by_name(group_type, set, name) if specified_by_name?(group_type) + return set_id_by_id_string(group_type, set, @attributes["#{group_type}_id"]) if specified_by_id?(group_type) + end + + def set_id_by_name(group_type, set, name) + group = set.where(:name => name).first_or_create + @attributes["#{group_type}_id"] = group.id + return group.new_record_before_save? + end + + def set_id_by_id_string(group_type, set, id) + begin + # be aware, this will replace the project_id/context_id (string) in @attributes with the new found id (int) + @attributes["#{group_type}_id"] = set.find(id).id + return false + rescue + @errors << { :attribute => group_type, :message => "unknown"} + end + end + + end +end \ No newline at end of file diff --git a/app/controllers/todos_controller.rb b/app/controllers/todos_controller.rb index 2c2630ce..e00f048a 100644 --- a/app/controllers/todos_controller.rb +++ b/app/controllers/todos_controller.rb @@ -83,58 +83,33 @@ class TodosController < ApplicationController if is_multiple create_multiple else - p = TodoCreateParamsHelper.new(params, prefs) + p = Todos::TodoCreateParamsHelper.new(params, current_user) p.parse_dates() unless mobile? tag_list = p.tag_list - predecessor_list = p.predecessor_list @todo = current_user.todos.build(p.attributes) - - if p.project_specified_by_name? - project = current_user.projects.where(:name => p.project_name).first_or_create - @new_project_created = project.new_record_before_save? - @not_done_todos = [@todo] if @new_project_created - @todo.project_id = project.id - elsif !(p.project_id.nil? || p.project_id.blank?) - project = current_user.projects.where(:id => p.project_id).first - @todo.errors[:project] << "unknown" if project.nil? - end - - if p.context_specified_by_name? - context = current_user.contexts.where(:name => p.context_name).first_or_create - @new_context_created = context.new_record_before_save? - @not_done_todos = [@todo] if @new_context_created - @todo.context_id = context.id - elsif !(p.context_id.nil? || p.context_id.blank?) - context = current_user.contexts.where(:id=>p.context_id).first - @todo.errors[:context] << "unknown" if context.nil? - end + p.add_errors(@todo) if @todo.errors.empty? - @todo.starred= (params[:new_todo_starred]||"").include? "true" if params[:new_todo_starred] - @todo.add_predecessor_list(predecessor_list) + @todo.add_predecessor_list(p.predecessor_list) @saved = @todo.save + @todo.tag_with(tag_list) if @saved && !tag_list.blank? @todo.update_state_from_project if @saved + @todo.block! if @todo.should_be_blocked? else @saved = false end - unless ( !@saved ) || tag_list.blank? - @todo.tag_with(tag_list) - @todo.tags.reload - end - - if @saved - @todo.block! unless @todo.uncompleted_predecessors.empty? || @todo.state == 'project_hidden' - @saved = @todo.save - end - - @todo.reload if @saved @todo_was_created_deferred = @todo.deferred? @todo_was_created_blocked = @todo.pending? + @not_done_todos = [@todo] if p.new_project_created || p.new_context_created + @new_project_created = p.new_project_created + @new_context_created = p.new_context_created respond_to do |format| - format.html { redirect_to :action => "index" } + format.html do + redirect_to :action => "index" + end format.m do @return_path=cookies[:mobile_url] ? cookies[:mobile_url] : mobile_path if @saved @@ -148,8 +123,8 @@ class TodosController < ApplicationController format.js do if @saved determine_down_count - @contexts = current_user.contexts if @new_context_created - @projects = current_user.projects if @new_project_created + @contexts = current_user.contexts + @projects = current_user.projects @initial_context_name = params['default_context_name'] @initial_project_name = params['default_project_name'] @initial_tags = params['initial_tag_list'] @@ -174,60 +149,38 @@ class TodosController < ApplicationController end def create_multiple - p = TodoCreateParamsHelper.new(params, prefs) - if p.project_specified_by_name? - project = current_user.projects.where(:name => params[:project_name]).first_or_create - @new_project_created = project.new_record_before_save? - @project_id = project.id - end + p = Todos::TodoCreateParamsHelper.new(params, current_user) + tag_list = p.tag_list - if p.context_specified_by_name? - context = current_user.contexts.where(:name => params[:context_name]).first_or_create - @new_context_created = context.new_record_before_save? - @not_done_todos = [] if @new_context_created - @context_id = context.id - end - - tag_list = params[:tag_list] - - @sequential = !params[:todos_sequential].blank? && params[:todos_sequential]=='true' - - @todos_init = [] + @not_done_todos, @build_todos, @todos, errors = [], [], [], [] @predecessor = nil validates = true - errors = [] # first build all todos and check if they would validate on save params[:todo][:multiple_todos].split("\n").map do |line| unless line.blank? #ignore blank lines - @todo = current_user.todos.build(:description => line) - @todo.project_id = @project_id - @todo.context_id = @context_id + @todo = current_user.todos.build({:description => line, :context_id => p.context_id, :project_id => p.project_id}) validates &&= @todo.valid? - @todos_init << @todo + @build_todos << @todo end end # if all todos validate, then save them and add predecessors and tags - @todos = [] if validates - @todos_init.each do |todo| + @build_todos.each do |todo| @saved = todo.save - validates = validates && @saved + validates &&= @saved - if @predecessor && @saved && @sequential + if @predecessor && @saved && p.sequential? todo.add_predecessor(@predecessor) todo.block! end - - unless (@saved == false) || tag_list.blank? - todo.tag_with(tag_list) - todo.tags.reload - end + + todo.tag_with(tag_list) unless (@saved == false) || tag_list.blank? @todos << todo - @not_done_todos << todo if @new_context_created + @not_done_todos << todo if p.new_context_created || p.new_project_created @predecessor = todo end else @@ -239,8 +192,8 @@ class TodosController < ApplicationController format.html { redirect_to :action => "index" } format.js do determine_down_count if @saved - @contexts = current_user.contexts if @new_context_created - @projects = current_user.projects if @new_project_created + @contexts = current_user.contexts if p.new_context_created + @projects = current_user.projects if p.new_project_created @initial_context_name = params['default_context_name'] @initial_project_name = params['default_project_name'] @initial_tags = params['initial_tag_list'] @@ -253,8 +206,8 @@ class TodosController < ApplicationController end @status_message = @todos.size > 1 ? t('todos.added_new_next_action_plural') : t('todos.added_new_next_action_singular') - @status_message = t('todos.added_new_project') + ' / ' + @status_message if @new_project_created - @status_message = t('todos.added_new_context') + ' / ' + @status_message if @new_context_created + @status_message = t('todos.added_new_project') + ' / ' + @status_message if p.new_project_created + @status_message = t('todos.added_new_context') + ' / ' + @status_message if p.new_context_created render :action => 'create_multiple' end @@ -1380,79 +1333,4 @@ class TodosController < ApplicationController return not_done_todos end - - class TodoCreateParamsHelper - - def initialize(params, prefs) - @params = params['request'] || params - @prefs = prefs - @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 which will collide with tags association, so rename tags to add_tags - @attributes[:add_tags] = @attributes[:tags] - @attributes.delete :tags - end - 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 project_id - @attributes['project_id'] - end - - def context_name - @params['context_name'].strip unless @params['context_name'].nil? - end - - def context_id - @attributes['context_id'] - end - - def tag_list - @params['tag_list'] - end - - def predecessor_list - @params['predecessor_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 +end \ No newline at end of file diff --git a/app/models/todo.rb b/app/models/todo.rb index 5cdeed15..65f56cdb 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -144,6 +144,10 @@ class Todo < ActiveRecord::Base return !uncompleted_predecessors.all.empty? end + def should_be_blocked? + return !( uncompleted_predecessors.empty? || state == 'project_hidden' ) + end + # Returns a string with description def specification project_name = self.project.is_a?(NullProject) ? "(none)" : self.project.name diff --git a/test/unit/todo_create_params_helper_test.rb b/test/unit/todo_create_params_helper_test.rb index 3251150a..812eda84 100644 --- a/test/unit/todo_create_params_helper_test.rb +++ b/test/unit/todo_create_params_helper_test.rb @@ -5,48 +5,44 @@ class TodoCreateParamsHelperTest < ActiveSupport::TestCase def test_works_with_request_as_root_hash_entry params = {'request' => { 'todo' => { 'description' => 'foo'}}} - prefs = users(:admin_user).prefs - params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs) + params_helper = Todos::TodoCreateParamsHelper.new(params, users(:admin_user)) assert_equal({'description' => 'foo'}, params_helper.attributes) end def test_works_with_todo_as_root_hash_entry params = { 'todo' => { 'description' => 'foo'}} - prefs = users(:admin_user).prefs - params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs) + params_helper = Todos::TodoCreateParamsHelper.new(params, users(:admin_user)) assert_equal({'description' => 'foo'}, params_helper.attributes) end def test_show_from_accessor expected_date = Time.now params = { 'todo' => { 'show_from' => expected_date}} - prefs = users(:admin_user).prefs - params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs) + params_helper = Todos::TodoCreateParamsHelper.new(params, users(:admin_user)) assert_equal(expected_date, params_helper.show_from) end def test_due_accessor expected_date = Time.now params = { 'todo' => { 'due' => expected_date}} - prefs = users(:admin_user).prefs - params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs) + params_helper = Todos::TodoCreateParamsHelper.new(params, users(:admin_user)) assert_equal(expected_date, params_helper.due) end def test_tag_list_accessor params = { 'todo' => { }, 'tag_list' => 'foo, bar'} - prefs = users(:admin_user).prefs - params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs) + params_helper = Todos::TodoCreateParamsHelper.new(params, users(:admin_user)) assert_equal('foo, bar', params_helper.tag_list) end def test_parse_dates_parses_show_from_date_based_on_prefs params = { 'todo' => { 'show_from' => '20/05/07', 'due' => '23/5/07'}} - prefs = users(:admin_user).prefs + user = users(:admin_user) + prefs = user.prefs prefs.date_format = "%d/%m/%y" # make sure the format matches the above - params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs) + params_helper = Todos::TodoCreateParamsHelper.new(params, user) params_helper.parse_dates() assert_equal Date.new(2007, 5, 20), params_helper.show_from.to_date end @@ -54,82 +50,73 @@ class TodoCreateParamsHelperTest < ActiveSupport::TestCase def test_parse_dates_parses_due_date_based_on_prefs params = { 'todo' => { 'show_from' => '20/5/07', 'due' => '23/5/07'}} - prefs = users(:admin_user).prefs + user = users(:admin_user) + prefs = user.prefs prefs.date_format = "%d/%m/%y" # make sure the format matches the above - params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs) + params_helper = Todos::TodoCreateParamsHelper.new(params, user) params_helper.parse_dates() assert_equal Date.new(2007, 5, 23), params_helper.due.to_date end def test_parse_dates_sets_due_to_empty_string_if_nil params = { 'todo' => { 'show_from' => '20/5/07', 'due' => nil}} - prefs = users(:admin_user).prefs - params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs) + params_helper = Todos::TodoCreateParamsHelper.new(params, users(:admin_user)) 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 = users(:admin_user).prefs - params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs) + params_helper = Todos::TodoCreateParamsHelper.new(params, users(:admin_user)) assert_equal 'Visit New Orleans', params_helper.project_name end def test_project_name_is_nil_when_unspecified params = { } - prefs = users(:admin_user).prefs - params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs) + params_helper = Todos::TodoCreateParamsHelper.new(params, users(:admin_user)) assert_nil params_helper.project_name end def test_context_name_is_stripped_of_leading_and_trailing_whitespace params = { 'context_name' => ' mobile phone ' } - prefs = users(:admin_user).prefs - params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs) + params_helper = Todos::TodoCreateParamsHelper.new(params, users(:admin_user)) assert_equal 'mobile phone', params_helper.context_name end def test_context_name_is_nil_when_unspecified params = { } - prefs = users(:admin_user).prefs - params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs) + params_helper = Todos::TodoCreateParamsHelper.new(params, users(:admin_user)) 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 = users(:admin_user).prefs - params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs) + params_helper = Todos::TodoCreateParamsHelper.new(params, users(:admin_user)) 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 = users(:admin_user).prefs - params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs) + params_helper = Todos::TodoCreateParamsHelper.new(params, users(:admin_user)) 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 = users(:admin_user).prefs - params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs) + params_helper = Todos::TodoCreateParamsHelper.new(params, users(:admin_user)) 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 = users(:admin_user).prefs - params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs) + params_helper = Todos::TodoCreateParamsHelper.new(params, users(:admin_user)) 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 = users(:admin_user).prefs - params_helper = TodosController::TodoCreateParamsHelper.new(params, prefs) + params_helper = Todos::TodoCreateParamsHelper.new(params, users(:admin_user)) assert_equal false, params_helper.context_specified_by_name? end