From 99ada7eea8db32f35a6386362933391b576919b4 Mon Sep 17 00:00:00 2001 From: Louise Crow Date: Thu, 16 Aug 2012 09:39:36 +0100 Subject: Add method all_can_view? which can be used to determine whether it is ok to cache the associated objects for an info request in the file cache which will be served up without authentication. --- app/models/info_request.rb | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) (limited to 'app/models/info_request.rb') diff --git a/app/models/info_request.rb b/app/models/info_request.rb index a41d6d2db..7e69a5cda 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -104,7 +104,7 @@ class InfoRequest < ActiveRecord::Base errors.add(:described_state, "is not a valid state") if !InfoRequest.enumerate_states.include? described_state end - + # The request must either be internal, in which case it has # a foreign key reference to a User object and no external_url or external_user_name, # or else be external in which case it has no user_id but does have an external_url, @@ -120,15 +120,15 @@ class InfoRequest < ActiveRecord::Base errors.add(:external_url, "must be null for an internal request") if !external_url.nil? end end - + def is_external? !external_url.nil? end - + def user_name is_external? ? external_user_name : user.name end - + def user_name_slug if is_external? if external_user_name.nil? @@ -708,10 +708,10 @@ public return self.public_body.is_followupable? end def recipient_name_and_email - return TMail::Address.address_from_name_and_email( - _("{{law_used}} requests at {{public_body}}", - :law_used => self.law_used_short, - :public_body => self.public_body.short_or_long_name), + return TMail::Address.address_from_name_and_email( + _("{{law_used}} requests at {{public_body}}", + :law_used => self.law_used_short, + :public_body => self.public_body.short_or_long_name), self.recipient_email).to_s end @@ -1035,6 +1035,12 @@ public return true end + # Is this request visible to everyone? + def all_can_view? + return true if ['normal', 'backpage'].include?(self.prominence) + return false + end + def indexed_by_search? if self.prominence == 'backpage' || self.prominence == 'hidden' || self.prominence == 'requester_only' return false -- cgit v1.2.3 From 6f66fe8a30083a2fa49618b4dad828c0cef3b310 Mon Sep 17 00:00:00 2001 From: Robin Houston Date: Mon, 20 Aug 2012 07:16:55 +0100 Subject: External requests ought not to be considered old_unclassified We certainly do not want to send reminder emails for such requests, for example, since we do not know the email address to send them to. --- app/models/info_request.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'app/models/info_request.rb') diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 2f4a89d91..6426f6ea8 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -942,7 +942,7 @@ public last_response_created_at = last_event_time_clause('response') age = extra_params[:age_in_days] ? extra_params[:age_in_days].days : OLD_AGE_IN_DAYS params = {:select => "*, #{last_response_created_at} as last_response_time", - :conditions => ["awaiting_description = ? and #{last_response_created_at} < ? and url_title != 'holding_pen'", + :conditions => ["awaiting_description = ? and #{last_response_created_at} < ? and url_title != 'holding_pen' and user_id is not null", true, Time.now() - age], :order => "last_response_time"} params[:limit] = extra_params[:limit] if extra_params[:limit] @@ -960,6 +960,7 @@ public end def is_old_unclassified? + return false if user_id.nil? return false if !awaiting_description return false if url_title == 'holding_pen' last_response_event = get_last_response_event -- cgit v1.2.3 From 99457f15722b3326ba11ecb3003c3dd50b70a9c4 Mon Sep 17 00:00:00 2001 From: Robin Houston Date: Mon, 20 Aug 2012 08:00:03 +0100 Subject: Fix tests Also make the InfoRequest#is_old_unclassified? method a little more conservative, by returning false only is the is_external? method returns true. This makes it subtly inconsistent with InfoRequest.find_old_unclassified, but it is better I think to be subtly inconsistent than to risk breaking things that used to work. --- app/models/info_request.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'app/models/info_request.rb') diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 6426f6ea8..e88c3a95f 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -960,7 +960,7 @@ public end def is_old_unclassified? - return false if user_id.nil? + return false if is_external? return false if !awaiting_description return false if url_title == 'holding_pen' last_response_event = get_last_response_event -- cgit v1.2.3 From dae632d5b6a636ee0976151c764831015a67f3d5 Mon Sep 17 00:00:00 2001 From: Robin Houston Date: Mon, 20 Aug 2012 11:08:01 +0100 Subject: Do not send email for external requests --- app/models/info_request.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'app/models/info_request.rb') diff --git a/app/models/info_request.rb b/app/models/info_request.rb index e88c3a95f..ba7e44c08 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -456,7 +456,7 @@ public if !allow if self.handle_rejected_responses == 'bounce' - RequestMailer.deliver_stopped_responses(self, email, raw_email_data) + RequestMailer.deliver_stopped_responses(self, email, raw_email_data) if !is_external? elsif self.handle_rejected_responses == 'holding_pen' InfoRequest.holding_pen_request.receive(email, raw_email_data, false, reason) elsif self.handle_rejected_responses == 'blackhole' @@ -565,7 +565,7 @@ public self.calculate_event_states - if self.requires_admin? + if self.requires_admin? && !self.is_external? RequestMailer.deliver_requires_admin(self, set_by) end end -- cgit v1.2.3 From f2115fe4357d232d0e8a373881790a9a38ee80f5 Mon Sep 17 00:00:00 2001 From: Robin Houston Date: Mon, 20 Aug 2012 15:25:00 +0100 Subject: Fail with NotFound if request slug doesn't exist Closes #554. --- app/models/info_request.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'app/models/info_request.rb') diff --git a/app/models/info_request.rb b/app/models/info_request.rb index ba7e44c08..23e18b858 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -223,7 +223,7 @@ class InfoRequest < ActiveRecord::Base incoming_message.clear_in_database_caches! end end - + # For debugging def InfoRequest.profile_search(query) t = Time.now.usec @@ -246,7 +246,9 @@ public # For request with same title as others, add on arbitary numeric identifier unique_url_title = url_title suffix_num = 2 # as there's already one without numeric suffix - while not InfoRequest.find_by_url_title(unique_url_title, :conditions => self.id.nil? ? nil : ["id <> ?", self.id] ).nil? + while not InfoRequest.find_by_url_title(unique_url_title, + :conditions => self.id.nil? ? nil : ["id <> ?", self.id] + ).nil? unique_url_title = url_title + "_" + suffix_num.to_s suffix_num = suffix_num + 1 end -- cgit v1.2.3 From bf4b97e16e0c1a699ddd2fc186ea3f8e962ec7bf Mon Sep 17 00:00:00 2001 From: Robin Houston Date: Tue, 21 Aug 2012 07:43:16 +0100 Subject: Email admins about external requests too If someone reports an external request as needing administrator attention, we should email the administrators about it. Thanks for spotting this, @crowbot. --- app/models/info_request.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'app/models/info_request.rb') diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 23e18b858..19ec949ba 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -567,8 +567,11 @@ public self.calculate_event_states - if self.requires_admin? && !self.is_external? - RequestMailer.deliver_requires_admin(self, set_by) + if self.requires_admin? + # Check there is someone to send the message "from" + if !set_by.nil? || !self.user.nil? + RequestMailer.deliver_requires_admin(self, set_by) + end end end -- cgit v1.2.3