From 4e29bf69f709356e4b5d2f1ae8d5069f914232e4 Mon Sep 17 00:00:00 2001 From: Reinier Balt Date: Wed, 18 Jul 2012 11:42:26 +0200 Subject: [PATCH] fix failing tests and small refactorings --- .../application_controller.rb.rails2 | 324 ------------------ app/controllers/todos_controller.rb | 6 +- app/helpers/feedlist_helper.rb | 15 +- app/helpers/projects_helper.rb | 24 +- app/models/message_gateway.rb | 33 +- app/models/project.rb | 3 + app/models/todo.rb | 4 +- test/functional/todos_controller_test.rb | 20 +- 8 files changed, 53 insertions(+), 376 deletions(-) delete mode 100644 app/controllers/application_controller.rb.rails2 diff --git a/app/controllers/application_controller.rb.rails2 b/app/controllers/application_controller.rb.rails2 deleted file mode 100644 index 761be1d9..00000000 --- a/app/controllers/application_controller.rb.rails2 +++ /dev/null @@ -1,324 +0,0 @@ -# The filters added to this controller will be run for all controllers in the -# application. Likewise will all the methods added be available for all -# controllers. - -require_dependency "login_system" -require_dependency "tracks/source_view" - -class ApplicationController < ActionController::Base - - protect_from_forgery - - helper :application - include LoginSystem - helper_method :current_user, :prefs, :format_date - - layout proc{ |controller| controller.mobile? ? "mobile" : "standard" } - exempt_from_layout /\.js\.erb$/ - - before_filter :check_for_deprecated_password_hash - before_filter :set_session_expiration - before_filter :set_time_zone - before_filter :set_zindex_counter - before_filter :set_locale - prepend_before_filter :login_required - prepend_before_filter :enable_mobile_content_negotiation - after_filter :set_charset - - # By default, sets the charset to UTF-8 if it isn't already set - def set_charset - headers["Content-Type"] ||= "text/html; charset=UTF-8" - end - - def set_locale - locale = params[:locale] # specifying a locale in the request takes precedence - locale = locale || prefs.locale unless current_user.nil? # otherwise, the locale of the currently logged in user takes over - locale = locale || request.env['HTTP_ACCEPT_LANGUAGE'].scan(/^[a-z]{2}/).first if request.env['HTTP_ACCEPT_LANGUAGE'] - I18n.locale = locale.nil? ? I18n.default_locale : (I18n::available_locales.include?(locale.to_sym) ? locale : I18n.default_locale) - logger.debug("Selected '#{I18n.locale}' as locale") - end - - def set_session_expiration - # http://wiki.rubyonrails.com/rails/show/HowtoChangeSessionOptions - unless session == nil - return if self.controller_name == 'feed' or session['noexpiry'] == "on" - # If the method is called by the feed controller (which we don't have - # under session control) or if we checked the box to keep logged in on - # login don't set the session expiry time. - if session - # Get expiry time (allow ten seconds window for the case where we have - # none) - expiry_time = session['expiry_time'] || Time.now + 10 - if expiry_time < Time.now - # Too late, matey... bang goes your session! - reset_session - else - # Okay, you get another hour - session['expiry_time'] = Time.now + (60*60) - end - end - end - end - - # Redirects to change_password_user_path if the current user uses a - # deprecated password hashing algorithm. - def check_for_deprecated_password_hash - if current_user and current_user.uses_deprecated_password? - notify :warning, t('users.you_have_to_reset_your_password') - redirect_to change_password_user_path current_user - end - end - - def render_failure message, status = 404 - render :text => message, :status => status - end - - # def rescue_action(exception) - # log_error(exception) if logger - # respond_to do |format| - # format.html do - # notify :warning, "An error occurred on the server." - # render :action => "index" - # end - # format.js { render :action => 'error' } - # format.xml { render :text => 'An error occurred on the server.' + $! } - # end - # end - - # Returns a count of next actions in the given context or project The result - # is count and a string descriptor, correctly pluralised if there are no - # actions or multiple actions - # - def count_undone_todos_phrase(todos_parent) - count = count_undone_todos(todos_parent) - deferred_count = count_deferred_todos(todos_parent) - if count == 0 && deferred_count > 0 - word = I18n.t('common.actions_midsentence', :count => deferred_count) - word = I18n.t('common.deferred') + " " + word - return deferred_count.to_s + " " + word - else - word = I18n.t('common.actions_midsentence', :count => count) - return count.to_s + " " + word - end - end - - def count_undone_todos(todos_parent) - if todos_parent.nil? - count = 0 - elsif (todos_parent.is_a?(Project) && todos_parent.hidden?) - count = eval "@project_project_hidden_todo_counts[#{todos_parent.id}]" - else - count = eval "@#{todos_parent.class.to_s.downcase}_not_done_counts[#{todos_parent.id}]" - end - count || 0 - end - - def count_deferred_todos(todos_parent) - if todos_parent.nil? - count = 0 - else - count = todos_parent.todos.deferred.count - end - end - - # Convert a date object to the format specified in the user's preferences in - # config/settings.yml - # - def format_date(date) - return date ? date.in_time_zone(prefs.time_zone).strftime("#{prefs.date_format}") : '' - end - - def for_autocomplete(coll, substr) - if substr # protect agains empty request - filtered = coll.find_all{|item| item.name.downcase.include? substr.downcase} - json_elems = Array[*filtered.map{ |e| {:id => e.id.to_s, :value => e.name} }].to_json - return json_elems - else - return "" - end - end - - def format_dependencies_as_json_for_auto_complete(entries) - json_elems = Array[*entries.map{ |e| {:value => e.id.to_s, :label => e.specification} }].to_json - return json_elems - end - - # Here's the concept behind this "mobile content negotiation" hack: In - # addition to the main, AJAXy Web UI, Tracks has a lightweight low-feature - # 'mobile' version designed to be suitablef or use from a phone or PDA. It - # makes some sense that tne pages of that mobile version are simply alternate - # representations of the same Todo resources. The implementation goal was to - # treat mobile as another format and be able to use respond_to to render both - # versions. Unfortunately, I ran into a lot of trouble simply registering a - # new mime type 'text/html' with format :m because :html already is linked to - # that mime type and the new registration was forcing all html requests to be - # rendered in the mobile view. The before_filter and after_filter hackery - # below accomplishs that implementation goal by using a 'fake' mime type - # during the processing and then setting it to 'text/html' in an - # 'after_filter' -LKM 2007-04-01 - def mobile? - return params[:format] == 'm' - end - - def enable_mobile_content_negotiation - if mobile? - request.format = :m - end - end - - def create_todo_from_recurring_todo(rt, date=nil) - # create todo and initialize with data from recurring_todo rt - todo = current_user.todos.build( { :description => rt.description, :notes => rt.notes, :project_id => rt.project_id, :context_id => rt.context_id}) - todo.recurring_todo_id = rt.id - - # set dates - todo.due = rt.get_due_date(date) - - show_from_date = rt.get_show_from_date(date) - if show_from_date.nil? - todo.show_from=nil - else - # make sure that show_from is not in the past - todo.show_from = show_from_date < Time.zone.now ? nil : show_from_date - end - - saved = todo.save - if saved - todo.tag_with(rt.tag_list) - todo.tags.reload - end - - # increate number of occurences created from recurring todo - rt.inc_occurences - - # mark recurring todo complete if there are no next actions left - checkdate = todo.due.nil? ? todo.show_from : todo.due - rt.toggle_completion! unless rt.has_next_todo(checkdate) - - return saved ? todo : nil - end - - def handle_unverified_request - unless request.format=="application/xml" - super # handle xml http auth via our own login code - end - end - - protected - - def admin_login_required - unless User.find_by_id_and_is_admin(session['user_id'], true) - render :text => t('errors.user_unauthorized'), :status => 401 - return false - end - end - - def redirect_back_or_home - respond_to do |format| - format.html { redirect_back_or_default home_url } - format.m { redirect_back_or_default mobile_url } - end - end - - def boolean_param(param_name) - return false if param_name.blank? - s = params[param_name] - return false if s.blank? || s == false || s =~ /^false$/i - return true if s == true || s =~ /^true$/i - raise ArgumentError.new("invalid value for Boolean: \"#{s}\"") - end - - def self.openid_enabled? - Tracks::Config.openid_enabled? - end - - def openid_enabled? - self.class.openid_enabled? - end - - def self.cas_enabled? - Tracks::Config.cas_enabled? - end - - def cas_enabled? - self.class.cas_enabled? - end - - def self.prefered_auth? - Tracks::Config.prefered_auth? - end - - def prefered_auth? - self.class.prefered_auth? - end - - # all completed todos [today@00:00, today@now] - def get_done_today(completed_todos, includes = {:include => Todo::DEFAULT_INCLUDES}) - start_of_this_day = Time.zone.now.beginning_of_day - completed_todos.completed_after(start_of_this_day).all(includes) - end - - # all completed todos [begin_of_week, start_of_today] - def get_done_this_week(completed_todos, includes = {:include => Todo::DEFAULT_INCLUDES}) - start_of_this_week = Time.zone.now.beginning_of_week - start_of_this_day = Time.zone.now.beginning_of_day - completed_todos.completed_before(start_of_this_day).completed_after(start_of_this_week).all(includes) - end - - # all completed todos [begin_of_month, begin_of_week] - def get_done_this_month(completed_todos, includes = {:include => Todo::DEFAULT_INCLUDES}) - start_of_this_month = Time.zone.now.beginning_of_month - start_of_this_week = Time.zone.now.beginning_of_week - completed_todos.completed_before(start_of_this_week).completed_after(start_of_this_month).all(includes) - end - - private - - def parse_date_per_user_prefs( s ) - prefs.parse_date(s) - end - - def init_data_for_sidebar - @completed_projects = current_user.projects.completed - @hidden_projects = current_user.projects.hidden - @active_projects = current_user.projects.active - - @active_contexts = current_user.contexts.active - @hidden_contexts = current_user.contexts.hidden - - init_not_done_counts - if prefs.show_hidden_projects_in_sidebar - init_project_hidden_todo_counts(['project']) - end - end - - def init_not_done_counts(parents = ['project','context']) - parents.each do |parent| - eval("@#{parent}_not_done_counts = @#{parent}_not_done_counts || current_user.todos.active.count(:group => :#{parent}_id)") - end - end - - def init_project_hidden_todo_counts(parents = ['project','context']) - parents.each do |parent| - eval("@#{parent}_project_hidden_todo_counts = @#{parent}_project_hidden_todo_counts || current_user.todos.count(:conditions => ['state = ? or state = ?', 'project_hidden', 'active'], :group => :#{parent}_id)") - end - end - - # Set the contents of the flash message from a controller Usage: notify - # :warning, "This is the message" Sets the flash of type 'warning' to "This is - # the message" - def notify(type, message) - flash[type] = message - logger.error("ERROR: #{message}") if type == :error - end - - def set_time_zone - Time.zone = current_user.prefs.time_zone if logged_in? - end - - def set_zindex_counter - # this counter can be used to handle the IE z-index bug - @z_index_counter = 500 - end - -end diff --git a/app/controllers/todos_controller.rb b/app/controllers/todos_controller.rb index 77977baf..852c5d41 100644 --- a/app/controllers/todos_controller.rb +++ b/app/controllers/todos_controller.rb @@ -27,7 +27,11 @@ class TodosController < ApplicationController @not_done_todos = @not_done_todos. reorder("todos.due IS NULL, todos.due ASC, todos.created_at ASC"). includes(Todo::DEFAULT_INCLUDES) - @not_done_todos = @not_done_todos.limit(sanitize(params[:limit])) if params[:limit] + + if params[:limit] + @not_done_todos = @not_done_todos.limit(sanitize(params[:limit])) + @todos = @todos.limit(sanitize(params[:limit])) + end if params[:due] due_within_when = Time.zone.now + params['due'].to_i.days diff --git a/app/helpers/feedlist_helper.rb b/app/helpers/feedlist_helper.rb index d6b086f4..1cb0da32 100644 --- a/app/helpers/feedlist_helper.rb +++ b/app/helpers/feedlist_helper.rb @@ -1,25 +1,26 @@ module FeedlistHelper + def linkoptions(format, options) + merge_hashes( {:format => format}, options, user_token_hash) + end + def rss_formatted_link(options = {}) image_tag = image_tag("feed-icon.png", :size => "16X16", :border => 0, :class => "rss-icon") - linkoptions = merge_hashes( {:format => 'rss'}, user_token_hash, options) - link_to(image_tag, linkoptions, :title => "RSS feed") + link_to(image_tag, linkoptions('rss', options), :title => "RSS feed") end def text_formatted_link(options = {}) - linkoptions = merge_hashes( {:format => 'txt'}, user_token_hash, options) - link_to(content_tag(:span, 'TXT', {:class => 'feed', :title => "Plain text feed"}), linkoptions) + link_to(content_tag(:span, 'TXT', {:class => 'feed', :title => "Plain text feed"}), linkoptions('txt', options)) end def ical_formatted_link(options = {}) - linkoptions = merge_hashes( {:format => 'ics'}, user_token_hash, options) - link_to(content_tag(:span, 'iCal', {:class=>"feed", :title => "iCal feed"}), linkoptions) + link_to(content_tag(:span, 'iCal', {:class=>"feed", :title => "iCal feed"}), linkoptions('ics', options)) end def feed_links(feeds, link_options, title) space = " " html = "" - html << rss_formatted_link(link_options)+space if feeds.include?(:rss) + html << rss_formatted_link(link_options) +space if feeds.include?(:rss) html << text_formatted_link(link_options)+space if feeds.include?(:txt) html << ical_formatted_link(link_options)+space if feeds.include?(:ical) html << title diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 15364485..c68268ca 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -2,29 +2,17 @@ module ProjectsHelper def project_next_prev html = "" - if @previous_project - project_name = truncate(@previous_project.name, :length => 40, :omission => "...") - html << link_to_project(@previous_project, "« #{project_name}".html_safe) - end + html << link_to_project(@previous_project, "« #{@previous_project.shortened_name}".html_safe) if @previous_project html << " | " if @previous_project && @next_project - if @next_project - project_name = truncate(@next_project.name, :length => 40, :omission => "...") - html << link_to_project(@next_project, "#{project_name} »".html_safe) - end - html.html_safe + html << link_to_project(@next_project, "#{@next_project.shortened_name} »".html_safe) if @next_project + return html.html_safe end def project_next_prev_mobile prev_project,next_project= "", "" - if @previous_project - project_name = truncate(@previous_project.name, :length => 40, :omission => "...") - prev_project = content_tag(:li, link_to_project_mobile(@previous_project, "5", project_name), :class=>"prev") - end - if @next_project - project_name = truncate(@next_project.name, :length => 40, :omission => "...") - next_project = content_tag(:li, link_to_project_mobile(@next_project, "6", project_name), :class=>"next") - end - return content_tag(:ul, "#{prev_project}#{next_project}".html_safe, :class=>"next-prev-project").html_safe + prev_project = content_tag(:li, link_to_project_mobile(@previous_project, "5", @previous_project.shortened_name), :class=>"prev") if @previous_project + next_project = content_tag(:li, link_to_project_mobile(@next_project, "6", @next_project.shortened_name), :class=>"next") if @next_project + return content_tag(:ul, "#{prev_project}#{next_project}".html_safe, :class=>"next-prev-project") end def link_to_delete_project(project, descriptor = sanitize(project.name)) diff --git a/app/models/message_gateway.rb b/app/models/message_gateway.rb index 43248afc..ed275b44 100644 --- a/app/models/message_gateway.rb +++ b/app/models/message_gateway.rb @@ -34,22 +34,27 @@ class MessageGateway < ActionMailer::Base return SITE_CONFIG['email_dispatch'] == 'to' ? email.to[0] : email.from[0] end - def get_user_from_email_address(email) - if SITE_CONFIG['email_dispatch'] == 'single_user' - Rails.logger.info "All received email goes to #{ENV['TRACKS_MAIL_RECEIVER']}" - user = User.find_by_login(ENV['TRACKS_MAIL_RECEIVER']) - Rails.logger.info "WARNING: Unknown user set for TRACKS_MAIL_RECEIVER (#{ENV['TRACKS_MAIL_RECEIVER']})" if user.nil? - else - 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 - Rails.logger.info(!user.nil? ? "Email belongs to #{user.login}" : "User unknown") - end + def get_user_from_env_setting + Rails.logger.info "All received email goes to #{ENV['TRACKS_MAIL_RECEIVER']}" + user = User.find_by_login(ENV['TRACKS_MAIL_RECEIVER']) + 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 + Rails.logger.info(!user.nil? ? "Email belongs to #{user.login}" : "User unknown") + return user + 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 diff --git a/app/models/project.rb b/app/models/project.rb index 0ce8b151..32681008 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -118,6 +118,9 @@ class Project < ActiveRecord::Base return self.todos.deferred_or_blocked.empty? && self.todos.not_deferred_or_blocked.empty? end + def shortened_name(length=40) + name.truncate(length, :omission => "...").html_safe + end def name=(value) self[:name] = value.gsub(/\s{2,}/, " ").strip diff --git a/app/models/todo.rb b/app/models/todo.rb index aaebce79..18219a34 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -209,13 +209,13 @@ class Todo < ActiveRecord::Base end def update_state_from_project - if self.state == 'project_hidden' && !self.project.hidden? + if self.project_hidden? && (!self.project.hidden?) if self.uncompleted_predecessors.empty? self.activate! else self.block! end - elsif self.state == 'active' && self.project.hidden? + elsif self.active? && self.project.hidden? self.hide! end self.save! diff --git a/test/functional/todos_controller_test.rb b/test/functional/todos_controller_test.rb index d9c7f4f4..3eb3666f 100644 --- a/test/functional/todos_controller_test.rb +++ b/test/functional/todos_controller_test.rb @@ -338,7 +338,7 @@ class TodosControllerTest < ActionController::TestCase assert_xml_select 'feed[xmlns="http://www.w3.org/2005/Atom"]' do assert_xml_select '>title', 'Tracks Actions' assert_xml_select '>subtitle', "Actions for #{users(:admin_user).display_name}" - assert_xml_select 'entry', 11 do + assert_xml_select 'entry', 17 do assert_xml_select 'title', /.+/ assert_xml_select 'content[type="html"]', /.*/ assert_xml_select 'published', /(#{Regexp.escape(todos(:book).updated_at.xmlschema)}|#{Regexp.escape(projects(:moremoney).updated_at.xmlschema)})/ @@ -506,7 +506,7 @@ class TodosControllerTest < ActionController::TestCase # link todo_1 and recurring_todo_1 recurring_todo_1 = RecurringTodo.find(1) - set_user_to_current_time_zone(recurring_todo_1.user) + #set_user_to_current_time_zone(recurring_todo_1.user) todo_1 = Todo.find_by_recurring_todo_id(1) today = Time.zone.now.at_midnight @@ -592,20 +592,20 @@ class TodosControllerTest < ActionController::TestCase p.hide! assert p.reload().hidden? todo = p.todos.first - assert_equal "project_hidden", todo.state + + assert todo.project_hidden?, "todo should be project_hidden" # clear project from todo: the todo should be unhidden - xhr :post, :update, :id => 5, :_source_view => 'todo', "project_name"=>"None", "todo"=>{} - todo.reload() - assert assigns['project_changed'] - assert_equal "active", todo.state + xhr :post, :update, :id => todo.id, :_source_view => 'todo', "project_name"=>"None", "todo"=>{} + + assert assigns['project_changed'], "the project of the todo should be changed" + assert todo.reload().active?, "todo should be active" end def test_url_with_slash_in_query_string_are_parsed_correctly # See http://blog.swivel.com/code/2009/06/rails-auto_link-and-certain-query-strings.html login_as(:admin_user) - user = users(:admin_user) - todo = user.todos.first + todo = users(:admin_user).todos.first url = "http://example.com/foo?bar=/baz" todo.notes = "foo #{url} bar" todo.save! @@ -661,7 +661,7 @@ class TodosControllerTest < ActionController::TestCase get :index assert_select("div#notes_todo_#{todo.id}", 'link me to onenote') assert_select("div#notes_todo_#{todo.id} a", 'link me to onenote') - assert_select("div#notes_todo_#{todo.id} a[href=onenote:///E:\\OneNote\\dir\\notes.one#PAGE&section-id={FD597D3A-3793-495F-8345-23D34A00DD3B}&page-id={1C95A1C7-6408-4804-B3B5-96C28426022B}&end]", 'link me to onenote') + assert_select("div#notes_todo_#{todo.id} a[href=onenote:///E:%5COneNote%5Cdir%5Cnotes.one#PAGE&section-id=%7BFD597D3A-3793-495F-8345-23D34A00DD3B%7D&page-id=%7B1C95A1C7-6408-4804-B3B5-96C28426022B%7D&end]", 'link me to onenote') end def test_get_boolean_expression_from_parameters_of_tag_view_single_tag