From e5855f7fd2657574c5af99890c63d530ca3bb5d0 Mon Sep 17 00:00:00 2001 From: Mark Longair Date: Mon, 19 Aug 2013 10:34:05 +0100 Subject: Improve calculation of PublicBody statistics columns On PublicBody, we don't need to update info_requests_count because that's already done with :counter_cache. On the other hand, info_requests_successful_count and info_requests_not_held_count can't be updated easily with counter_cache (since they need conditions to be attached). Instead we update them in post_save and post_destroy, as suggested here: http://blog.douglasfshearer.com/post/17495285851/custom-counter-cache-with-conditions This also adds tests to ensure that the after_(save|destroy) callbacks are called and that they modify the counts correctly. --- spec/models/info_request_spec.rb | 66 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) (limited to 'spec/models') diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index 3451e018f..3f2e02189 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -857,4 +857,70 @@ describe InfoRequest do end end end + + describe 'when saving an info_request' do + + before do + @info_request = InfoRequest.new(:external_url => 'http://www.example.com', + :external_user_name => 'Example User', + :title => 'Some request or other', + :public_body => public_bodies(:geraldine_public_body)) + end + + it "should call purge_in_cache and update_counter_cache" do + @info_request.should_receive(:purge_in_cache) + # Twice - once for save, once for destroy: + @info_request.should_receive(:update_counter_cache).twice + @info_request.save! + @info_request.destroy + end + + end + + describe 'when destroying an info_request' do + + before do + @info_request = InfoRequest.new(:external_url => 'http://www.example.com', + :external_user_name => 'Example User', + :title => 'Some request or other', + :public_body => public_bodies(:geraldine_public_body)) + end + + it "should call update_counter_cache" do + @info_request.save! + @info_request.should_receive(:update_counter_cache) + @info_request.destroy + end + + end + + describe 'when changing a described_state' do + + it "should change the counts on its PublicBody" do + pb = public_bodies(:geraldine_public_body) + old_successful_count = pb.info_requests_successful_count + old_not_held_count = pb.info_requests_not_held_count + ir = InfoRequest.new(:external_url => 'http://www.example.com', + :external_user_name => 'Example User', + :title => 'Some request or other', + :described_state => 'partially_successful', + :public_body => pb) + ir.save! + pb.info_requests_successful_count.should == (old_successful_count + 1) + ir.described_state = 'not_held' + ir.save! + pb.info_requests_successful_count.should == old_successful_count + pb.info_requests_not_held_count.should == (old_not_held_count + 1) + ir.described_state = 'successful' + ir.save! + pb.info_requests_successful_count.should == (old_successful_count + 1) + pb.info_requests_not_held_count.should == old_not_held_count + ir.destroy + pb.info_requests_successful_count.should == old_successful_count + pb.info_requests_successful_count.should == old_not_held_count + end + + end + + end -- cgit v1.2.3 From 2f1cca7df58c93ef7844831af892a716b9b17f48 Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Thu, 5 Sep 2013 15:39:19 +0100 Subject: Don't dirty every attribute in checking for whitespace. Check to see if the stripped version is different before setting it on the record. If we don't do this, the subsequent call to write_attribute in Globalize3 which uses attribute_will_change! means we're storing versions when there hasn't really been any change. --- spec/models/public_body_spec.rb | 46 +++++++++++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 15 deletions(-) (limited to 'spec/models') diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb index 90affaaaa..3578c0e9c 100644 --- a/spec/models/public_body_spec.rb +++ b/spec/models/public_body_spec.rb @@ -136,36 +136,32 @@ describe PublicBody, " when saving" do @public_body = PublicBody.new end + def set_default_attributes(public_body) + public_body.name = "Testing Public Body" + public_body.short_name = "TPB" + public_body.request_email = "request@localhost" + public_body.last_edit_editor = "*test*" + public_body.last_edit_comment = "This is a test" + end + it "should not be valid without setting some parameters" do @public_body.should_not be_valid end it "should not be valid with misformatted request email" do - @public_body.name = "Testing Public Body" - @public_body.short_name = "TPB" + set_default_attributes(@public_body) @public_body.request_email = "requestBOOlocalhost" - @public_body.last_edit_editor = "*test*" - @public_body.last_edit_comment = "This is a test" @public_body.should_not be_valid @public_body.should have(1).errors_on(:request_email) end it "should save" do - @public_body.name = "Testing Public Body" - @public_body.short_name = "TPB" - @public_body.request_email = "request@localhost" - @public_body.last_edit_editor = "*test*" - @public_body.last_edit_comment = "This is a test" + set_default_attributes(@public_body) @public_body.save! end it "should update first_letter" do - @public_body.name = "Testing Public Body" - @public_body.short_name = "TPB" - @public_body.request_email = "request@localhost" - @public_body.last_edit_editor = "*test*" - @public_body.last_edit_comment = "This is a test" - + set_default_attributes(@public_body) @public_body.first_letter.should be_nil @public_body.save! @public_body.first_letter.should == 'T' @@ -178,6 +174,26 @@ describe PublicBody, " when saving" do public_body.name.should == "Mark's Public Body" end + + it 'should not create a new version when nothing has changed' do + @public_body.versions.size.should == 0 + set_default_attributes(@public_body) + @public_body.save! + @public_body.versions.size.should == 1 + @public_body.save! + @public_body.versions.size.should == 1 + end + + it 'should create a new version if something has changed' do + @public_body.versions.size.should == 0 + set_default_attributes(@public_body) + @public_body.save! + @public_body.versions.size.should == 1 + @public_body.name = 'Test' + @public_body.save! + @public_body.versions.size.should == 2 + end + end describe PublicBody, "when searching" do -- cgit v1.2.3 From c5c815d6450b4ecd4be004ae20fb0ecb1b62fa0d Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Thu, 5 Sep 2013 16:39:57 +0100 Subject: Save cached columns once updated. --- spec/models/info_request_spec.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index 3f2e02189..ab36a201c 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -896,8 +896,9 @@ describe InfoRequest do describe 'when changing a described_state' do - it "should change the counts on its PublicBody" do + it "should change the counts on its PublicBody without saving a new version" do pb = public_bodies(:geraldine_public_body) + old_version_count = pb.versions.count old_successful_count = pb.info_requests_successful_count old_not_held_count = pb.info_requests_not_held_count ir = InfoRequest.new(:external_url => 'http://www.example.com', @@ -909,15 +910,19 @@ describe InfoRequest do pb.info_requests_successful_count.should == (old_successful_count + 1) ir.described_state = 'not_held' ir.save! + pb.reload pb.info_requests_successful_count.should == old_successful_count pb.info_requests_not_held_count.should == (old_not_held_count + 1) ir.described_state = 'successful' ir.save! + pb.reload pb.info_requests_successful_count.should == (old_successful_count + 1) pb.info_requests_not_held_count.should == old_not_held_count ir.destroy + pb.reload pb.info_requests_successful_count.should == old_successful_count pb.info_requests_successful_count.should == old_not_held_count + pb.versions.count.should == old_version_count end end -- cgit v1.2.3 From 136cf239bfa0230d33d741aa4b31c0291f20fe10 Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Wed, 31 Jul 2013 15:34:12 +0100 Subject: Re-annotate models with database fields --- spec/models/censor_rule_spec.rb | 17 ++++++++++++++++ spec/models/foi_attachment_spec.rb | 15 ++++++++++++++ spec/models/holiday_spec.rb | 9 ++++++++ spec/models/incoming_message_spec.rb | 20 ++++++++++++++++++ spec/models/info_request_event_spec.rb | 17 ++++++++++++++++ spec/models/info_request_spec.rb | 24 ++++++++++++++++++++++ spec/models/mail_server_log_spec.rb | 13 ++++++++++++ spec/models/outgoing_message_spec.rb | 16 +++++++++++++++ spec/models/post_redirect_spec.rb | 16 +++++++++++++++ spec/models/profile_photo_spec.rb | 10 +++++++++ spec/models/public_body_spec.rb | 23 +++++++++++++++++++++ spec/models/purge_request_spec.rb | 11 ++++++++++ spec/models/raw_email_spec.rb | 7 +++++++ spec/models/track_thing_spec.rb | 16 +++++++++++++++ spec/models/track_things_sent_email_spec.rb | 13 ++++++++++++ spec/models/user_info_request_sent_alert_spec.rb | 11 ++++++++++ spec/models/user_spec.rb | 26 ++++++++++++++++++++++++ 17 files changed, 264 insertions(+) (limited to 'spec/models') diff --git a/spec/models/censor_rule_spec.rb b/spec/models/censor_rule_spec.rb index 3782cc630..5b41cc0d4 100644 --- a/spec/models/censor_rule_spec.rb +++ b/spec/models/censor_rule_spec.rb @@ -1,3 +1,20 @@ +# == Schema Information +# +# Table name: censor_rules +# +# id :integer not null, primary key +# info_request_id :integer +# user_id :integer +# public_body_id :integer +# text :text not null +# replacement :text not null +# last_edit_editor :string(255) not null +# last_edit_comment :text not null +# created_at :datetime not null +# updated_at :datetime not null +# regexp :boolean +# + require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe CensorRule, "substituting things" do diff --git a/spec/models/foi_attachment_spec.rb b/spec/models/foi_attachment_spec.rb index 9b0115c44..882723d1e 100644 --- a/spec/models/foi_attachment_spec.rb +++ b/spec/models/foi_attachment_spec.rb @@ -1,3 +1,18 @@ +# == Schema Information +# +# Table name: foi_attachments +# +# id :integer not null, primary key +# content_type :text +# filename :text +# charset :text +# display_size :text +# url_part_number :integer +# within_rfc822_subject :text +# incoming_message_id :integer +# hexdigest :string(32) +# + require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe FoiAttachment do diff --git a/spec/models/holiday_spec.rb b/spec/models/holiday_spec.rb index 5d3f76d24..89849abb7 100644 --- a/spec/models/holiday_spec.rb +++ b/spec/models/holiday_spec.rb @@ -1,3 +1,12 @@ +# == Schema Information +# +# Table name: holidays +# +# id :integer not null, primary key +# day :date +# description :text +# + require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe Holiday, " when calculating due date" do diff --git a/spec/models/incoming_message_spec.rb b/spec/models/incoming_message_spec.rb index ff6a8e34e..9455f20db 100644 --- a/spec/models/incoming_message_spec.rb +++ b/spec/models/incoming_message_spec.rb @@ -1,4 +1,24 @@ # coding: utf-8 +# == Schema Information +# +# Table name: incoming_messages +# +# id :integer not null, primary key +# info_request_id :integer not null +# created_at :datetime not null +# updated_at :datetime not null +# raw_email_id :integer not null +# cached_attachment_text_clipped :text +# cached_main_body_text_folded :text +# cached_main_body_text_unfolded :text +# subject :text +# mail_from_domain :text +# valid_to_reply_to :boolean +# last_parsed :datetime +# mail_from :text +# sent_at :datetime +# + require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe IncomingMessage, " when dealing with incoming mail" do diff --git a/spec/models/info_request_event_spec.rb b/spec/models/info_request_event_spec.rb index eb0de8c86..7f485454d 100644 --- a/spec/models/info_request_event_spec.rb +++ b/spec/models/info_request_event_spec.rb @@ -1,4 +1,21 @@ # coding: utf-8 +# == Schema Information +# +# Table name: info_request_events +# +# id :integer not null, primary key +# info_request_id :integer not null +# event_type :text not null +# params_yaml :text not null +# created_at :datetime not null +# described_state :string(255) +# calculated_state :string(255) +# last_described_at :datetime +# incoming_message_id :integer +# outgoing_message_id :integer +# comment_id :integer +# + require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe InfoRequestEvent do diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index c9ee57c74..2846fa6dc 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -1,3 +1,27 @@ +# == Schema Information +# +# Table name: info_requests +# +# id :integer not null, primary key +# title :text not null +# user_id :integer +# public_body_id :integer not null +# created_at :datetime not null +# updated_at :datetime not null +# described_state :string(255) not null +# awaiting_description :boolean default(FALSE), not null +# prominence :string(255) default("normal"), not null +# url_title :text not null +# law_used :string(255) default("foi"), not null +# allow_new_responses_from :string(255) default("anybody"), not null +# handle_rejected_responses :string(255) default("bounce"), not null +# idhash :string(255) not null +# external_user_name :string(255) +# external_url :string(255) +# attention_requested :boolean default(FALSE) +# comments_allowed :boolean default(TRUE), not null +# + require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe InfoRequest do diff --git a/spec/models/mail_server_log_spec.rb b/spec/models/mail_server_log_spec.rb index 2b44a4559..67709b130 100644 --- a/spec/models/mail_server_log_spec.rb +++ b/spec/models/mail_server_log_spec.rb @@ -1,3 +1,16 @@ +# == Schema Information +# +# Table name: mail_server_logs +# +# id :integer not null, primary key +# mail_server_log_done_id :integer +# info_request_id :integer +# order :integer not null +# line :text not null +# created_at :datetime not null +# updated_at :datetime not null +# + require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe MailServerLog do diff --git a/spec/models/outgoing_message_spec.rb b/spec/models/outgoing_message_spec.rb index 60164fb31..d0816d01f 100644 --- a/spec/models/outgoing_message_spec.rb +++ b/spec/models/outgoing_message_spec.rb @@ -1,3 +1,19 @@ +# == Schema Information +# +# Table name: outgoing_messages +# +# id :integer not null, primary key +# info_request_id :integer not null +# body :text not null +# status :string(255) not null +# message_type :string(255) not null +# created_at :datetime not null +# updated_at :datetime not null +# last_sent_at :datetime +# incoming_message_followup_id :integer +# what_doing :string(255) not null +# + require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe OutgoingMessage, " when making an outgoing message" do diff --git a/spec/models/post_redirect_spec.rb b/spec/models/post_redirect_spec.rb index 5f51b6de5..73740e914 100644 --- a/spec/models/post_redirect_spec.rb +++ b/spec/models/post_redirect_spec.rb @@ -1,3 +1,19 @@ +# == Schema Information +# +# Table name: post_redirects +# +# id :integer not null, primary key +# token :text not null +# uri :text not null +# post_params_yaml :text +# created_at :datetime not null +# updated_at :datetime not null +# email_token :text not null +# reason_params_yaml :text +# user_id :integer +# circumstance :text default("normal"), not null +# + require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe PostRedirect, " when constructing" do diff --git a/spec/models/profile_photo_spec.rb b/spec/models/profile_photo_spec.rb index 892cccd08..0e157e2c5 100644 --- a/spec/models/profile_photo_spec.rb +++ b/spec/models/profile_photo_spec.rb @@ -1,3 +1,13 @@ +# == Schema Information +# +# Table name: profile_photos +# +# id :integer not null, primary key +# data :binary not null +# user_id :integer +# draft :boolean default(FALSE), not null +# + require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe ProfilePhoto, "when constructing a new photo" do diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb index 34a91a2c9..d1fdd2c47 100644 --- a/spec/models/public_body_spec.rb +++ b/spec/models/public_body_spec.rb @@ -1,3 +1,26 @@ +# == Schema Information +# +# Table name: public_bodies +# +# id :integer not null, primary key +# name :text not null +# short_name :text not null +# request_email :text not null +# version :integer not null +# last_edit_editor :string(255) not null +# last_edit_comment :text not null +# created_at :datetime not null +# updated_at :datetime not null +# url_name :text not null +# home_page :text default(""), not null +# notes :text default(""), not null +# first_letter :string(255) not null +# publication_scheme :text default(""), not null +# api_key :string(255) not null +# info_requests_count :integer default(0), not null +# disclosure_log :text default(""), not null +# + require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe PublicBody, " using tags" do diff --git a/spec/models/purge_request_spec.rb b/spec/models/purge_request_spec.rb index 7b67fca52..02b3d685d 100644 --- a/spec/models/purge_request_spec.rb +++ b/spec/models/purge_request_spec.rb @@ -1,3 +1,14 @@ +# == Schema Information +# +# Table name: purge_requests +# +# id :integer not null, primary key +# url :string(255) +# created_at :datetime not null +# model :string(255) not null +# model_id :integer not null +# + require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') require 'fakeweb' diff --git a/spec/models/raw_email_spec.rb b/spec/models/raw_email_spec.rb index ff2830a62..f86b35e99 100644 --- a/spec/models/raw_email_spec.rb +++ b/spec/models/raw_email_spec.rb @@ -1,3 +1,10 @@ +# == Schema Information +# +# Table name: raw_emails +# +# id :integer not null, primary key +# + require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe User, "manipulating a raw email" do diff --git a/spec/models/track_thing_spec.rb b/spec/models/track_thing_spec.rb index c42eb5e8b..86d3c0cda 100644 --- a/spec/models/track_thing_spec.rb +++ b/spec/models/track_thing_spec.rb @@ -1,3 +1,19 @@ +# == Schema Information +# +# Table name: track_things +# +# id :integer not null, primary key +# tracking_user_id :integer not null +# track_query :string(255) not null +# info_request_id :integer +# tracked_user_id :integer +# public_body_id :integer +# track_medium :string(255) not null +# track_type :string(255) default("internal_error"), not null +# created_at :datetime +# updated_at :datetime +# + require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe TrackThing, "when tracking changes" do diff --git a/spec/models/track_things_sent_email_spec.rb b/spec/models/track_things_sent_email_spec.rb index 6166f42ab..4675d0847 100644 --- a/spec/models/track_things_sent_email_spec.rb +++ b/spec/models/track_things_sent_email_spec.rb @@ -1,3 +1,16 @@ +# == Schema Information +# +# Table name: track_things_sent_emails +# +# id :integer not null, primary key +# track_thing_id :integer not null +# info_request_event_id :integer +# user_id :integer +# public_body_id :integer +# created_at :datetime +# updated_at :datetime +# + require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe TrackThingsSentEmail, "when tracking things sent email" do diff --git a/spec/models/user_info_request_sent_alert_spec.rb b/spec/models/user_info_request_sent_alert_spec.rb index 971c5b8c1..69be1092b 100644 --- a/spec/models/user_info_request_sent_alert_spec.rb +++ b/spec/models/user_info_request_sent_alert_spec.rb @@ -1,3 +1,14 @@ +# == Schema Information +# +# Table name: user_info_request_sent_alerts +# +# id :integer not null, primary key +# user_id :integer not null +# info_request_id :integer not null +# alert_type :string(255) not null +# info_request_event_id :integer +# + require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe UserInfoRequestSentAlert, " when blah" do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 96c169604..f380f6f13 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -1,3 +1,29 @@ +# == Schema Information +# +# Table name: users +# +# id :integer not null, primary key +# email :string(255) not null +# name :string(255) not null +# hashed_password :string(255) not null +# salt :string(255) not null +# created_at :datetime not null +# updated_at :datetime not null +# email_confirmed :boolean default(FALSE), not null +# url_name :text not null +# last_daily_track_email :datetime default(2000-01-01 00:00:00 UTC) +# admin_level :string(255) default("none"), not null +# ban_text :text default(""), not null +# about_me :text default(""), not null +# locale :string(255) +# email_bounced_at :datetime +# email_bounce_message :text default(""), not null +# no_limit :boolean default(FALSE), not null +# receive_email_alerts :boolean default(TRUE), not null +# address :string(255) +# dob :date +# + require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe User, "making up the URL name" do -- cgit v1.2.3 From 4e2c4501587b7bf59a2f639a4891581c681d553e Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Wed, 31 Jul 2013 16:12:51 +0100 Subject: Add prominence to incoming message. --- spec/models/incoming_message_spec.rb | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'spec/models') diff --git a/spec/models/incoming_message_spec.rb b/spec/models/incoming_message_spec.rb index 9455f20db..eda96f2a7 100644 --- a/spec/models/incoming_message_spec.rb +++ b/spec/models/incoming_message_spec.rb @@ -17,10 +17,31 @@ # last_parsed :datetime # mail_from :text # sent_at :datetime +# prominence :string(255) default("normal"), not null # require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') +describe IncomingMessage, 'when validating' do + + it 'should be valid with valid prominence values' do + ['hidden', 'requester_only', 'normal'].each do |prominence| + incoming_message = IncomingMessage.new(:raw_email => RawEmail.new, + :info_request => InfoRequest.new, + :prominence => prominence) + incoming_message.valid?.should be_true + end + end + + it 'should not be valid with an invalid prominence value' do + incoming_message = IncomingMessage.new(:raw_email => RawEmail.new, + :info_request => InfoRequest.new, + :prominence => 'norman') + incoming_message.valid?.should be_false + end + +end + describe IncomingMessage, " when dealing with incoming mail" do before(:each) do -- cgit v1.2.3 From fee41e5045ebee47c12ea6f9d2e6a39ad8110cae Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Wed, 31 Jul 2013 17:16:56 +0100 Subject: Add response_event helper Add a convenience method for getting the 'response' event associated with an incoming message. --- spec/models/incoming_message_spec.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'spec/models') diff --git a/spec/models/incoming_message_spec.rb b/spec/models/incoming_message_spec.rb index eda96f2a7..f249ee87e 100644 --- a/spec/models/incoming_message_spec.rb +++ b/spec/models/incoming_message_spec.rb @@ -42,6 +42,18 @@ describe IncomingMessage, 'when validating' do end +describe IncomingMessage, 'when getting a response event' do + + it 'should return an event with event_type "response"' do + incoming_message = IncomingMessage.new + ['comment', 'response'].each do |event_type| + incoming_message.info_request_events << InfoRequestEvent.new(:event_type => event_type) + end + incoming_message.response_event.event_type.should == 'response' + end + +end + describe IncomingMessage, " when dealing with incoming mail" do before(:each) do -- cgit v1.2.3 From af6bc758faf62eb44e5822ed0eb6c1871db6ac91 Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Wed, 31 Jul 2013 18:36:55 +0100 Subject: Add a method to ask whether a user can view an incoming message. --- spec/models/incoming_message_spec.rb | 57 ++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) (limited to 'spec/models') diff --git a/spec/models/incoming_message_spec.rb b/spec/models/incoming_message_spec.rb index f249ee87e..a45bf303e 100644 --- a/spec/models/incoming_message_spec.rb +++ b/spec/models/incoming_message_spec.rb @@ -54,6 +54,63 @@ describe IncomingMessage, 'when getting a response event' do end +describe IncomingMessage, 'when asked if a user can view it', :focus => true do + + before do + @user = mock_model(User) + @info_request = mock_model(InfoRequest) + @incoming_message = IncomingMessage.new(:info_request => @info_request) + end + + context 'if the prominence is hidden' do + + before do + @incoming_message.prominence = 'hidden' + end + + it 'should return true if the user can view hidden things' do + User.stub!(:view_hidden?).with(@user).and_return(true) + @incoming_message.user_can_view?(@user).should be_true + end + + it 'should return false if the user cannot view hidden things' do + User.stub!(:view_hidden?).with(@user).and_return(false) + @incoming_message.user_can_view?(@user).should be_false + end + + end + + context 'if the prominence is requester_only' do + + before do + @incoming_message.prominence = 'requester_only' + end + + it 'should return true if the user owns the associated request' do + @info_request.stub!(:is_owning_user?).with(@user).and_return(true) + @incoming_message.user_can_view?(@user).should be_true + end + + it 'should return false if the user does not own the associated request' do + @info_request.stub!(:is_owning_user?).with(@user).and_return(false) + @incoming_message.user_can_view?(@user).should be_false + end + end + + context 'if the prominence is normal' do + + before do + @incoming_message.prominence = 'normal' + end + + it 'should return true' do + @incoming_message.user_can_view?(@user).should be_true + end + + end + +end + describe IncomingMessage, " when dealing with incoming mail" do before(:each) do -- cgit v1.2.3 From b8965db2ed79e5b79e77716371de02a0297d425e Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Wed, 31 Jul 2013 18:47:03 +0100 Subject: Refactor common logic about prominence and access. Move it into the Ability module. --- spec/models/incoming_message_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/incoming_message_spec.rb b/spec/models/incoming_message_spec.rb index a45bf303e..ad4def0bb 100644 --- a/spec/models/incoming_message_spec.rb +++ b/spec/models/incoming_message_spec.rb @@ -54,7 +54,7 @@ describe IncomingMessage, 'when getting a response event' do end -describe IncomingMessage, 'when asked if a user can view it', :focus => true do +describe IncomingMessage, 'when asked if a user can view it' do before do @user = mock_model(User) -- cgit v1.2.3 From 20bd7353b9d30a00e130ebf3e8208b9b227c16bc Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Thu, 1 Aug 2013 16:43:47 +0100 Subject: Adding prominence_reason to IncomingMessage. --- spec/models/incoming_message_spec.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'spec/models') diff --git a/spec/models/incoming_message_spec.rb b/spec/models/incoming_message_spec.rb index ad4def0bb..51df16194 100644 --- a/spec/models/incoming_message_spec.rb +++ b/spec/models/incoming_message_spec.rb @@ -18,6 +18,7 @@ # mail_from :text # sent_at :datetime # prominence :string(255) default("normal"), not null +# prominence_reason :text # require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') -- cgit v1.2.3 From a34e6b1e727c05052f3b4056d881622993d1da9a Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Thu, 1 Aug 2013 17:56:05 +0100 Subject: Don't index hidden and requester_only incoming messages. --- spec/models/incoming_message_spec.rb | 23 +++++++++++++++++++ spec/models/info_request_event_spec.rb | 41 ++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) (limited to 'spec/models') diff --git a/spec/models/incoming_message_spec.rb b/spec/models/incoming_message_spec.rb index 51df16194..c0a7e5340 100644 --- a/spec/models/incoming_message_spec.rb +++ b/spec/models/incoming_message_spec.rb @@ -112,6 +112,29 @@ describe IncomingMessage, 'when asked if a user can view it' do end +describe 'when asked if it is indexed by search' do + + before do + @incoming_message = IncomingMessage.new + end + + it 'should return false if it has prominence "hidden"' do + @incoming_message.prominence = 'hidden' + @incoming_message.indexed_by_search?.should be_false + end + + it 'should return false if it has prominence "requester_only"' do + @incoming_message.prominence = 'requester_only' + @incoming_message.indexed_by_search?.should be_false + end + + it 'should return true if it has prominence "normal"' do + @incoming_message.prominence = 'normal' + @incoming_message.indexed_by_search?.should be_true + end + +end + describe IncomingMessage, " when dealing with incoming mail" do before(:each) do diff --git a/spec/models/info_request_event_spec.rb b/spec/models/info_request_event_spec.rb index 7f485454d..5feb3560a 100644 --- a/spec/models/info_request_event_spec.rb +++ b/spec/models/info_request_event_spec.rb @@ -32,6 +32,47 @@ describe InfoRequestEvent do end + describe 'when deciding if it is indexed by search' do + + before do + @comment = mock_model(Comment) + @incoming_message = mock_model(IncomingMessage) + @info_request = mock_model(InfoRequest, :indexed_by_search? => true) + end + + it 'should return false for a comment that is not visible' do + @comment.stub!(:visible).and_return(false) + @info_request_event = InfoRequestEvent.new(:event_type => 'comment', + :comment => @comment, + :info_request => @info_request) + @info_request_event.indexed_by_search?.should be_false + end + + it 'should return true for a comment that is visible' do + @comment.stub!(:visible).and_return(true) + @info_request_event = InfoRequestEvent.new(:event_type => 'comment', + :comment => @comment, + :info_request => @info_request) + @info_request_event.indexed_by_search?.should be_true + end + + it 'should return false for an incoming message that is not indexed by search' do + @incoming_message.stub!(:indexed_by_search?).and_return false + @info_request_event = InfoRequestEvent.new(:event_type => 'response', + :incoming_message => @incoming_message, + :info_request => @info_request) + @info_request_event.indexed_by_search?.should be_false + end + + it 'should return true for an incoming message with prominence "normal"' do + @incoming_message.stub!(:indexed_by_search?).and_return true + @info_request_event = InfoRequestEvent.new(:event_type => 'response', + :incoming_message => @incoming_message, + :info_request => @info_request) + @info_request_event.indexed_by_search?.should be_true + end + end + describe 'after saving' do it 'should mark the model for reindexing in xapian if there is no no_xapian_reindex flag on the object' do -- cgit v1.2.3 From 798ba1a031c162b36bb1b4a61a431c7026fe1223 Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Tue, 27 Aug 2013 12:26:33 +0100 Subject: Add some tests for user_can_view? on outgoing message. --- spec/models/outgoing_message_spec.rb | 52 ++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) (limited to 'spec/models') diff --git a/spec/models/outgoing_message_spec.rb b/spec/models/outgoing_message_spec.rb index d0816d01f..4bd9361a7 100644 --- a/spec/models/outgoing_message_spec.rb +++ b/spec/models/outgoing_message_spec.rb @@ -66,6 +66,58 @@ describe OutgoingMessage, " when making an outgoing message" do outgoing_message.body.should include(expected_text) end + describe 'when asked if a user can view it' do + + before do + @info_request = FactoryGirl.create(:info_request) + @outgoing_message = @info_request.outgoing_messages.first + end + + context 'if the prominence is hidden' do + + before do + @outgoing_message.prominence = 'hidden' + end + + it 'should return true for an admin user' do + @outgoing_message.user_can_view?(FactoryGirl.create(:admin_user)).should be_true + end + + it 'should return false for a non-admin user' do + @outgoing_message.user_can_view?(FactoryGirl.create(:user)).should be_false + end + + end + + context 'if the prominence is requester_only' do + + before do + @outgoing_message.prominence = 'requester_only' + end + + it 'should return true if the user owns the associated request' do + @outgoing_message.user_can_view?(@info_request.user).should be_true + end + + it 'should return false if the user does not own the associated request' do + @outgoing_message.user_can_view?(FactoryGirl.create(:user)).should be_false + end + end + + context 'if the prominence is normal' do + + before do + @outgoing_message.prominence = 'normal' + end + + it 'should return true for a non-admin user' do + @outgoing_message.user_can_view?(FactoryGirl.create(:user)).should be_true + end + + end + + end + end -- cgit v1.2.3 From e91443d8fab05b805fa781c701ca8616aeff563f Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Tue, 27 Aug 2013 12:28:10 +0100 Subject: Move indexed_by_search to MessageProminence Add some tests that it's working on the outgoing message model. --- spec/models/outgoing_message_spec.rb | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/outgoing_message_spec.rb b/spec/models/outgoing_message_spec.rb index 4bd9361a7..1e05e09f1 100644 --- a/spec/models/outgoing_message_spec.rb +++ b/spec/models/outgoing_message_spec.rb @@ -118,6 +118,29 @@ describe OutgoingMessage, " when making an outgoing message" do end + describe 'when asked if it is indexed by search' do + + before do + @info_request = FactoryGirl.create(:info_request) + @outgoing_message = @info_request.outgoing_messages.first + end + + it 'should return false if it has prominence "hidden"' do + @outgoing_message.prominence = 'hidden' + @outgoing_message.indexed_by_search?.should be_false + end + + it 'should return false if it has prominence "requester_only"' do + @outgoing_message.prominence = 'requester_only' + @outgoing_message.indexed_by_search?.should be_false + end + + it 'should return true if it has prominence "normal"' do + @outgoing_message.prominence = 'normal' + @outgoing_message.indexed_by_search?.should be_true + end + + end end @@ -140,5 +163,3 @@ describe IncomingMessage, " when censoring data" do @om.body.should match(/fancy cat/) end end - - -- cgit v1.2.3 From fca2037a243a8e0b0afaf7fc779a96bb86f39b7f Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Tue, 27 Aug 2013 12:29:05 +0100 Subject: InfoRequestEvent.indexed_by_search consults OutgoingMessage. --- spec/models/info_request_event_spec.rb | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/info_request_event_spec.rb b/spec/models/info_request_event_spec.rb index 5feb3560a..53c83bd46 100644 --- a/spec/models/info_request_event_spec.rb +++ b/spec/models/info_request_event_spec.rb @@ -37,6 +37,7 @@ describe InfoRequestEvent do before do @comment = mock_model(Comment) @incoming_message = mock_model(IncomingMessage) + @outgoing_message = mock_model(OutgoingMessage) @info_request = mock_model(InfoRequest, :indexed_by_search? => true) end @@ -64,13 +65,29 @@ describe InfoRequestEvent do @info_request_event.indexed_by_search?.should be_false end - it 'should return true for an incoming message with prominence "normal"' do + it 'should return true for an incoming message that is indexed by search' do @incoming_message.stub!(:indexed_by_search?).and_return true @info_request_event = InfoRequestEvent.new(:event_type => 'response', :incoming_message => @incoming_message, :info_request => @info_request) @info_request_event.indexed_by_search?.should be_true end + + it 'should return false for an outgoing message that is not indexed by search' do + @outgoing_message.stub!(:indexed_by_search?).and_return false + @info_request_event = InfoRequestEvent.new(:event_type => 'followup_sent', + :outgoing_message => @outgoing_message, + :info_request => @info_request) + @info_request_event.indexed_by_search?.should be_false + end + + it 'should return true for an outgoing message that is indexed by search' do + @outgoing_message.stub!(:indexed_by_search?).and_return true + @info_request_event = InfoRequestEvent.new(:event_type => 'followup_sent', + :outgoing_message => @outgoing_message, + :info_request => @info_request) + @info_request_event.indexed_by_search?.should be_true + end end describe 'after saving' do -- cgit v1.2.3 From bc743d9fc8c8f740f37b91cbe374c6ae20b10619 Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Tue, 27 Aug 2013 16:13:02 +0100 Subject: Add public criteria for message event access methods get_last_response_event and get_last_outgoing_event are used in various places to determine which events to link to, use in queries etc. Restrict them to refer to the last publicly visible event of the relevant type, and rename them to make that clear. --- spec/models/info_request_spec.rb | 63 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index 2846fa6dc..a6f877b20 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -451,8 +451,14 @@ describe InfoRequest do before do Time.stub!(:now).and_return(Time.utc(2007, 11, 9, 23, 59)) - @mock_comment_event = mock_model(InfoRequestEvent, :created_at => Time.now - 23.days, :event_type => 'comment', :response? => false) - @mock_response_event = mock_model(InfoRequestEvent, :created_at => Time.now - 22.days, :event_type => 'response', :response? => true) + @mock_comment_event = mock_model(InfoRequestEvent, :created_at => Time.now - 23.days, + :event_type => 'comment', + :response? => false) + mock_incoming_message = mock_model(IncomingMessage, :all_can_view? => true) + @mock_response_event = mock_model(InfoRequestEvent, :created_at => Time.now - 22.days, + :event_type => 'response', + :response? => true, + :incoming_message => mock_incoming_message) @info_request = InfoRequest.new(:prominence => 'normal', :awaiting_description => true, :info_request_events => [@mock_response_event, @mock_comment_event]) @@ -589,6 +595,59 @@ describe InfoRequest do end end + describe 'when asked for the last public response event' do + + before do + @info_request = FactoryGirl.create(:info_request_with_incoming) + @incoming_message = @info_request.incoming_messages.first + end + + it 'should not return an event with a hidden prominence message' do + @incoming_message.prominence = 'hidden' + @incoming_message.save! + @info_request.get_last_public_response_event.should == nil + end + + it 'should not return an event with a requester_only prominence message' do + @incoming_message.prominence = 'requester_only' + @incoming_message.save! + @info_request.get_last_public_response_event.should == nil + end + + it 'should return an event with a normal prominence message' do + @incoming_message.prominence = 'normal' + @incoming_message.save! + @info_request.get_last_public_response_event.should == @incoming_message.response_event + end + end + + describe 'when asked for the last public outgoing event' do + + before do + @info_request = FactoryGirl.create(:info_request) + @outgoing_message = @info_request.outgoing_messages.first + end + + it 'should not return an event with a hidden prominence message' do + @outgoing_message.prominence = 'hidden' + @outgoing_message.save! + @info_request.get_last_public_outgoing_event.should == nil + end + + it 'should not return an event with a requester_only prominence message' do + @outgoing_message.prominence = 'requester_only' + @outgoing_message.save! + @info_request.get_last_public_outgoing_event.should == nil + end + + it 'should return an event with a normal prominence message' do + @outgoing_message.prominence = 'normal' + @outgoing_message.save! + @info_request.get_last_public_outgoing_event.should == @outgoing_message.info_request_events.first + end + + end + describe 'when generating json for the api' do before do -- cgit v1.2.3 From 939f08a5396b65778748417c26b54c214fe35883 Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Tue, 27 Aug 2013 17:04:23 +0100 Subject: Only include public messages in who_can_followup_to --- spec/models/info_request_spec.rb | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) (limited to 'spec/models') diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index a6f877b20..e03bf6856 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -648,6 +648,43 @@ describe InfoRequest do end + describe 'when asked who can be sent a followup' do + + before do + @info_request = FactoryGirl.create(:info_request_with_plain_incoming) + @incoming_message = @info_request.incoming_messages.first + @public_body = @info_request.public_body + end + + it 'should not include details from a hidden prominence response' do + @incoming_message.prominence = 'hidden' + @incoming_message.save! + @info_request.who_can_followup_to.should == [[@public_body.name, + @public_body.request_email, + nil]] + end + + it 'should not include details from a requester_only prominence response' do + @incoming_message.prominence = 'requester_only' + @incoming_message.save! + @info_request.who_can_followup_to.should == [[@public_body.name, + @public_body.request_email, + nil]] + end + + it 'should include details from a normal prominence response' do + @incoming_message.prominence = 'normal' + @incoming_message.save! + @info_request.who_can_followup_to.should == [[@public_body.name, + @public_body.request_email, + nil], + ['Bob Responder', + "bob@example.com", + @incoming_message.id]] + end + + end + describe 'when generating json for the api' do before do -- cgit v1.2.3 From eb16ca53ae1b1cfa5f23bb56d10e9eecc50b00e6 Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Wed, 28 Aug 2013 09:37:58 +0100 Subject: Exclude hidden responses when calculating old_unclassified Make old_unclassified_params method consistent with last_public_response_event and associated methods. --- spec/models/info_request_spec.rb | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) (limited to 'spec/models') diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index e03bf6856..d80eabace 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -400,10 +400,12 @@ describe InfoRequest do it 'should add extra conditions if supplied' do expected_conditions = ["awaiting_description = ? - AND (SELECT created_at - FROM info_request_events + AND (SELECT info_request_events.created_at + FROM info_request_events, incoming_messages WHERE info_request_events.info_request_id = info_requests.id AND info_request_events.event_type = 'response' + AND incoming_messages.id = info_request_events.incoming_message_id + AND incoming_messages.prominence = 'normal' ORDER BY created_at desc LIMIT 1) < ? AND url_title != 'holding_pen' AND user_id IS NOT NULL @@ -418,21 +420,25 @@ describe InfoRequest do InfoRequest.find_old_unclassified({:conditions => ["prominence != 'backpage'"]}) end - it 'should ask the database for requests that are awaiting description, have a last response older + it 'should ask the database for requests that are awaiting description, have a last public response older than 21 days old, have a user, are not the holding pen and are not backpaged' do expected_conditions = ["awaiting_description = ? - AND (SELECT created_at - FROM info_request_events + AND (SELECT info_request_events.created_at + FROM info_request_events, incoming_messages WHERE info_request_events.info_request_id = info_requests.id AND info_request_events.event_type = 'response' + AND incoming_messages.id = info_request_events.incoming_message_id + AND incoming_messages.prominence = 'normal' ORDER BY created_at desc LIMIT 1) < ? AND url_title != 'holding_pen' AND user_id IS NOT NULL".split(' ').join(' '), true, Time.now - 21.days] - expected_select = "*, (SELECT created_at - FROM info_request_events + expected_select = "*, (SELECT info_request_events.created_at + FROM info_request_events, incoming_messages WHERE info_request_events.info_request_id = info_requests.id AND info_request_events.event_type = 'response' + AND incoming_messages.id = info_request_events.incoming_message_id + AND incoming_messages.prominence = 'normal' ORDER BY created_at desc LIMIT 1) AS last_response_time".split(' ').join(' ') InfoRequest.should_receive(:find) do |all, query_params| -- cgit v1.2.3 From 09d6cc3313b9ec1f495ff09c74fe91e1b667f8b9 Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Tue, 17 Sep 2013 17:04:27 +0100 Subject: Add a failing spec for handling a race condition The spec uses a hook method to simulate the insertion of an acts_as_xapian_job in another process for the model. Credit to: http://stackoverflow.com/questions/2017587/simulating-race-conditions-in-rspec-unit-tests Conflicts: vendor/plugins/acts_as_xapian/lib/acts_as_xapian.rb --- spec/models/xapian_spec.rb | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) (limited to 'spec/models') diff --git a/spec/models/xapian_spec.rb b/spec/models/xapian_spec.rb index 7aab9cdc6..c7c21e3a0 100644 --- a/spec/models/xapian_spec.rb +++ b/spec/models/xapian_spec.rb @@ -400,3 +400,34 @@ describe ActsAsXapian::Search, "#words_to_highlight" do end end + +describe InfoRequestEvent, " when faced with a race condition during xapian_mark_needs_index" do + + 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 + end + +end -- cgit v1.2.3 From 777ed1016b23d577f4cee62f49961e1d44bc346d Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Mon, 9 Sep 2013 16:02:01 +0100 Subject: Allow a unicode-only title in validation --- spec/models/info_request_spec.rb | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'spec/models') diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index ab36a201c..6d026e286 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -1,7 +1,29 @@ +# encoding: utf-8 require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe InfoRequest do + describe 'when validating', :focus => true do + + it 'should accept a summary with ascii characters' do + info_request = InfoRequest.new(:title => 'abcde') + info_request.valid? + info_request.errors[:title].should be_empty + end + + it 'should accept a summary with unicode characters' do + info_request = InfoRequest.new(:title => 'кажете') + info_request.valid? + info_request.errors[:title].should be_empty + end + + it 'should not accept a summary with no ascii or unicode characters' do + info_request = InfoRequest.new(:title => '55555') + info_request.valid? + info_request.errors[:title].should_not be_empty + end + end + describe 'when generating a user name slug' do before do -- cgit v1.2.3