From 7c4fdfb4944e3c4f9729c86a0e8a9c445e51b688 Mon Sep 17 00:00:00 2001 From: Matthew Landauer Date: Fri, 1 Mar 2013 12:35:33 +1100 Subject: Inline method --- app/models/info_request.rb | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'app/models/info_request.rb') diff --git a/app/models/info_request.rb b/app/models/info_request.rb index cee9eb959..98b45af85 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -543,13 +543,8 @@ public # states which require administrator action (hence email administrators # when they are entered, and offer state change dialog to them) - def InfoRequest.requires_admin_states - return ['requires_admin', 'error_message', 'attention_requested'] - end - def requires_admin? - return true if InfoRequest.requires_admin_states.include?(described_state) - return false + ['requires_admin', 'error_message', 'attention_requested'].include?(described_state) end # change status, including for last event for later historical purposes -- cgit v1.2.3 From d88b79d1aa24a78ed95255a37a0403d7e3c0117e Mon Sep 17 00:00:00 2001 From: Matthew Landauer Date: Fri, 1 Mar 2013 13:04:02 +1100 Subject: Inline method --- app/models/info_request.rb | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) (limited to 'app/models/info_request.rb') diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 98b45af85..f7391a60a 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -551,7 +551,7 @@ public def set_described_state(new_state, set_by = nil) ActiveRecord::Base.transaction do self.awaiting_description = false - last_event = self.get_last_event + last_event = self.info_request_events.last last_event.described_state = new_state self.described_state = new_state last_event.save! @@ -789,16 +789,6 @@ public end end - # Returns last event - def get_last_event - events = self.info_request_events - if events.size == 0 - return nil - else - return events[-1] - end - end - # Get previous email sent to def get_previous_email_sent_to(info_request_event) last_email = nil -- cgit v1.2.3 From 66279a9373226f49d0c47ab651f61deb2aebf1a7 Mon Sep 17 00:00:00 2001 From: Matthew Landauer Date: Fri, 1 Mar 2013 14:54:09 +1100 Subject: Simplify logic in method --- app/models/info_request.rb | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'app/models/info_request.rb') diff --git a/app/models/info_request.rb b/app/models/info_request.rb index f7391a60a..e0ade0eb6 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -972,13 +972,8 @@ public end def is_old_unclassified? - return false if is_external? - return false if !awaiting_description - return false if url_title == 'holding_pen' - last_response_event = get_last_response_event - return false unless last_response_event - return false if last_response_event.created_at >= Time.now - OLD_AGE_IN_DAYS - return true + !is_external? && awaiting_description && url_title != 'holding_pen' && get_last_response_event && + Time.now > get_last_response_event.created_at + OLD_AGE_IN_DAYS end # List of incoming messages to followup, by unique email -- cgit v1.2.3 From 3291fe2a287f09d2a1bc88ee0f5617cece3b3ee7 Mon Sep 17 00:00:00 2001 From: Matthew Landauer Date: Fri, 1 Mar 2013 14:55:44 +1100 Subject: Simplify methods --- app/models/info_request.rb | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) (limited to 'app/models/info_request.rb') diff --git a/app/models/info_request.rb b/app/models/info_request.rb index e0ade0eb6..88770c684 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -714,31 +714,19 @@ public self.info_request_events.create!(:event_type => type, :params => params) end + def response_events + self.info_request_events.select{|e| e.event_type == 'response'} + end + # The last response is the default one people might want to reply to def get_last_response_event_id - for e in self.info_request_events.reverse - if e.event_type == 'response' - return e.id - end - end - return nil - + get_last_response_event.id if get_last_response_event end def get_last_response_event - for e in self.info_request_events.reverse - if e.event_type == 'response' - return e - end - end - return nil + response_events.last end def get_last_response - last_response_event = self.get_last_response_event - if last_response_event.nil? - return nil - else - return last_response_event.incoming_message - end + get_last_response_event.incoming_message if get_last_response_event end # The last outgoing message -- cgit v1.2.3 From 450c8e766c943cd212d706ae53768f2bae563c67 Mon Sep 17 00:00:00 2001 From: Matthew Landauer Date: Fri, 1 Mar 2013 15:12:00 +1100 Subject: Extract method --- app/models/info_request.rb | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'app/models/info_request.rb') diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 88770c684..4951ea3b8 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -729,14 +729,13 @@ public get_last_response_event.incoming_message if get_last_response_event end + def outgoing_events + info_request_events.select{|e| [ 'sent', 'followup_sent' ].include?(e.event_type) } + end + # The last outgoing message def get_last_outgoing_event - for e in self.info_request_events.reverse - if [ 'sent', 'followup_sent' ].include?(e.event_type) - return e - end - end - return nil + outgoing_events.last end # Text from the the initial request, for use in summary display -- cgit v1.2.3 From 82f9d1f8618dd22bb7bb34456446ceaf8c742d54 Mon Sep 17 00:00:00 2001 From: Matthew Landauer Date: Fri, 1 Mar 2013 15:15:20 +1100 Subject: InfoRequestEvent should know about its event types --- 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 4951ea3b8..b1a5c905c 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -715,7 +715,7 @@ public end def response_events - self.info_request_events.select{|e| e.event_type == 'response'} + self.info_request_events.select{|e| e.response?} end # The last response is the default one people might want to reply to @@ -730,7 +730,7 @@ public end def outgoing_events - info_request_events.select{|e| [ 'sent', 'followup_sent' ].include?(e.event_type) } + info_request_events.select{|e| e.outgoing? } end # The last outgoing message -- cgit v1.2.3 From ac7cc9ec63a6161ddda6a7ff679395dfffb29aad Mon Sep 17 00:00:00 2001 From: Matthew Landauer Date: Fri, 1 Mar 2013 15:39:09 +1100 Subject: Simplify method by using array to store mapping --- app/models/info_request.rb | 63 ++++++++++++++++++---------------------------- 1 file changed, 24 insertions(+), 39 deletions(-) (limited to 'app/models/info_request.rb') diff --git a/app/models/info_request.rb b/app/models/info_request.rb index b1a5c905c..c663f6152 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -793,46 +793,31 @@ public # Display version of status def InfoRequest.get_status_description(status) - if status == 'waiting_classification' - _("Awaiting classification.") - elsif status == 'waiting_response' - _("Awaiting response.") - elsif status == 'waiting_response_overdue' - _("Delayed.") - elsif status == 'waiting_response_very_overdue' - _("Long overdue.") - elsif status == 'not_held' - _("Information not held.") - elsif status == 'rejected' - _("Refused.") - elsif status == 'partially_successful' - _("Partially successful.") - elsif status == 'successful' - _("Successful.") - elsif status == 'waiting_clarification' - _("Waiting clarification.") - elsif status == 'gone_postal' - _("Handled by post.") - elsif status == 'internal_review' - _("Awaiting internal review.") - elsif status == 'error_message' - _("Delivery error") - elsif status == 'requires_admin' - _("Unusual response.") - elsif status == 'attention_requested' - _("Reported for administrator attention.") - elsif status == 'user_withdrawn' - _("Withdrawn by the requester.") - elsif status == 'vexatious' - _("Considered by administrators as vexatious and hidden from site.") - elsif status == 'not_foi' - _("Considered by administrators as not an FOI request and hidden from site.") + descriptions = { + 'waiting_classification' => _("Awaiting classification."), + 'waiting_response' => _("Awaiting response."), + 'waiting_response_overdue' => _("Delayed."), + 'waiting_response_very_overdue' => _("Long overdue."), + 'not_held' => _("Information not held."), + 'rejected' => _("Refused."), + 'partially_successful' => _("Partially successful."), + 'successful' => _("Successful."), + 'waiting_clarification' => _("Waiting clarification."), + 'gone_postal' => _("Handled by post."), + 'internal_review' => _("Awaiting internal review."), + 'error_message' => _("Delivery error"), + 'requires_admin' => _("Unusual response."), + 'attention_requested' => _("Reported for administrator attention."), + 'user_withdrawn' => _("Withdrawn by the requester."), + 'vexatious' => _("Considered by administrators as vexatious and hidden from site."), + 'not_foi' => _("Considered by administrators as not an FOI request and hidden from site."), + } + if descriptions[status] + descriptions[status] + elsif respond_to?(:theme_display_status) + theme_display_status(status) else - begin - return self.theme_display_status(status) - rescue NoMethodError - raise _("unknown status ") + status - end + raise _("unknown status ") + status end end -- cgit v1.2.3 From d9fd0afaca38c0818ebd1221bd4843014ad6992c Mon Sep 17 00:00:00 2001 From: Matthew Landauer Date: Thu, 28 Feb 2013 15:08:36 +1100 Subject: When setting a state optionally pass a messsage --- 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 c663f6152..191f10018 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -548,7 +548,7 @@ public end # change status, including for last event for later historical purposes - def set_described_state(new_state, set_by = nil) + def set_described_state(new_state, set_by = nil, message = "") ActiveRecord::Base.transaction do self.awaiting_description = false last_event = self.info_request_events.last @@ -563,7 +563,7 @@ public 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) + RequestMailer.deliver_requires_admin(self, set_by, message) end end end -- cgit v1.2.3 From f60ada47d4e7aabe0dce152109cb0d91865929da Mon Sep 17 00:00:00 2001 From: Matthew Landauer Date: Mon, 4 Mar 2013 14:16:37 +1100 Subject: Refactor functionality from controller to model --- app/models/info_request.rb | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'app/models/info_request.rb') diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 191f10018..237364f56 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -549,6 +549,7 @@ public # change status, including for last event for later historical purposes def set_described_state(new_state, set_by = nil, message = "") + old_described_state = described_state ActiveRecord::Base.transaction do self.awaiting_description = false last_event = self.info_request_events.last @@ -566,6 +567,20 @@ public RequestMailer.deliver_requires_admin(self, set_by, message) end end + + unless set_by.nil? || is_actual_owning_user?(set_by) || described_state == 'attention_requested' + # Log the status change by someone other than the requester + event = log_event("status_update", + { :user_id => set_by.id, + :old_described_state => old_described_state, + :described_state => described_state, + }) + # Create a classification event for league tables + RequestClassification.create!(:user_id => set_by.id, + :info_request_event_id => event.id) + + RequestMailer.deliver_old_unclassified_updated(self) if !is_external? + end end # Work out what the situation of the request is. In addition to values of -- cgit v1.2.3