From 7106744372abce3c00dd326d4bc5d5ce5cac9d78 Mon Sep 17 00:00:00 2001 From: Matthew Landauer Date: Wed, 16 Jan 2013 19:07:54 +1100 Subject: Set locale in controller test by passing parameter in get Also using I18n.locale to pass the current locale around. --- app/models/public_body.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/models') diff --git a/app/models/public_body.rb b/app/models/public_body.rb index 168b9f4c7..7ba36c725 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -106,7 +106,7 @@ 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 + locale = I18n.locale PublicBody.with_locale(locale) do found = PublicBody.find(:all, :conditions => ["public_body_translations.url_name=?", name], -- cgit v1.2.3 From 4dec7290c10537ebcccba491ba5dddcefd81e43a Mon Sep 17 00:00:00 2001 From: Matthew Landauer Date: Wed, 16 Jan 2013 19:16:02 +1100 Subject: Small refactoring in PublicBody.self.find_by_url_name_with_historic just use the current locale --- app/models/public_body.rb | 41 +++++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 22 deletions(-) (limited to 'app/models') diff --git a/app/models/public_body.rb b/app/models/public_body.rb index 7ba36c725..6fe5cc3e5 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -106,28 +106,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 = 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 -- cgit v1.2.3 From 5d22285eef67627c58320f031d03d2bffd0ba55e Mon Sep 17 00:00:00 2001 From: Matthew Landauer Date: Thu, 17 Jan 2013 05:26:54 +1100 Subject: Set locale with I18n rather than through globalize Conflicts: app/controllers/general_controller.rb --- app/models/public_body.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'app/models') diff --git a/app/models/public_body.rb b/app/models/public_body.rb index 6fe5cc3e5..199440a28 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -326,7 +326,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( @@ -364,7 +364,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 @@ -407,7 +407,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}" @@ -442,7 +442,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}" -- cgit v1.2.3 From f6f1afd6d4d7908fd56173637fd7afbd76a8aba4 Mon Sep 17 00:00:00 2001 From: Matthew Landauer Date: Thu, 17 Jan 2013 05:49:55 +1100 Subject: Refactor using I18.with_locale --- app/models/track_mailer.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'app/models') diff --git a/app/models/track_mailer.rb b/app/models/track_mailer.rb index 51440e4d7..7262c82f3 100644 --- a/app/models/track_mailer.rb +++ b/app/models/track_mailer.rb @@ -91,10 +91,9 @@ class TrackMailer < ApplicationMailer if email_about_things.size > 0 # Send the email - previous_locale = I18n.locale - I18n.locale = user.get_locale - TrackMailer.deliver_event_digest(user, email_about_things) - I18n.locale = previous_locale + I18n.with_locale(user.get_locale) do + TrackMailer.deliver_event_digest(user, email_about_things) + end end # Record that we've now sent those alerts to that user -- cgit v1.2.3 From d7fe6deb9d573889274797039c8472bb40d08669 Mon Sep 17 00:00:00 2001 From: Matthew Landauer Date: Thu, 17 Jan 2013 05:50:32 +1100 Subject: Small refactor --- app/models/user.rb | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'app/models') diff --git a/app/models/user.rb b/app/models/user.rb index e6c666e47..612ac7fa2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -98,12 +98,7 @@ class User < ActiveRecord::Base end def get_locale - if !self.locale.nil? - locale = self.locale - else - locale = I18n.locale - end - return locale.to_s + (self.locale || I18n.locale).to_s end def visible_comments -- cgit v1.2.3 From f0b73deefec594c9cd35985f19b66cc3bb836788 Mon Sep 17 00:00:00 2001 From: Matthew Landauer Date: Thu, 17 Jan 2013 05:51:09 +1100 Subject: As much as possible use I18n.locale for getting the locale --- app/models/public_body.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'app/models') diff --git a/app/models/public_body.rb b/app/models/public_body.rb index 199440a28..b4d36fe91 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -240,13 +240,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 -- cgit v1.2.3 From 20605630197686023576768f7abbc211a3044202 Mon Sep 17 00:00:00 2001 From: Matthew Landauer Date: Fri, 18 Jan 2013 15:02:33 +1100 Subject: In translation strings replace %{} with {{}} formatting --- app/models/profile_photo.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/models') diff --git a/app/models/profile_photo.rb b/app/models/profile_photo.rb index 6e605651d..bcb0390b5 100644 --- a/app/models/profile_photo.rb +++ b/app/models/profile_photo.rb @@ -97,7 +97,7 @@ class ProfilePhoto < ActiveRecord::Base end if !self.draft && (self.image.columns != WIDTH || self.image.rows != HEIGHT) - errors.add(:data, N_("Failed to convert image to the correct size: at %{cols}x%{rows}, need %{width}x%{height}" % { :cols => self.image.columns, :rows => self.image.rows, :width => WIDTH, :height => HEIGHT })) + errors.add(:data, _("Failed to convert image to the correct size: at {{cols}}x{{rows}}, need {{width}}x{{height}}", :cols => self.image.columns, :rows => self.image.rows, :width => WIDTH, :height => HEIGHT)) end if self.draft && self.user_id -- cgit v1.2.3 From 77ecc648e9d68927c2ddc8c5a30906d5946db5fc Mon Sep 17 00:00:00 2001 From: Matthew Landauer Date: Mon, 21 Jan 2013 14:00:20 +1100 Subject: In translation strings replace %{} with {{}} formatting --- app/models/outgoing_message.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/models') diff --git a/app/models/outgoing_message.rb b/app/models/outgoing_message.rb index 441813e5f..3cb0ebf49 100644 --- a/app/models/outgoing_message.rb +++ b/app/models/outgoing_message.rb @@ -152,7 +152,7 @@ class OutgoingMessage < ActiveRecord::Base 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 })) + 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.')) -- cgit v1.2.3 From 768ea68463b5fc063f1516e12537195dc2a975cd Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Tue, 16 Apr 2013 14:16:01 +0100 Subject: Update outgoing message signoff translation --- app/models/outgoing_message.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/models') diff --git a/app/models/outgoing_message.rb b/app/models/outgoing_message.rb index c75894e6a..248125808 100644 --- a/app/models/outgoing_message.rb +++ b/app/models/outgoing_message.rb @@ -268,7 +268,7 @@ class OutgoingMessage < ActiveRecord::Base 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 })) + 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.')) -- cgit v1.2.3 From 0287ca92fb6bd39bade5bfb44f52ac6e006139d0 Mon Sep 17 00:00:00 2001 From: Mark Longair Date: Fri, 12 Apr 2013 18:48:52 +0100 Subject: Fix the deadlock on dealing with incoming email Fixes #336 There was an occasional deadlock when two emails for the same request came in near-simultaneously; two processes would be started via script/mailin, each to deal with one email which are both updating the same InfoRequest. The error would look like: 2013-04-07 09:19:03 BST [13398]: [2-1] DETAIL: Process 13398 waits for ShareLock on transaction 36193647; blocked by process 13397. Process 13397 waits for ExclusiveLock on tuple (390,35) of relation 32918788 of database 32918687; blocked by process 13398. Process 13398: UPDATE "info_requests" SET "updated_at" = '2013-04-07 08:19:02.139515', "awaiting_description" = 't' WHERE "id" = 156200 Process 13397: UPDATE "info_requests" SET "updated_at" = '2013-04-07 08:19:02.143624', "awaiting_description" = 't' WHERE "id" = 156200 This arose from the following section of code: ActiveRecord::Base.transaction do raw_email = RawEmail.new incoming_message.raw_email = raw_email incoming_message.info_request = self incoming_message.save! raw_email.data = raw_email_data raw_email.save! self.awaiting_description = true params = { :incoming_message_id => incoming_message.id } if !rejected_reason.empty? params[:rejected_reason] = rejected_reason.to_str end self.log_event("response", params) self.save! end Matthew Somerville explained what was happening here in the issue report; to repeat his explanation from the bug report, both processes enter the transaction block and acquire a ShareLock on self with: incoming_message.info_request = self incoming_message.save! However, in order to update the self.awaiting_description field of the InfoRequest, with: self.awaiting_description = true [...] self.save! ... the ShareLock needs to be upgraded to an ExclusiveLock, but both will wait until the other's ShareLock is released, which would only happen at the end of the transaction. We can avoid this deadlock by using SELECT ... FOR UPDATE for the row in info_requests. In Rails 3.2.0 there is ActiveRecord support for this (via with_lock and lock! on a model instance) but so as not to require upgrading rails, I'm just using raw SQL. --- app/models/info_request.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'app/models') diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 237364f56..eaed25a61 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -474,6 +474,17 @@ public incoming_message = IncomingMessage.new ActiveRecord::Base.transaction do + + # To avoid a deadlock when simultaneously dealing with two + # incoming emails that refer to the same InfoRequest, we + # lock the row for update. In Rails 3.2.0 and later this + # can be done with info_request.with_lock or + # info_request.lock!, but upgrading to that version of + # Rails creates many other problems at the moment. In the + # interim, just use raw SQL to do the SELECT ... FOR UPDATE + raw_sql = "SELECT * FROM info_requests WHERE id = #{self.id} LIMIT 1 FOR UPDATE" + ActiveRecord::Base.connection.execute(raw_sql) + raw_email = RawEmail.new incoming_message.raw_email = raw_email incoming_message.info_request = self -- cgit v1.2.3 From 0010d44962185ed42fd616d19f864cb20a6630f5 Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Tue, 23 Apr 2013 13:43:20 +0100 Subject: Restore call to N_ --- app/models/profile_photo.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/models') diff --git a/app/models/profile_photo.rb b/app/models/profile_photo.rb index 47ce221b2..f6aec6338 100644 --- a/app/models/profile_photo.rb +++ b/app/models/profile_photo.rb @@ -101,7 +101,7 @@ class ProfilePhoto < ActiveRecord::Base end if !self.draft && (self.image.columns != WIDTH || self.image.rows != HEIGHT) - errors.add(:data, _("Failed to convert image to the correct size: at {{cols}}x{{rows}}, need {{width}}x{{height}}", :cols => self.image.columns, :rows => self.image.rows, :width => WIDTH, :height => HEIGHT)) + errors.add(:data, N_("Failed to convert image to the correct size: at {{cols}}x{{rows}}, need {{width}}x{{height}}", :cols => self.image.columns, :rows => self.image.rows, :width => WIDTH, :height => HEIGHT)) end if self.draft && self.user_id -- cgit v1.2.3