diff options
Diffstat (limited to 'app/models')
| -rw-r--r-- | app/models/application_mailer.rb | 9 | ||||
| -rw-r--r-- | app/models/censor_rule.rb | 10 | ||||
| -rw-r--r-- | app/models/comment.rb | 2 | ||||
| -rw-r--r-- | app/models/incoming_message.rb | 4 | ||||
| -rw-r--r-- | app/models/info_request.rb | 56 | ||||
| -rw-r--r-- | app/models/outgoing_message.rb | 19 | ||||
| -rw-r--r-- | app/models/post_redirect.rb | 26 | ||||
| -rw-r--r-- | app/models/profile_photo.rb | 43 | ||||
| -rw-r--r-- | app/models/public_body.rb | 53 | ||||
| -rw-r--r-- | app/models/user.rb | 49 |
10 files changed, 147 insertions, 124 deletions
diff --git a/app/models/application_mailer.rb b/app/models/application_mailer.rb index cdb279c3c..859e8a0e2 100644 --- a/app/models/application_mailer.rb +++ b/app/models/application_mailer.rb @@ -67,7 +67,8 @@ class ApplicationMailer < ActionMailer::Base return nil end - if ActionMailer::VERSION::MAJOR == 2 + # 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! # modified to allow templates to be selected correctly for multipart @@ -142,9 +143,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/comment.rb b/app/models/comment.rb index 5507910e2..bcd1efca8 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -68,7 +68,7 @@ class Comment < ActiveRecord::Base text = CGI.escapeHTML(text) text = MySociety::Format.make_clickable(text, :contract => 1) text = text.gsub(/\n/, '<br>') - return text + return text.html_safe end # When posting a new comment, use this to check user hasn't double submitted. diff --git a/app/models/incoming_message.rb b/app/models/incoming_message.rb index a02d2456a..ee8b92f11 100644 --- a/app/models/incoming_message.rb +++ b/app/models/incoming_message.rb @@ -631,7 +631,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 @@ -694,7 +694,7 @@ class IncomingMessage < ActiveRecord::Base text = text.gsub(/\n/, '<br>') text = text.gsub(/(?:<br>\s*){2,}/, '<br><br>') # remove excess linebreaks that unnecessarily space it out - return text + return text.html_safe end diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 3355b9443..08b331c26 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 2e98e1021..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}/ @@ -252,7 +247,7 @@ class OutgoingMessage < ActiveRecord::Base text = MySociety::Format.make_clickable(text, :contract => 1) text.gsub!(/\[(email address|mobile number)\]/, '[<a href="/help/officers#mobiles">\1</a>]') text = text.gsub(/\n/, '<br>') - return text + return text.html_safe end def fully_destroy @@ -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 57fe27767..b48c57228 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}" } @@ -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 @@ -301,7 +298,7 @@ class PublicBody < ActiveRecord::Base ret = ret + " and " end ret = ret + types[-1] - return ret + return ret.html_safe else return _("A public authority") 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/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? |