diff options
25 files changed, 142 insertions, 60 deletions
@@ -49,7 +49,7 @@ gem 'globalize3', :git => 'git://github.com/henare/globalize3.git', :branch => ' gem 'locale' gem 'routing-filter' gem 'unicode' -gem 'unidecode' +gem 'unidecoder' group :test do gem 'fakeweb' diff --git a/Gemfile.lock b/Gemfile.lock index 4494c2342..9accf0283 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -136,7 +136,7 @@ GEM net-ssh (2.6.7) net-ssh-gateway (1.2.0) net-ssh (>= 2.6.5) - newrelic_rpm (3.6.2.96) + newrelic_rpm (3.6.8.164) nokogiri (1.5.9) paper_trail (2.7.2) activerecord (~> 3.0) @@ -236,7 +236,7 @@ GEM polyglot (>= 0.3.1) tzinfo (0.3.37) unicode (0.4.4) - unidecode (1.0.0) + unidecoder (1.1.2) vpim (0.695) webrat (0.7.3) nokogiri (>= 1.2.0) @@ -293,7 +293,7 @@ DEPENDENCIES statistics2 (~> 0.54) syslog_protocol unicode - unidecode + unidecoder vpim webrat will_paginate diff --git a/app/controllers/public_body_controller.rb b/app/controllers/public_body_controller.rb index 02f0ceb19..1f7032eed 100644 --- a/app/controllers/public_body_controller.rb +++ b/app/controllers/public_body_controller.rb @@ -113,8 +113,8 @@ class PublicBodyController < ApplicationController elsif @tag == 'other' category_list = PublicBodyCategories::get().tags().map{|c| "'"+c+"'"}.join(",") where_condition += base_tag_condition + " AND has_tag_string_tags.name in (#{category_list})) = 0" - elsif @tag.size == 1 - @tag.upcase! + elsif @tag.scan(/./mu).size == 1 + @tag = Unicode.upcase @tag # The first letter queries have to be done on # translations, so just indicate to add that later: first_letter = true diff --git a/app/models/info_request.rb b/app/models/info_request.rb index aaced91a2..eba620f53 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -271,15 +271,9 @@ public # Subject lines for emails about the request def email_subject_request - # XXX pull out this general_register_office specialisation - # 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.html_safe) - else - _('{{law_used_full}} request - {{title}}',:law_used_full=>self.law_used_full,:title=>self.title.html_safe) - end + _('{{law_used_full}} request - {{title}}',:law_used_full=>self.law_used_full,:title=>self.title.html_safe) end + def email_subject_followup(incoming_message = nil) if incoming_message.nil? || !incoming_message.valid_to_reply_to? || !incoming_message.subject 'Re: ' + self.email_subject_request diff --git a/app/models/info_request_event.rb b/app/models/info_request_event.rb index 67cdda1b4..e268b28ca 100644 --- a/app/models/info_request_event.rb +++ b/app/models/info_request_event.rb @@ -339,6 +339,9 @@ class InfoRequestEvent < ActiveRecord::Base end raise _("unknown status ") + status end + # TRANSLATORS: "Follow up" in this context means a further + # message sent by the requester to the authority after + # the initial request return _("Follow up") end diff --git a/app/models/public_body.rb b/app/models/public_body.rb index 828e8c94a..485a794b0 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -71,7 +71,7 @@ class PublicBody < ActiveRecord::Base def PublicBody.set_first_letter(instance) unless instance.name.nil? or instance.name.empty? # we use a regex to ensure it works with utf-8/multi-byte - first_letter = instance.name.scan(/^./mu)[0].upcase + first_letter = Unicode.upcase instance.name.scan(/^./mu)[0] if first_letter != instance.first_letter instance.first_letter = first_letter end diff --git a/app/views/general/_advanced_search_tips.html.erb b/app/views/general/_advanced_search_tips.html.erb index 08ce04439..5e19c41e0 100644 --- a/app/views/general/_advanced_search_tips.html.erb +++ b/app/views/general/_advanced_search_tips.html.erb @@ -40,7 +40,10 @@ <p><%= _("All the options below can use <strong>variety</strong> or <strong>latest_variety</strong> before the colon. For example, <strong>variety:sent</strong> will match requests which have <em>ever</em> been sent; <strong>latest_variety:sent</strong> will match only requests that are <em>currently</em> marked as sent.") %></p> <table class="status_table"> <tr><td><strong><%=search_link('variety:sent')%></strong></td><td><%= _('Original request sent') %></td></tr> - <tr><td><strong><%=search_link('variety:followup_sent')%></strong></td><td><%= _('Follow up message sent by requester') %></td></tr> + <tr><td><strong><%=search_link('variety:followup_sent')%></strong></td><td><%= # TRANSLATORS: "Follow up message" in this context means a + # further message sent by the requester to the authority after + # the initial request + _('Follow up message sent by requester') %></td></tr> <tr><td><strong><%=search_link('variety:response')%></strong></td><td><%= _('Response from a public authority') %></td></tr> <tr><td><strong><%=search_link('variety:comment')%></strong></td><td><%= _('Annotation added to request') %></td></tr> <tr><td><strong><%=search_link('variety:authority')%></strong></td><td><%= _('A public authority') %></td></tr> diff --git a/app/views/request/_describe_state.html.erb b/app/views/request/_describe_state.html.erb index 05cce013e..7b6fa9683 100644 --- a/app/views/request/_describe_state.html.erb +++ b/app/views/request/_describe_state.html.erb @@ -80,7 +80,7 @@ <% if @update_status %> <div> <%= radio_button "incoming_message", "described_state", "requires_admin", :id => 'requires_admin' + id_suffix %> - <label for="error_message<%=id_suffix%>"> + <label for="requires_admin<%=id_suffix%>"> <%= _('This request <strong>requires administrator attention</strong>') %> </label> </div> diff --git a/app/views/request/_followup.html.erb b/app/views/request/_followup.html.erb index bb099ff15..2643b767f 100644 --- a/app/views/request/_followup.html.erb +++ b/app/views/request/_followup.html.erb @@ -45,7 +45,12 @@ </div> <% end %> <% if @info_request.allow_new_responses_from == 'nobody' %> - <p><%= _('Follow ups and new responses to this request have been stopped to prevent spam. Please <a href="{{url}}">contact us</a> if you are {{user_link}} and need to send a follow up.',:user_link=>user_link(@info_request.user), :url=>help_contact_path) %></p> + + <p><%= + # TRANSLATORS: "Follow ups" in this context means further + # messages sent by the requester to the authority after + # the initial request + _('Follow ups and new responses to this request have been stopped to prevent spam. Please <a href="{{url}}">contact us</a> if you are {{user_link}} and need to send a follow up.',:user_link=>user_link(@info_request.user), :url=>help_contact_path) %></p> <% else %> <% if @internal_review %> <p> diff --git a/app/views/request/_sidebar.html.erb b/app/views/request/_sidebar.html.erb index e08f43eaa..8d4a4a2d8 100644 --- a/app/views/request/_sidebar.html.erb +++ b/app/views/request/_sidebar.html.erb @@ -60,7 +60,6 @@ <% if @xapian_similar_more %> <p><%= link_to _("More similar requests"), similar_request_path(@info_request.url_title) %></p> <% end %> - <!-- Important terms: <%= @xapian_similar.important_terms.join(" ") %> --> <% end %> <p><%= link_to _('Event history details'), request_details_path(@info_request) %></p> diff --git a/app/views/request/similar.html.erb b/app/views/request/similar.html.erb index eb7ff636d..5bdefc494 100644 --- a/app/views/request/similar.html.erb +++ b/app/views/request/similar.html.erb @@ -12,8 +12,6 @@ <%- end %> </h1> -<!-- Important terms: <%= @xapian_object.important_terms.join(" ") %> --> - <% if @xapian_object.results.empty? %> <p><%= _('No similar requests found.')%></p> <% else %> diff --git a/commonlib b/commonlib -Subproject 9462a28fe12b25637d6e67d7140d444632e3ff7 +Subproject 77a6b09daa5da3808be4431799521f8bee5ab21 diff --git a/lib/tasks/stats.rake b/lib/tasks/stats.rake index 4eda27289..eb36204c6 100644 --- a/lib/tasks/stats.rake +++ b/lib/tasks/stats.rake @@ -94,7 +94,7 @@ namespace :stats do desc 'Update statistics in the public_bodies table' task :update_public_bodies_stats => :environment do verbose = ENV['VERBOSE'] == '1' - PublicBody.all.each do |public_body| + PublicBody.find_each(:batch_size => 10) do |public_body| puts "Counting overdue requests for #{public_body.name}" if verbose # Look for values of 'waiting_response_overdue' and @@ -102,7 +102,8 @@ namespace :stats do # described_state column, and instead need to be calculated: overdue_count = 0 very_overdue_count = 0 - InfoRequest.find_each(:conditions => {:public_body_id => public_body.id}) do |ir| + InfoRequest.find_each(:batch_size => 200, + :conditions => {:public_body_id => public_body.id}) do |ir| case ir.calculate_status when 'waiting_response_very_overdue' very_overdue_count += 1 diff --git a/spec/controllers/admin_incoming_message_controller_spec.rb b/spec/controllers/admin_incoming_message_controller_spec.rb index b969a8a3f..21c744e5b 100644 --- a/spec/controllers/admin_incoming_message_controller_spec.rb +++ b/spec/controllers/admin_incoming_message_controller_spec.rb @@ -50,6 +50,17 @@ describe AdminIncomingMessageController, "when administering incoming messages" :url_title => destination_info_request.url_title end + it 'should succeed, even if a duplicate xapian indexing job is created' do + + with_duplicate_xapian_job_creation do + current_info_request = info_requests(:fancy_dog_request) + destination_info_request = info_requests(:naughty_chicken_request) + incoming_message = incoming_messages(:useless_incoming_message) + post :redeliver, :redeliver_incoming_message_id => incoming_message.id, + :url_title => destination_info_request.url_title + end + + end end diff --git a/spec/controllers/admin_public_body_controller_spec.rb b/spec/controllers/admin_public_body_controller_spec.rb index 8a72db724..fe5087d7c 100644 --- a/spec/controllers/admin_public_body_controller_spec.rb +++ b/spec/controllers/admin_public_body_controller_spec.rb @@ -55,7 +55,8 @@ describe AdminPublicBodyController, "when administering public bodies" do end it "mass assigns tags" do - n = PublicBody.count + condition = "public_body_translations.locale = ?" + n = PublicBody.joins(:translations).where([condition, "en"]).count post :mass_tag_add, { :new_tag => "department", :table_name => "substring" } request.flash[:notice].should == "Added tag to table of bodies." response.should redirect_to(:action=>'list') diff --git a/spec/controllers/public_body_controller_spec.rb b/spec/controllers/public_body_controller_spec.rb index 2d1b1466f..025ebfdba 100644 --- a/spec/controllers/public_body_controller_spec.rb +++ b/spec/controllers/public_body_controller_spec.rb @@ -250,6 +250,13 @@ describe PublicBodyController, "when listing bodies" do response.code.should == '406' end + it "should list authorities starting with a multibyte first letter" do + get :list, {:tag => "å", :show_locale => 'cs'} + response.should render_template('list') + assigns[:public_bodies].should == [ public_bodies(:accented_public_body) ] + assigns[:tag].should == "Å" + end + end describe PublicBodyController, "when showing JSON version for API" do diff --git a/spec/fixtures/files/fake-authority-type.csv b/spec/fixtures/files/fake-authority-type.csv index cb25050c6..a320941c7 100644 --- a/spec/fixtures/files/fake-authority-type.csv +++ b/spec/fixtures/files/fake-authority-type.csv @@ -2,3 +2,5 @@ ,Scottish Fake Authority,scottish_foi@localhost ,Fake Authority of Northern Ireland,ni_foi@localhost ,Gobierno de Aragón,spain_foi@localhost +,Nordic æøå,no_foi@localhost + diff --git a/spec/fixtures/public_bodies.yml b/spec/fixtures/public_bodies.yml index 1fa016d3a..71fe0a379 100644 --- a/spec/fixtures/public_bodies.yml +++ b/spec/fixtures/public_bodies.yml @@ -128,3 +128,21 @@ other_public_body: info_requests_successful_count: 0 info_requests_not_held_count: 0 info_requests_overdue_count: 0 +accented_public_body: + id: 8 + version: 1 + name: 'Åčçèñtéd Authority' + first_letter: Å + request_email: accented@localhost + short_name: 'Åčçèñtéd Authority' + url_name: accented_authority + notes: This is to test unicode handling in body names + updated_at: 2008-10-25 10:51:01.161639 + last_edit_comment: Another edit + last_edit_editor: louise + created_at: 2008-10-25 10:51:01.161639 + api_key: 7 + info_requests_count: 0 + info_requests_successful_count: 0 + info_requests_not_held_count: 0 + info_requests_overdue_count: 0 diff --git a/spec/fixtures/public_body_translations.yml b/spec/fixtures/public_body_translations.yml index de1bf2f18..2030804ac 100644 --- a/spec/fixtures/public_body_translations.yml +++ b/spec/fixtures/public_body_translations.yml @@ -115,3 +115,15 @@ humpadink_he_IL_public_body_translation: publication_scheme: "" disclosure_log: "" +accented_public_body_translation: + id: 10 + public_body_id: 8 + locale: cs + name: "Åčçèñtéd Authority" + first_letter: 'Å' + request_email: accented@localhost + short_name: "Åčçèñtéd Authority" + url_name: accented_authority + notes: This is to test unicode handling in body names + publication_scheme: "" + disclosure_log: "" diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index 7281b74b6..64ad1972e 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -770,6 +770,16 @@ describe InfoRequest do end end + describe 'when working out a subject for request emails' do + + it 'should create a standard request subject' do + info_request = FactoryGirl.build(:info_request) + expected_text = "Freedom of Information request - #{info_request.title}" + info_request.email_subject_request.should == expected_text + end + + end + describe 'when working out a subject for a followup emails' do it "should not be confused by an nil subject in the incoming message" do diff --git a/spec/models/outgoing_message_spec.rb b/spec/models/outgoing_message_spec.rb index 20f07e8fe..8c000665b 100644 --- a/spec/models/outgoing_message_spec.rb +++ b/spec/models/outgoing_message_spec.rb @@ -144,7 +144,7 @@ describe OutgoingMessage, " when making an outgoing message" do end -describe IncomingMessage, " when censoring data" do +describe OutgoingMessage, " when censoring data" do before do @om = outgoing_messages(:useless_outgoing_message) diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb index 582f1430e..7a2c60722 100644 --- a/spec/models/public_body_spec.rb +++ b/spec/models/public_body_spec.rb @@ -190,6 +190,17 @@ describe PublicBody, " when saving" do @public_body.first_letter.should == 'T' end + it "should update first letter, even if it's a multibyte character" do + pb = PublicBody.new(:name => 'åccents, lower-case', + :short_name => 'ALC', + :request_email => 'foo@localhost', + :last_edit_editor => 'test', + :last_edit_comment => '') + pb.first_letter.should be_nil + pb.save! + pb.first_letter.should == 'Å' + end + it "should save the name when renaming an existing public body" do public_body = public_bodies(:geraldine_public_body) public_body.name = "Mark's Public Body" @@ -300,7 +311,7 @@ describe PublicBody, " when loading CSV files" do errors.should == [] notes.size.should == 2 notes[0].should == "line 1: creating new authority 'aBody' (locale: en):\n\t{\"name\":\"aBody\"}" - notes[1].should =~ /Notes: Some bodies are in database, but not in CSV file:\n( [A-Za-z ]+\n)*You may want to delete them manually.\n/ + notes[1].should =~ /Notes: Some bodies are in database, but not in CSV file:\n( .+\n)*You may want to delete them manually.\n/ end it "should do a dry run successfully" do @@ -309,14 +320,15 @@ describe PublicBody, " when loading CSV files" do csv_contents = normalize_string_to_utf8(load_file_fixture("fake-authority-type.csv")) errors, notes = PublicBody.import_csv(csv_contents, '', 'replace', true, 'someadmin') # true means dry run errors.should == [] - notes.size.should == 5 - notes[0..3].should == [ + notes.size.should == 6 + notes[0..4].should == [ "line 1: creating new authority 'North West Fake Authority' (locale: en):\n\t\{\"name\":\"North West Fake Authority\",\"request_email\":\"north_west_foi@localhost\"\}", "line 2: creating new authority 'Scottish Fake Authority' (locale: en):\n\t\{\"name\":\"Scottish Fake Authority\",\"request_email\":\"scottish_foi@localhost\"\}", "line 3: creating new authority 'Fake Authority of Northern Ireland' (locale: en):\n\t\{\"name\":\"Fake Authority of Northern Ireland\",\"request_email\":\"ni_foi@localhost\"\}", "line 4: creating new authority 'Gobierno de Aragón' (locale: en):\n\t\{\"name\":\"Gobierno de Arag\\u00f3n\",\"request_email\":\"spain_foi@localhost\"}", + "line 5: creating new authority 'Nordic æøå' (locale: en):\n\t{\"name\":\"Nordic \\u00e6\\u00f8\\u00e5\",\"request_email\":\"no_foi@localhost\"}" ] - notes[4].should =~ /Notes: Some bodies are in database, but not in CSV file:\n( [A-Za-z ]+\n)*You may want to delete them manually.\n/ + notes[5].should =~ /Notes: Some bodies are in database, but not in CSV file:\n( .+\n)*You may want to delete them manually.\n/ PublicBody.count.should == original_count end @@ -327,16 +339,17 @@ describe PublicBody, " when loading CSV files" do csv_contents = normalize_string_to_utf8(load_file_fixture("fake-authority-type.csv")) errors, notes = PublicBody.import_csv(csv_contents, '', 'replace', false, 'someadmin') # false means real run errors.should == [] - notes.size.should == 5 - notes[0..3].should == [ + notes.size.should == 6 + notes[0..4].should == [ "line 1: creating new authority 'North West Fake Authority' (locale: en):\n\t\{\"name\":\"North West Fake Authority\",\"request_email\":\"north_west_foi@localhost\"\}", "line 2: creating new authority 'Scottish Fake Authority' (locale: en):\n\t\{\"name\":\"Scottish Fake Authority\",\"request_email\":\"scottish_foi@localhost\"\}", "line 3: creating new authority 'Fake Authority of Northern Ireland' (locale: en):\n\t\{\"name\":\"Fake Authority of Northern Ireland\",\"request_email\":\"ni_foi@localhost\"\}", "line 4: creating new authority 'Gobierno de Aragón' (locale: en):\n\t\{\"name\":\"Gobierno de Arag\\u00f3n\",\"request_email\":\"spain_foi@localhost\"}", + "line 5: creating new authority 'Nordic æøå' (locale: en):\n\t{\"name\":\"Nordic \\u00e6\\u00f8\\u00e5\",\"request_email\":\"no_foi@localhost\"}" ] - notes[4].should =~ /Notes: Some bodies are in database, but not in CSV file:\n( [A-Za-z ]+\n)*You may want to delete them manually.\n/ + notes[5].should =~ /Notes: Some bodies are in database, but not in CSV file:\n( .+\n)*You may want to delete them manually.\n/ - PublicBody.count.should == original_count + 4 + PublicBody.count.should == original_count + 5 end it "should do imports without a tag successfully" do @@ -345,15 +358,16 @@ describe PublicBody, " when loading CSV files" do csv_contents = normalize_string_to_utf8(load_file_fixture("fake-authority-type.csv")) errors, notes = PublicBody.import_csv(csv_contents, '', 'replace', false, 'someadmin') # false means real run errors.should == [] - notes.size.should == 5 - notes[0..3].should == [ + notes.size.should == 6 + notes[0..4].should == [ "line 1: creating new authority 'North West Fake Authority' (locale: en):\n\t\{\"name\":\"North West Fake Authority\",\"request_email\":\"north_west_foi@localhost\"\}", "line 2: creating new authority 'Scottish Fake Authority' (locale: en):\n\t\{\"name\":\"Scottish Fake Authority\",\"request_email\":\"scottish_foi@localhost\"\}", "line 3: creating new authority 'Fake Authority of Northern Ireland' (locale: en):\n\t\{\"name\":\"Fake Authority of Northern Ireland\",\"request_email\":\"ni_foi@localhost\"\}", "line 4: creating new authority 'Gobierno de Aragón' (locale: en):\n\t\{\"name\":\"Gobierno de Arag\\u00f3n\",\"request_email\":\"spain_foi@localhost\"}", + "line 5: creating new authority 'Nordic æøå' (locale: en):\n\t{\"name\":\"Nordic \\u00e6\\u00f8\\u00e5\",\"request_email\":\"no_foi@localhost\"}" ] - notes[4].should =~ /Notes: Some bodies are in database, but not in CSV file:\n( [A-Za-z ]+\n)*You may want to delete them manually.\n/ - PublicBody.count.should == original_count + 4 + notes[5].should =~ /Notes: Some bodies are in database, but not in CSV file:\n( .+\n)*You may want to delete them manually.\n/ + PublicBody.count.should == original_count + 5 end it "should handle a field list and fields out of order" do @@ -368,7 +382,7 @@ describe PublicBody, " when loading CSV files" do "line 3: creating new authority 'Scottish Fake Authority' (locale: en):\n\t\{\"name\":\"Scottish Fake Authority\",\"request_email\":\"scottish_foi@localhost\",\"home_page\":\"http://scottish.org\",\"tag_string\":\"scottish\"\}", "line 4: creating new authority 'Fake Authority of Northern Ireland' (locale: en):\n\t\{\"name\":\"Fake Authority of Northern Ireland\",\"request_email\":\"ni_foi@localhost\",\"tag_string\":\"fake aTag\"\}", ] - notes[3].should =~ /Notes: Some bodies are in database, but not in CSV file:\n( [A-Za-z ]+\n)*You may want to delete them manually.\n/ + notes[3].should =~ /Notes: Some bodies are in database, but not in CSV file:\n( .+\n)*You may want to delete them manually.\n/ PublicBody.count.should == original_count end @@ -425,7 +439,7 @@ describe PublicBody, " when loading CSV files" do "line 4: creating new authority 'Fake Authority of Northern Ireland' (locale: en):\n\t{\"name\":\"Fake Authority of Northern Ireland\",\"request_email\":\"ni_foi@localhost\",\"tag_string\":\"fake aTag\"}", "line 4: creating new authority 'Fake Authority of Northern Ireland' (locale: es):\n\t{\"name\":\"Autoridad Irlandesa\"}", ] - notes[6].should =~ /Notes: Some bodies are in database, but not in CSV file:\n( [A-Za-z ]+\n)*You may want to delete them manually.\n/ + notes[6].should =~ /Notes: Some bodies are in database, but not in CSV file:\n( .+\n)*You may want to delete them manually.\n/ PublicBody.count.should == original_count + 3 @@ -451,7 +465,7 @@ describe PublicBody, " when loading CSV files" do "line 3: creating new authority 'Scottish Fake Authority' (locale: en):\n\t{\"name\":\"Scottish Fake Authority\",\"request_email\":\"scottish_foi@localhost\",\"home_page\":\"http://scottish.org\",\"tag_string\":\"scottish\"}", "line 4: creating new authority 'Fake Authority of Northern Ireland' (locale: en):\n\t{\"name\":\"Fake Authority of Northern Ireland\",\"request_email\":\"ni_foi@localhost\",\"tag_string\":\"fake aTag\"}", ] - notes[3].should =~ /Notes: Some bodies are in database, but not in CSV file:\n( [A-Za-z ]+\n)*You may want to delete them manually.\n/ + notes[3].should =~ /Notes: Some bodies are in database, but not in CSV file:\n( .+\n)*You may want to delete them manually.\n/ PublicBody.count.should == original_count end diff --git a/spec/models/xapian_spec.rb b/spec/models/xapian_spec.rb index c7c21e3a0..3c9fff784 100644 --- a/spec/models/xapian_spec.rb +++ b/spec/models/xapian_spec.rb @@ -406,28 +406,13 @@ describe InfoRequestEvent, " when faced with a race condition during xapian_mark before(:each) do load_raw_emails_data get_fixtures_xapian_index - # Use the before create job hook to simulate a race condition with another process - # by creating an acts_as_xapian_job record for the same model - class InfoRequestEvent - def xapian_before_create_job_hook(action, model, model_id) - ActsAsXapian::ActsAsXapianJob.create!(:model => model, - :model_id => model_id, - :action => action) - end - end - end - - after(:each) do - # Reset the before create job hook - class InfoRequestEvent - def xapian_before_create_job_hook(action, model, model_id) - end - end end it 'should not raise an error but should fail silently' do - ir = info_requests(:naughty_chicken_request) - ir.reindex_request_events + with_duplicate_xapian_job_creation do + ir = info_requests(:naughty_chicken_request) + ir.reindex_request_events + end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 0d8f8fac5..6e65018f1 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -125,6 +125,25 @@ Spork.prefork do rebuild_xapian_index end + # Use the before create job hook to simulate a race condition with + # another process by creating an acts_as_xapian_job record for the + # same model: + def with_duplicate_xapian_job_creation + InfoRequestEvent.module_eval do + def xapian_before_create_job_hook(action, model, model_id) + ActsAsXapian::ActsAsXapianJob.create!(:model => model, + :model_id => model_id, + :action => action) + end + end + yield + ensure + InfoRequestEvent.module_eval do + def xapian_before_create_job_hook(action, model, model_id) + end + end + end + def with_env_tz(new_tz = 'US/Eastern') old_tz, ENV['TZ'] = ENV['TZ'], new_tz yield diff --git a/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb b/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb index fcf7a778d..2e486f328 100644 --- a/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb +++ b/vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb @@ -928,7 +928,7 @@ module ActsAsXapian def xapian_create_job(action, model, model_id) begin - ActiveRecord::Base.transaction do + ActiveRecord::Base.transaction(:requires_new => true) do ActsAsXapianJob.delete_all([ "model = ? and model_id = ?", model, model_id]) xapian_before_create_job_hook(action, model, model_id) ActsAsXapianJob.create!(:model => model, |