diff options
Diffstat (limited to 'app')
31 files changed, 276 insertions, 264 deletions
diff --git a/app/controllers/admin_public_body_controller.rb b/app/controllers/admin_public_body_controller.rb index ac12e97b2..c41d05c8d 100644 --- a/app/controllers/admin_public_body_controller.rb +++ b/app/controllers/admin_public_body_controller.rb @@ -14,7 +14,7 @@ class AdminPublicBodyController < AdminController def _lookup_query_internal @locale = self.locale_from_params() - PublicBody.with_locale(@locale) do + I18n.with_locale(@locale) do @query = params[:query] if @query == "" @query = nil @@ -75,7 +75,7 @@ class AdminPublicBodyController < AdminController def show @locale = self.locale_from_params() - PublicBody.with_locale(@locale) do + I18n.with_locale(@locale) do @public_body = PublicBody.find(params[:id]) render end @@ -87,7 +87,7 @@ class AdminPublicBodyController < AdminController end def create - PublicBody.with_locale(I18n.default_locale) do + I18n.with_locale(I18n.default_locale) do params[:public_body][:last_edit_editor] = admin_current_user() @public_body = PublicBody.new(params[:public_body]) if @public_body.save @@ -106,7 +106,7 @@ class AdminPublicBodyController < AdminController end def update - PublicBody.with_locale(I18n.default_locale) do + I18n.with_locale(I18n.default_locale) do params[:public_body][:last_edit_editor] = admin_current_user() @public_body = PublicBody.find(params[:id]) if @public_body.update_attributes(params[:public_body]) @@ -120,7 +120,7 @@ class AdminPublicBodyController < AdminController def destroy @locale = self.locale_from_params() - PublicBody.with_locale(@locale) do + I18n.with_locale(@locale) do public_body = PublicBody.find(params[:id]) if public_body.info_requests.size > 0 diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index a946526b8..9b39d5178 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -27,9 +27,6 @@ class ApplicationController < ActionController::Base before_filter :set_vary_header before_filter :set_popup_banner - # scrub sensitive parameters from the logs - filter_parameter_logging :password - def set_vary_header response.headers['Vary'] = 'Cookie' end @@ -74,9 +71,6 @@ class ApplicationController < ActionController::Base end end - # scrub sensitive parameters from the logs - filter_parameter_logging :password - helper_method :locale_from_params # Help work out which request causes RAM spike. @@ -154,19 +148,20 @@ class ApplicationController < ActionController::Base render :template => "general/exception_caught.rhtml", :status => @status end - # For development sites. - alias original_rescue_action_locally rescue_action_locally - def rescue_action_locally(exception) - # Make sure expiry time for session is set (before_filters are - # otherwise missed by this override) - session_remember_me + # FIXME: This was disabled during the Rails 3 upgrade as this is now handled by Rack + # # For development sites. + # alias original_rescue_action_locally rescue_action_locally + # def rescue_action_locally(exception) + # # Make sure expiry time for session is set (before_filters are + # # otherwise missed by this override) + # session_remember_me - # Make sure the locale is set correctly too - set_gettext_locale + # # Make sure the locale is set correctly too + # set_gettext_locale - # Display default, detailed error for developers - original_rescue_action_locally(exception) - end + # # Display default, detailed error for developers + # original_rescue_action_locally(exception) + # end def local_request? false @@ -258,7 +253,7 @@ class ApplicationController < ActionController::Base # Check the user is logged in def authenticated?(reason_params) unless session[:user_id] - post_redirect = PostRedirect.new(:uri => request.request_uri, :post_params => params, + post_redirect = PostRedirect.new(:uri => request.fullpath, :post_params => params, :reason_params => reason_params) post_redirect.save! # 'modal' controls whether the sign-in form will be displayed in the typical full-blown diff --git a/app/controllers/general_controller.rb b/app/controllers/general_controller.rb index 3ba636e29..faf34aa04 100644 --- a/app/controllers/general_controller.rb +++ b/app/controllers/general_controller.rb @@ -19,53 +19,51 @@ class GeneralController < ApplicationController # New, improved front page! def frontpage medium_cache - behavior_cache :tag => [session[:user_id], request.url] do - # get some example searches and public bodies to display - # either from config, or based on a (slow!) query if not set - body_short_names = Configuration::frontpage_publicbody_examples.split(/\s*;\s*/).map{|s| "'%s'" % s.gsub(/'/, "''") }.join(", ") - @locale = self.locale_from_params() - locale_condition = 'public_body_translations.locale = ?' - conditions = [locale_condition, @locale] - PublicBody.with_locale(@locale) do - if body_short_names.empty? - # This is too slow - @popular_bodies = PublicBody.visible.find(:all, - :order => "info_requests_count desc", - :limit => 32, - :conditions => conditions, - :joins => :translations - ) - else - conditions[0] += " and public_bodies.url_name in (" + body_short_names + ")" - @popular_bodies = PublicBody.find(:all, - :conditions => conditions, - :joins => :translations) - end + # get some example searches and public bodies to display + # either from config, or based on a (slow!) query if not set + body_short_names = Configuration::frontpage_publicbody_examples.split(/\s*;\s*/).map{|s| "'%s'" % s.gsub(/'/, "''") }.join(", ") + @locale = self.locale_from_params() + locale_condition = 'public_body_translations.locale = ?' + conditions = [locale_condition, @locale] + I18n.with_locale(@locale) do + if body_short_names.empty? + # This is too slow + @popular_bodies = PublicBody.visible.find(:all, + :order => "info_requests_count desc", + :limit => 32, + :conditions => conditions, + :joins => :translations + ) + else + conditions[0] += " and public_bodies.url_name in (" + body_short_names + ")" + @popular_bodies = PublicBody.find(:all, + :conditions => conditions, + :joins => :translations) end - # Get some successful requests - begin - query = 'variety:response (status:successful OR status:partially_successful)' - sortby = "newest" - max_count = 5 - xapian_object = perform_search([InfoRequestEvent], query, sortby, 'request_title_collapse', max_count) - @request_events = xapian_object.results.map { |r| r[:model] } - - # If there are not yet enough successful requests, fill out the list with - # other requests - if @request_events.count < max_count - @request_events_all_successful = false - query = 'variety:sent' - xapian_object = perform_search([InfoRequestEvent], query, sortby, 'request_title_collapse', max_count-@request_events.count) - more_events = xapian_object.results.map { |r| r[:model] } - @request_events += more_events - # Overall we still want the list sorted with the newest first - @request_events.sort!{|e1,e2| e2.created_at <=> e1.created_at} - else - @request_events_all_successful = true - end - rescue - @request_events = [] + end + # Get some successful requests + begin + query = 'variety:response (status:successful OR status:partially_successful)' + sortby = "newest" + max_count = 5 + xapian_object = perform_search([InfoRequestEvent], query, sortby, 'request_title_collapse', max_count) + @request_events = xapian_object.results.map { |r| r[:model] } + + # If there are not yet enough successful requests, fill out the list with + # other requests + if @request_events.count < max_count + @request_events_all_successful = false + query = 'variety:sent' + xapian_object = perform_search([InfoRequestEvent], query, sortby, 'request_title_collapse', max_count-@request_events.count) + more_events = xapian_object.results.map { |r| r[:model] } + @request_events += more_events + # Overall we still want the list sorted with the newest first + @request_events.sort!{|e1,e2| e2.created_at <=> e1.created_at} + else + @request_events_all_successful = true end + rescue + @request_events = [] end end diff --git a/app/controllers/help_controller.rb b/app/controllers/help_controller.rb index cf90f45bb..d13b2655f 100644 --- a/app/controllers/help_controller.rb +++ b/app/controllers/help_controller.rb @@ -50,7 +50,7 @@ class HelpController < ApplicationController end @contact = ContactValidator.new(params[:contact]) if @contact.valid? && !params[:remove] - ContactMailer.deliver_message( + ContactMailer.deliver_to_admin_message( params[:contact][:name], params[:contact][:email], params[:contact][:subject], diff --git a/app/controllers/public_body_controller.rb b/app/controllers/public_body_controller.rb index 8a4a65820..5265706bf 100644 --- a/app/controllers/public_body_controller.rb +++ b/app/controllers/public_body_controller.rb @@ -16,7 +16,7 @@ class PublicBodyController < ApplicationController return end @locale = self.locale_from_params() - PublicBody.with_locale(@locale) do + I18n.with_locale(@locale) do @public_body = PublicBody.find_by_url_name_with_historic(params[:url_name]) raise ActiveRecord::RecordNotFound.new("None found") if @public_body.nil? if @public_body.url_name.nil? @@ -71,7 +71,7 @@ class PublicBodyController < ApplicationController @public_body = PublicBody.find_by_url_name_with_historic(params[:url_name]) raise ActiveRecord::RecordNotFound.new("None found") if @public_body.nil? - PublicBody.with_locale(self.locale_from_params()) do + I18n.with_locale(self.locale_from_params()) do if params[:submitted_view_email] if verify_recaptcha flash.discard(:error) @@ -129,7 +129,7 @@ class PublicBodyController < ApplicationController @description = _("in the category ‘{{category_name}}’", :category_name=>category_name) end end - PublicBody.with_locale(@locale) do + I18n.with_locale(@locale) do @public_bodies = PublicBody.paginate( :order => "public_body_translations.name", :page => params[:page], :per_page => 100, :conditions => conditions, diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index 3e8c0a5f6..6106d68da 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -52,7 +52,7 @@ class RequestController < ApplicationController medium_cache end @locale = self.locale_from_params() - PublicBody.with_locale(@locale) do + I18n.with_locale(@locale) do # Look up by old style numeric identifiers if params[:url_title].match(/^[0-9]+$/) @@ -99,15 +99,13 @@ class RequestController < ApplicationController # Sidebar stuff # ... requests that have similar imporant terms - behavior_cache :tag => ['similar', @info_request.id] do - begin - limit = 10 - @xapian_similar = ::ActsAsXapian::Similar.new([InfoRequestEvent], @info_request.info_request_events, - :limit => limit, :collapse_by_prefix => 'request_collapse') - @xapian_similar_more = (@xapian_similar.matches_estimated > limit) - rescue - @xapian_similar = nil - end + begin + limit = 10 + @xapian_similar = ::ActsAsXapian::Similar.new([InfoRequestEvent], @info_request.info_request_events, + :limit => limit, :collapse_by_prefix => 'request_collapse') + @xapian_similar_more = (@xapian_similar.matches_estimated > limit) + rescue + @xapian_similar = nil end # Track corresponding to this page @@ -180,13 +178,10 @@ class RequestController < ApplicationController query = make_query_from_params @title = _("View and search requests") sortby = "newest" - @cache_tag = Digest::MD5.hexdigest(query + @page.to_s + I18n.locale.to_s) - behavior_cache :tag => [@cache_tag] do - xapian_object = perform_search([InfoRequestEvent], query, sortby, 'request_collapse') - @list_results = xapian_object.results.map { |r| r[:model] } - @matches_estimated = xapian_object.matches_estimated - @show_no_more_than = (@matches_estimated > MAX_RESULTS) ? MAX_RESULTS : @matches_estimated - end + xapian_object = perform_search([InfoRequestEvent], query, sortby, 'request_collapse') + @list_results = xapian_object.results.map { |r| r[:model] } + @matches_estimated = xapian_object.matches_estimated + @show_no_more_than = (@matches_estimated > MAX_RESULTS) ? MAX_RESULTS : @matches_estimated @title = @title + " (page " + @page.to_s + ")" if (@page > 1) @track_thing = TrackThing.create_track_for_search_query(query) @@ -817,7 +812,7 @@ class RequestController < ApplicationController # FOI officers can upload a response def upload_response @locale = self.locale_from_params() - PublicBody.with_locale(@locale) do + I18n.with_locale(@locale) do @info_request = InfoRequest.find_by_url_title!(params[:url_title]) @reason_params = { @@ -874,7 +869,7 @@ class RequestController < ApplicationController def download_entire_request @locale = self.locale_from_params() - PublicBody.with_locale(@locale) do + I18n.with_locale(@locale) do @info_request = InfoRequest.find_by_url_title!(params[:url_title]) # Test for whole request being hidden or requester-only if !@info_request.all_can_view? diff --git a/app/models/application_mailer.rb b/app/models/application_mailer.rb index cdb279c3c..84b045795 100644 --- a/app/models/application_mailer.rb +++ b/app/models/application_mailer.rb @@ -30,9 +30,11 @@ class ApplicationMailer < ActionMailer::Base # will be initialized according to the named method. If not, the mailer will # remain uninitialized (useful when you only need to invoke the "receive" # method, for instance). - def initialize(method_name=nil, *parameters) #:nodoc: - create!(method_name, *parameters) if method_name - end + + # TEMPORARY: commented out method below while upgrading to Rails 3 + #def initialize(method_name=nil, *parameters) #:nodoc: + # create!(method_name, *parameters) if method_name + #end # For each multipart template (e.g. "the_template_file.text.html.erb") available, # add the one from the view path with the highest priority as a part to the mail @@ -67,6 +69,7 @@ class ApplicationMailer < ActionMailer::Base return nil end + # FIXME: This check was disabled temporarily during the Rails 3 upgrade if ActionMailer::VERSION::MAJOR == 2 # This method is a customised version of ActionMailer::Base.create! @@ -142,9 +145,9 @@ class ApplicationMailer < ActionMailer::Base # build the mail object itself @mail = create_mail end - else - raise "ApplicationMailer.create! is obsolete - find another way to ensure that themes can override mail templates for multipart mails" - end + else + #raise "ApplicationMailer.create! is obsolete - find another way to ensure that themes can override mail templates for multipart mails" + end end diff --git a/app/models/censor_rule.rb b/app/models/censor_rule.rb index f40ab6fbb..ec66074b7 100644 --- a/app/models/censor_rule.rb +++ b/app/models/censor_rule.rb @@ -33,13 +33,15 @@ class CensorRule < ActiveRecord::Base validate :require_valid_regexp, :if => proc{ |rule| rule.regexp? == true } validates_presence_of :text - named_scope :global, {:conditions => {:info_request_id => nil, - :user_id => nil, - :public_body_id => nil}} + scope :global, {:conditions => {:info_request_id => nil, + :user_id => nil, + :public_body_id => nil}} def require_user_request_or_public_body if self.info_request.nil? && self.user.nil? && self.public_body.nil? - errors.add("Censor must apply to an info request a user or a body; ") + [:info_request, :user, :public_body].each do |a| + errors.add(a, "Rule must apply to an info request, a user or a body") + end end end diff --git a/app/models/contact_mailer.rb b/app/models/contact_mailer.rb index 16aae2f15..abde64928 100644 --- a/app/models/contact_mailer.rb +++ b/app/models/contact_mailer.rb @@ -7,7 +7,7 @@ class ContactMailer < ApplicationMailer # Send message to administrator - def message(name, email, subject, message, logged_in_user, last_request, last_body) + def to_admin_message(name, email, subject, message, logged_in_user, last_request, last_body) @from = name + " <" + email + ">" @recipients = contact_from_name_and_email @subject = subject diff --git a/app/models/incoming_message.rb b/app/models/incoming_message.rb index 3f551f420..f70b8c0cb 100644 --- a/app/models/incoming_message.rb +++ b/app/models/incoming_message.rb @@ -605,7 +605,7 @@ class IncomingMessage < ActiveRecord::Base content_type = 'application/octet-stream' end hexdigest = Digest::MD5.hexdigest(content) - attachment = self.foi_attachments.find_or_create_by_hexdigest(:hexdigest => hexdigest) + attachment = self.foi_attachments.find_or_create_by_hexdigest(hexdigest) attachment.update_attributes(:filename => filename, :content_type => content_type, :body => content, @@ -632,7 +632,7 @@ class IncomingMessage < ActiveRecord::Base attachment_attributes = MailHandler.get_attachment_attributes(self.mail(force)) attachments = [] attachment_attributes.each do |attrs| - attachment = self.foi_attachments.find_or_create_by_hexdigest(:hexdigest => attrs[:hexdigest]) + attachment = self.foi_attachments.find_or_create_by_hexdigest(attrs[:hexdigest]) body = attrs.delete(:body) attachment.update_attributes(attrs) # Set the body separately as its handling can depend on the value of charset diff --git a/app/models/info_request.rb b/app/models/info_request.rb index cee9eb959..c85a9701d 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -27,7 +27,7 @@ require 'digest/sha1' class InfoRequest < ActiveRecord::Base include ActionView::Helpers::UrlHelper - include ActionController::UrlWriter + include Rails.application.routes.url_helpers strip_attributes! @@ -51,7 +51,7 @@ class InfoRequest < ActiveRecord::Base has_tag_string - named_scope :visible, :conditions => {:prominence => "normal"} + scope :visible, :conditions => {:prominence => "normal"} # user described state (also update in info_request_event, admin_request/edit.rhtml) validate :must_be_valid_state @@ -81,6 +81,11 @@ class InfoRequest < ActiveRecord::Base 'blackhole' # just dump them ] + # only check on create, so existing models with mixed case are allowed + validate :title_formatting, :on => :create + + after_initialize :set_defaults + def self.enumerate_states states = [ 'waiting_response', @@ -156,31 +161,8 @@ class InfoRequest < ActiveRecord::Base rescue MissingSourceFile, NameError end - # only check on create, so existing models with mixed case are allowed - def validate_on_create - if !self.title.nil? && !MySociety::Validate.uses_mixed_capitals(self.title, 10) - errors.add(:title, _('Please write the summary using a mixture of capital and lower case letters. This makes it easier for others to read.')) - end - if !self.title.nil? && title.size > 200 - errors.add(:title, _('Please keep the summary short, like in the subject of an email. You can use a phrase, rather than a full sentence.')) - end - if !self.title.nil? && self.title =~ /^(FOI|Freedom of Information)\s*requests?$/i - errors.add(:title, _('Please describe more what the request is about in the subject. There is no need to say it is an FOI request, we add that on anyway.')) - end - end - OLD_AGE_IN_DAYS = 21.days - def after_initialize - if self.described_state.nil? - self.described_state = 'waiting_response' - end - # FOI or EIR? - if !self.public_body.nil? && self.public_body.eir_only? - self.law_used = 'eir' - end - end - def visible_comments self.comments.find(:all, :conditions => 'visible') end @@ -1155,5 +1137,29 @@ public yield(column.human_name, self.send(column.name), column.type.to_s, column.name) end end + + private + + def set_defaults + if self.described_state.nil? + self.described_state = 'waiting_response' + end + # FOI or EIR? + if !self.public_body.nil? && self.public_body.eir_only? + self.law_used = 'eir' + end + end + + def title_formatting + if !self.title.nil? && !MySociety::Validate.uses_mixed_capitals(self.title, 10) + errors.add(:title, _('Please write the summary using a mixture of capital and lower case letters. This makes it easier for others to read.')) + end + if !self.title.nil? && title.size > 200 + errors.add(:title, _('Please keep the summary short, like in the subject of an email. You can use a phrase, rather than a full sentence.')) + end + if !self.title.nil? && self.title =~ /^(FOI|Freedom of Information)\s*requests?$/i + errors.add(:title, _('Please describe more what the request is about in the subject. There is no need to say it is an FOI request, we add that on anyway.')) + end + end end diff --git a/app/models/outgoing_message.rb b/app/models/outgoing_message.rb index 441813e5f..23b5c904b 100644 --- a/app/models/outgoing_message.rb +++ b/app/models/outgoing_message.rb @@ -50,6 +50,8 @@ class OutgoingMessage < ActiveRecord::Base end end + after_initialize :set_default_letter + # How the default letter starts and ends def get_salutation ret = "" @@ -129,13 +131,6 @@ class OutgoingMessage < ActiveRecord::Base MySociety::Validate.contains_postcode?(self.body) end - # Set default letter - def after_initialize - if self.body.nil? - self.body = get_default_message - end - end - # Check have edited letter def validate if self.body.empty? || self.body =~ /\A#{get_salutation}\s+#{get_signoff}/ || self.body =~ /#{get_internal_review_insert_here_note}/ @@ -275,6 +270,14 @@ class OutgoingMessage < ActiveRecord::Base yield(column.human_name, self.send(column.name), column.type.to_s, column.name) end end + + private + + def set_default_letter + if self.body.nil? + self.body = get_default_message + end + end end diff --git a/app/models/post_redirect.rb b/app/models/post_redirect.rb index 31f08c21a..dfca936e2 100644 --- a/app/models/post_redirect.rb +++ b/app/models/post_redirect.rb @@ -32,6 +32,8 @@ class PostRedirect < ActiveRecord::Base # Optional, does a login confirm before redirect for use in email links. belongs_to :user + after_initialize :generate_token + # We store YAML version of POST parameters in the database def post_params=(params) self.post_params_yaml = params.to_yaml @@ -62,18 +64,6 @@ class PostRedirect < ActiveRecord::Base MySociety::Util.generate_token end - # Make the token - def after_initialize - # The token is used to return you to what you are doing after the login form. - if not self.token - self.token = PostRedirect.generate_random_token - end - # There is a separate token to use in the URL if we send a confirmation email. - if not self.email_token - self.email_token = PostRedirect.generate_random_token - end - end - # Used by (rspec) test code only def self.get_last_post_redirect # XXX yeuch - no other easy way of getting the token so we can check @@ -89,6 +79,18 @@ class PostRedirect < ActiveRecord::Base PostRedirect.delete_all "updated_at < (now() - interval '2 months')" end + private + + def generate_token + # The token is used to return you to what you are doing after the login form. + if not self.token + self.token = PostRedirect.generate_random_token + end + # There is a separate token to use in the URL if we send a confirmation email. + if not self.email_token + self.email_token = PostRedirect.generate_random_token + end + end end diff --git a/app/models/profile_photo.rb b/app/models/profile_photo.rb index 6e605651d..41cb298b3 100644 --- a/app/models/profile_photo.rb +++ b/app/models/profile_photo.rb @@ -23,29 +23,15 @@ class ProfilePhoto < ActiveRecord::Base belongs_to :user + validate :data_and_draft_checks + # deliberately don't strip_attributes, so keeps raw photo properly attr_accessor :x, :y, :w, :h - # convert binary data blob into ImageMagick image when assigned attr_accessor :image - def after_initialize - if data.nil? - self.image = nil - return - end - image_list = Magick::ImageList.new - begin - image_list.from_blob(data) - rescue Magick::ImageMagickError - self.image = nil - return - end - - self.image = image_list[0] # XXX perhaps take largest image or somesuch if there were multiple in the file? - self.convert_image - end + after_initialize :convert_data_to_image # make image valid format and size def convert_image @@ -81,7 +67,9 @@ class ProfilePhoto < ActiveRecord::Base end end - def validate + private + + def data_and_draft_checks if self.data.nil? errors.add(:data, N_("Please choose a file containing your photo.")) return @@ -108,6 +96,25 @@ class ProfilePhoto < ActiveRecord::Base raise "Internal error, real pictures must have a user" end end + + # Convert binary data blob into ImageMagick image when assigned + def convert_data_to_image + if data.nil? + self.image = nil + return + end + + image_list = Magick::ImageList.new + begin + image_list.from_blob(data) + rescue Magick::ImageMagickError + self.image = nil + return + end + + self.image = image_list[0] # XXX perhaps take largest image or somesuch if there were multiple in the file? + self.convert_image + end end diff --git a/app/models/public_body.rb b/app/models/public_body.rb index f71520ee6..5084ad6e8 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -43,7 +43,7 @@ class PublicBody < ActiveRecord::Base before_save :set_api_key, :set_default_publication_scheme # Every public body except for the internal admin one is visible - named_scope :visible, lambda { + scope :visible, lambda { { :conditions => "public_bodies.id <> #{PublicBody.internal_admin_body.id}" } @@ -52,7 +52,7 @@ class PublicBody < ActiveRecord::Base translates :name, :short_name, :request_email, :url_name, :notes, :first_letter, :publication_scheme # Convenience methods for creating/editing translations via forms - def translation(locale) + def find_translation_by_locale(locale) self.translations.find_by_locale(locale) end @@ -104,28 +104,25 @@ class PublicBody < ActiveRecord::Base # like find_by_url_name but also search historic url_name if none found def self.find_by_url_name_with_historic(name) - locale = self.locale || I18n.locale - PublicBody.with_locale(locale) do - found = PublicBody.find(:all, - :conditions => ["public_body_translations.url_name=?", name], - :joins => :translations, - :readonly => false) - # If many bodies are found (usually because the url_name is the same across - # locales) return any of them - return found.first if found.size >= 1 - - # If none found, then search the history of short names - old = PublicBody::Version.find_all_by_url_name(name) - # Find unique public bodies in it - old = old.map { |x| x.public_body_id } - old = old.uniq - # Maybe return the first one, so we show something relevant, - # rather than throwing an error? - raise "Two bodies with the same historical URL name: #{name}" if old.size > 1 - return unless old.size == 1 - # does acts_as_versioned provide a method that returns the current version? - return PublicBody.find(old.first) - end + found = PublicBody.find(:all, + :conditions => ["public_body_translations.url_name=?", name], + :joins => :translations, + :readonly => false) + # If many bodies are found (usually because the url_name is the same across + # locales) return any of them + return found.first if found.size >= 1 + + # If none found, then search the history of short names + old = PublicBody::Version.find_all_by_url_name(name) + # Find unique public bodies in it + old = old.map { |x| x.public_body_id } + old = old.uniq + # Maybe return the first one, so we show something relevant, + # rather than throwing an error? + raise "Two bodies with the same historical URL name: #{name}" if old.size > 1 + return unless old.size == 1 + # does acts_as_versioned provide a method that returns the current version? + return PublicBody.find(old.first) end # Set the first letter, which is used for faster queries @@ -250,13 +247,13 @@ class PublicBody < ActiveRecord::Base # When name or short name is changed, also change the url name def short_name=(short_name) - globalize.write(self.class.locale || I18n.locale, :short_name, short_name) + globalize.write(I18n.locale, :short_name, short_name) self[:short_name] = short_name self.update_url_name end def name=(name) - globalize.write(self.class.locale || I18n.locale, :name, name) + globalize.write(I18n.locale, :name, name) self[:name] = name self.update_url_name end @@ -336,7 +333,7 @@ class PublicBody < ActiveRecord::Base # The "internal admin" is a special body for internal use. def PublicBody.internal_admin_body - PublicBody.with_locale(I18n.default_locale) do + I18n.with_locale(I18n.default_locale) do pb = PublicBody.find_by_url_name("internal_admin_authority") if pb.nil? pb = PublicBody.new( @@ -374,7 +371,7 @@ class PublicBody < ActiveRecord::Base # of updating them bodies_by_name = {} set_of_existing = Set.new() - PublicBody.with_locale(I18n.default_locale) do + I18n.with_locale(I18n.default_locale) do bodies = (tag.nil? || tag.empty?) ? PublicBody.find(:all) : PublicBody.find_by_tag(tag) for existing_body in bodies # Hide InternalAdminBody from import notes @@ -417,7 +414,7 @@ class PublicBody < ActiveRecord::Base if public_body = bodies_by_name[name] # Existing public body available_locales.each do |locale| - PublicBody.with_locale(locale) do + I18n.with_locale(locale) do changed = ActiveSupport::OrderedHash.new field_list.each do |field_name| localized_field_name = (locale.to_s == I18n.default_locale.to_s) ? field_name : "#{field_name}.#{locale}" @@ -452,7 +449,7 @@ class PublicBody < ActiveRecord::Base else # New public body public_body = PublicBody.new(:name=>"", :short_name=>"", :request_email=>"") available_locales.each do |locale| - PublicBody.with_locale(locale) do + I18n.with_locale(locale) do changed = ActiveSupport::OrderedHash.new field_list.each do |field_name| localized_field_name = (locale.to_s == I18n.default_locale.to_s) ? field_name : "#{field_name}.#{locale}" diff --git a/app/models/request_mailer.rb b/app/models/request_mailer.rb index 493d6961c..f07e3c3d8 100644 --- a/app/models/request_mailer.rb +++ b/app/models/request_mailer.rb @@ -76,15 +76,17 @@ class RequestMailer < ApplicationMailer def new_response(info_request, incoming_message) # Don't use login link here, just send actual URL. This is # because people tend to forward these emails amongst themselves. - url = main_url(incoming_message_url(incoming_message)) + @url = main_url(incoming_message_url(incoming_message)) + @incoming_message, @info_request = incoming_message, info_request - @from = contact_from_name_and_email - headers 'Return-Path' => blackhole_email, 'Reply-To' => @from, # not much we can do if the user's email is broken + headers 'Return-Path' => blackhole_email, 'Auto-Submitted' => 'auto-generated', # http://tools.ietf.org/html/rfc3834 'X-Auto-Response-Suppress' => 'OOF' - @recipients = info_request.user.name_and_email - @subject = _("New response to your FOI request - ") + info_request.title - @body = { :incoming_message => incoming_message, :info_request => info_request, :url => url } + mail(:from => contact_from_name_and_email, :to => info_request.user.name_and_email, + :subject => _("New response to your FOI request - ") + info_request.title, + :charset => "UTF-8", + # not much we can do if the user's email is broken + :reply_to => contact_from_name_and_email) end # Tell the requester that the public body is late in replying diff --git a/app/models/user.rb b/app/models/user.rb index 6e1e21481..490587c39 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -50,12 +50,17 @@ class User < ActiveRecord::Base 'super', ], :message => N_('Admin level is not included in list') + validate :email_and_name_are_valid + acts_as_xapian :texts => [ :name, :about_me ], :values => [ [ :created_at_numeric, 1, "created_at", :number ] # for sorting ], :terms => [ [ :variety, 'V', "variety" ] ], :if => :indexed_by_search? + + after_initialize :set_defaults + def created_at_numeric # format it here as no datetime support in Xapian's value ranges return self.created_at.strftime("%Y%m%d%H%M%S") @@ -65,17 +70,6 @@ class User < ActiveRecord::Base "user" end - def after_initialize - if self.admin_level.nil? - self.admin_level = 'none' - end - if self.new_record? - # make alert emails go out at a random time for each new user, so - # overall they are spread out throughout the day. - self.last_daily_track_email = User.random_time_in_last_day - end - end - # requested_by: and commented_by: search queries also need updating after save after_update :reindex_referencing_models def reindex_referencing_models @@ -108,15 +102,6 @@ class User < ActiveRecord::Base self.comments.find(:all, :conditions => 'visible') end - def validate - if self.email != "" && !MySociety::Validate.is_valid_email(self.email) - errors.add(:email, _("Please enter a valid email address")) - end - if MySociety::Validate.is_valid_email(self.name) - errors.add(:name, _("Please enter your name, not your email address, in the name field.")) - end - end - # Don't display any leading/trailing spaces # XXX we have strip_attributes! now, so perhaps this can be removed (might # be still needed for existing cases) @@ -148,14 +133,14 @@ class User < ActiveRecord::Base if user # There is user with email, check password if !user.has_this_password?(params[:password]) - user.errors.add_to_base(auth_fail_message) + user.errors.add(:base, auth_fail_message) end else # No user of same email, make one (that we don't save in the database) # for the forms code to use. user = User.new(params) # deliberately same message as above so as not to leak whether registered - user.errors.add_to_base(auth_fail_message) + user.errors.add(:base, auth_fail_message) end user end @@ -413,6 +398,26 @@ class User < ActiveRecord::Base self.salt = self.object_id.to_s + rand.to_s end + def set_defaults + if self.admin_level.nil? + self.admin_level = 'none' + end + if self.new_record? + # make alert emails go out at a random time for each new user, so + # overall they are spread out throughout the day. + self.last_daily_track_email = User.random_time_in_last_day + end + end + + def email_and_name_are_valid + if self.email != "" && !MySociety::Validate.is_valid_email(self.email) + errors.add(:email, _("Please enter a valid email address")) + end + if MySociety::Validate.is_valid_email(self.name) + errors.add(:name, _("Please enter your name, not your email address, in the name field.")) + end + end + ## Class methods def User.encrypted_password(password, salt) string_to_hash = password + salt # XXX need to add a secret here too? diff --git a/app/views/admin_public_body/_form.rhtml b/app/views/admin_public_body/_form.rhtml index 0d6ae51e2..7392b5bf3 100644 --- a/app/views/admin_public_body/_form.rhtml +++ b/app/views/admin_public_body/_form.rhtml @@ -18,7 +18,7 @@ prefix = "public_body[translated_versions][]" object = @public_body.new_record? ? PublicBody::Translation.new : - @public_body.translation(locale.to_s) || PublicBody::Translation.new + @public_body.find_translation_by_locale(locale.to_s) || PublicBody::Translation.new end fields_for prefix, object do |t| diff --git a/app/views/contact_mailer/message.rhtml b/app/views/contact_mailer/to_admin_message.rhtml index 9c0a74c02..9c0a74c02 100644 --- a/app/views/contact_mailer/message.rhtml +++ b/app/views/contact_mailer/to_admin_message.rhtml diff --git a/app/views/general/frontpage.rhtml b/app/views/general/frontpage.rhtml index acc7f4095..bf5261d15 100644 --- a/app/views/general/frontpage.rhtml +++ b/app/views/general/frontpage.rhtml @@ -1,4 +1,4 @@ -<% view_cache :ttl => 5.minutes.to_i, :tag => I18n.locale do %> +<% # TODO: Cache for 5 minutes %> <div id="frontpage_splash"> <div id="left_column"> <%= render :partial => "frontpage_new_request" %> @@ -17,4 +17,3 @@ <%= render :partial => "frontpage_bodies_list" %> <%= render :partial => "frontpage_requests_list" %> </div> -<% end %> diff --git a/app/views/help/contact.rhtml b/app/views/help/contact.rhtml index fab5017b8..385c24a62 100644 --- a/app/views/help/contact.rhtml +++ b/app/views/help/contact.rhtml @@ -46,7 +46,7 @@ <p> <label class="form_label" for="contact_name">Your name:</label> <%= f.text_field :name, :size => 20 %> - (or <%= link_to "sign in", signin_url(:r => request.request_uri) %>) + (or <%= link_to "sign in", signin_url(:r => request.fullpath) %>) </p> <p> diff --git a/app/views/layouts/default.rhtml b/app/views/layouts/default.rhtml index 6ac7064a7..28d099984 100644 --- a/app/views/layouts/default.rhtml +++ b/app/views/layouts/default.rhtml @@ -32,7 +32,7 @@ <% end %> <% end %> <% if @has_json %> - <link rel="alternate" type="application/json" title="JSON version of this page" href="<%=h main_url(request.request_uri, '.json') %>"> + <link rel="alternate" type="application/json" title="JSON version of this page" href="<%=h main_url(request.fullpath, '.json') %>"> <% end %> <% if @no_crawl %> @@ -92,9 +92,9 @@ <% end %> - <%= link_to _("Sign out"), signout_url(:r => request.request_uri) %> + <%= link_to _("Sign out"), signout_url(:r => request.fullpath) %> <% else %> - <%= link_to _("Sign in or sign up"), signin_url(:r => request.request_uri) %> + <%= link_to _("Sign in or sign up"), signin_url(:r => request.fullpath) %> <% end %> </div> <% end %> diff --git a/app/views/request/_describe_state.rhtml b/app/views/request/_describe_state.rhtml index 5b6004e81..f70e5ed8b 100644 --- a/app/views/request/_describe_state.rhtml +++ b/app/views/request/_describe_state.rhtml @@ -108,7 +108,7 @@ <%= _('We don\'t know whether the most recent response to this request contains information or not – - if you are {{user_link}} please <a href="{{url}}">sign in</a> and let everyone know.',:user_link=>user_link(@info_request.user), :url=>signin_url(:r => request.request_uri)) %> + if you are {{user_link}} please <a href="{{url}}">sign in</a> and let everyone know.',:user_link=>user_link(@info_request.user), :url=>signin_url(:r => request.fullpath)) %> <% end %> <% end %> diff --git a/app/views/request/_hidden_correspondence.rhtml b/app/views/request/_hidden_correspondence.rhtml index 0873b312f..a5e680385 100644 --- a/app/views/request/_hidden_correspondence.rhtml +++ b/app/views/request/_hidden_correspondence.rhtml @@ -8,20 +8,20 @@ <div class="correspondence" id="incoming-<%=incoming_message.id.to_s%>"> <p> <%= raw(_('This response has been hidden. See annotations to find out why. - If you are the requester, then you may <a href="%s">sign in</a> to view the response.') % [signin_url(:r => request.request_uri)]) %> + If you are the requester, then you may <a href="%s">sign in</a> to view the response.') % [signin_url(:r => request.fullpath)]) %> </p> </div> <% elsif [ 'sent', 'followup_sent', 'resent', 'followup_resent' ].include?(info_request_event.event_type) %> <div class="correspondence" id="outgoing-<%=outgoing_message.id.to_s%>"> <p> <%= raw(_('This outgoing message has been hidden. See annotations to - find out why. If you are the requester, then you may <a href="%s">sign in</a> to view the response.') % [signin_url(:r => request.request_uri)]) %> + find out why. If you are the requester, then you may <a href="%s">sign in</a> to view the response.') % [signin_url(:r => request.fullpath)]) %> </p> </div> <% elsif info_request_event.event_type == 'comment' %> <div class="comment_in_request" id="comment-<%=comment.id.to_s%>"> <p><%= raw(_('This comment has been hidden. See annotations to - find out why. If you are the requester, then you may <a href="%s">sign in</a> to view the response.') % [signin_url(:r => request.request_uri)]) %> + find out why. If you are the requester, then you may <a href="%s">sign in</a> to view the response.') % [signin_url(:r => request.fullpath)]) %> </p> </div> <% end %> diff --git a/app/views/request/_sidebar.rhtml b/app/views/request/_sidebar.rhtml index b669278f9..d349bee14 100644 --- a/app/views/request/_sidebar.rhtml +++ b/app/views/request/_sidebar.rhtml @@ -46,8 +46,8 @@ <%= render :partial => 'request/next_actions' %> - <% view_cache :ttl => 1.day.to_i, :tag => ['similar', @info_request.id, I18n.locale] do %> - <% if !@xapian_similar.nil? && @xapian_similar.results.size > 0 %> + <% # TODO: Cache for 1 day %> + <% if !@xapian_similar.nil? && @xapian_similar.results.size > 0 %> <h2><%= _('Similar requests')%></h2> <% for result in @xapian_similar.results %> <%= render :partial => 'request/request_listing_short_via_event', :locals => { :event => result[:model], :info_request => result[:model].info_request } %> @@ -56,8 +56,7 @@ <p><%= link_to _("More similar requests"), request_similar_url(@info_request) %></p> <% end %> <!-- Important terms: <%= @xapian_similar.important_terms.join(" ") %> --> - <% end %> - <% end %> + <% end %> <p><%= link_to _('Event history details'), request_details_url(@info_request) %></p> diff --git a/app/views/request/hidden.rhtml b/app/views/request/hidden.rhtml index 2d038a663..41b2ff7e4 100644 --- a/app/views/request/hidden.rhtml +++ b/app/views/request/hidden.rhtml @@ -12,7 +12,7 @@ various reasons why we might have done this, sorry we can\'t be more specific he </p> <% if @info_request.prominence == 'requester_only' %> <p> - <%= raw(_('If you are the requester, then you may <a href="%s">sign in</a> to view the request.') % [signin_url(:r => request.request_uri)]) %> + <%= raw(_('If you are the requester, then you may <a href="%s">sign in</a> to view the request.') % [signin_url(:r => request.fullpath)]) %> </p> <% end %> diff --git a/app/views/request/list.rhtml b/app/views/request/list.rhtml index 7cbd982f1..062b77c3e 100644 --- a/app/views/request/list.rhtml +++ b/app/views/request/list.rhtml @@ -14,22 +14,21 @@ <div style="clear:both"></div> <div class="results_section"> - <% view_cache :ttl => 5.minutes.to_i, :tag => [@cache_tag] do %> - <% if @list_results.empty? %> - <p> <%= _('No requests of this sort yet.')%></p> - <% else %> - <h2 class="foi_results"><%= _('{{count}} FOI requests found', :count => @matches_estimated) %></h2> - <div class="results_block"> - <% for result in @list_results%> - <% if result.class.to_s == 'InfoRequestEvent' %> - <%= render :partial => 'request/request_listing_via_event', :locals => { :event => result, :info_request => result.info_request } %> - <% else %> - <p><strong><%= _('Unexpected search result type') %> <%=result.class.to_s%></strong></p> - <% end %> - <% end %> - </div> - <% end %> + <% # TODO: Cache for 5 minutes %> + <% if @list_results.empty? %> + <p> <%= _('No requests of this sort yet.')%></p> + <% else %> + <h2 class="foi_results"><%= _('{{count}} FOI requests found', :count => @matches_estimated) %></h2> + <div class="results_block"> + <% for result in @list_results%> + <% if result.class.to_s == 'InfoRequestEvent' %> + <%= render :partial => 'request/request_listing_via_event', :locals => { :event => result, :info_request => result.info_request } %> + <% else %> + <p><strong><%= _('Unexpected search result type') %> <%=result.class.to_s%></strong></p> + <% end %> + <% end %> + </div> + <% end %> - <%= will_paginate WillPaginate::Collection.new(@page, @per_page, @show_no_more_than) %> - <% end %> + <%= will_paginate WillPaginate::Collection.new(@page, @per_page, @show_no_more_than) %> </div> diff --git a/app/views/request/new_please_describe.rhtml b/app/views/request/new_please_describe.rhtml index ff27405b8..6a193e70d 100644 --- a/app/views/request/new_please_describe.rhtml +++ b/app/views/request/new_please_describe.rhtml @@ -13,7 +13,7 @@ if they are successful yet or not.') %> </ul> <p> - <%= raw(_('When you\'re done, <strong>come back here</strong>, <a href="%s">reload this page</a> and file your new request.') % [request.request_uri]) %> + <%= raw(_('When you\'re done, <strong>come back here</strong>, <a href="%s">reload this page</a> and file your new request.') % [request.fullpath]) %> </p> <p> diff --git a/app/views/request/show.rhtml b/app/views/request/show.rhtml index 0cae3a9aa..fa75a6529 100644 --- a/app/views/request/show.rhtml +++ b/app/views/request/show.rhtml @@ -106,7 +106,7 @@ <%= _('The request is <strong>waiting for clarification</strong>.') %> <% if !@info_request.is_external? %> <%= _('If you are {{user_link}}, please',:user_link=>user_link_for_request(@info_request)) %> - <%= link_to _("sign in"), signin_url(:r => request.request_uri) %> <%= _('to send a follow up message.') %> + <%= link_to _("sign in"), signin_url(:r => request.fullpath) %> <%= _('to send a follow up message.') %> <% end %> <% end %> <% elsif @status == 'gone_postal' %> diff --git a/app/views/track/_tracking_links.rhtml b/app/views/track/_tracking_links.rhtml index 06e87ac74..ee18ec475 100644 --- a/app/views/track/_tracking_links.rhtml +++ b/app/views/track/_tracking_links.rhtml @@ -9,7 +9,7 @@ <% elsif existing_track %> <p><%= track_thing.params[:verb_on_page_already] %></p> <div class="feed_link feed_link_<%=location%>"> - <%= link_to _("Unsubscribe"), {:controller => 'track', :action => 'update', :track_id => existing_track.id, :track_medium => "delete", :r => request.request_uri}, :class => "link_button_green" %> + <%= link_to _("Unsubscribe"), {:controller => 'track', :action => 'update', :track_id => existing_track.id, :track_medium => "delete", :r => request.fullpath}, :class => "link_button_green" %> </div> <% elsif track_thing %> <div class="feed_link feed_link_<%=location%>"> diff --git a/app/views/user/show.rhtml b/app/views/user/show.rhtml index 31ea2a70b..e8e59b541 100644 --- a/app/views/user/show.rhtml +++ b/app/views/user/show.rhtml @@ -97,7 +97,7 @@ <% if not @is_you %> <p id="user_not_logged_in"> - <%= raw(_('<a href="%s">Sign in</a> to change password, subscriptions and more ({{user_name}} only)',:user_name=>h(@display_user.name)) % [signin_url(:r => request.request_uri)]) %> + <%= raw(_('<a href="%s">Sign in</a> to change password, subscriptions and more ({{user_name}} only)',:user_name=>h(@display_user.name)) % [signin_url(:r => request.fullpath)]) %> </p> <% end %> </div> @@ -186,7 +186,7 @@ <%=TrackThing.track_type_description(@track_things[0].track_type)%> <%= hidden_field_tag 'track_type', @track_things[0].track_type %> <%= hidden_field_tag 'user', @display_user.id %> - <%= hidden_field_tag 'r', request.request_uri %> + <%= hidden_field_tag 'r', request.fullpath %> <% if @track_things.size > 1 %> <%= submit_tag _('unsubscribe all') %> <% end %> @@ -200,7 +200,7 @@ <%=TrackThing.track_type_description(track_type)%> <%= hidden_field_tag 'track_type', track_type %> <%= hidden_field_tag 'user', @display_user.id %> - <%= hidden_field_tag 'r', request.request_uri %> + <%= hidden_field_tag 'r', request.fullpath %> <% if track_things.size > 1 %> <%= submit_tag _('unsubscribe all')%> <% end %> @@ -215,7 +215,7 @@ <div> <%= track_thing.params[:list_description] %> <%= hidden_field_tag 'track_medium', "delete", { :id => 'track_medium_' + track_thing.id.to_s } %> - <%= hidden_field_tag 'r', request.request_uri, { :id => 'r_' + track_thing.id.to_s } %> + <%= hidden_field_tag 'r', request.fullpath, { :id => 'r_' + track_thing.id.to_s } %> <%= submit_tag _('unsubscribe') %> </div> <% end %> |