diff options
| -rw-r--r-- | README.md | 2 | ||||
| -rw-r--r-- | app/controllers/request_controller.rb | 2 | ||||
| -rw-r--r-- | app/controllers/track_controller.rb | 1 | ||||
| -rw-r--r-- | app/models/incoming_message.rb | 6 | ||||
| -rw-r--r-- | app/models/user.rb | 10 | ||||
| -rw-r--r-- | app/views/user/rate_limited.rhtml | 6 | ||||
| -rw-r--r-- | spec/controllers/request_controller_spec.rb | 25 | ||||
| -rw-r--r-- | spec/controllers/track_controller_spec.rb | 7 | ||||
| -rw-r--r-- | spec/controllers/user_controller_spec.rb | 2 | ||||
| -rw-r--r-- | spec/models/has_tag_string_tag_spec.rb | 2 | ||||
| -rw-r--r-- | spec/models/incoming_message_spec.rb | 34 |
11 files changed, 58 insertions, 39 deletions
@@ -14,7 +14,7 @@ norm, rather than the exception. Please join our mailing list at https://groups.google.com/group/alaveteli-dev and introduce yourself. -Some documentation can be found in the [`doc/` folder](./alaveteli/doc/). There's +Some documentation can be found in the [`doc/` folder](https://github.com/sebbacon/alaveteli/tree/master/doc). There's background information and a little more documentation on [our wiki](https://github.com/sebbacon/alaveteli/wiki/Home/), and hopefully we'll make [our homepage](http://alaveteli.org) more useful diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index 2295d6718..313a57d7d 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -220,6 +220,8 @@ class RequestController < ApplicationController render :template => 'user/banned' return end + # User did exceed limit + @next_request_permitted_at = authenticated_user.next_request_permitted_at end # First time we get to the page, just display it diff --git a/app/controllers/track_controller.rb b/app/controllers/track_controller.rb index e39a0489d..d858ab233 100644 --- a/app/controllers/track_controller.rb +++ b/app/controllers/track_controller.rb @@ -66,6 +66,7 @@ class TrackController < ApplicationController # Track a user def track_user @track_user = User.find_by_url_name(params[:url_name]) + raise ActiveRecord::RecordNotFound.new("No such user") if @track_user.nil? @track_thing = TrackThing.create_track_for_user(@track_user) return atom_feed_internal if params[:feed] == 'feed' diff --git a/app/models/incoming_message.rb b/app/models/incoming_message.rb index 131970ba6..cbbcf5aa6 100644 --- a/app/models/incoming_message.rb +++ b/app/models/incoming_message.rb @@ -57,7 +57,7 @@ class IncomingMessage < ActiveRecord::Base validates_presence_of :raw_email has_many :outgoing_message_followups, :foreign_key => 'incoming_message_followup_id', :class_name => 'OutgoingMessage' - has_many :foi_attachments + has_many :foi_attachments, :order => 'id' has_many :info_request_events # never really has many, but could in theory belongs_to :raw_email @@ -773,12 +773,12 @@ class IncomingMessage < ActiveRecord::Base # which is really messy. ensure_parts_counted attachments = [] - for leaf in leaves + for leaf in leaves body = leaf.body # As leaf.body causes MIME decoding which uses lots of RAM, do garbage collection here # to prevent excess memory use. XXX not really sure if this helps reduce # peak RAM use overall. Anyway, maybe there is something better to do than this. - GC.start + GC.start if leaf.within_rfc822_attachment within_rfc822_subject = leaf.within_rfc822_attachment.subject # Test to see if we are in the first part of the attached diff --git a/app/models/user.rb b/app/models/user.rb index 8c4b35fe6..28d130c46 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -288,6 +288,16 @@ class User < ActiveRecord::Base return (recent_requests >= daily_limit) end + def next_request_permitted_at + return nil if self.no_limit + + daily_limit = MySociety::Config.get("MAX_REQUESTS_PER_USER_PER_DAY") + n_most_recent_requests = InfoRequest.all(:conditions => ["user_id = ? and created_at > now() - '1 day'::interval", self.id], :order => "created_at DESC", :limit => daily_limit) + return nil if n_most_recent_requests.size < daily_limit + + nth_most_recent_request = n_most_recent_requests[-1] + return nth_most_recent_request.created_at + 1.day + end def can_make_followup? self.ban_text.empty? end diff --git a/app/views/user/rate_limited.rhtml b/app/views/user/rate_limited.rhtml index c1e8f360e..2a770d62e 100644 --- a/app/views/user/rate_limited.rhtml +++ b/app/views/user/rate_limited.rhtml @@ -2,11 +2,9 @@ <h1><%=@title%></h1> -<p><%= _("There is a limit on the number of requests that you can make in any one day. You can make more requests tomorrow.")%></p> +<p><%= _("You have hit the rate limit on new requests. Users are ordinarily limited to {{max_requests_per_user_per_day}} requests in any rolling 24-hour period. You will be able to make another request in {{can_make_another_request}}.", :max_requests_per_user_per_day => MySociety::Config.get("MAX_REQUESTS_PER_USER_PER_DAY"), :can_make_another_request => distance_of_time_in_words(Time.now, @next_request_permitted_at))%></p> -<!-- Insert explanation of why we have a limit --> - -<p><%= _("If you need to make more requests than this, <a href='%s'>get in touch</a> and we’ll consider it.") % [help_contact_path] %></p> +<p><%= _("There is a limit on the number of requests you can make in a day, because we don’t want public authorities to be bombarded with large numbers of inappropriate requests. If you feel you have a good reason to ask for the limit to be lifted in your case, please <a href='{{help_contact_path}}'>get in touch</a>.", :help_contact_path => help_contact_path) %></p> <% if @info_request %> <p><%= _("Here is the message you wrote, in case you would like to copy the text and save it for later.") %></p> diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index 25dce3f22..9018f76fe 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -300,7 +300,7 @@ describe RequestController, "when showing one request" do }.should raise_error(ActiveRecord::RecordNotFound) end - it "should generate valid HTML verson of PDF attachments " do + it "should generate valid HTML verson of PDF attachments" do ir = info_requests(:fancy_dog_request) receive_incoming_mail('incoming-request-pdf-attachment.email', ir.incoming_email) ir.reload @@ -309,7 +309,7 @@ describe RequestController, "when showing one request" do response.should have_text(/Walberswick Parish Council/) end - it "should not cause a reparsing of the raw email, even when the result would be a 404 " do + it "should not cause a reparsing of the raw email, even when the result would be a 404" do ir = info_requests(:fancy_dog_request) receive_incoming_mail('incoming-request-two-same-name.email', ir.incoming_email) ir.reload @@ -1319,9 +1319,10 @@ describe RequestController, "sending overdue request alerts" do RequestMailer.alert_overdue_requests - deliveries = ActionMailer::Base.deliveries - deliveries.size.should == 2 - mail = deliveries[1] + chicken_mails = ActionMailer::Base.deliveries.select{|x| x.body =~ /chickens/} + chicken_mails.size.should == 1 + mail = chicken_mails[0] + mail.body.should =~ /promptly, as normally/ mail.to_addrs.first.to_s.should == info_requests(:naughty_chicken_request).user.name_and_email @@ -1347,9 +1348,10 @@ describe RequestController, "sending overdue request alerts" do RequestMailer.alert_overdue_requests - deliveries = ActionMailer::Base.deliveries - deliveries.size.should == 2 - mail = deliveries[1] + chicken_mails = ActionMailer::Base.deliveries.select{|x| x.body =~ /chickens/} + chicken_mails.size.should == 1 + mail = chicken_mails[0] + mail.body.should =~ /promptly, as normally/ mail.to_addrs.first.to_s.should == info_requests(:naughty_chicken_request).user.name_and_email end @@ -1372,9 +1374,10 @@ describe RequestController, "sending overdue request alerts" do RequestMailer.alert_overdue_requests - deliveries = ActionMailer::Base.deliveries - deliveries.size.should == 2 - mail = deliveries[1] + chicken_mails = ActionMailer::Base.deliveries.select{|x| x.body =~ /chickens/} + chicken_mails.size.should == 1 + mail = chicken_mails[0] + mail.body.should =~ /required by law/ mail.to_addrs.first.to_s.should == info_requests(:naughty_chicken_request).user.name_and_email diff --git a/spec/controllers/track_controller_spec.rb b/spec/controllers/track_controller_spec.rb index 38a447640..0dc5db607 100644 --- a/spec/controllers/track_controller_spec.rb +++ b/spec/controllers/track_controller_spec.rb @@ -25,7 +25,7 @@ describe TrackController, "when making a new track on a request" do response.should redirect_to(:controller => 'user', :action => 'signin', :token => post_redirect.token) end - it "should save the track and redirect if you are logged in " do + it "should save the track and redirect if you are logged in" do session[:user_id] = @user.id @track_thing.should_receive(:save!) get :track_request, :url_title => @ir.url_title, :feed => 'track' @@ -134,6 +134,11 @@ describe TrackController, "when viewing RSS feed for a track" do assigns[:xapian_object].results[2][:model].should == info_request_events(:useless_outgoing_message_event) # created_at 2007-10-14 10:41:12.686264 end + it "should return NotFound for a non-existent user" do + lambda { + get :track_user, :feed => 'feed', :url_name => "there_is_no_such_user" + }.should raise_error(ActiveRecord::RecordNotFound) + end end describe TrackController, "when viewing JSON version of a track feed" do diff --git a/spec/controllers/user_controller_spec.rb b/spec/controllers/user_controller_spec.rb index 1a701ad43..fbe33c529 100644 --- a/spec/controllers/user_controller_spec.rb +++ b/spec/controllers/user_controller_spec.rb @@ -233,7 +233,7 @@ describe UserController, "when signing up" do deliveries[0].body.should include("No revelaremos su dirección de correo") end - it "should send special 'already signed up' mail if you fill the form in with existing registered email " do + it "should send special 'already signed up' mail if you fill the form in with existing registered email" do post :signup, { :user_signup => { :email => 'silly@localhost', :name => 'New Person', :password => 'sillypassword', :password_confirmation => 'sillypassword' } } diff --git a/spec/models/has_tag_string_tag_spec.rb b/spec/models/has_tag_string_tag_spec.rb index 57c301471..759b3396f 100644 --- a/spec/models/has_tag_string_tag_spec.rb +++ b/spec/models/has_tag_string_tag_spec.rb @@ -1,6 +1,6 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') -describe HasTagString::HasTagStringTag, " when fiddling with tag strings " do +describe HasTagString::HasTagStringTag, " when fiddling with tag strings" do it "should be able to make a new tag and save it" do @tag = HasTagString::HasTagStringTag.new diff --git a/spec/models/incoming_message_spec.rb b/spec/models/incoming_message_spec.rb index b6fee7898..6cfd2eb64 100644 --- a/spec/models/incoming_message_spec.rb +++ b/spec/models/incoming_message_spec.rb @@ -362,6 +362,7 @@ describe IncomingMessage, " when uudecoding bad messages" do im = incoming_messages(:useless_incoming_message) im.stub!(:mail).and_return(mail) im.extract_attachments! + attachments = im.foi_attachments attachments.size.should == 2 attachments[1].filename.should == 'moo.txt' @@ -385,9 +386,9 @@ describe IncomingMessage, " when uudecoding bad messages" do ir.censor_rules << @censor_rule im.extract_attachments! - attachments = im.get_attachments_for_display - attachments.size.should == 1 - attachments[0].display_filename.should == 'bah.txt' + im.get_attachments_for_display.map(&:display_filename).should == [ + 'bah.txt', + ] end end @@ -409,10 +410,11 @@ describe IncomingMessage, "when messages are attached to messages" do im.extract_attachments! attachments = im.get_attachments_for_display - attachments.size.should == 3 - attachments[0].display_filename.should == 'Same attachment twice.txt' - attachments[1].display_filename.should == 'hello.txt' - attachments[2].display_filename.should == 'hello.txt' + attachments.map(&:display_filename).should == [ + 'Same attachment twice.txt', + 'hello.txt', + 'hello.txt', + ] end end @@ -431,10 +433,10 @@ describe IncomingMessage, "when Outlook messages are attached to messages" do im.stub!(:mail).and_return(mail) im.extract_attachments! - attachments = im.get_attachments_for_display - attachments.size.should == 2 - attachments[0].display_filename.should == 'test.html' # picks HTML rather than text by default, as likely to render better - attachments[1].display_filename.should == 'attach.txt' + im.get_attachments_for_display.map(&:display_filename).should == [ + 'test.html', # picks HTML rather than text by default, as likely to render better + 'attach.txt', + ] end end @@ -453,12 +455,10 @@ describe IncomingMessage, "when TNEF attachments are attached to messages" do im.stub!(:mail).and_return(mail) im.extract_attachments! - attachments = im.get_attachments_for_display - attachments.size.should == 2 - attachments[0].display_filename.should == 'FOI 09 02976i.doc' - attachments[1].display_filename.should == 'FOI 09 02976iii.doc' + im.get_attachments_for_display.map(&:display_filename).should == [ + 'FOI 09 02976i.doc', + 'FOI 09 02976iii.doc', + ] end end - - |