diff options
Diffstat (limited to 'app/models')
-rw-r--r-- | app/models/about_me_validator.rb | 8 | ||||
-rw-r--r-- | app/models/change_email_validator.rb | 5 | ||||
-rw-r--r-- | app/models/comment.rb | 41 | ||||
-rw-r--r-- | app/models/contact_mailer.rb | 2 | ||||
-rw-r--r-- | app/models/foi_attachment.rb | 2 | ||||
-rw-r--r-- | app/models/incoming_message.rb | 5 | ||||
-rw-r--r-- | app/models/info_request.rb | 4 | ||||
-rw-r--r-- | app/models/outgoing_message.rb | 56 | ||||
-rw-r--r-- | app/models/profile_photo.rb | 6 | ||||
-rw-r--r-- | app/models/public_body.rb | 23 | ||||
-rw-r--r-- | app/models/track_thing.rb | 13 | ||||
-rw-r--r-- | app/models/user.rb | 23 |
12 files changed, 106 insertions, 82 deletions
diff --git a/app/models/about_me_validator.rb b/app/models/about_me_validator.rb index 5e04c2761..8ee505ac8 100644 --- a/app/models/about_me_validator.rb +++ b/app/models/about_me_validator.rb @@ -17,10 +17,14 @@ class AboutMeValidator < ActiveRecord::BaseWithoutTable column :about_me, :text, "I...", false - def validate + # TODO: Switch to built in validations + validate :length_of_about_me + + private + + def length_of_about_me if !self.about_me.blank? && self.about_me.size > 500 errors.add(:about_me, _("Please keep it shorter than 500 characters")) end end - end diff --git a/app/models/change_email_validator.rb b/app/models/change_email_validator.rb index 9ef25217d..2ddebb177 100644 --- a/app/models/change_email_validator.rb +++ b/app/models/change_email_validator.rb @@ -28,12 +28,15 @@ class ChangeEmailValidator < ActiveRecord::BaseWithoutTable validates_presence_of :old_email, :message => N_("Please enter your old email address") validates_presence_of :new_email, :message => N_("Please enter your new email address") validates_presence_of :password, :message => N_("Please enter your password"), :unless => :changing_email + validate :password_and_format_of_email def changing_email() self.user_circumstance == 'change_email' end - def validate + private + + def password_and_format_of_email if !self.old_email.blank? && !MySociety::Validate.is_valid_email(self.old_email) errors.add(:old_email, _("Old email doesn't look like a valid address")) end diff --git a/app/models/comment.rb b/app/models/comment.rb index 5507910e2..70f3ba00d 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -24,13 +24,13 @@ class Comment < ActiveRecord::Base strip_attributes! belongs_to :user - #validates_presence_of :user # breaks during construction of new ones :( - - validates_inclusion_of :comment_type, :in => [ 'request' ] belongs_to :info_request - has_many :info_request_events # in practice only ever has one + #validates_presence_of :user # breaks during construction of new ones :( + validates_inclusion_of :comment_type, :in => [ 'request' ] + validate :body_of_comment + def body ret = read_attribute(:body) if ret.nil? @@ -40,6 +40,7 @@ class Comment < ActiveRecord::Base ret = ret.gsub(/(?:\n\s*){2,}/, "\n\n") # remove excess linebreaks that unnecessarily space it out ret end + def raw_body read_attribute(:body) end @@ -52,23 +53,13 @@ class Comment < ActiveRecord::Base end end - # Check have edited comment - def validate - if self.body.empty? || self.body =~ /^\s+$/ - errors.add(:body, _("Please enter your annotation")) - end - if !MySociety::Validate.uses_mixed_capitals(self.body) - errors.add(:body, _('Please write your annotation using a mixture of capital and lower case letters. This makes it easier for others to read.')) - end - end - # Return body for display as HTML def get_body_for_html_display text = self.body.strip 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. @@ -82,9 +73,21 @@ class Comment < ActiveRecord::Base return Comment.find(:first, :conditions => [ "info_request_id = ? and body = ?", info_request_id, body ]) end end - def for_admin_column - self.class.content_columns.each do |column| - yield(column.human_name, self.send(column.name), column.type.to_s, column.name) + + def for_admin_column + self.class.content_columns.each do |column| + yield(column.human_name, self.send(column.name), column.type.to_s, column.name) + end + end + + private + + def body_of_comment + if self.body.empty? || self.body =~ /^\s+$/ + errors.add(:body, _("Please enter your annotation")) + end + if !MySociety::Validate.uses_mixed_capitals(self.body) + errors.add(:body, _('Please write your annotation using a mixture of capital and lower case letters. This makes it easier for others to read.')) + end 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/foi_attachment.rb b/app/models/foi_attachment.rb index 2f8a9ab04..723bc4abb 100644 --- a/app/models/foi_attachment.rb +++ b/app/models/foi_attachment.rb @@ -219,7 +219,7 @@ class FoiAttachment < ActiveRecord::Base def ensure_filename! - if self.filename.nil? + if self.filename.blank? calc_ext = AlaveteliFileTypes.mimetype_to_extension(self.content_type) if !calc_ext calc_ext = "bin" diff --git a/app/models/incoming_message.rb b/app/models/incoming_message.rb index a02d2456a..5c845ead3 100644 --- a/app/models/incoming_message.rb +++ b/app/models/incoming_message.rb @@ -284,6 +284,7 @@ class IncomingMessage < ActiveRecord::Base # Lotus notes quoting yeuch! def remove_lotus_quoting(text, replacement = "FOLDED_QUOTED_SECTION") text = text.dup + return text if self.info_request.user_name.nil? name = Regexp.escape(self.info_request.user_name) # To end of message sections @@ -534,7 +535,7 @@ class IncomingMessage < ActiveRecord::Base text = Iconv.conv('utf-8//IGNORE', source_charset, text) + _("\n\n[ {{site_name}} note: The above text was badly encoded, and has had strange characters removed. ]", :site_name => Configuration::site_name) - rescue Iconv::InvalidEncoding, Iconv::IllegalSequence + rescue Iconv::InvalidEncoding, Iconv::IllegalSequence, Iconv::InvalidCharacter if source_charset != "utf-8" source_charset = "utf-8" retry @@ -694,7 +695,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..cee9eb959 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -284,9 +284,9 @@ public # into some sort of separate jurisdiction dependent file if self.public_body.url_name == 'general_register_office' # without GQ in the subject, you just get an auto response - _('{{law_used_full}} request GQ - {{title}}',:law_used_full=>self.law_used_full,:title=>self.title) + _('{{law_used_full}} request GQ - {{title}}',:law_used_full=>self.law_used_full,:title=>self.title.html_safe) else - _('{{law_used_full}} request - {{title}}',:law_used_full=>self.law_used_full,:title=>self.title) + _('{{law_used_full}} request - {{title}}',:law_used_full=>self.law_used_full,:title=>self.title.html_safe) end end def email_subject_followup(incoming_message = nil) diff --git a/app/models/outgoing_message.rb b/app/models/outgoing_message.rb index 2e98e1021..c75894e6a 100644 --- a/app/models/outgoing_message.rb +++ b/app/models/outgoing_message.rb @@ -30,6 +30,7 @@ class OutgoingMessage < ActiveRecord::Base validates_inclusion_of :status, :in => ['ready', 'sent', 'failed'] validates_inclusion_of :message_type, :in => ['initial_request', 'followup' ] #, 'complaint'] + validate :format_of_body belongs_to :incoming_message_followup, :foreign_key => 'incoming_message_followup_id', :class_name => 'IncomingMessage' @@ -136,32 +137,6 @@ class OutgoingMessage < ActiveRecord::Base 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}/ - if self.message_type == 'followup' - if self.what_doing == 'internal_review' - errors.add(:body, _("Please give details explaining why you want a review")) - else - errors.add(:body, _("Please enter your follow up message")) - end - elsif - errors.add(:body, _("Please enter your letter requesting information")) - else - raise "Message id #{self.id} has type '#{self.message_type}' which validate can't handle" - end - end - if self.body =~ /#{get_signoff}\s*\Z/m - errors.add(:body, _("Please sign at the bottom with your name, or alter the \"%{signoff}\" signature" % { :signoff => get_signoff })) - end - if !MySociety::Validate.uses_mixed_capitals(self.body) - errors.add(:body, _('Please write your message using a mixture of capital and lower case letters. This makes it easier for others to read.')) - end - if self.what_doing.nil? || !['new_information', 'internal_review', 'normal_sort'].include?(self.what_doing) - errors.add(:what_doing_dummy, _('Please choose what sort of reply you are making.')) - end - end - # Deliver outgoing message # Note: You can test this from script/console with, say: # InfoRequest.find(1).outgoing_messages[0].send_message @@ -252,7 +227,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 +250,33 @@ class OutgoingMessage < ActiveRecord::Base yield(column.human_name, self.send(column.name), column.type.to_s, column.name) end end + + private + + def format_of_body + if self.body.empty? || self.body =~ /\A#{get_salutation}\s+#{get_signoff}/ || self.body =~ /#{get_internal_review_insert_here_note}/ + if self.message_type == 'followup' + if self.what_doing == 'internal_review' + errors.add(:body, _("Please give details explaining why you want a review")) + else + errors.add(:body, _("Please enter your follow up message")) + end + elsif + errors.add(:body, _("Please enter your letter requesting information")) + else + raise "Message id #{self.id} has type '#{self.message_type}' which validate can't handle" + end + end + if self.body =~ /#{get_signoff}\s*\Z/m + errors.add(:body, _("Please sign at the bottom with your name, or alter the \"%{signoff}\" signature" % { :signoff => get_signoff })) + end + if !MySociety::Validate.uses_mixed_capitals(self.body) + errors.add(:body, _('Please write your message using a mixture of capital and lower case letters. This makes it easier for others to read.')) + end + if self.what_doing.nil? || !['new_information', 'internal_review', 'normal_sort'].include?(self.what_doing) + errors.add(:what_doing_dummy, _('Please choose what sort of reply you are making.')) + end + end end diff --git a/app/models/profile_photo.rb b/app/models/profile_photo.rb index 6e605651d..73d7ca12b 100644 --- a/app/models/profile_photo.rb +++ b/app/models/profile_photo.rb @@ -23,6 +23,8 @@ 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 @@ -81,7 +83,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 diff --git a/app/models/public_body.rb b/app/models/public_body.rb index 57fe27767..168b9f4c7 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -35,6 +35,8 @@ class PublicBody < ActiveRecord::Base validates_uniqueness_of :short_name, :message => N_("Short name is already taken"), :if => Proc.new { |pb| pb.short_name != "" } validates_uniqueness_of :name, :message => N_("Name is already taken") + validate :request_email_if_requestable + has_many :info_requests, :order => 'created_at desc' has_many :track_things, :order => 'created_at desc' has_many :censor_rules, :order => 'created_at desc' @@ -135,15 +137,6 @@ class PublicBody < ActiveRecord::Base self.first_letter = self.name.scan(/./mu)[0].upcase end - def validate - # Request_email can be blank, meaning we don't have details - if self.is_requestable? - unless MySociety::Validate.is_valid_email(self.request_email) - errors.add(:request_email, "Request email doesn't look like a valid email address") - end - end - end - # If tagged "not_apply", then FOI/EIR no longer applies to authority at all def not_apply? return self.has_tag?('not_apply') @@ -301,7 +294,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 @@ -520,6 +513,8 @@ class PublicBody < ActiveRecord::Base 'Version', ] public_bodies.each do |public_body| + # Skip bodies we use only for site admin + next if public_body.has_tag?('site_administration') csv << [ public_body.name, public_body.short_name, @@ -640,4 +635,12 @@ class PublicBody < ActiveRecord::Base end end + def request_email_if_requestable + # Request_email can be blank, meaning we don't have details + if self.is_requestable? + unless MySociety::Validate.is_valid_email(self.request_email) + errors.add(:request_email, "Request email doesn't look like a valid email address") + end + end + end end diff --git a/app/models/track_thing.rb b/app/models/track_thing.rb index 2a61eb858..dfe92b7fe 100644 --- a/app/models/track_thing.rb +++ b/app/models/track_thing.rb @@ -199,11 +199,12 @@ class TrackThing < ActiveRecord::Base if self.track_type == 'request_updates' @params = { # Website - :list_description => _("'{{link_to_request}}', a request", :link_to_request => "<a href=\"/request/" + CGI.escapeHTML(self.info_request.url_title) + "\">" + CGI.escapeHTML(self.info_request.title) + "</a>"), # XXX yeuch, sometimes I just want to call view helpers from the model, sorry! can't work out how + :list_description => _("'{{link_to_request}}', a request", + :link_to_request => ("<a href=\"/request/" + CGI.escapeHTML(self.info_request.url_title) + "\">" + CGI.escapeHTML(self.info_request.title) + "</a>").html_safe), # XXX yeuch, sometimes I just want to call view helpers from the model, sorry! can't work out how :verb_on_page => _("Follow this request"), :verb_on_page_already => _("You are already following this request"), # Email - :title_in_email => _("New updates for the request '{{request_title}}'", :request_title => self.info_request.title), + :title_in_email => _("New updates for the request '{{request_title}}'", :request_title => self.info_request.title.html_safe), :title_in_rss => _("New updates for the request '{{request_title}}'", :request_title => self.info_request.title), # Authentication :web => _("To follow the request '{{request_title}}'", :request_title => CGI.escapeHTML(self.info_request.title)), @@ -250,7 +251,7 @@ class TrackThing < ActiveRecord::Base elsif self.track_type == 'public_body_updates' @params = { # Website - :list_description => _("'{{link_to_authority}}', a public authority", :link_to_authority => "<a href=\"/body/" + CGI.escapeHTML(self.public_body.url_name) + "\">" + CGI.escapeHTML(self.public_body.name) + "</a>"), # XXX yeuch, sometimes I just want to call view helpers from the model, sorry! can't work out how + :list_description => _("'{{link_to_authority}}', a public authority", :link_to_authority => ("<a href=\"/body/" + CGI.escapeHTML(self.public_body.url_name) + "\">" + CGI.escapeHTML(self.public_body.name) + "</a>").html_safe), # XXX yeuch, sometimes I just want to call view helpers from the model, sorry! can't work out how :verb_on_page => _("Follow requests to {{public_body_name}}",:public_body_name=>CGI.escapeHTML(self.public_body.name)), :verb_on_page_already => _("You are already following requests to {{public_body_name}}", :public_body_name=>CGI.escapeHTML(self.public_body.name)), # Email @@ -266,11 +267,11 @@ class TrackThing < ActiveRecord::Base elsif self.track_type == 'user_updates' @params = { # Website - :list_description => _("'{{link_to_user}}', a person", :link_to_user => "<a href=\"/user/" + CGI.escapeHTML(self.tracked_user.url_name) + "\">" + CGI.escapeHTML(self.tracked_user.name) + "</a>"), # XXX yeuch, sometimes I just want to call view helpers from the model, sorry! can't work out how + :list_description => _("'{{link_to_user}}', a person", :link_to_user => ("<a href=\"/user/" + CGI.escapeHTML(self.tracked_user.url_name) + "\">" + CGI.escapeHTML(self.tracked_user.name) + "</a>").html_safe), # XXX yeuch, sometimes I just want to call view helpers from the model, sorry! can't work out how :verb_on_page => _("Follow this person"), :verb_on_page_already => _("You are already following this person"), # Email - :title_in_email => _("FOI requests by '{{user_name}}'", :user_name=>self.tracked_user.name), + :title_in_email => _("FOI requests by '{{user_name}}'", :user_name=>self.tracked_user.name.html_safe), :title_in_rss => _("FOI requests by '{{user_name}}'", :user_name=>self.tracked_user.name), # Authentication :web => _("To follow requests by '{{user_name}}'", :user_name=>CGI.escapeHTML(self.tracked_user.name)), @@ -282,7 +283,7 @@ class TrackThing < ActiveRecord::Base elsif self.track_type == 'search_query' @params = { # Website - :list_description => "<a href=\"/search/" + CGI.escapeHTML(self.track_query) + "/newest/advanced\">" + self.track_query_description + "</a>", # XXX yeuch, sometimes I just want to call view helpers from the model, sorry! can't work out how + :list_description => ("<a href=\"/search/" + CGI.escapeHTML(self.track_query) + "/newest/advanced\">" + CGI.escapeHTML(self.track_query_description) + "</a>").html_safe, # XXX yeuch, sometimes I just want to call view helpers from the model, sorry! can't work out how :verb_on_page => _("Follow things matching this search"), :verb_on_page_already => _("You are already following things matching this search"), # Email diff --git a/app/models/user.rb b/app/models/user.rb index 6e1e21481..e6c666e47 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -50,6 +50,8 @@ 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 @@ -108,15 +110,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) @@ -361,12 +354,13 @@ class User < ActiveRecord::Base end # Return about me text for display as HTML + # TODO: Move this to a view helper def get_about_me_for_html_display text = self.about_me.strip text = CGI.escapeHTML(text) text = MySociety::Format.make_clickable(text, :contract => 1) text = text.gsub(/\n/, '<br>') - return text + return text.html_safe end def json_for_api @@ -413,6 +407,15 @@ class User < ActiveRecord::Base self.salt = self.object_id.to_s + rand.to_s 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? |