From 2accbd0a3257c2e4a99fab2f3fb23f3fc64c603f Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Sat, 1 Oct 2011 03:45:41 +0200 Subject: [PATCH] start changing param parsing to allow and and or of tags --- app/controllers/todos_controller.rb | 63 +++++++++++++++++++++++---- app/models/todo.rb | 1 + test/functional/todos_tagging_test.rb | 62 ++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 9 deletions(-) create mode 100644 test/functional/todos_tagging_test.rb diff --git a/app/controllers/todos_controller.rb b/app/controllers/todos_controller.rb index b626653f..6294a82b 100644 --- a/app/controllers/todos_controller.rb +++ b/app/controllers/todos_controller.rb @@ -584,27 +584,72 @@ class TodosController < ApplicationController redirect_to project_todos_path(project, :format => 'm') end + def get_ids_from_tag_expr(tag_expr) + ids = [] + tag_expr.each do |tag_list| + id_list = [] + tag_list.each do |tag| + tag = Tag.find_by_name(tag) + id_list << tag.id if tag + end + ids << id_list + end + return ids + end + + def get_params_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] + + i = 1 + while params['and'+i.to_s] + @tag_expr << sanitize(params['and'+i.to_s]).split(',') + i=i+1 + end + + @single_tag = @tag_expr.size == 1 && @tag_expr[0].size == 1 + @tag_name = @tag_expr[0][0] # if @single_tag + end + + def find_todos_with_tag_ids(tag_ids) + todos = current_user.todos + tag_ids.each do |ids| + todos = todos.with_tags(ids) unless ids.nil? || ids.empty? + end + return todos + end + # /todos/tag/[tag_name] shows all the actions tagged with tag_name def tag - init_data_for_sidebar unless mobile? - @source_view = params['_source_view'] || 'tag' - @tag_name = sanitize(params[:name]) # sanitize to prevent XSS vunerability! @page_title = t('todos.tagged_page_title', :tag_name => @tag_name) + @source_view = params['_source_view'] || 'tag' - # mobile tags are routed with :name ending on .m. So we need to chomp it - @tag_name = @tag_name.chomp('.m') if mobile? + get_params_for_tag_view + + 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 @tag = Tag.find_by_name(@tag_name) @tag = Tag.new(:name => @tag_name) if @tag.nil? - @not_done_todos = current_user.todos.with_tag(@tag).active.not_hidden.find(:all, + @tag_ids = get_ids_from_tag_expr(@tag_expr) + todos_with_tag_ids = find_todos_with_tag_ids(@tag_ids) + + @not_done_todos = todos_with_tag_ids.active.not_hidden.find(:all, :order => 'todos.due IS NULL, todos.due ASC, todos.created_at ASC', :include => Todo::DEFAULT_INCLUDES) - @hidden_todos = current_user.todos.with_tag(@tag).hidden.find(:all, + @hidden_todos = todos_with_tag_ids.hidden.find(:all, :include => Todo::DEFAULT_INCLUDES, :order => 'todos.completed_at DESC, todos.created_at DESC') - @deferred = current_user.todos.with_tag(@tag).deferred.find(:all, + @deferred = todos_with_tag_ids.deferred.find(:all, :order => 'show_from ASC, todos.created_at DESC', :include => Todo::DEFAULT_INCLUDES) - @pending = current_user.todos.with_tag(@tag).blocked.find(:all, + @pending = todos_with_tag_ids.blocked.find(:all, :order => 'show_from ASC, todos.created_at DESC', :include => Todo::DEFAULT_INCLUDES) # If you've set no_completed to zero, the completed items box isn't shown on diff --git a/app/models/todo.rb b/app/models/todo.rb index ea6e17c9..80481546 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -39,6 +39,7 @@ class Todo < ActiveRecord::Base # other scopes named_scope :are_due, :conditions => ['NOT (todos.due IS NULL)'] named_scope :with_tag, lambda { |tag| {:joins => :taggings, :conditions => ["taggings.tag_id = ? ", tag.id] } } + named_scope :with_tags, lambda { |tag_ids| {:joins => :taggings, :conditions => ["taggings.tag_id IN (?) ", tag_ids] } } named_scope :of_user, lambda { |user_id| {:conditions => ["todos.user_id = ? ", user_id] } } named_scope :completed_after, lambda { |date| {:conditions => ["todos.completed_at > ? ", date] } } named_scope :completed_before, lambda { |date| {:conditions => ["todos.completed_at < ? ", date] } } diff --git a/test/functional/todos_tagging_test.rb b/test/functional/todos_tagging_test.rb new file mode 100644 index 00000000..46dd1627 --- /dev/null +++ b/test/functional/todos_tagging_test.rb @@ -0,0 +1,62 @@ +require File.dirname(__FILE__) + '/../test_helper' +require 'todos_controller' + +# Re-raise errors caught by the controller. +class TodosController; def rescue_action(e) raise e end; end + +class TodosControllerTest < ActionController::TestCase + fixtures :users, :preferences, :projects, :contexts, :todos, :tags, :taggings, :recurring_todos + + def test_get_boolean_expression_from_parameters_of_tag_view_single_tag + login_as(:admin_user) + get :tag, :name => "single" + assert_equal true, assigns['single_tag'], "should recognize it is a single tag name" + assert_equal "single", assigns['tag_expr'][0][0], "should store the single tag" + end + + def test_get_boolean_expression_from_parameters_of_tag_view_multiple_tags + login_as(:admin_user) + get :tag, :name => "multiple", :and => "tags", :and1 => "present", :and2 => "here" + assert_equal false, assigns['single_tag'], "should recognize it has multiple tags" + assert_equal 4, assigns['tag_expr'].size, "should have 4 AND expressions" + end + + def test_get_boolean_expression_from_parameters_of_tag_view_multiple_tags_without_digitless_and + login_as(:admin_user) + get :tag, :name => "multiple", :and1 => "tags", :and2 => "present", :and3 => "here" + assert_equal false, assigns['single_tag'], "should recognize it has multiple tags" + assert_equal 4, assigns['tag_expr'].size, "should have 4 AND expressions" + end + + def test_get_boolean_expression_from_parameters_of_tag_view_multiple_ORs + login_as(:admin_user) + get :tag, :name => "multiple,tags,present" + assert_equal false, assigns['single_tag'], "should recognize it has multiple tags" + assert_equal 1, assigns['tag_expr'].size, "should have 1 expressions" + assert_equal 3, assigns['tag_expr'][0].size, "should have 3 ORs in 1st expression" + end + + def test_get_boolean_expression_from_parameters_of_tag_view_multiple_ORs_and_ANDS + login_as(:admin_user) + get :tag, :name => "multiple,tags,present", :and => "here,is,two", :and1=>"and,three" + assert_equal false, assigns['single_tag'], "should recognize it has multiple tags" + assert_equal 3, assigns['tag_expr'].size, "should have 3 expressions" + assert_equal 3, assigns['tag_expr'][0].size, "should have 3 ORs in 1st expression" + assert_equal 3, assigns['tag_expr'][1].size, "should have 3 ORs in 2nd expression" + assert_equal 2, assigns['tag_expr'][2].size, "should have 2 ORs in 3rd expression" + end + + def test_get_ids_from_tag_expr + login_as(:admin_user) + + # make sure the tags exits + # "multiple,tags,present,here,is,two,and,three".split(',').each { |tag| Tag.find_or_create_by_name(:name=>tag)} + + get :tag, :name => "foo,bar", :and => "baz" + + assert_equal 1, assigns['tag_ids'][0][0], "first id should be 1 for foo" + assert_equal 2, assigns['tag_ids'][0][1], "second id should be 2 for bar" + assert_equal 3, assigns['tag_ids'][1][0], "third id should be 3 for baz" + end + +end \ No newline at end of file