diff options
57 files changed, 231 insertions, 259 deletions
diff --git a/app/controllers/admin_comment_controller.rb b/app/controllers/admin_comment_controller.rb index dc505be9f..eed328b34 100644 --- a/app/controllers/admin_comment_controller.rb +++ b/app/controllers/admin_comment_controller.rb @@ -21,7 +21,7 @@ class AdminCommentController < AdminController if @comment.update_attributes(params[:comment]) @comment.info_request.log_event("edit_comment", { :comment_id => @comment.id, - :editor => admin_current_user(), + :editor => admin_current_user, :old_body => old_body, :body => @comment.body, :old_visible => old_visible, diff --git a/app/controllers/admin_incoming_message_controller.rb b/app/controllers/admin_incoming_message_controller.rb index 8dfd53662..db7bed34c 100644 --- a/app/controllers/admin_incoming_message_controller.rb +++ b/app/controllers/admin_incoming_message_controller.rb @@ -14,7 +14,7 @@ class AdminIncomingMessageController < AdminController if @incoming_message.save @incoming_message.info_request.log_event('edit_incoming', :incoming_message_id => @incoming_message.id, - :editor => admin_current_user(), + :editor => admin_current_user, :old_prominence => old_prominence, :prominence => @incoming_message.prominence, :old_prominence_reason => old_prominence_reason, @@ -34,7 +34,7 @@ class AdminIncomingMessageController < AdminController @incoming_message.fully_destroy @incoming_message.info_request.log_event("destroy_incoming", - { :editor => admin_current_user(), :deleted_incoming_message_id => incoming_message_id }) + { :editor => admin_current_user, :deleted_incoming_message_id => incoming_message_id }) # expire cached files expire_for_request(@info_request) flash[:notice] = 'Incoming message successfully destroyed.' @@ -64,7 +64,7 @@ class AdminIncomingMessageController < AdminController incoming_message_id = incoming_message.id incoming_message.info_request.log_event("redeliver_incoming", { - :editor => admin_current_user(), + :editor => admin_current_user, :destination_request => destination_request.id, :deleted_incoming_message_id => incoming_message_id }) diff --git a/app/controllers/admin_outgoing_message_controller.rb b/app/controllers/admin_outgoing_message_controller.rb index 6c0b23ceb..56e27b108 100644 --- a/app/controllers/admin_outgoing_message_controller.rb +++ b/app/controllers/admin_outgoing_message_controller.rb @@ -12,7 +12,7 @@ class AdminOutgoingMessageController < AdminController @outgoing_message.fully_destroy @outgoing_message.info_request.log_event("destroy_outgoing", - { :editor => admin_current_user(), :deleted_outgoing_message_id => outgoing_message_id }) + { :editor => admin_current_user, :deleted_outgoing_message_id => outgoing_message_id }) flash[:notice] = 'Outgoing message successfully destroyed.' redirect_to admin_request_url(@info_request) @@ -30,7 +30,7 @@ class AdminOutgoingMessageController < AdminController if @outgoing_message.save @outgoing_message.info_request.log_event("edit_outgoing", { :outgoing_message_id => @outgoing_message.id, - :editor => admin_current_user(), + :editor => admin_current_user, :old_body => old_body, :body => @outgoing_message.body, :old_prominence => old_prominence, diff --git a/app/controllers/admin_public_body_controller.rb b/app/controllers/admin_public_body_controller.rb index efd90f4ca..0565bc775 100644 --- a/app/controllers/admin_public_body_controller.rb +++ b/app/controllers/admin_public_body_controller.rb @@ -12,7 +12,7 @@ class AdminPublicBodyController < AdminController end def show - @locale = self.locale_from_params() + @locale = locale_from_params I18n.with_locale(@locale) do @public_body = PublicBody.find(params[:id]) @info_requests = @public_body.info_requests.paginate :order => "created_at desc", @@ -44,7 +44,7 @@ class AdminPublicBodyController < AdminController if params[:change_request_id] @change_request = PublicBodyChangeRequest.find(params[:change_request_id]) end - params[:public_body][:last_edit_editor] = admin_current_user() + params[:public_body][:last_edit_editor] = admin_current_user @public_body = PublicBody.new(params[:public_body]) if @public_body.save if @change_request @@ -85,7 +85,7 @@ class AdminPublicBodyController < AdminController @change_request = PublicBodyChangeRequest.find(params[:change_request_id]) end I18n.with_locale(I18n.default_locale) do - params[:public_body][:last_edit_editor] = admin_current_user() + params[:public_body][:last_edit_editor] = admin_current_user @public_body = PublicBody.find(params[:id]) if @public_body.update_attributes(params[:public_body]) if @change_request @@ -102,7 +102,7 @@ class AdminPublicBodyController < AdminController end def destroy - @locale = self.locale_from_params() + @locale = locale_from_params I18n.with_locale(@locale) do public_body = PublicBody.find(params[:id]) @@ -178,7 +178,7 @@ class AdminPublicBodyController < AdminController params[:tag], params[:tag_behaviour], true, - admin_current_user(), + admin_current_user, I18n.available_locales) if errors.size == 0 @@ -192,7 +192,7 @@ class AdminPublicBodyController < AdminController params[:tag], params[:tag_behaviour], false, - admin_current_user(), + admin_current_user, I18n.available_locales) if errors.size != 0 raise "dry run mismatched real run" @@ -235,7 +235,7 @@ class AdminPublicBodyController < AdminController end def lookup_query - @locale = self.locale_from_params() + @locale = locale_from_params underscore_locale = @locale.gsub '-', '_' I18n.with_locale(@locale) do @query = params[:query] diff --git a/app/controllers/admin_request_controller.rb b/app/controllers/admin_request_controller.rb index 4dcb0681f..59eb5e6b8 100644 --- a/app/controllers/admin_request_controller.rb +++ b/app/controllers/admin_request_controller.rb @@ -57,7 +57,7 @@ class AdminRequestController < AdminController if @info_request.valid? @info_request.save! @info_request.log_event("edit", - { :editor => admin_current_user(), + { :editor => admin_current_user, :old_title => old_title, :title => @info_request.title, :old_prominence => old_prominence, :prominence => @info_request.prominence, :old_described_state => old_described_state, :described_state => params[:info_request][:described_state], @@ -105,7 +105,7 @@ class AdminRequestController < AdminController info_request.user = destination_user info_request.save! info_request.log_event("move_request", { - :editor => admin_current_user(), + :editor => admin_current_user, :old_user_url_name => old_user.url_name, :user_url_name => destination_user.url_name }) @@ -176,7 +176,7 @@ class AdminRequestController < AdminController info_request.prominence = "requester_only" info_request.log_event("hide", { - :editor => admin_current_user(), + :editor => admin_current_user, :reason => params[:reason], :subject => subject, :explanation => explanation diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 779ae04a6..3b8991f28 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -42,7 +42,7 @@ class ApiController < ApplicationController :status => 'ready', :message_type => 'initial_request', :body => json["body"], - :last_sent_at => Time.now(), + :last_sent_at => Time.now, :what_doing => 'normal_sort', :info_request => request ) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 07956d370..a76e6630a 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -206,7 +206,7 @@ class ApplicationController < ActionController::Base def foi_fragment_cache_part_path(param) path = url_for(param) id = param['id'] || param[:id] - first_three_digits = id.to_s()[0..2] + first_three_digits = id.to_s[0..2] path = path.sub("/request/", "/request/" + first_three_digits + "/") return path end diff --git a/app/controllers/general_controller.rb b/app/controllers/general_controller.rb index 324f5f2e4..53288c2f4 100644 --- a/app/controllers/general_controller.rb +++ b/app/controllers/general_controller.rb @@ -15,7 +15,7 @@ class GeneralController < ApplicationController # New, improved front page! def frontpage medium_cache - @locale = self.locale_from_params() + @locale = locale_from_params successful_query = InfoRequestEvent.make_query_from_params( :latest_status => ['successful'] ) @track_thing = TrackThing.create_track_for_search_query(successful_query) @feed_autodetect = [ { :url => do_track_url(@track_thing, 'feed'), @@ -33,7 +33,7 @@ class GeneralController < ApplicationController @feed_autodetect = [] @feed_url = AlaveteliConfiguration::blog_feed separator = @feed_url.include?('?') ? '&' : '?' - @feed_url = "#{@feed_url}#{separator}lang=#{self.locale_from_params()}" + @feed_url = "#{@feed_url}#{separator}lang=#{locale_from_params}" @blog_items = [] if not @feed_url.empty? content = quietly_try_to_open(@feed_url) diff --git a/app/controllers/public_body_controller.rb b/app/controllers/public_body_controller.rb index 754d91da9..1b01dc837 100644 --- a/app/controllers/public_body_controller.rb +++ b/app/controllers/public_body_controller.rb @@ -24,7 +24,7 @@ class PublicBodyController < ApplicationController redirect_to :url_name => MySociety::Format.simplify_url_part(params[:url_name], 'body'), :status => :moved_permanently return end - @locale = self.locale_from_params() + @locale = locale_from_params I18n.with_locale(@locale) do @public_body = PublicBody.find_by_url_name_with_historic(params[:url_name]) raise ActiveRecord::RecordNotFound.new("None found") if @public_body.nil? @@ -85,7 +85,7 @@ class PublicBodyController < ApplicationController @public_body = PublicBody.find_by_url_name_with_historic(params[:url_name]) raise ActiveRecord::RecordNotFound.new("None found") if @public_body.nil? - I18n.with_locale(self.locale_from_params()) do + I18n.with_locale(locale_from_params) do if params[:submitted_view_email] if verify_recaptcha flash.discard(:error) @@ -108,7 +108,7 @@ class PublicBodyController < ApplicationController @tag = params[:tag] - @locale = self.locale_from_params + @locale = locale_from_params underscore_locale = @locale.gsub '-', '_' underscore_default_locale = I18n.default_locale.to_s.gsub '-', '_' diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index aa88aeeeb..bdb309dd8 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -75,7 +75,7 @@ class RequestController < ApplicationController else medium_cache end - @locale = self.locale_from_params() + @locale = locale_from_params I18n.with_locale(@locale) do # Look up by old style numeric identifiers @@ -164,7 +164,7 @@ class RequestController < ApplicationController def list medium_cache @view = params[:view] - @locale = self.locale_from_params() + @locale = locale_from_params @page = get_search_page_from_params if !@page # used in cache case, as perform_search sets @page as side effect @per_page = PER_PAGE @max_results = MAX_RESULTS @@ -848,7 +848,7 @@ class RequestController < ApplicationController # FOI officers can upload a response def upload_response - @locale = self.locale_from_params() + @locale = locale_from_params I18n.with_locale(@locale) do @info_request = InfoRequest.find_by_url_title!(params[:url_title]) @@ -914,7 +914,7 @@ class RequestController < ApplicationController end def download_entire_request - @locale = self.locale_from_params() + @locale = locale_from_params I18n.with_locale(@locale) do @info_request = InfoRequest.find_by_url_title!(params[:url_title]) if authenticated?( diff --git a/app/controllers/request_game_controller.rb b/app/controllers/request_game_controller.rb index cb3ba1f3a..2915b5c2f 100644 --- a/app/controllers/request_game_controller.rb +++ b/app/controllers/request_game_controller.rb @@ -23,7 +23,7 @@ class RequestGameController < ApplicationController :site_name => site_name) end - @league_table_28_days = RequestClassification.league_table(10, [ "created_at >= ?", Time.now() - 28.days ]) + @league_table_28_days = RequestClassification.league_table(10, [ "created_at >= ?", Time.now - 28.days ]) @league_table_all_time = RequestClassification.league_table(10) @play_urls = true end diff --git a/app/controllers/user_controller.rb b/app/controllers/user_controller.rb index 91eb7baf4..47041a969 100644 --- a/app/controllers/user_controller.rb +++ b/app/controllers/user_controller.rb @@ -373,7 +373,7 @@ class UserController < ApplicationController if (not session[:user_circumstance]) or (session[:user_circumstance] != "change_email") # don't store the password in the db params[:signchangeemail].delete(:password) - post_redirect = PostRedirect.new(:uri => signchangeemail_url(), + post_redirect = PostRedirect.new(:uri => signchangeemail_url, :post_params => params, :circumstance => "change_email" # special login that lets you change your email ) @@ -510,7 +510,7 @@ class UserController < ApplicationController else flash[:notice] = _("<p>Thanks for updating your profile photo.</p> <p><strong>Next...</strong> You can put some text about you and your research on your profile.</p>") - redirect_to set_profile_about_me_url() + redirect_to set_profile_about_me_url end else render :template => 'user/set_draft_profile_photo' @@ -596,7 +596,7 @@ class UserController < ApplicationController else flash[:notice] = _("<p>Thanks for changing the text about you on your profile.</p> <p><strong>Next...</strong> You can upload a profile photograph too.</p>") - redirect_to set_profile_photo_url() + redirect_to set_profile_photo_url end end diff --git a/app/helpers/public_body_helper.rb b/app/helpers/public_body_helper.rb index 3fb04d45a..e094d98b4 100644 --- a/app/helpers/public_body_helper.rb +++ b/app/helpers/public_body_helper.rb @@ -41,8 +41,8 @@ module PublicBodyHelper def type_of_authority(public_body) first = true types = public_body.tags.each.map do |tag| - if PublicBodyCategory.get().by_tag().include?(tag.name) - desc = PublicBodyCategory.get().singular_by_tag()[tag.name] + if PublicBodyCategory.get.by_tag.include?(tag.name) + desc = PublicBodyCategory.get.singular_by_tag[tag.name] if first desc = desc.sub(/\S/) { |m| Unicode.upcase(m) } first = false diff --git a/app/mailers/request_mailer.rb b/app/mailers/request_mailer.rb index f97556915..f12b4a0d6 100644 --- a/app/mailers/request_mailer.rb +++ b/app/mailers/request_mailer.rb @@ -100,13 +100,8 @@ class RequestMailer < ApplicationMailer @url = confirm_url(:email_token => post_redirect.email_token) @info_request = info_request - headers('Return-Path' => blackhole_email, 'Reply-To' => contact_from_name_and_email, # not much we can do if the user's email is broken - 'Auto-Submitted' => 'auto-generated', # http://tools.ietf.org/html/rfc3834 - 'X-Auto-Response-Suppress' => 'OOF') - - mail(:from => contact_from_name_and_email, - :to => user.name_and_email, - :subject => _("Delayed response to your FOI request - ") + info_request.title.html_safe) + auto_generated_headers + mail_user_with_info_request_title(user, _("Delayed response to your FOI request - "), info_request) end # Tell the requester that the public body is very late in replying @@ -120,13 +115,8 @@ class RequestMailer < ApplicationMailer @url = confirm_url(:email_token => post_redirect.email_token) @info_request = info_request - headers('Return-Path' => blackhole_email, 'Reply-To' => contact_from_name_and_email, # not much we can do if the user's email is broken - 'Auto-Submitted' => 'auto-generated', # http://tools.ietf.org/html/rfc3834 - 'X-Auto-Response-Suppress' => 'OOF') - - mail(:from => contact_from_name_and_email, - :to => user.name_and_email, - :subject => _("You're long overdue a response to your FOI request - ") + info_request.title.html_safe) + auto_generated_headers + mail_user_with_info_request_title(user, _("You're long overdue a response to your FOI request - "), info_request) end # Tell the requester that they need to say if the new response @@ -142,13 +132,8 @@ class RequestMailer < ApplicationMailer @incoming_message = incoming_message @info_request = info_request - headers('Return-Path' => blackhole_email, 'Reply-To' => contact_from_name_and_email, # not much we can do if the user's email is broken - 'Auto-Submitted' => 'auto-generated', # http://tools.ietf.org/html/rfc3834 - 'X-Auto-Response-Suppress' => 'OOF') - - mail(:from => contact_from_name_and_email, - :to => info_request.user.name_and_email, - :subject => _("Was the response you got to your FOI request any good?")) + auto_generated_headers + mail_user(info_request.user, _("Was the response you got to your FOI request any good?")) end # Tell the requester that someone updated their old unclassified request @@ -156,13 +141,8 @@ class RequestMailer < ApplicationMailer @url = request_url(info_request) @info_request = info_request - headers('Return-Path' => blackhole_email, 'Reply-To' => contact_from_name_and_email, # not much we can do if the user's email is broken - 'Auto-Submitted' => 'auto-generated', # http://tools.ietf.org/html/rfc3834 - 'X-Auto-Response-Suppress' => 'OOF') - - mail(:from => contact_from_name_and_email, - :to => info_request.user.name_and_email, - :subject => _("Someone has updated the status of your request")) + auto_generated_headers + mail_user(info_request.user, _("Someone has updated the status of your request")) end # Tell the requester that they need to clarify their request @@ -178,13 +158,8 @@ class RequestMailer < ApplicationMailer @incoming_message = incoming_message @info_request = info_request - headers('Return-Path' => blackhole_email, 'Reply-To' => contact_from_name_and_email, # not much we can do if the user's email is broken - 'Auto-Submitted' => 'auto-generated', # http://tools.ietf.org/html/rfc3834 - 'X-Auto-Response-Suppress' => 'OOF') - - mail(:from => contact_from_name_and_email, - :to => info_request.user.name_and_email, - :subject => _("Clarify your FOI request - ") + info_request.title.html_safe) + auto_generated_headers + mail_user_with_info_request_title(info_request.user, _("Clarify your FOI request - "), info_request) end # Tell requester that somebody add an annotation to their request @@ -192,25 +167,15 @@ class RequestMailer < ApplicationMailer @comment, @info_request = comment, info_request @url = comment_url(comment) - headers('Return-Path' => blackhole_email, 'Reply-To' => contact_from_name_and_email, # not much we can do if the user's email is broken - 'Auto-Submitted' => 'auto-generated', # http://tools.ietf.org/html/rfc3834 - 'X-Auto-Response-Suppress' => 'OOF') - - mail(:from => contact_from_name_and_email, - :to => info_request.user.name_and_email, - :subject => _("Somebody added a note to your FOI request - ") + info_request.title.html_safe) + auto_generated_headers + mail_user_with_info_request_title(info_request.user, _("Somebody added a note to your FOI request - "), info_request) end def comment_on_alert_plural(info_request, count, earliest_unalerted_comment) @count, @info_request = count, info_request @url = comment_url(earliest_unalerted_comment) - headers('Return-Path' => blackhole_email, 'Reply-To' => contact_from_name_and_email, # not much we can do if the user's email is broken - 'Auto-Submitted' => 'auto-generated', # http://tools.ietf.org/html/rfc3834 - 'X-Auto-Response-Suppress' => 'OOF') - - mail(:from => contact_from_name_and_email, - :to => info_request.user.name_and_email, - :subject => _("Some notes have been added to your FOI request - ") + info_request.title.html_safe) + auto_generated_headers + mail_user_with_info_request_title(info_request.user, _("Some notes have been added to your FOI request - "), info_request) end # Class function, called by script/mailin with all incoming responses. @@ -269,7 +234,7 @@ class RequestMailer < ApplicationMailer end # Send email alerts for overdue requests - def self.alert_overdue_requests() + def self.alert_overdue_requests info_requests = InfoRequest.find(:all, :conditions => [ "described_state = 'waiting_response' @@ -373,8 +338,8 @@ class RequestMailer < ApplicationMailer # Send email alerts for requests which need clarification. Goes out 3 days # after last update of event. - def self.alert_not_clarified_request() - info_requests = InfoRequest.find(:all, :conditions => [ "awaiting_description = ? and described_state = 'waiting_clarification' and info_requests.updated_at < ?", false, Time.now() - 3.days ], :include => [ :user ], :order => "info_requests.id" ) + def self.alert_not_clarified_request + info_requests = InfoRequest.find(:all, :conditions => [ "awaiting_description = ? and described_state = 'waiting_clarification' and info_requests.updated_at < ?", false, Time.now - 3.days ], :include => [ :user ], :order => "info_requests.id" ) for info_request in info_requests alert_event_id = info_request.get_last_public_response_event_id last_response_message = info_request.get_last_public_response @@ -401,7 +366,7 @@ class RequestMailer < ApplicationMailer end # Send email alert to request submitter for new comments on the request. - def self.alert_comment_on_request() + def self.alert_comment_on_request # We only check comments made in the last month - this means if the # cron jobs broke for more than a month events would be lost, but no @@ -468,7 +433,27 @@ class RequestMailer < ApplicationMailer end end + private -end + def auto_generated_headers + headers({ + 'Return-Path' => blackhole_email, + 'Reply-To' => contact_from_name_and_email, # not much we can do if the user's email is broken + 'Auto-Submitted' => 'auto-generated', # http://tools.ietf.org/html/rfc3834 + 'X-Auto-Response-Suppress' => 'OOF', + }) + end + def mail_user_with_info_request_title(user, subject, info_request) + mail_user(user, subject + info_request.title.html_safe) + end + + def mail_user(user, subject) + mail({ + :from => contact_from_name_and_email, + :to => user.name_and_email, + :subject => subject, + }) + end +end diff --git a/app/mailers/track_mailer.rb b/app/mailers/track_mailer.rb index fc7895203..3f5de6a1b 100644 --- a/app/mailers/track_mailer.rb +++ b/app/mailers/track_mailer.rb @@ -39,7 +39,7 @@ class TrackMailer < ApplicationMailer # User.find(:all, :conditions => [ "last_daily_track_email < ?", Time.now - 1.day ]).size def self.alert_tracks done_something = false - now = Time.now() + now = Time.now one_week_ago = now - 7.days User.find_each(:conditions => [ "last_daily_track_email < ?", now - 1.day ]) do |user| diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 38627df50..f11b3a854 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -197,19 +197,6 @@ class InfoRequest < ActiveRecord::Base rescue MissingSourceFile, NameError end - # only check on create, so existing models with mixed case are allowed - def validate_on_create - if !self.title.nil? && !MySociety::Validate.uses_mixed_capitals(self.title, 10) - errors.add(:title, _('Please write the summary using a mixture of capital and lower case letters. This makes it easier for others to read.')) - end - if !self.title.nil? && title.size > 200 - errors.add(:title, _('Please keep the summary short, like in the subject of an email. You can use a phrase, rather than a full sentence.')) - end - if !self.title.nil? && self.title =~ /^(FOI|Freedom of Information)\s*requests?$/i - errors.add(:title, _('Please describe more what the request is about in the subject. There is no need to say it is an FOI request, we add that on anyway.')) - end - end - OLD_AGE_IN_DAYS = 21.days def visible_comments @@ -553,7 +540,7 @@ public :status => 'ready', :message_type => 'initial_request', :body => 'This is the holding pen request. It shows responses that were sent to invalid addresses, and need moving to the correct request by an adminstrator.', - :last_sent_at => Time.now(), + :last_sent_at => Time.now, :what_doing => 'normal_sort' }) @@ -669,11 +656,11 @@ public if !curr_state.nil? && event.event_type == 'response' if event.calculated_state != curr_state event.calculated_state = curr_state - event.last_described_at = Time.now() + event.last_described_at = Time.now event.save! end if event.last_described_at.nil? # TODO: actually maybe this isn't needed - event.last_described_at = Time.now() + event.last_described_at = Time.now event.save! end curr_state = nil @@ -685,7 +672,7 @@ public # indexed. if event.calculated_state != event.described_state event.calculated_state = event.described_state - event.last_described_at = Time.now() + event.last_described_at = Time.now event.save! end @@ -702,7 +689,7 @@ public # case there is a preceding response that the described state should be applied to. if event.calculated_state != event.described_state event.calculated_state = event.described_state - event.last_described_at = Time.now() + event.last_described_at = Time.now event.save! end end @@ -995,20 +982,20 @@ public LIMIT 1)" end - def InfoRequest.last_public_response_clause() + def InfoRequest.last_public_response_clause join_clause = "incoming_messages.id = info_request_events.incoming_message_id AND incoming_messages.prominence = 'normal'" last_event_time_clause('response', 'incoming_messages', join_clause) end def InfoRequest.old_unclassified_params(extra_params, include_last_response_time=false) - last_response_created_at = last_public_response_clause() + last_response_created_at = last_public_response_clause age = extra_params[:age_in_days] ? extra_params[:age_in_days].days : OLD_AGE_IN_DAYS params = { :conditions => ["awaiting_description = ? AND #{last_response_created_at} < ? AND url_title != 'holding_pen' AND user_id IS NOT NULL", - true, Time.now() - age] } + true, Time.now - age] } if include_last_response_time params[:select] = "*, #{last_response_created_at} AS last_response_time" params[:order] = 'last_response_time' @@ -1018,21 +1005,13 @@ public def InfoRequest.count_old_unclassified(extra_params={}) params = old_unclassified_params(extra_params) - if extra_params[:conditions] - condition_string = extra_params[:conditions].shift - params[:conditions][0] += " AND #{condition_string}" - params[:conditions] += extra_params[:conditions] - end + add_conditions_from_extra_params(params, extra_params) count(:all, params) end def InfoRequest.get_random_old_unclassified(limit, extra_params) params = old_unclassified_params({}) - if extra_params[:conditions] - condition_string = extra_params[:conditions].shift - params[:conditions][0] += " AND #{condition_string}" - params[:conditions] += extra_params[:conditions] - end + add_conditions_from_extra_params(params, extra_params) params[:limit] = limit params[:order] = "random()" find(:all, params) @@ -1047,15 +1026,11 @@ public params[:order] = extra_params[:order] params.delete(:select) end - if extra_params[:conditions] - condition_string = extra_params[:conditions].shift - params[:conditions][0] += " AND #{condition_string}" - params[:conditions] += extra_params[:conditions] - end + add_conditions_from_extra_params(params, extra_params) find(:all, params) end - def InfoRequest.download_zip_dir() + def InfoRequest.download_zip_dir File.join(Rails.root, "cache", "zips", "#{Rails.env}") end @@ -1073,7 +1048,7 @@ public end def request_dirs - first_three_digits = id.to_s()[0..2] + first_three_digits = id.to_s[0..2] File.join(first_three_digits.to_s, id.to_s) end @@ -1082,7 +1057,7 @@ public end def make_zip_cache_path(user) - cache_file_dir = File.join(InfoRequest.download_zip_dir(), + cache_file_dir = File.join(InfoRequest.download_zip_dir, "download", request_dirs, last_update_hash) @@ -1255,7 +1230,7 @@ public :model => self.class.base_class.to_s, :model_id => self.id) end - req.save() + req.save end end @@ -1411,7 +1386,7 @@ public self.described_state = 'waiting_response' end rescue ActiveModel::MissingAttributeError - # this should only happen on Model.exists?() call. It can be safely ignored. + # this should only happen on Model.exists? call. It can be safely ignored. # See http://www.tatvartha.com/2011/03/activerecordmissingattributeerror-missing-attribute-a-bug-or-a-features/ end @@ -1432,5 +1407,13 @@ public errors.add(:title, _('Please describe more what the request is about in the subject. There is no need to say it is an FOI request, we add that on anyway.')) end end + + def self.add_conditions_from_extra_params(params, extra_params) + if extra_params[:conditions] + condition_string = extra_params[:conditions].shift + params[:conditions][0] += " AND #{condition_string}" + params[:conditions] += extra_params[:conditions] + end + end end diff --git a/app/models/info_request_batch.rb b/app/models/info_request_batch.rb index 5a20edce8..b831f3042 100644 --- a/app/models/info_request_batch.rb +++ b/app/models/info_request_batch.rb @@ -70,7 +70,7 @@ class InfoRequestBatch < ActiveRecord::Base info_request end - def InfoRequestBatch.send_batches() + def InfoRequestBatch.send_batches find_each(:conditions => "sent_at IS NULL") do |info_request_batch| unrequestable = info_request_batch.create_batch! mail_message = InfoRequestBatchMailer.batch_sent(info_request_batch, diff --git a/app/models/info_request_event.rb b/app/models/info_request_event.rb index 285e54f9a..29c1ce965 100644 --- a/app/models/info_request_event.rb +++ b/app/models/info_request_event.rb @@ -326,9 +326,17 @@ class InfoRequestEvent < ActiveRecord::Base end - def is_incoming_message?() not self.incoming_message_selective_columns("incoming_messages.id").nil? end - def is_outgoing_message?() not self.outgoing_message.nil? end - def is_comment?() not self.comment.nil? end + def is_incoming_message? + !self.incoming_message_selective_columns("incoming_messages.id").nil? + end + + def is_outgoing_message? + !self.outgoing_message.nil? + end + + def is_comment? + !self.comment.nil? + end # Display version of status def display_status diff --git a/app/models/mail_server_log.rb b/app/models/mail_server_log.rb index 1c69635f3..ca3ea5d19 100644 --- a/app/models/mail_server_log.rb +++ b/app/models/mail_server_log.rb @@ -178,7 +178,7 @@ class MailServerLog < ActiveRecord::Base # Get all requests sent for from 2 to 10 days ago. The 2 day gap is # because we load mail server log lines via cron at best an hour after they # are made) - irs = InfoRequest.find(:all, :conditions => [ "created_at < ? and created_at > ? and user_id is not null", Time.now() - 2.day, Time.now() - 10.days ] ) + irs = InfoRequest.find(:all, :conditions => [ "created_at < ? and created_at > ? and user_id is not null", Time.now - 2.day, Time.now - 10.days ] ) # Go through each request and check it ok = true diff --git a/app/models/public_body.rb b/app/models/public_body.rb index feec7cdd5..103d86bb4 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -387,7 +387,7 @@ class PublicBody < ActiveRecord::Base # matching names won't work afterwards, and we'll create new bodies instead # of updating them bodies_by_name = {} - set_of_existing = Set.new() + set_of_existing = Set.new internal_admin_body_id = PublicBody.internal_admin_body.id I18n.with_locale(I18n.default_locale) do bodies = (tag.nil? || tag.empty?) ? PublicBody.find(:all, :include => :translations) : PublicBody.find_by_tag(tag) @@ -400,7 +400,7 @@ class PublicBody < ActiveRecord::Base end end - set_of_importing = Set.new() + set_of_importing = Set.new # Default values in case no field list is given field_names = { 'name' => 1, 'request_email' => 2 } line = 0 diff --git a/app/models/public_body_category/category_collection.rb b/app/models/public_body_category/category_collection.rb index 3938722c0..7d5732a82 100644 --- a/app/models/public_body_category/category_collection.rb +++ b/app/models/public_body_category/category_collection.rb @@ -14,19 +14,19 @@ class PublicBodyCategory::CategoryCollection end def with_description - @categories.select() { |a| a.instance_of?(Array) } + @categories.select { |a| a.instance_of?(Array) } end def tags - tags = with_description.map() { |a| a[0] } + tags = with_description.map { |a| a[0] } end def by_tag - Hash[*with_description.map() { |a| a[0..1] }.flatten] + Hash[*with_description.map { |a| a[0..1] }.flatten] end def singular_by_tag - Hash[*with_description.map() { |a| [a[0],a[2]] }.flatten] + Hash[*with_description.map { |a| [a[0],a[2]] }.flatten] end def by_heading diff --git a/app/views/public_body_change_requests/new.html.erb b/app/views/public_body_change_requests/new.html.erb index b52d583be..9abe03732 100644 --- a/app/views/public_body_change_requests/new.html.erb +++ b/app/views/public_body_change_requests/new.html.erb @@ -12,7 +12,7 @@ <p> <label class="form_label" for="user_email"> - <%= ("Your email:") %> + <%= _("Your email:") %> </label> <%= f.text_field :user_email %> </p> diff --git a/commonlib b/commonlib -Subproject 2a0271d3a6aaba9c429261b8c94e0e7acb6bd68 +Subproject dcc2e223ae1f76ed7db80b45ea12297e01bf5d4 diff --git a/config/initializers/alaveteli.rb b/config/initializers/alaveteli.rb index f4def4c2a..cda163a9b 100644 --- a/config/initializers/alaveteli.rb +++ b/config/initializers/alaveteli.rb @@ -11,7 +11,7 @@ load "debug_helpers.rb" load "util.rb" # Application version -ALAVETELI_VERSION = '0.21.0.30' +ALAVETELI_VERSION = '0.21.0.32' # Add new inflection rules using the following format # (all these examples are active by default): diff --git a/lib/acts_as_xapian/acts_as_xapian.rb b/lib/acts_as_xapian/acts_as_xapian.rb index 7076fc586..380c882d9 100644 --- a/lib/acts_as_xapian/acts_as_xapian.rb +++ b/lib/acts_as_xapian/acts_as_xapian.rb @@ -137,10 +137,10 @@ module ActsAsXapian prepare_environment # We need to reopen the database each time, so Xapian gets changes to it. - # Calling reopen() does not always pick up changes for reasons that I can + # Calling reopen does not always pick up changes for reasons that I can # only speculate about at the moment. (It is easy to reproduce this by - # changing the code below to use reopen() rather than open() followed by - # close(), and running rake spec.) + # changing the code below to use reopen rather than open followed by + # close, and running rake spec.) if !@@db.nil? @@db.close end @@ -260,7 +260,7 @@ module ActsAsXapian # for indexing @@writable_db = Xapian::WritableDatabase.new(full_path, Xapian::DB_CREATE_OR_OPEN) @@enquire = Xapian::Enquire.new(@@writable_db) - @@term_generator = Xapian::TermGenerator.new() + @@term_generator = Xapian::TermGenerator.new @@term_generator.set_flags(Xapian::TermGenerator::FLAG_SPELLING, 0) @@term_generator.database = @@writable_db @@term_generator.stemmer = @@stemmer @@ -336,7 +336,7 @@ module ActsAsXapian delay *= 2 delay = MSET_MAX_DELAY if delay > MSET_MAX_DELAY - ActsAsXapian.db.reopen() + ActsAsXapian.db.reopen retry else raise @@ -559,7 +559,7 @@ module ActsAsXapian matches = ActsAsXapian.enquire.mset(0, 100, 100) # TODO: so this whole method will only work with 100 docs # Get set of relevant terms for those documents - selection = Xapian::RSet.new() + selection = Xapian::RSet.new iter = matches._begin while not iter.equals(matches._end) selection.add_document(iter) diff --git a/lib/has_tag_string/has_tag_string.rb b/lib/has_tag_string/has_tag_string.rb index a1af8c597..70dbbda5d 100644 --- a/lib/has_tag_string/has_tag_string.rb +++ b/lib/has_tag_string/has_tag_string.rb @@ -152,7 +152,7 @@ module HasTagString ###################################################################### # Main entry point, add has_tag_string to your model. module HasMethods - def has_tag_string() + def has_tag_string has_many :tags, :conditions => "model = '" + self.to_s + "'", :foreign_key => "model_id", :class_name => 'HasTagString::HasTagStringTag' include InstanceMethods diff --git a/lib/health_checks/checks/days_ago_check.rb b/lib/health_checks/checks/days_ago_check.rb index 9e574fe95..3c1cb784f 100644 --- a/lib/health_checks/checks/days_ago_check.rb +++ b/lib/health_checks/checks/days_ago_check.rb @@ -20,7 +20,7 @@ module HealthChecks "#{ super }: #{ subject.call }" end - def check + def ok? subject.call >= days.days.ago end diff --git a/lib/health_checks/health_checkable.rb b/lib/health_checks/health_checkable.rb index f71ca36ca..1e324c1c7 100644 --- a/lib/health_checks/health_checkable.rb +++ b/lib/health_checks/health_checkable.rb @@ -13,12 +13,8 @@ module HealthChecks self.class.to_s end - def check - raise NotImplementedError - end - def ok? - check ? true : false + raise NotImplementedError end def message diff --git a/lib/health_checks/health_checks.rb b/lib/health_checks/health_checks.rb index 54cb0d96b..6c98365fc 100644 --- a/lib/health_checks/health_checks.rb +++ b/lib/health_checks/health_checks.rb @@ -32,7 +32,7 @@ module HealthChecks private def assert_valid_check(check) - check.respond_to?(:check) + check.respond_to?(:ok?) end end diff --git a/lib/mail_handler/backends/mail_backend.rb b/lib/mail_handler/backends/mail_backend.rb index 89fc24bb9..5a7e0ef65 100644 --- a/lib/mail_handler/backends/mail_backend.rb +++ b/lib/mail_handler/backends/mail_backend.rb @@ -36,7 +36,7 @@ module MailHandler module Backends module MailBackend - def backend() + def backend 'Mail' end @@ -124,7 +124,7 @@ module MailHandler envelope_to = mail['envelope-to'] ? [mail['envelope-to'].value.to_s] : [] ((mail.to || []) + (mail.cc || []) + - (envelope_to || [])).uniq + (envelope_to || [])).compact.uniq end def empty_return_path?(mail) diff --git a/lib/mail_handler/mail_handler.rb b/lib/mail_handler/mail_handler.rb index 0c60fd3f5..313869d16 100644 --- a/lib/mail_handler/mail_handler.rb +++ b/lib/mail_handler/mail_handler.rb @@ -134,7 +134,7 @@ module MailHandler begin zip_file = Zip::ZipFile.open(tempfile.path) text += get_attachment_text_from_zip_file(zip_file) - zip_file.close() + zip_file.close rescue $stderr.puts("Error processing zip file: #{$!.inspect}") end diff --git a/lib/strip_attributes/test/test_helper.rb b/lib/strip_attributes/test/test_helper.rb index b69440715..6a4f6136a 100644 --- a/lib/strip_attributes/test/test_helper.rb +++ b/lib/strip_attributes/test/test_helper.rb @@ -10,7 +10,7 @@ require "#{PLUGIN_ROOT}/init" class ActiveRecord::Base alias_method :save, :valid? - def self.columns() + def self.columns @columns ||= [] end diff --git a/lib/world_foi_websites.rb b/lib/world_foi_websites.rb index b63d0860d..a1e705c82 100644 --- a/lib/world_foi_websites.rb +++ b/lib/world_foi_websites.rb @@ -78,7 +78,11 @@ class WorldFOIWebsites {:name => "Слободен пристап", :country_name => "Република Македонија", :country_iso_code => "MK", - :url => "http://www.slobodenpristap.mk/"} + :url => "http://www.slobodenpristap.mk/"}, + {:name => "Imamo pravo znati", + :country_name => "Republika Hrvatska", + :country_iso_code => "HR", + :url => "http://imamopravoznati.org/"} ] return world_foi_websites end diff --git a/spec/controllers/admin_censor_rule_controller_spec.rb b/spec/controllers/admin_censor_rule_controller_spec.rb index a2014808c..b9f936836 100644 --- a/spec/controllers/admin_censor_rule_controller_spec.rb +++ b/spec/controllers/admin_censor_rule_controller_spec.rb @@ -585,7 +585,7 @@ describe AdminCensorRuleController, "when making censor rules from the admin int :replacement => "tofu", :last_edit_comment => "none" } - PurgeRequest.all().first.model_id.should == ir.id + PurgeRequest.all.first.model_id.should == ir.id end end diff --git a/spec/controllers/admin_public_body_controller_spec.rb b/spec/controllers/admin_public_body_controller_spec.rb index 035c478bf..1b960ccc3 100644 --- a/spec/controllers/admin_public_body_controller_spec.rb +++ b/spec/controllers/admin_public_body_controller_spec.rb @@ -651,17 +651,17 @@ describe AdminPublicBodyController, "when administering public bodies and paying render_views before do - config = MySociety::Config.load_default() + config = MySociety::Config.load_default config['SKIP_ADMIN_AUTH'] = false basic_auth_login @request end after do - config = MySociety::Config.load_default() + config = MySociety::Config.load_default config['SKIP_ADMIN_AUTH'] = true end def setup_emergency_credentials(username, password) - config = MySociety::Config.load_default() + config = MySociety::Config.load_default config['SKIP_ADMIN_AUTH'] = false config['ADMIN_USERNAME'] = username config['ADMIN_PASSWORD'] = password @@ -678,7 +678,7 @@ describe AdminPublicBodyController, "when administering public bodies and paying end it "skips admin authorisation when SKIP_ADMIN_AUTH set" do - config = MySociety::Config.load_default() + config = MySociety::Config.load_default config['SKIP_ADMIN_AUTH'] = true @request.env["HTTP_AUTHORIZATION"] = "" n = PublicBody.count @@ -758,7 +758,7 @@ describe AdminPublicBodyController, "when administering public bodies and paying end it 'returns the REMOTE_USER value from the request environment when skipping admin auth' do - config = MySociety::Config.load_default() + config = MySociety::Config.load_default config['SKIP_ADMIN_AUTH'] = true @request.env["HTTP_AUTHORIZATION"] = "" @request.env["REMOTE_USER"] = "i_am_admin" diff --git a/spec/controllers/general_controller_spec.rb b/spec/controllers/general_controller_spec.rb index d466625b7..495624403 100644 --- a/spec/controllers/general_controller_spec.rb +++ b/spec/controllers/general_controller_spec.rb @@ -112,7 +112,7 @@ describe GeneralController, "when showing the frontpage" do it "should render the front page with default language and ignore the browser setting" do - config = MySociety::Config.load_default() + config = MySociety::Config.load_default config['USE_DEFAULT_BROWSER_LANGUAGE'] = false accept_language = "en-GB,en-US;q=0.8,en;q=0.6" request.env['HTTP_ACCEPT_LANGUAGE'] = accept_language @@ -123,7 +123,7 @@ describe GeneralController, "when showing the frontpage" do end it "should render the front page with browser-selected language when there's no default set" do - config = MySociety::Config.load_default() + config = MySociety::Config.load_default config['USE_DEFAULT_BROWSER_LANGUAGE'] = true accept_language = "es-ES,en-GB,en-US;q=0.8,en;q=0.6" request.env['HTTP_ACCEPT_LANGUAGE'] = accept_language diff --git a/spec/controllers/public_body_controller_spec.rb b/spec/controllers/public_body_controller_spec.rb index 02e37684c..c5c94a45c 100644 --- a/spec/controllers/public_body_controller_spec.rb +++ b/spec/controllers/public_body_controller_spec.rb @@ -336,7 +336,7 @@ end describe PublicBodyController, "when showing public body statistics" do it "should render the right template with the right data" do - config = MySociety::Config.load_default() + config = MySociety::Config.load_default config['MINIMUM_REQUESTS_FOR_STATISTICS'] = 1 config['PUBLIC_BODY_STATISTICS_PAGE'] = true get :statistics diff --git a/spec/controllers/request_controller_spec.rb b/spec/controllers/request_controller_spec.rb index eb4ea5e7d..a5534e9ff 100644 --- a/spec/controllers/request_controller_spec.rb +++ b/spec/controllers/request_controller_spec.rb @@ -40,41 +40,41 @@ describe RequestController, "when changing things that appear on the request pag it "should purge the downstream cache when mail is received" do ir = info_requests(:fancy_dog_request) receive_incoming_mail('incoming-request-plain.email', ir.incoming_email) - PurgeRequest.all().first.model_id.should == ir.id + PurgeRequest.all.first.model_id.should == ir.id end it "should purge the downstream cache when a comment is added" do ir = info_requests(:fancy_dog_request) new_comment = info_requests(:fancy_dog_request).add_comment('I also love making annotations.', users(:bob_smith_user)) - PurgeRequest.all().first.model_id.should == ir.id + PurgeRequest.all.first.model_id.should == ir.id end it "should purge the downstream cache when a followup is made" do session[:user_id] = users(:bob_smith_user).id ir = info_requests(:fancy_dog_request) post :show_response, :outgoing_message => { :body => "What a useless response! You suck.", :what_doing => 'normal_sort' }, :id => ir.id, :submitted_followup => 1 - PurgeRequest.all().first.model_id.should == ir.id + PurgeRequest.all.first.model_id.should == ir.id end it "should purge the downstream cache when the request is categorised" do ir = info_requests(:fancy_dog_request) ir.set_described_state('waiting_clarification') - PurgeRequest.all().first.model_id.should == ir.id + PurgeRequest.all.first.model_id.should == ir.id end it "should purge the downstream cache when the authority data is changed" do ir = info_requests(:fancy_dog_request) ir.public_body.name = "Something new" ir.public_body.save! - PurgeRequest.all().map{|x| x.model_id}.should =~ ir.public_body.info_requests.map{|x| x.id} + PurgeRequest.all.map{|x| x.model_id}.should =~ ir.public_body.info_requests.map{|x| x.id} end it "should purge the downstream cache when the user name is changed" do ir = info_requests(:fancy_dog_request) ir.user.name = "Something new" ir.user.save! - PurgeRequest.all().map{|x| x.model_id}.should =~ ir.user.info_requests.map{|x| x.id} + PurgeRequest.all.map{|x| x.model_id}.should =~ ir.user.info_requests.map{|x| x.id} end it "should not purge the downstream cache when non-visible user details are changed" do ir = info_requests(:fancy_dog_request) ir.user.hashed_password = "some old hash" ir.user.save! - PurgeRequest.all().count.should == 0 + PurgeRequest.all.count.should == 0 end it "should purge the downstream cache when censor rules have changed" do # TODO: really, CensorRules should execute expiry logic as part @@ -86,17 +86,17 @@ describe RequestController, "when changing things that appear on the request pag ir = info_requests(:fancy_dog_request) ir.prominence = 'hidden' ir.save! - PurgeRequest.all().first.model_id.should == ir.id + PurgeRequest.all.first.model_id.should == ir.id end it "should not create more than one entry for any given resource" do ir = info_requests(:fancy_dog_request) ir.prominence = 'hidden' ir.save! - PurgeRequest.all().count.should == 1 + PurgeRequest.all.count.should == 1 ir = info_requests(:fancy_dog_request) ir.prominence = 'hidden' ir.save! - PurgeRequest.all().count.should == 1 + PurgeRequest.all.count.should == 1 end end @@ -611,7 +611,7 @@ describe RequestController, "when showing one request" do it "should censor attachments downloaded as binary" do ir = info_requests(:fancy_dog_request) - censor_rule = CensorRule.new() + censor_rule = CensorRule.new censor_rule.text = "Second" censor_rule.replacement = "Mouse" censor_rule.last_edit_editor = "unknown" @@ -632,7 +632,7 @@ describe RequestController, "when showing one request" do it "should censor with rules on the user (rather than the request)" do ir = info_requests(:fancy_dog_request) - censor_rule = CensorRule.new() + censor_rule = CensorRule.new censor_rule.text = "Second" censor_rule.replacement = "Mouse" censor_rule.last_edit_editor = "unknown" @@ -675,7 +675,7 @@ describe RequestController, "when showing one request" do s.should contain /hello world.txt/m end - censor_rule = CensorRule.new() + censor_rule = CensorRule.new # Note that the censor rule applies to the original filename, # not the display_filename: censor_rule.text = "hello-world.txt" @@ -1471,7 +1471,7 @@ describe RequestController, "when classifying an information request" do it 'should record a classification' do event = mock_model(InfoRequestEvent) - @dog_request.stub!(:log_event).with("status_update", anything()).and_return(event) + @dog_request.stub!(:log_event).with("status_update", anything).and_return(event) RequestClassification.should_receive(:create!).with(:user_id => @admin_user.id, :info_request_event_id => event.id) post_status('rejected') @@ -1915,7 +1915,7 @@ describe RequestController, "sending overdue request alerts" do it "should send an overdue alert mail to creators of overdue requests" do chicken_request = info_requests(:naughty_chicken_request) - chicken_request.outgoing_messages[0].last_sent_at = Time.now() - 30.days + chicken_request.outgoing_messages[0].last_sent_at = Time.now - 30.days chicken_request.outgoing_messages[0].save! RequestMailer.alert_overdue_requests @@ -1941,7 +1941,7 @@ describe RequestController, "sending overdue request alerts" do it "should include clause for schools when sending an overdue alert mail to creators of overdue requests" do chicken_request = info_requests(:naughty_chicken_request) - chicken_request.outgoing_messages[0].last_sent_at = Time.now() - 30.days + chicken_request.outgoing_messages[0].last_sent_at = Time.now - 30.days chicken_request.outgoing_messages[0].save! chicken_request.public_body.tag_string = "school" @@ -1972,7 +1972,7 @@ describe RequestController, "sending overdue request alerts" do it "should send a very overdue alert mail to creators of very overdue requests" do chicken_request = info_requests(:naughty_chicken_request) - chicken_request.outgoing_messages[0].last_sent_at = Time.now() - 60.days + chicken_request.outgoing_messages[0].last_sent_at = Time.now - 60.days chicken_request.outgoing_messages[0].save! RequestMailer.alert_overdue_requests @@ -1998,7 +1998,7 @@ describe RequestController, "sending overdue request alerts" do it "should not resend alerts to people who've already received them" do chicken_request = info_requests(:naughty_chicken_request) - chicken_request.outgoing_messages[0].last_sent_at = Time.now() - 60.days + chicken_request.outgoing_messages[0].last_sent_at = Time.now - 60.days chicken_request.outgoing_messages[0].save! RequestMailer.alert_overdue_requests chicken_mails = ActionMailer::Base.deliveries.select{|x| x.body =~ /chickens/} @@ -2011,7 +2011,7 @@ describe RequestController, "sending overdue request alerts" do it 'should send alerts for requests where the last event forming the initial request is a followup being sent following a request for clarification' do chicken_request = info_requests(:naughty_chicken_request) - chicken_request.outgoing_messages[0].last_sent_at = Time.now() - 60.days + chicken_request.outgoing_messages[0].last_sent_at = Time.now - 60.days chicken_request.outgoing_messages[0].save! RequestMailer.alert_overdue_requests chicken_mails = ActionMailer::Base.deliveries.select{|x| x.body =~ /chickens/} @@ -2048,7 +2048,7 @@ describe RequestController, "sending overdue request alerts" do chicken_mails.size.should == 1 # Make the followup older - outgoing_message.last_sent_at = Time.now() - 60.days + outgoing_message.last_sent_at = Time.now - 60.days outgoing_message.save! # Now it should be alerted on diff --git a/spec/controllers/services_controller_spec.rb b/spec/controllers/services_controller_spec.rb index d4a3e5939..6ab527bc9 100644 --- a/spec/controllers/services_controller_spec.rb +++ b/spec/controllers/services_controller_spec.rb @@ -9,7 +9,7 @@ describe ServicesController, "when returning a message for people in other count # store and restore the locale in the context of the test suite to isolate # changes made in these tests before do - @old_locale = FastGettext.locale() + @old_locale = FastGettext.locale end it 'keeps the flash' do @@ -21,7 +21,7 @@ describe ServicesController, "when returning a message for people in other count end it "should show no alaveteli message when in the deployed country" do - config = MySociety::Config.load_default() + config = MySociety::Config.load_default config['ISO_COUNTRY_CODE'] = "DE" controller.stub!(:country_from_ip).and_return('DE') get :other_country_message @@ -29,7 +29,7 @@ describe ServicesController, "when returning a message for people in other count end it "should show an alaveteli message when not in the deployed country and in a country with no FOI website" do - config = MySociety::Config.load_default() + config = MySociety::Config.load_default config['ISO_COUNTRY_CODE'] = "DE" controller.stub!(:country_from_ip).and_return('ZZ') get :other_country_message @@ -37,7 +37,7 @@ describe ServicesController, "when returning a message for people in other count end it "should show link to other FOI website when not in the deployed country" do - config = MySociety::Config.load_default() + config = MySociety::Config.load_default config['ISO_COUNTRY_CODE'] = "ZZ" controller.stub!(:country_from_ip).and_return('ES') request.env['HTTP_ACCEPT_LANGUAGE'] = "es" @@ -60,7 +60,7 @@ describe ServicesController, "when returning a message for people in other count end it "should return the 'another country' message if the service responds OK" do - config = MySociety::Config.load_default() + config = MySociety::Config.load_default config['ISO_COUNTRY_CODE'] = "DE" AlaveteliConfiguration.stub!(:gaze_url).and_return('http://denmark.com') FakeWeb.register_uri(:get, %r|denmark.com|, :body => "DK") diff --git a/spec/controllers/user_controller_spec.rb b/spec/controllers/user_controller_spec.rb index 2d3b36f70..fb03615f8 100644 --- a/spec/controllers/user_controller_spec.rb +++ b/spec/controllers/user_controller_spec.rb @@ -76,11 +76,11 @@ describe UserController, "when redirecting a show request to a canonical url" do end it 'should not redirect a long canonical name that has a numerical suffix' do - User.stub!(:find).with(:first, anything()).and_return(mock_model(User, + User.stub!(:find).with(:first, anything).and_return(mock_model(User, :url_name => 'bob_smithbob_smithbob_smithbob_s_2', :name => 'Bob Smith Bob Smith Bob Smith Bob Smith', :info_requests => [])) - User.stub!(:find).with(:all, anything()).and_return([]) + User.stub!(:find).with(:all, anything).and_return([]) get :show, :url_name => 'bob_smithbob_smithbob_smithbob_s_2' response.should be_success end diff --git a/spec/lib/alaveteli_text_masker_spec.rb b/spec/lib/alaveteli_text_masker_spec.rb index 8f7c530b3..f2d52c1cc 100644 --- a/spec/lib/alaveteli_text_masker_spec.rb +++ b/spec/lib/alaveteli_text_masker_spec.rb @@ -60,7 +60,7 @@ describe AlaveteliTextMasker do end def pdf_replacement_test(use_ghostscript_compression) - config = MySociety::Config.load_default() + config = MySociety::Config.load_default previous = config['USE_GHOSTSCRIPT_COMPRESSION'] config['USE_GHOSTSCRIPT_COMPRESSION'] = use_ghostscript_compression orig_pdf = load_file_fixture('tfl.pdf') diff --git a/spec/lib/health_checks/checks/days_ago_check_spec.rb b/spec/lib/health_checks/checks/days_ago_check_spec.rb index 829b8e71e..4fbc1913b 100644 --- a/spec/lib/health_checks/checks/days_ago_check_spec.rb +++ b/spec/lib/health_checks/checks/days_ago_check_spec.rb @@ -16,16 +16,16 @@ describe HealthChecks::Checks::DaysAgoCheck do expect(check.days).to eq(4) end - describe :check do + describe :ok? do it 'is successful if the subject is in the last day' do check = HealthChecks::Checks::DaysAgoCheck.new { Time.now } - expect(check.check).to be_true + expect(check.ok?).to be_true end it 'fails if the subject is over a day ago' do check = HealthChecks::Checks::DaysAgoCheck.new { 2.days.ago } - expect(check.check).to be_false + expect(check.ok?).to be_false end end diff --git a/spec/lib/health_checks/health_checkable_spec.rb b/spec/lib/health_checks/health_checkable_spec.rb index 301585bbd..59d76c337 100644 --- a/spec/lib/health_checks/health_checkable_spec.rb +++ b/spec/lib/health_checks/health_checkable_spec.rb @@ -32,24 +32,10 @@ describe HealthChecks::HealthCheckable do end - describe :check do - - it 'is intended to be overridden by the includer' do - expect{ @subject.check }.to raise_error(NotImplementedError) - end - - end - describe :ok? do - it 'returns true if the check was successful' do - @subject.stub(:check => true) - expect(@subject.ok?).to be_true - end - - it 'returns false if the check failed' do - @subject.stub(:check => false) - expect(@subject.ok?).to be_false + it 'is intended to be overridden by the includer' do + expect{ @subject.ok? }.to raise_error(NotImplementedError) end end @@ -93,7 +79,7 @@ describe HealthChecks::HealthCheckable do context 'if the check succeeds' do before(:each) do - @subject.stub(:check => true) + @subject.stub(:ok? => true) end it 'returns the default success message' do @@ -110,7 +96,7 @@ describe HealthChecks::HealthCheckable do context 'if the check fails' do before(:each) do - @subject.stub(:check => false) + @subject.stub(:ok? => false) end it 'returns the default failure message' do diff --git a/spec/lib/health_checks/health_checks_spec.rb b/spec/lib/health_checks/health_checks_spec.rb index 335e10b3a..0b97725db 100644 --- a/spec/lib/health_checks/health_checks_spec.rb +++ b/spec/lib/health_checks/health_checks_spec.rb @@ -7,7 +7,7 @@ describe HealthChecks do describe :add do it 'adds a check to the collection and returns the check' do - check = double('MockCheck', :check => true) + check = double('MockCheck', :ok? => true) expect(add(check)).to eq(check) end @@ -21,8 +21,8 @@ describe HealthChecks do describe :all do it 'returns all the checks' do - check1 = double('MockCheck', :check => true) - check2 = double('AnotherCheck', :check => false) + check1 = double('MockCheck', :ok? => true) + check2 = double('AnotherCheck', :ok? => false) add(check1) add(check2) expect(all).to include(check1, check2) diff --git a/spec/lib/mail_handler/mail_handler_spec.rb b/spec/lib/mail_handler/mail_handler_spec.rb index 7f0070a8a..27a7a3db4 100644 --- a/spec/lib/mail_handler/mail_handler_spec.rb +++ b/spec/lib/mail_handler/mail_handler_spec.rb @@ -158,6 +158,16 @@ describe 'when asked for all the addresses a mail has been sent to' do mail = MailHandler.mail_from_raw_email(mail_data) MailHandler.get_all_addresses(mail).should == ["request-5555-xxxxxxxx@whatdotheyknow.com"] end + + it 'should not return invalid addresses' do + mail_data = load_file_fixture('autoresponse-header.email') + mail_data.gsub!('To: FOI Person <EMAIL_TO>', + 'To: <request-5555-xxxxxxxx>') + mail = MailHandler.mail_from_raw_email(mail_data) + MailHandler.get_all_addresses(mail).should == [] + end + + end describe 'when asked for auto_submitted' do diff --git a/spec/mailers/application_mailer_spec.rb b/spec/mailers/application_mailer_spec.rb index a5c5a5cf2..1854e4741 100644 --- a/spec/mailers/application_mailer_spec.rb +++ b/spec/mailers/application_mailer_spec.rb @@ -14,7 +14,7 @@ describe ApplicationMailer do end def add_mail_methods(method_names) - method_names.each{ |method_name| ApplicationMailer.send(:define_method, method_name){ mail() } } + method_names.each{ |method_name| ApplicationMailer.send(:define_method, method_name){ mail } } end def remove_mail_methods(method_names) diff --git a/spec/mailers/request_mailer_spec.rb b/spec/mailers/request_mailer_spec.rb index 5c8b8a3de..12d83ca62 100644 --- a/spec/mailers/request_mailer_spec.rb +++ b/spec/mailers/request_mailer_spec.rb @@ -301,7 +301,7 @@ describe RequestMailer, "when sending reminders to requesters to classify a resp ORDER BY created_at desc LIMIT 1) < ? AND url_title != 'holding_pen' AND user_id IS NOT NULL".split(' ').join(' '), - true, Time.now() - 7.days ] + true, Time.now - 7.days ] # compare the query string ignoring any spacing differences InfoRequest.should_receive(:find) do |all, query_params| diff --git a/spec/models/incoming_message_spec.rb b/spec/models/incoming_message_spec.rb index f4b450370..10bb3de62 100644 --- a/spec/models/incoming_message_spec.rb +++ b/spec/models/incoming_message_spec.rb @@ -263,7 +263,7 @@ describe IncomingMessage, " when dealing with incoming mail" do incoming_message = InfoRequest.holding_pen_request.incoming_messages[0] # This will raise an error if the bug in TMail hasn't been fixed - incoming_message.get_body_for_html_display() + incoming_message.get_body_for_html_display end @@ -282,7 +282,7 @@ end describe IncomingMessage, " display attachments" do it "should not show slashes in filenames" do - foi_attachment = FoiAttachment.new() + foi_attachment = FoiAttachment.new # http://www.whatdotheyknow.com/request/post_commercial_manager_librarie#incoming-17233 foi_attachment.filename = "FOI/09/066 RESPONSE TO FOI REQUEST RECEIVED 21st JANUARY 2009.txt" expected_display_filename = foi_attachment.filename.gsub(/\//, " ") @@ -290,7 +290,7 @@ describe IncomingMessage, " display attachments" do end it "should not show slashes in subject generated filenames" do - foi_attachment = FoiAttachment.new() + foi_attachment = FoiAttachment.new # http://www.whatdotheyknow.com/request/post_commercial_manager_librarie#incoming-17233 foi_attachment.within_rfc822_subject = "FOI/09/066 RESPONSE TO FOI REQUEST RECEIVED 21st JANUARY 2009" foi_attachment.content_type = 'text/plain' @@ -312,20 +312,20 @@ describe IncomingMessage, " folding quoted parts of emails" do it 'should fold a plain text lotus notes quoted part correctly' do text = "FOI Team\n\n\nInfo Requester <xxx@whatdotheyknow.com>=20\nSent by: Info Requester <request-bounce-xxxxx@whatdotheyknow.com>\n06/03/08 10:00\nPlease respond to\nInfo Requester <request-xxxx@whatdotheyknow.com>" - @incoming_message = IncomingMessage.new() + @incoming_message = IncomingMessage.new @incoming_message.stub_chain(:info_request, :user_name).and_return("Info Requester") @incoming_message.remove_lotus_quoting(text).should match(/FOLDED_QUOTED_SECTION/) end it 'should not error when trying to fold lotus notes quoted parts on a request with no user_name' do text = "hello" - @incoming_message = IncomingMessage.new() + @incoming_message = IncomingMessage.new @incoming_message.stub_chain(:info_request, :user_name).and_return(nil) @incoming_message.remove_lotus_quoting(text).should == 'hello' end it "cope with [ in user names properly" do - @incoming_message = IncomingMessage.new() + @incoming_message = IncomingMessage.new @incoming_message.stub_chain(:info_request, :user_name).and_return("Sir [ Bobble") # this gives a warning if [ is in the name text = @incoming_message.remove_lotus_quoting("Sir [ Bobble \nSent by: \n") @@ -357,7 +357,7 @@ describe IncomingMessage, " checking validity to reply to" do MailHandler.stub!(:get_from_address).and_return(email) MailHandler.stub!(:empty_return_path?).with(@mail).and_return(empty_return_path) MailHandler.stub!(:get_auto_submitted).with(@mail).and_return(autosubmitted) - @incoming_message = IncomingMessage.new() + @incoming_message = IncomingMessage.new @incoming_message.stub!(:mail).and_return(@mail) @incoming_message._calculate_valid_to_reply_to.should == result end @@ -431,21 +431,21 @@ describe IncomingMessage, " when censoring data" do @im = incoming_messages(:useless_incoming_message) - @censor_rule_1 = CensorRule.new() + @censor_rule_1 = CensorRule.new @censor_rule_1.text = "Stilton" @censor_rule_1.replacement = "Jarlsberg" @censor_rule_1.last_edit_editor = "unknown" @censor_rule_1.last_edit_comment = "none" @im.info_request.censor_rules << @censor_rule_1 - @censor_rule_2 = CensorRule.new() + @censor_rule_2 = CensorRule.new @censor_rule_2.text = "blue" @censor_rule_2.replacement = "yellow" @censor_rule_2.last_edit_editor = "unknown" @censor_rule_2.last_edit_comment = "none" @im.info_request.censor_rules << @censor_rule_2 - @regex_censor_rule = CensorRule.new() + @regex_censor_rule = CensorRule.new @regex_censor_rule.text = 'm[a-z][a-z][a-z]e' @regex_censor_rule.regexp = true @regex_censor_rule.replacement = 'cat' @@ -477,7 +477,7 @@ describe IncomingMessage, " when censoring whole users" do @im = incoming_messages(:useless_incoming_message) - @censor_rule_1 = CensorRule.new() + @censor_rule_1 = CensorRule.new @censor_rule_1.text = "Stilton" @censor_rule_1.replacement = "Gorgonzola" @censor_rule_1.last_edit_editor = "unknown" @@ -534,7 +534,7 @@ describe IncomingMessage, " when uudecoding bad messages" do im.stub!(:mail).and_return(mail) ir = info_requests(:fancy_dog_request) - @censor_rule = CensorRule.new() + @censor_rule = CensorRule.new @censor_rule.text = "moo" @censor_rule.replacement = "bah" @censor_rule.last_edit_editor = "unknown" diff --git a/spec/models/info_request_event_spec.rb b/spec/models/info_request_event_spec.rb index 401a5f712..ff20ab059 100644 --- a/spec/models/info_request_event_spec.rb +++ b/spec/models/info_request_event_spec.rb @@ -111,7 +111,7 @@ describe InfoRequestEvent do describe "should know" do it "that it's an incoming message" do - event = InfoRequestEvent.new() + event = InfoRequestEvent.new event.stub!(:incoming_message_selective_columns).and_return(1) event.is_incoming_message?.should be_true event.is_outgoing_message?.should be_false diff --git a/spec/models/info_request_spec.rb b/spec/models/info_request_spec.rb index cddf1f880..18120fbb5 100644 --- a/spec/models/info_request_spec.rb +++ b/spec/models/info_request_spec.rb @@ -31,7 +31,7 @@ describe InfoRequest do describe :new do it 'sets the default law used' do - expect(InfoRequest.new().law_used).to eq('foi') + expect(InfoRequest.new.law_used).to eq('foi') end it 'sets the default law used if a body is eir-only' do diff --git a/spec/models/outgoing_message_spec.rb b/spec/models/outgoing_message_spec.rb index 336e5a605..8d43e2ef1 100644 --- a/spec/models/outgoing_message_spec.rb +++ b/spec/models/outgoing_message_spec.rb @@ -114,7 +114,7 @@ describe OutgoingMessage, " when making an outgoing message" do :status => 'ready', :message_type => 'initial_request', :body => 'This request contains a foo@bar.com email address', - :last_sent_at => Time.now(), + :last_sent_at => Time.now, :what_doing => 'normal_sort' }) end diff --git a/spec/models/public_body_category/category_collection_spec.rb b/spec/models/public_body_category/category_collection_spec.rb index 29fa6a699..9ee684982 100644 --- a/spec/models/public_body_category/category_collection_spec.rb +++ b/spec/models/public_body_category/category_collection_spec.rb @@ -16,7 +16,7 @@ describe PublicBodyCategory::CategoryCollection do describe 'when asked for headings' do it 'should return a list of headings' do - @categories.headings().should == ['Local and regional', 'Miscellaneous'] + @categories.headings.should == ['Local and regional', 'Miscellaneous'] end end @@ -31,7 +31,7 @@ describe PublicBodyCategory::CategoryCollection do "Miscellaneous", ["other", "Miscellaneous", "miscellaneous"]] - @categories.with_headings().should == expected_categories + @categories.with_headings.should == expected_categories end end @@ -39,7 +39,7 @@ describe PublicBodyCategory::CategoryCollection do describe 'when asked for tags by headings' do it 'should return a hash of tags keyed by heading' do - @categories.by_heading().should == {'Local and regional' => ['local_council'], + @categories.by_heading.should == {'Local and regional' => ['local_council'], 'Miscellaneous' => ['other']} end end @@ -51,19 +51,19 @@ describe PublicBodyCategory::CategoryCollection do ["local_council", "Local councils", "a local council"], ["other", "Miscellaneous", "miscellaneous"] ] - @categories.with_description().should == expected_categories + @categories.with_description.should == expected_categories end end describe 'when asked for tags' do it 'should return a list of tags' do - @categories.tags().should == ["local_council", "other"] + @categories.tags.should == ["local_council", "other"] end end describe 'when asked for categories by tag' do it 'should return a hash of categories keyed by tag' do - @categories.by_tag().should == { + @categories.by_tag.should == { "local_council" => "Local councils", "other" => "Miscellaneous" } @@ -72,7 +72,7 @@ describe PublicBodyCategory::CategoryCollection do describe 'when asked for singular_by_tag' do it 'should return a hash of category descriptions keyed by tag' do - @categories.singular_by_tag().should == { + @categories.singular_by_tag.should == { "local_council" => "a local council", "other" => "miscellaneous" } diff --git a/spec/models/public_body_change_request_spec.rb b/spec/models/public_body_change_request_spec.rb index 90abf0b6b..e35ffa692 100644 --- a/spec/models/public_body_change_request_spec.rb +++ b/spec/models/public_body_change_request_spec.rb @@ -22,7 +22,7 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe PublicBodyChangeRequest, 'when validating' do it 'should not be valid without a public body name' do - change_request = PublicBodyChangeRequest.new() + change_request = PublicBodyChangeRequest.new change_request.valid?.should be_false change_request.errors[:public_body_name].should == ['Please enter the name of the authority'] end diff --git a/spec/models/purge_request_spec.rb b/spec/models/purge_request_spec.rb index 617e082a8..642d5d2e2 100644 --- a/spec/models/purge_request_spec.rb +++ b/spec/models/purge_request_spec.rb @@ -23,23 +23,23 @@ describe PurgeRequest, "purging things" do req = PurgeRequest.new(:url => "/begone_from_here", :model => "don't care", :model_id => "don't care") - req.save() - PurgeRequest.all().count.should == 1 - PurgeRequest.purge_all() - PurgeRequest.all().count.should == 0 + req.save + PurgeRequest.all.count.should == 1 + PurgeRequest.purge_all + PurgeRequest.all.count.should == 0 end it 'should fail silently for a misconfigured server' do FakeWeb.register_uri(:get, %r|brokenv|, :body => "BROKEN") - config = MySociety::Config.load_default() + config = MySociety::Config.load_default config['VARNISH_HOST'] = "brokencache" req = PurgeRequest.new(:url => "/begone_from_here", :model => "don't care", :model_id => "don't care") - req.save() - PurgeRequest.all().count.should == 1 - PurgeRequest.purge_all() - PurgeRequest.all().count.should == 0 + req.save + PurgeRequest.all.count.should == 1 + PurgeRequest.purge_all + PurgeRequest.all.count.should == 0 end end diff --git a/spec/models/spam_address_spec.rb b/spec/models/spam_address_spec.rb index edef79bed..670b969b0 100644 --- a/spec/models/spam_address_spec.rb +++ b/spec/models/spam_address_spec.rb @@ -16,7 +16,7 @@ describe SpamAddress do describe :new do it 'requres an email address' do - SpamAddress.new().should_not be_valid + SpamAddress.new.should_not be_valid SpamAddress.new(:email => 'spam@example.org').should be_valid end diff --git a/spec/models/xapian_spec.rb b/spec/models/xapian_spec.rb index b8ef341bd..b3f2e2b3c 100644 --- a/spec/models/xapian_spec.rb +++ b/spec/models/xapian_spec.rb @@ -102,7 +102,7 @@ describe PublicBody, " when indexing requests by body they are to" do end # if you index via the Xapian TermGenerator, it ignores terms of this length, - # this checks we're using Document:::add_term() instead + # this checks we're using Document:::add_term instead it "should work with URL names that are longer than 64 characters" do # change the URL name of the body body = public_bodies(:geraldine_public_body) diff --git a/spec/support/xapian_index.rb b/spec/support/xapian_index.rb index bfdffd281..3f5f900fd 100644 --- a/spec/support/xapian_index.rb +++ b/spec/support/xapian_index.rb @@ -23,7 +23,7 @@ end # Copy the xapian index created in create_fixtures_xapian_index to a temporary # copy at the same level and point xapian at the copy -def get_fixtures_xapian_index() +def get_fixtures_xapian_index # Create a base index for the fixtures if not already created $existing_xapian_db ||= create_fixtures_xapian_index # Store whatever the xapian db path is originally |