From 067db90d58f46d5a2dedde2ad51d5ad8a9382b5d Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Sun, 22 Sep 2013 17:34:58 +0200 Subject: [PATCH 1/9] various small refactorings --- app/helpers/feedlist_helper.rb | 9 +++- app/models/message_gateway.rb | 88 ++++++++++++++++++--------------- lib/is_taggable.rb | 8 ++- test/models/is_taggable_test.rb | 28 +++++++++++ 4 files changed, 90 insertions(+), 43 deletions(-) diff --git a/app/helpers/feedlist_helper.rb b/app/helpers/feedlist_helper.rb index 1cb0da32..507e219b 100644 --- a/app/helpers/feedlist_helper.rb +++ b/app/helpers/feedlist_helper.rb @@ -27,12 +27,16 @@ module FeedlistHelper return html.html_safe end + def all_feed_links(object, symbol) + feed_links([:rss, :txt, :ical], { :controller=> 'todos', :action => 'index', symbol => object.to_param }, content_tag(:strong, object.name)) + end + def all_feed_links_for_project(project) - feed_links([:rss, :txt, :ical], { :controller=> 'todos', :action => 'index', :project_id => project.to_param }, content_tag(:strong, project.name)) + all_feed_links(project, :project_id) end def all_feed_links_for_context(context) - feed_links([:rss, :txt, :ical], { :controller=> 'todos', :action => 'index', :context_id => context.to_param }, content_tag(:strong, context.name)) + all_feed_links(context, :context_id) end protected @@ -44,5 +48,6 @@ module FeedlistHelper def user_token_hash { :token => current_user.token } end + end diff --git a/app/models/message_gateway.rb b/app/models/message_gateway.rb index e6c3855d..fb42dc92 100644 --- a/app/models/message_gateway.rb +++ b/app/models/message_gateway.rb @@ -3,27 +3,14 @@ class MessageGateway < ActionMailer::Base extend ActionView::Helpers::SanitizeHelper::ClassMethods def receive(email) - user = get_user_from_email_address(email) + user = get_receiving_user_from_email_address(email) return false if user.nil? + return false unless check_sender_is_in_mailmap(user, email) context = user.prefs.sms_context - description = nil - notes = nil - - if email.multipart? - description = get_text_or_nil(email.subject) - notes = get_first_text_plain_part(email) - else - if email.subject.blank? - description = get_decoded_text_or_nil(email.body) - notes = nil - else - description = get_text_or_nil(email.subject) - notes = get_decoded_text_or_nil(email.body) - end - end + todo_params = get_todo_params(email) - todo_builder = TodoFromRichMessage.new(user, context.id, description, notes) + todo_builder = TodoFromRichMessage.new(user, context.id, todo_params[:description], todo_params[:notes]) todo = todo_builder.construct todo.save! Rails.logger.info "Saved email as todo for user #{user.login} in context #{context.name}" @@ -32,32 +19,60 @@ class MessageGateway < ActionMailer::Base private - def get_address(email) - return SITE_CONFIG['email_dispatch'] == 'to' ? email.to[0] : email.from[0] - end - - def get_user_from_env_setting + def get_todo_params(email) + params = {} + + if email.multipart? + params[:description] = get_text_or_nil(email.subject) + params[:notes] = get_first_text_plain_part(email) + else + if email.subject.blank? + params[:description] = get_decoded_text_or_nil(email.body) + params[:notes] = nil + else + params[:description] = get_text_or_nil(email.subject) + params[:notes] = get_decoded_text_or_nil(email.body) + end + end + params + end + + def get_receiving_user_from_email_address(email) + SITE_CONFIG['email_dispatch'] == 'single_user' ? get_receiving_user_from_env_setting : get_receiving_user_from_mail_header(email) + end + + def get_receiving_user_from_env_setting Rails.logger.info "All received email goes to #{ENV['TRACKS_MAIL_RECEIVER']}" user = User.where(:login => ENV['TRACKS_MAIL_RECEIVER']).first Rails.logger.info "WARNING: Unknown user set for TRACKS_MAIL_RECEIVER (#{ENV['TRACKS_MAIL_RECEIVER']})" if user.nil? return user end - def get_user_from_mail_header(email) - address = get_address(email) - Rails.logger.info "Looking for user with email #{address}" - user = User.where("preferences.sms_email" => address.strip).includes(:preference).first - if user.nil? - user = User.where("preferences.sms_email" => address.strip[1.100]).includes(:preference).first - end - if user.present? and !sender_is_in_mailmap?(user,email) - Rails.logger.warn "#{email.from[0]} not found in mailmap for #{user.login}" - return nil - end - Rails.logger.info(!user.nil? ? "Email belongs to #{user.login}" : "User unknown") + def get_receiving_user_from_mail_header(email) + user = get_receiving_user_from_sms_email( get_address(email) ) + Rails.logger.info(user.nil? ? "User unknown": "Email belongs to #{user.login}") return user end + def get_address(email) + return SITE_CONFIG['email_dispatch'] == 'to' ? email.to[0] : email.from[0] + end + + def get_receiving_user_from_sms_email(address) + Rails.logger.info "Looking for user with email #{address}" + user = User.where("preferences.sms_email" => address.strip).includes(:preference).first + user = User.where("preferences.sms_email" => address.strip[1.100]).includes(:preference).first if user.nil? + return user + end + + def check_sender_is_in_mailmap(user, email) + if user.present? and !sender_is_in_mailmap?(user,email) + Rails.logger.warn "#{email.from[0]} not found in mailmap for #{user.login}" + return false + end + return true + end + def sender_is_in_mailmap?(user,email) if SITE_CONFIG['mailmap'].is_a? Hash and SITE_CONFIG['email_dispatch'] == 'to' # Look for the sender in the map of allowed senders @@ -69,11 +84,6 @@ class MessageGateway < ActionMailer::Base end end - - def get_user_from_email_address(email) - SITE_CONFIG['email_dispatch'] == 'single_user' ? get_user_from_env_setting : get_user_from_mail_header(email) - end - def get_text_or_nil(text) return text ? sanitize(text.strip) : nil end diff --git a/lib/is_taggable.rb b/lib/is_taggable.rb index 950ed95c..f98895f1 100644 --- a/lib/is_taggable.rb +++ b/lib/is_taggable.rb @@ -80,7 +80,11 @@ module IsTaggable end end - def tag_cast_to_string obj + def tag_cast_to_string(obj) + tag_array_from_obj(obj).flatten.compact.map(&:downcase).uniq + end + + def tag_array_from_obj(obj) case obj when Array obj.map! { |item| get_tag_name_from_item(item) } @@ -88,7 +92,7 @@ module IsTaggable obj.split(Tag::DELIMITER).map { |tag_name| tag_name.strip.squeeze(" ") } else raise "Invalid object of class #{obj.class} as tagging method parameter" - end.flatten.compact.map(&:downcase).uniq + end end end diff --git a/test/models/is_taggable_test.rb b/test/models/is_taggable_test.rb index 12d4828d..dfd97400 100644 --- a/test/models/is_taggable_test.rb +++ b/test/models/is_taggable_test.rb @@ -42,5 +42,33 @@ class IsTaggableTest < ActiveSupport::TestCase t.tag_with "a, b, e, f" assert_equal "a, b, e, f", t.tag_list, "should add e and f and remove c and d" end + + def test_editing_using_taglist + t = Todo.create(:description => "test", :context => Context.first) + t.tag_list = "a, b, c" + assert_equal 3, t.tags.count + assert_equal "a, b, c", t.tag_list + + + t.tag_list = "d, e" + assert_equal 2, t.tags.count + assert_equal "d, e", t.tag_list + end + def test_tag_cast_to_string + t = Todo.create(:description => "test", :context => Context.first) + + obj = "tag" + assert_equal ["tag"], t.tag_cast_to_string(obj) + + obj = ["tag1", Tag.new(name: "tag2")] + assert_equal ["tag1", "tag2"], t.tag_cast_to_string(obj) + + obj = {a: "hash"} + assert_raise(RuntimeError) { t.tag_cast_to_string(obj) } + + obj = ["string", {a: "hash"}] + assert_raise(RuntimeError) { t.tag_cast_to_string(obj) } + end + end \ No newline at end of file From 07a3962d7d2c2c2003a7108fc0aa186c22dea229 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Sun, 22 Sep 2013 19:29:30 +0200 Subject: [PATCH 2/9] remove simplecov since we now have coverage reporting on Code Climate --- Gemfile | 1 - features/support/env.rb | 6 ++++-- test/minimal_test_helper.rb | 5 +++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Gemfile b/Gemfile index c14b5709..965374dd 100644 --- a/Gemfile +++ b/Gemfile @@ -57,7 +57,6 @@ group :test do gem "mocha", :require => false gem "aruba", git: 'https://github.com/cucumber/aruba', :require => false # need 0.5.4 for piping files; 0.5.3 is latest - gem "simplecov" gem "timecop", "~> 0.6.2" # Note that > 2.14 has problems, see: diff --git a/features/support/env.rb b/features/support/env.rb index 376c2cf3..3a1145c8 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -3,8 +3,10 @@ # newer version of cucumber-rails. Consider adding your own code to a new file # instead of editing this one. Cucumber will automatically load all features/**/*.rb # files. -require 'simplecov' -SimpleCov.start 'rails' + +# test coverage from codeclimate +require "codeclimate-test-reporter" +CodeClimate::TestReporter.start require 'cucumber/rails' require 'aruba/cucumber' diff --git a/test/minimal_test_helper.rb b/test/minimal_test_helper.rb index 1216a193..ca8e59a2 100644 --- a/test/minimal_test_helper.rb +++ b/test/minimal_test_helper.rb @@ -1,5 +1,6 @@ -require 'simplecov' -SimpleCov.start 'rails' +# test coverage from codeclimate +require "codeclimate-test-reporter" +CodeClimate::TestReporter.start ENV["RAILS_ENV"] = "test" require 'test/unit' From bba86e51d7dd8bbe97eb2d75a12f5514a1254807 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Mon, 23 Sep 2013 16:49:59 +0200 Subject: [PATCH 3/9] re-add simplecov by request --- Gemfile | 1 + Gemfile.lock | 6 +++--- features/support/env.rb | 4 ++++ test/minimal_test_helper.rb | 4 ++++ test/test_helper.rb | 4 ++++ 5 files changed, 16 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index 965374dd..9222533d 100644 --- a/Gemfile +++ b/Gemfile @@ -70,6 +70,7 @@ group :test do #gem "capybara-screenshot" #gem "launchy" + gem "simplecov" # get test coverage info on codeclimate gem "codeclimate-test-reporter", group: :test, require: nil end diff --git a/Gemfile.lock b/Gemfile.lock index f90a4549..7f3b6b77 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -62,7 +62,7 @@ GEM xpath (~> 2.0) childprocess (0.3.9) ffi (~> 1.0, >= 1.0.11) - codeclimate-test-reporter (0.0.11) + codeclimate-test-reporter (0.1.1) simplecov (>= 0.7.1, < 1.0.0) coffee-rails (4.0.0) coffee-script (>= 2.2.0) @@ -130,7 +130,7 @@ GEM bundler (>= 1.3.0, < 2.0) railties (= 4.0.0) sprockets-rails (~> 2.0.0) - rails_autolink (1.1.3) + rails_autolink (1.1.4) rails (> 3.1) railties (4.0.0) actionpack (= 4.0.0) @@ -139,7 +139,7 @@ GEM thor (>= 0.18.1, < 2.0) rake (10.1.0) ref (1.0.5) - rspec-expectations (2.14.2) + rspec-expectations (2.14.3) diff-lcs (>= 1.1.3, < 2.0) rubyzip (0.9.9) safe_yaml (0.9.7) diff --git a/features/support/env.rb b/features/support/env.rb index 3a1145c8..cc4f56e3 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -8,6 +8,10 @@ require "codeclimate-test-reporter" CodeClimate::TestReporter.start +# local test coverage +require 'simplecov' +SimpleCov.start 'rails' + require 'cucumber/rails' require 'aruba/cucumber' diff --git a/test/minimal_test_helper.rb b/test/minimal_test_helper.rb index ca8e59a2..a230f3b0 100644 --- a/test/minimal_test_helper.rb +++ b/test/minimal_test_helper.rb @@ -2,6 +2,10 @@ require "codeclimate-test-reporter" CodeClimate::TestReporter.start +# local test coverage +require 'simplecov' +SimpleCov.start 'rails' + ENV["RAILS_ENV"] = "test" require 'test/unit' require 'mocha/setup' diff --git a/test/test_helper.rb b/test/test_helper.rb index 74c19d9c..e3ab13cd 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -4,6 +4,10 @@ ENV["RAILS_ENV"] ||= "test" require "codeclimate-test-reporter" CodeClimate::TestReporter.start +# local test coverage +require 'simplecov' +SimpleCov.start 'rails' + require File.expand_path('../../config/environment', __FILE__) require 'rails/test_help' From ae11f09d2fc351b192028b779b31b6c238fb047e Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Mon, 23 Sep 2013 16:52:29 +0200 Subject: [PATCH 4/9] fix calendar_test todos in rest of month means all todos after next week in the current month. The test failed if the todo was created in next week and that week being the last week in the month. In that case the todo should not be returned by rest_of_month since it is in rest_of_week --- test/models/todos/calendar_test.rb | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/test/models/todos/calendar_test.rb b/test/models/todos/calendar_test.rb index 8ac6f7ec..5148f47c 100644 --- a/test/models/todos/calendar_test.rb +++ b/test/models/todos/calendar_test.rb @@ -32,10 +32,21 @@ module Todos assert_equal [due_next_week], @calendar.due_next_week end - def test_due_this_month - due_this_month = create_todo(Time.zone.now.end_of_month) + def test_due_this_month_at_start_month + # should return 1 todo + Timecop.travel(2013,9,1) do + due_this_month = create_todo(Time.zone.now.end_of_month) + assert_equal [due_this_month], @calendar.due_this_month + end + end - assert_equal [due_this_month], @calendar.due_this_month + def test_due_this_month_at_end_month + # the todo is due next week and is thus left out for todos due rest + # of month (i.e. after next week, but in this month) + Timecop.travel(2013,9,23) do + due_this_month = create_todo(Time.zone.now.end_of_month) + assert_equal 0, @calendar.due_this_month.size + end end def test_due_after_this_month From 406eb47db78802e88b22aa6b27182bdc35ef630a Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Tue, 24 Sep 2013 09:48:21 +0200 Subject: [PATCH 5/9] move coverage to ci rake task --- features/support/env.rb | 8 -------- lib/tasks/continuous_integration.rake | 17 ++++++++++++++++- test/minimal_test_helper.rb | 10 +--------- test/test_helper.rb | 8 -------- 4 files changed, 17 insertions(+), 26 deletions(-) diff --git a/features/support/env.rb b/features/support/env.rb index cc4f56e3..a9e3e6c9 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -4,14 +4,6 @@ # instead of editing this one. Cucumber will automatically load all features/**/*.rb # files. -# test coverage from codeclimate -require "codeclimate-test-reporter" -CodeClimate::TestReporter.start - -# local test coverage -require 'simplecov' -SimpleCov.start 'rails' - require 'cucumber/rails' require 'aruba/cucumber' diff --git a/lib/tasks/continuous_integration.rake b/lib/tasks/continuous_integration.rake index 67b5f161..5a393c11 100644 --- a/lib/tasks/continuous_integration.rake +++ b/lib/tasks/continuous_integration.rake @@ -1 +1,16 @@ -task :ci => ['db:migrate', :test, :cucumber] +task :ci do |t| + ENV['RAILS_ENV'] ||= "test" + + # test coverage from codeclimate + require "codeclimate-test-reporter" + CodeClimate::TestReporter.start + + # local test coverage + require 'simplecov' + SimpleCov.start 'rails' + + [:environment, 'db:migrate', 'test:all', 'cucumber'].each do |t| + print "running '#{t}'\n" + Rake::Task[t].invoke + end +end \ No newline at end of file diff --git a/test/minimal_test_helper.rb b/test/minimal_test_helper.rb index a230f3b0..d1d73ae9 100644 --- a/test/minimal_test_helper.rb +++ b/test/minimal_test_helper.rb @@ -1,12 +1,4 @@ -# test coverage from codeclimate -require "codeclimate-test-reporter" -CodeClimate::TestReporter.start - -# local test coverage -require 'simplecov' -SimpleCov.start 'rails' - -ENV["RAILS_ENV"] = "test" +ENV["RAILS_ENV"] ||= "test" require 'test/unit' require 'mocha/setup' diff --git a/test/test_helper.rb b/test/test_helper.rb index e3ab13cd..22b639f1 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -1,13 +1,5 @@ ENV["RAILS_ENV"] ||= "test" -# test coverage from codeclimate -require "codeclimate-test-reporter" -CodeClimate::TestReporter.start - -# local test coverage -require 'simplecov' -SimpleCov.start 'rails' - require File.expand_path('../../config/environment', __FILE__) require 'rails/test_help' From 23d92411d3e734a20e6531f7324964823a1d6a43 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Wed, 25 Sep 2013 13:40:30 +0200 Subject: [PATCH 6/9] avoid name clash --- doc/tracks_template_cli.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/tracks_template_cli.rb b/doc/tracks_template_cli.rb index 53fbee6c..92d63b3e 100644 --- a/doc/tracks_template_cli.rb +++ b/doc/tracks_template_cli.rb @@ -215,7 +215,7 @@ end class Error < StandardError; end class InvalidParser < StandardError; end -class ConsoleOptions +class ConsoleOptionsForTemplate attr_reader :parser, :options, :keywords def initialize @@ -307,5 +307,5 @@ class ConsoleOptions end if $0 == __FILE__ - ConsoleOptions.new.run(ARGV) + ConsoleOptionsForTemplate.new.run(ARGV) end From 11bc4294a839196e3ec57339f9b4d92132a0b5c5 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Wed, 25 Sep 2013 15:08:25 +0200 Subject: [PATCH 7/9] add tests for done_todos --- lib/tracks/done_todos.rb | 39 ++++++++++++----- test/models/todos/done_todos_test.rb | 64 ++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 11 deletions(-) create mode 100644 test/models/todos/done_todos_test.rb diff --git a/lib/tracks/done_todos.rb b/lib/tracks/done_todos.rb index 89cb1e3c..179f6661 100644 --- a/lib/tracks/done_todos.rb +++ b/lib/tracks/done_todos.rb @@ -1,32 +1,32 @@ class DoneTodos + + def self.done_todos_for_container(user) completed_todos = user.todos.completed return done_today(completed_todos), done_rest_of_week(completed_todos), done_rest_of_month(completed_todos) end def self.done_today(todos, includes = {:include => Todo::DEFAULT_INCLUDES}) - start_of_this_day = Time.zone.now.beginning_of_day # TODO: refactor to remove outer hash from includes param - todos.completed_after(start_of_this_day).includes(includes[:include]) + todos.completed_after(beginning_of_day).includes(includes[:include]) end def self.done_rest_of_week(todos, includes = {:include => Todo::DEFAULT_INCLUDES}) - done_between(todos, includes, Time.zone.now.beginning_of_day, Time.zone.now.beginning_of_week) + done_between(todos, includes, beginning_of_day, beginning_of_week) end def self.done_rest_of_month(todos, includes = {:include => Todo::DEFAULT_INCLUDES}) - done_between(todos, includes, Time.zone.now.beginning_of_week, Time.zone.now.beginning_of_month) + done_between(todos, includes, beginning_of_week, beginning_of_month) end def self.completed_period(date) - return nil if date.nil? + return nil if date.nil? - period = nil - period = "rest_of_month" if date > Time.zone.now.beginning_of_month - period = "rest_of_week" if date > Time.zone.now.beginning_of_week - period = "today" if date > Time.zone.now.beginning_of_day - - return period + return "today" if date >= end_of_day # treat todos with completed_at in future as done today (happens in tests) + return "today" if date.between?(beginning_of_day, end_of_day) + return "rest_of_week" if date >= beginning_of_week + return "rest_of_month" if date >= beginning_of_month + return nil end def self.remaining_in_container(user, period) @@ -41,4 +41,21 @@ class DoneTodos # TODO: refactor to remove outer hash from includes param todos.completed_before(start_date).completed_after(end_date).includes(includes[:include]) end + + def self.beginning_of_day + Time.zone.now.beginning_of_day + end + + def self.end_of_day + Time.zone.now.end_of_day + end + + def self.beginning_of_week + Time.zone.now.beginning_of_week + end + + def self.beginning_of_month + Time.zone.now.beginning_of_month + end + end diff --git a/test/models/todos/done_todos_test.rb b/test/models/todos/done_todos_test.rb new file mode 100644 index 00000000..b3b066e4 --- /dev/null +++ b/test/models/todos/done_todos_test.rb @@ -0,0 +1,64 @@ +require_relative '../../test_helper' + +module Todos + class DoneTodosTest < ActiveSupport::TestCase + + def test_completed_period + Timecop.travel(2013,1,23,12,00,00) do # wednesday at 12:00; + assert_equal "today", DoneTodos.completed_period(Time.zone.local(2013,1,23,9,00)) # today at 9:00 + assert_equal "rest_of_week", DoneTodos.completed_period(Time.zone.local(2013,1,21)) # monday this week + assert_equal "rest_of_month", DoneTodos.completed_period(Time.zone.local(2013,1,8)) # tuestday in first week of jan + + assert_nil DoneTodos.completed_period(nil) + assert_nil DoneTodos.completed_period(Time.zone.local(2012,12,1)) # older than this month + + assert_equal "today", DoneTodos.completed_period(Time.zone.local(2013,12,1)) # date in future -> act as if today + end + end + + def test_done_today + todos = users(:admin_user).todos + assert 0, DoneTodos.done_today(todos, {}).count + + t = users(:admin_user).todos.active.first + t.complete! + + assert 0, DoneTodos.done_today(todos.reload, {}).count + end + + def test_done_rest_of_week + todos = users(:admin_user).todos + + # When I mark a todo complete on jan 1 + Timecop.travel(2013,1,1,0,0) do + t = users(:admin_user).todos.active.first + t.complete! + end + + # Then I should be in rest_of_week on jan 2 + Timecop.travel(2013,1,2,0,0) do + assert 0, DoneTodos.done_today(todos.reload, {}).count + assert 1, DoneTodos.done_rest_of_week(todos.reload, {}).count + end + end + + def test_done_rest_of_month + todos = users(:admin_user).todos + + # When I mark a todo complete on jan 1 + Timecop.travel(2013,1,1,0,0) do + t = users(:admin_user).todos.active.first + t.complete! + end + + # Then I should be in rest_of_month on jan 21 + Timecop.travel(2013,1,21,0,0) do + assert 0, DoneTodos.done_today(todos.reload, {}).count + assert 0, DoneTodos.done_rest_of_week(todos.reload, {}).count + assert 1, DoneTodos.done_rest_of_month(todos.reload, {}).count + end + end + + + end +end From 9f55a45ec67cd43bed1fab85ac34c70ba5d5e8a2 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Wed, 25 Sep 2013 15:38:51 +0200 Subject: [PATCH 8/9] refactor search controller --- app/controllers/search_controller.rb | 41 +++------------------ app/models/search/search_results.rb | 54 ++++++++++++++++++++++++++++ app/views/search/results.html.erb | 20 +++++------ 3 files changed, 68 insertions(+), 47 deletions(-) create mode 100644 app/models/search/search_results.rb diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index f4cbb776..42e43e67 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -5,19 +5,12 @@ class SearchController < ApplicationController def results @source_view = params['_source_view'] || 'search' @page_title = "TRACKS::Search Results for #{params[:search]}" - terms = "%#{params[:search]}%" - @found_not_complete_todos = incomplete_todos(terms) - @found_complete_todos = complete_todos(terms) - @found_todos = @found_not_complete_todos + @found_complete_todos + searcher = Search::SearchResults.new(current_user, params[:search]) + searcher.search - @found_projects = current_user.projects.with_name_or_description(terms); - @found_notes = current_user.notes.with_body(terms); - @found_contexts = current_user.contexts.with_name(terms) - - # TODO: limit search to tags on todos - @found_tags = todo_tags_by_name(current_user, terms) - @count = @found_todos.size + @found_projects.size + @found_notes.size + @found_contexts.size + @found_tags.size + @results = searcher.results + @count = searcher.number_of_finds init_not_done_counts init_project_hidden_todo_counts @@ -26,30 +19,4 @@ class SearchController < ApplicationController def index @page_title = "TRACKS::Search" end - -private - def incomplete_todos(terms) - current_user.todos. - where("(todos.description LIKE ? OR todos.notes LIKE ?) AND todos.completed_at IS NULL", terms, terms). - includes(Todo::DEFAULT_INCLUDES). - reorder("todos.due IS NULL, todos.due ASC, todos.created_at ASC") - end - - def complete_todos(terms) - current_user.todos. - where("(todos.description LIKE ? OR todos.notes LIKE ?) AND NOT (todos.completed_at IS NULL)", terms, terms). - includes(Todo::DEFAULT_INCLUDES). - reorder("todos.completed_at DESC") - end - - def todo_tags_by_name(current_user, terms) - Tagging.find_by_sql([ - "SELECT DISTINCT tags.name as name "+ - "FROM tags "+ - "LEFT JOIN taggings ON tags.id = taggings.tag_id "+ - "LEFT JOIN todos ON taggings.taggable_id = todos.id "+ - "WHERE todos.user_id=? "+ - "AND tags.name LIKE ? ", current_user.id, terms]) - - end end diff --git a/app/models/search/search_results.rb b/app/models/search/search_results.rb new file mode 100644 index 00000000..692d0fd0 --- /dev/null +++ b/app/models/search/search_results.rb @@ -0,0 +1,54 @@ +module Search + + class SearchResults + attr_reader :results + + def initialize(user, terms) + @user = user + @terms = "%#{terms}%" + @results = {} + end + + def search + results[:not_complete_todos] = incomplete_todos(@terms) + results[:complete_todos] = complete_todos(@terms) + results[:todos] = results[:not_complete_todos] + results[:complete_todos] + results[:projects] = @user.projects.with_name_or_description(@terms) + results[:notes] = @user.notes.with_body(@terms) + results[:contexts] = @user.contexts.with_name(@terms) + results[:tags] = todo_tags_by_name(@terms) + end + + def number_of_finds + results[:todos].size + results[:projects].size + results[:notes].size + results[:contexts].size + results[:tags].size + end + + private + + def incomplete_todos(terms) + @user.todos. + where("(todos.description LIKE ? OR todos.notes LIKE ?) AND todos.completed_at IS NULL", terms, terms). + includes(Todo::DEFAULT_INCLUDES). + reorder("todos.due IS NULL, todos.due ASC, todos.created_at ASC") + end + + def complete_todos(terms) + @user.todos. + where("(todos.description LIKE ? OR todos.notes LIKE ?) AND NOT (todos.completed_at IS NULL)", terms, terms). + includes(Todo::DEFAULT_INCLUDES). + reorder("todos.completed_at DESC") + end + + def todo_tags_by_name(terms) + Tagging.find_by_sql([ + "SELECT DISTINCT tags.name as name "+ + "FROM tags "+ + "LEFT JOIN taggings ON tags.id = taggings.tag_id "+ + "LEFT JOIN todos ON taggings.taggable_id = todos.id "+ + "WHERE todos.user_id=? "+ + "AND tags.name LIKE ? ", @user.id, terms]) + end + + end + +end diff --git a/app/views/search/results.html.erb b/app/views/search/results.html.erb index c73d9761..e97431ca 100644 --- a/app/views/search/results.html.erb +++ b/app/views/search/results.html.erb @@ -2,24 +2,24 @@

<%= t('search.no_results') %>

<% else -%> - <%= render :layout => 'show_results_collection', :object => @found_todos, :locals => { :collection_name => "found-todos", :collection_title => t('search.todos_matching_query')} do %> - <%= render :partial => "todos/todo", :collection => @found_todos, :locals => { :parent_container_type => 'search', :suppress_context => false, :suppress_project => false, :suppress_edit_button => false } %> + <%= render :layout => 'show_results_collection', :object => @results[:todos], :locals => { :collection_name => "found-todos", :collection_title => t('search.todos_matching_query')} do %> + <%= render :partial => "todos/todo", :collection => @results[:todos], :locals => { :parent_container_type => 'search', :suppress_context => false, :suppress_project => false, :suppress_edit_button => false } %> <% end -%> - <%= render :layout => 'show_results_collection', :object => @found_projects, :locals => { :collection_name => "found-project", :collection_title => t('search.projects_matching_query')} do %> - <%= render :partial => "projects/project_listing", :collection => @found_projects, :locals => { :suppress_drag_handle => true, :suppress_edit_button => true, :suppress_delete_button => true } %> + <%= render :layout => 'show_results_collection', :object => @results[:projects], :locals => { :collection_name => "found-project", :collection_title => t('search.projects_matching_query')} do %> + <%= render :partial => "projects/project_listing", :collection => @results[:projects], :locals => { :suppress_drag_handle => true, :suppress_edit_button => true, :suppress_delete_button => true } %> <% end -%> - <%= render :layout => 'show_results_collection', :object => @found_notes, :locals => { :collection_name => "found-notes", :collection_title => t('search.notes_matching_query')} do %> - <%= render :partial => "notes/notes_summary", :collection=> @found_notes %> + <%= render :layout => 'show_results_collection', :object => @results[:notes], :locals => { :collection_name => "found-notes", :collection_title => t('search.notes_matching_query')} do %> + <%= render :partial => "notes/notes_summary", :collection=> @results[:notes] %> <% end -%> - <%= render :layout => 'show_results_collection', :object => @found_contexts, :locals => { :collection_name => "found-contexts", :collection_title => t('search.contexts_matching_query')} do %> - <%= render :partial => "contexts/context_listing", :collection => @found_contexts, :locals => { :suppress_drag_handle => true, :suppress_edit_button => true } %> + <%= render :layout => 'show_results_collection', :object => @results[:contexts], :locals => { :collection_name => "found-contexts", :collection_title => t('search.contexts_matching_query')} do %> + <%= render :partial => "contexts/context_listing", :collection => @results[:contexts], :locals => { :suppress_drag_handle => true, :suppress_edit_button => true } %> <% end -%> - <%= render :layout => 'show_results_collection', :object => @found_tags, :locals => { :collection_name => "found-tags", :collection_title => t('search.tags_matching_query')} do %> - <% @found_tags.each do |tag| -%> + <%= render :layout => 'show_results_collection', :object => @results[:tags], :locals => { :collection_name => "found-tags", :collection_title => t('search.tags_matching_query')} do %> + <% @results[:tags].each do |tag| -%> <%= link_to tag.name, tag_path(tag.name) -%> <% end %>

From edb2ad7077a0200e4771be461ccc7ddb3d157712 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Wed, 25 Sep 2013 16:22:06 +0200 Subject: [PATCH 9/9] add missing tests for todo --- test/models/todo_test.rb | 43 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/test/models/todo_test.rb b/test/models/todo_test.rb index ed9bf57e..5d87210c 100644 --- a/test/models/todo_test.rb +++ b/test/models/todo_test.rb @@ -201,6 +201,24 @@ class TodoTest < ActiveSupport::TestCase assert t.project.is_a?(NullProject) end + def test_update_from_project + # Given a hidden project + assert_not_nil @not_completed1.project + project = @not_completed1.project + project.hide! + assert project.hidden? + assert @not_completed1.reload.hidden? + + # When I manually create a new todo in the hidden projct + new_todo = @not_completed1.user.todos.build(description: "test", context: @not_completed1.context, project: project) + new_todo.save! + assert new_todo.active? + # And I update the state of the todo from its project + new_todo.update_state_from_project + # Then the todo should be hidden + assert new_todo.hidden? + end + def test_initial_state_defaults_to_active t = Todo.new t.description = 'foo' @@ -328,6 +346,31 @@ class TodoTest < ActiveSupport::TestCase assert @not_completed1.active?, "removing last predecessor should activate todo" end + def test_removing_precesessor_using_new_dependency_list + # Given three active todos (@not_completed{1,2.3}) + @completed.activate! + @not_completed3 = @completed + + #When I add two todos as dependency to one todo + @not_completed1.add_predecessor_list("#{@not_completed2.id}, #{@not_completed3.id}") + @not_completed1.save_predecessors + # blocking is not done automagically + @not_completed1.block! + + # Then @completed1 should have predecessors and should be blocked + assert @not_completed1.uncompleted_predecessors? + assert @not_completed1.pending?, "a todo with predecessors should be pending" + + # When I set the predecessors to only todo2 + @not_completed1.add_predecessor_list("#{@not_completed2.id}") # + @not_completed1.save_predecessors + + # Then todo1 should have only one predecessor and it should be todo2 + assert @not_completed1.uncompleted_predecessors? + assert_equal 1, @not_completed1.predecessors.count + assert_equal @not_completed2, @not_completed1.predecessors.first + end + def test_finding_todos_with_a_tag todo = @not_completed1 todo.tag_list = "a, b, c"