From 741a7e33cf2af9f233a046bdb96700c3663ee77a Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Fri, 7 Mar 2014 15:47:04 +0000 Subject: Make search description specs match actual cases In the largest Alaveteli instance, WDTK, only a fraction of tracks use filters and these specs represent those that have actually been used commonly. --- spec/models/track_thing_spec.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'spec/models') diff --git a/spec/models/track_thing_spec.rb b/spec/models/track_thing_spec.rb index 1c582564b..3edf2d1ad 100644 --- a/spec/models/track_thing_spec.rb +++ b/spec/models/track_thing_spec.rb @@ -51,10 +51,11 @@ describe TrackThing, "when tracking changes" do end it "will make some sane descriptions of search-based tracks" do - tests = { 'bob variety:user' => "users matching text 'bob'", - 'bob (variety:sent OR variety:followup_sent OR variety:response OR variety:comment) (latest_status:successful OR latest_status:partially_successful OR latest_status:rejected OR latest_status:not_held)' => "comments or requests which are successful or unsuccessful matching text 'bob'", - '(latest_status:waiting_response OR latest_status:waiting_clarification OR waiting_classification:true)' => 'requests which are awaiting a response', - ' (variety:sent OR variety:followup_sent OR variety:response OR variety:comment)' => 'all requests or comments' } + tests = { ' (variety:sent OR variety:followup_sent OR variety:response OR variety:comment)' => 'all requests or comments', + 'bob (variety:sent OR variety:followup_sent OR variety:response OR variety:comment)' => "all requests or comments matching text 'bob'", + 'bob (latest_status:successful OR latest_status:partially_successful)' => "requests which are successful matching text 'bob'", + '(latest_status:successful OR latest_status:partially_successful)' => 'requests which are successful', + 'bob' => "anything matching text 'bob'" } tests.each do |query, description| track_thing = TrackThing.create_track_for_search_query(query) track_thing.track_query_description.should == description -- cgit v1.2.3 From d6d8a95db5797e8e55283e0ab9e1e7c28147699c Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Tue, 25 Mar 2014 12:30:55 +0000 Subject: Add SpamAddress model The volume of spam in the holding pen in WDTK has increased. Over a few weeks in January 2014 the pattern was roughly: - 8 were sent "To" the same address, which was a nearly valid old request address - correct hash, but missing the second hyphen. - 1 was sent "To" an invalid request address (nearly correct hash) - 1 was sent "BCC" a valid request address - 1 was sent "BCC" request@whatdotheyknow.com If a spam was sent "To" an old valid request address then it would be rejected. It's not entirely safe to just reject mails to old requests with any hash, because sometimes authorities miss out a digit in the request number, though perhaps simply getting a failure bounce would cause them to check. In any case that wouldn't trivially catch the most frequent case above as it doesn't have an obvious request number. --- We looked at greylisting and configuring the MTA with an RBL. Greylisting was rejected as it would slow down the responsiveness of the application when people email in. This could be revisited if/when emails are parsed through a queue system depending on how we find the performance there. An RBL is already configured, but this ticket refers more to where the email is sent rather than who it came from. --- We elected to: - Create spam_address model - Add code to RequestMailer.receive to check the list of spam addresses and silently discard an incoming mail if it's addressed to one of them - Add page to admin interface for adding/removing spam addresses --- Thanks to Ganesh Sittampalam for the research and Louise Crow for the implementation strategy. --- spec/models/spam_address_spec.rb | 49 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 spec/models/spam_address_spec.rb (limited to 'spec/models') diff --git a/spec/models/spam_address_spec.rb b/spec/models/spam_address_spec.rb new file mode 100644 index 000000000..79a1afd11 --- /dev/null +++ b/spec/models/spam_address_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe SpamAddress do + + describe :new do + + it 'requres an email address' do + SpamAddress.new().should_not be_valid + SpamAddress.new(:email => 'spam@example.org').should be_valid + end + + it 'must have a unique email address' do + existing = FactoryGirl.create(:spam_address) + SpamAddress.new(:email => existing.email).should_not be_valid + end + + end + + describe '.spam?' do + + before(:each) do + @spam_address = FactoryGirl.create(:spam_address) + end + + it 'is a spam address if the address is stored' do + SpamAddress.spam?(@spam_address.email).should be_true + end + + it 'is not a spam address if the adress is not stored' do + SpamAddress.spam?('genuine-email@example.com').should be_false + end + + describe 'when accepting an array of emails' do + + it 'is spam if any of the emails are stored' do + emails = ['genuine-email@example.com', @spam_address.email] + SpamAddress.spam?(emails).should be_true + end + + it 'is not spam if none of the emails are stored' do + emails = ['genuine-email@example.com', 'genuine-email@example.org'] + SpamAddress.spam?(emails).should be_false + end + + end + + end + +end -- cgit v1.2.3 From 47dadad195826be1f4cccabf98f004cde0cf737b Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Wed, 26 Mar 2014 16:08:49 +0000 Subject: Add specs for ContactValidator --- spec/models/contact_validator_spec.rb | 49 +++++++++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 2 deletions(-) (limited to 'spec/models') diff --git a/spec/models/contact_validator_spec.rb b/spec/models/contact_validator_spec.rb index 9ea0fac49..0f5403967 100644 --- a/spec/models/contact_validator_spec.rb +++ b/spec/models/contact_validator_spec.rb @@ -1,8 +1,53 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') -describe ContactValidator, " when blah" do - before do +describe ContactValidator do + + describe :new do + + let(:valid_params) do + { :name => "Vinny Vanilli", + :email => "vinny@localhost", + :subject => "Why do I have such an ace name?", + :message => "You really should know!!!\n\nVinny" } + end + + it 'validates specified attributes' do + ContactValidator.new(valid_params).should be_valid + end + + it 'validates name is present' do + valid_params.except!(:name) + validator = ContactValidator.new(valid_params) + expect(validator).to have(1).error_on(:name) + end + + it 'validates email is present' do + valid_params.except!(:email) + validator = ContactValidator.new(valid_params) + # We have 2 errors on email because of the format validator + expect(validator).to have(2).errors_on(:email) + end + + it 'validates email format' do + valid_params.merge!({:email => 'not-an-email'}) + validator = ContactValidator.new(valid_params) + expect(validator.errors_on(:email)).to include("Email doesn't look like a valid address") + end + + it 'validates subject is present' do + valid_params.except!(:subject) + validator = ContactValidator.new(valid_params) + expect(validator).to have(1).error_on(:subject) + end + + it 'validates message is present' do + valid_params.except!(:message) + validator = ContactValidator.new(valid_params) + expect(validator).to have(1).error_on(:message) + end + end + end -- cgit v1.2.3 From aabf6e5967bf24f47c8c25d638ab8ba2c2db5968 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Tue, 8 Apr 2014 16:19:28 +0100 Subject: Annotate models Should have been run after related migrations. Could automate this to always run after migrations. --- spec/models/info_request_batch_spec.rb | 13 ++++++++++++ spec/models/info_request_spec.rb | 1 + spec/models/public_body_spec.rb | 2 +- spec/models/spam_address_spec.rb | 10 +++++++++ spec/models/user_spec.rb | 37 +++++++++++++++++----------------- 5 files changed, 44 insertions(+), 19 deletions(-) (limited to 'spec/models') diff --git a/spec/models/info_request_batch_spec.rb b/spec/models/info_request_batch_spec.rb index 53158ebe2..2881e7745 100644 --- a/spec/models/info_request_batch_spec.rb +++ b/spec/models/info_request_batch_spec.rb @@ -1,3 +1,16 @@ +# == Schema Information +# +# Table name: info_request_batches +# +# id :integer not null, primary key +# title :text not null +# user_id :integer not null +# created_at :datetime not null +# updated_at :datetime not null +# body :text +# sent_at :datetime +# + require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe InfoRequestBatch, "when validating" do diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index 9766f928f..12499f50a 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -21,6 +21,7 @@ # external_url :string(255) # attention_requested :boolean default(FALSE) # comments_allowed :boolean default(TRUE), not null +# info_request_batch_id :integer # require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb index dc09bdfa6..efd170013 100644 --- a/spec/models/public_body_spec.rb +++ b/spec/models/public_body_spec.rb @@ -5,7 +5,7 @@ # # id :integer not null, primary key # name :text not null -# short_name :text not null +# short_name :text default(""), not null # request_email :text not null # version :integer not null # last_edit_editor :string(255) not null diff --git a/spec/models/spam_address_spec.rb b/spec/models/spam_address_spec.rb index 79a1afd11..f28440121 100644 --- a/spec/models/spam_address_spec.rb +++ b/spec/models/spam_address_spec.rb @@ -1,3 +1,13 @@ +# == Schema Information +# +# Table name: spam_addresses +# +# id :integer not null, primary key +# email :string(255) not null +# created_at :datetime not null +# updated_at :datetime not null +# + require 'spec_helper' describe SpamAddress do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index b6f48dad3..c54043092 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2,24 +2,25 @@ # # 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 +# 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(Sat Jan 01 00:00:00 UTC 2000) +# 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 +# can_make_batch_requests :boolean default(FALSE), not null # require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') -- cgit v1.2.3 From f1fd69bff059cf0dbd4c0e543dbd9d9079ea3be8 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Mon, 14 Apr 2014 11:22:24 +0100 Subject: Add missing validation to PublicBody There's a unique index on public_bodies url_name, so we should have a validation for that. --- spec/models/public_body_spec.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'spec/models') diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb index efd170013..c443f0d6a 100644 --- a/spec/models/public_body_spec.rb +++ b/spec/models/public_body_spec.rb @@ -205,6 +205,12 @@ describe PublicBody, " when saving" do pb.first_letter.should == 'Å' end + it "should not save if the url_name is already taken" do + existing = FactoryGirl.create(:public_body) + pb = PublicBody.new(existing.attributes) + pb.should have(1).errors_on(:url_name) + 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" -- cgit v1.2.3 From 97075669802ec9e99bbcb5b52f539b3e3bbf2487 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Mon, 14 Apr 2014 12:02:48 +0100 Subject: Handle validation errors in PublicBody.import_csv Specifically using save! so that anything other than an ActiveRecord::RecordInvalid doesn't get missed Note that ActiveModel::Errors#full_messages includes the attribute key in the message. This is by design, so we should consider whether we can improve the way that we use translated validation messages. --- spec/models/public_body_spec.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'spec/models') diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb index c443f0d6a..38e31783d 100644 --- a/spec/models/public_body_spec.rb +++ b/spec/models/public_body_spec.rb @@ -533,6 +533,19 @@ describe PublicBody, " when loading CSV files" do PublicBody.count.should == original_count + 3 end + it "should handle active record validation errors" do + csv = <<-CSV +#name,request_email,short_name +Foobar,a@example.com,foobar +Foobar Test,b@example.com,foobar +CSV + + csv_contents = normalize_string_to_utf8(csv) + errors, notes = PublicBody.import_csv(csv_contents, '', 'replace', true, 'someadmin') # true means dry run + + errors.should include("error: line 3: Url name URL name is already taken for authority 'Foobar Test'") + end + end describe PublicBody do -- cgit v1.2.3