From 76de470b1424be57934e58275a6116afb8eb9b3c Mon Sep 17 00:00:00 2001 From: Seb Bacon Date: Fri, 9 Mar 2012 11:48:38 +0000 Subject: first stab at sending PURGE requests to upstream varnish for request pages. Next step: making it asynchronous, e.g. with a queue of things to purge via a cron job. --- 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 b5a1cd833..3a1f4b9f3 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -1042,6 +1042,21 @@ public end return ret end + + before_save(:mark_view_is_dirty) + def mark_view_is_dirty + self.view_is_dirty = true + self.save! + end + + def self.purge_varnish + for info_request in InfoRequest.find_by_view_is_dirty(true) + url = "/request/#{info_request.url_title}" + purge(url) + info_request.view_is_dirty = true + info_request.save! + end + end end -- cgit v1.2.3 From 4533b28661d6ce6973becf5877e82ca5656c37d2 Mon Sep 17 00:00:00 2001 From: Seb Bacon Date: Fri, 20 Apr 2012 19:10:10 +0100 Subject: More changes and refactoring to make purges work. --- app/models/info_request.rb | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) (limited to 'app/models/info_request.rb') diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 3a1f4b9f3..9b129e74b 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -453,7 +453,6 @@ public # An annotation (comment) is made def add_comment(body, user) comment = Comment.new - ActiveRecord::Base.transaction do comment.body = body comment.user = user @@ -1043,18 +1042,14 @@ public return ret end - before_save(:mark_view_is_dirty) - def mark_view_is_dirty - self.view_is_dirty = true - self.save! - end - - def self.purge_varnish - for info_request in InfoRequest.find_by_view_is_dirty(true) - url = "/request/#{info_request.url_title}" - purge(url) - info_request.view_is_dirty = true - info_request.save! + before_save :purge_in_cache + def purge_in_cache + if !MySociety::Config.get('VARNISH_HOST').nil? && !self.id.nil? + # we only do this for existing info_requests (new ones have a nil id) + req = PurgeRequest.new(:url => "/request/#{self.url_title}", + :model => self.class.base_class.to_s, + :model_id => self.id) + req.save() end end end -- cgit v1.2.3 From f2367311f662ae5b8da40e59d980b4a6e0726a6b Mon Sep 17 00:00:00 2001 From: Seb Bacon Date: Wed, 2 May 2012 08:13:29 +0100 Subject: Use routing system to work out URL of request to purge, rather than hard coding it. --- app/models/info_request.rb | 6 +++++- 1 file changed, 5 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 9b129e74b..78121f5ea 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -23,6 +23,9 @@ require 'digest/sha1' class InfoRequest < ActiveRecord::Base + include ActionView::Helpers::UrlHelper + include ActionController::UrlWriter + strip_attributes! validates_presence_of :title, :message => N_("Please enter a summary of your request") @@ -1046,7 +1049,8 @@ public def purge_in_cache if !MySociety::Config.get('VARNISH_HOST').nil? && !self.id.nil? # we only do this for existing info_requests (new ones have a nil id) - req = PurgeRequest.new(:url => "/request/#{self.url_title}", + path = url_for(:controller => 'request', :action => 'show', :url_title => self.url_title, :only_path => true, :locale => :none) + req = PurgeRequest.new(:url => path, :model => self.class.base_class.to_s, :model_id => self.id) req.save() -- cgit v1.2.3 From b0b1aebb371211e0deb5e9eac600817e90f83301 Mon Sep 17 00:00:00 2001 From: Seb Bacon Date: Fri, 4 May 2012 11:53:36 +0100 Subject: Don't ever create more than one entry for each URL that we want Varnish to purge. --- app/models/info_request.rb | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'app/models/info_request.rb') diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 78121f5ea..e570150bb 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -1050,9 +1050,12 @@ public if !MySociety::Config.get('VARNISH_HOST').nil? && !self.id.nil? # we only do this for existing info_requests (new ones have a nil id) path = url_for(:controller => 'request', :action => 'show', :url_title => self.url_title, :only_path => true, :locale => :none) - req = PurgeRequest.new(:url => path, - :model => self.class.base_class.to_s, - :model_id => self.id) + req = PurgeRequest.find_by_url(path) + if req.nil? + req = PurgeRequest.new(:url => path, + :model => self.class.base_class.to_s, + :model_id => self.id) + end req.save() end end -- cgit v1.2.3 From 014c5a221a3ac47d89de71cfd81054f39ac3759d Mon Sep 17 00:00:00 2001 From: Seb Bacon Date: Tue, 15 May 2012 09:52:11 +0100 Subject: Remove trailing whitespace (to make a cleaner forthcoming merge with wombleton:feature/440_sparkly_admin_css) --- app/models/info_request.rb | 68 +++++++++++++++++++++++----------------------- 1 file changed, 34 insertions(+), 34 deletions(-) (limited to 'app/models/info_request.rb') diff --git a/app/models/info_request.rb b/app/models/info_request.rb index e570150bb..3b86f4cb3 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -51,14 +51,14 @@ class InfoRequest < ActiveRecord::Base # user described state (also update in info_request_event, admin_request/edit.rhtml) validate :must_be_valid_state - validates_inclusion_of :prominence, :in => [ - 'normal', + validates_inclusion_of :prominence, :in => [ + 'normal', 'backpage', 'hidden', 'requester_only' ] - validates_inclusion_of :law_used, :in => [ + validates_inclusion_of :law_used, :in => [ 'foi', # Freedom of Information Act 'eir', # Environmental Information Regulations ] @@ -77,13 +77,13 @@ class InfoRequest < ActiveRecord::Base ] def self.enumerate_states - states = [ + states = [ 'waiting_response', - 'waiting_clarification', + 'waiting_clarification', 'gone_postal', 'not_held', 'rejected', # this is called 'refused' in UK FOI law and the user interface, but 'rejected' internally for historic reasons - 'successful', + 'successful', 'partially_successful', 'internal_review', 'error_message', @@ -97,7 +97,7 @@ class InfoRequest < ActiveRecord::Base end def must_be_valid_state - errors.add(:described_state, "is not a valid state") if + errors.add(:described_state, "is not a valid state") if !InfoRequest.enumerate_states.include? described_state end @@ -123,7 +123,7 @@ class InfoRequest < ActiveRecord::Base 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 after_initialize @@ -169,7 +169,7 @@ class InfoRequest < ActiveRecord::Base end end # Force reindex when tag string changes - alias_method :orig_tag_string=, :tag_string= + alias_method :orig_tag_string=, :tag_string= def tag_string=(tag_string) ret = self.orig_tag_string=(tag_string) reindex_request_events @@ -222,7 +222,7 @@ public end # Email which public body should use to respond to request. This is in - # the format PREFIXrequest-ID-HASH@DOMAIN. Here ID is the id of the + # the format PREFIXrequest-ID-HASH@DOMAIN. Here ID is the id of the # FOI request, and HASH is a signature for that id. def incoming_email return self.magic_email("request-") @@ -254,7 +254,7 @@ public end end - # Two sorts of laws for requests, FOI or EIR + # Two sorts of laws for requests, FOI or EIR def law_used_full if self.law_used == 'foi' return _("Freedom of Information") @@ -309,7 +309,7 @@ public guesses = [] # 1. Try to guess based on the email address(es) addresses = - (incoming_message.mail.to || []) + + (incoming_message.mail.to || []) + (incoming_message.mail.cc || []) + (incoming_message.mail.envelope_to || []) addresses.uniq! @@ -541,7 +541,7 @@ public self.base_calculate_status end end - + def base_calculate_status return 'waiting_classification' if self.awaiting_description return described_state unless self.described_state == "waiting_response" @@ -561,13 +561,13 @@ public curr_state = nil for event in self.info_request_events.reverse event.xapian_mark_needs_index # we need to reindex all events in order to update their latest_* terms - if curr_state.nil? + if curr_state.nil? if !event.described_state.nil? curr_state = event.described_state end end - if !curr_state.nil? && event.event_type == 'response' + 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() @@ -581,7 +581,7 @@ public elsif !curr_state.nil? && (event.event_type == 'followup_sent' || event.event_type == 'sent') && !event.described_state.nil? && (event.described_state == 'waiting_response' || event.described_state == 'internal_review') # Followups can set the status to waiting response / internal # review. Initial requests ('sent') set the status to waiting response. - + # We want to store that in calculated_state state so it gets # indexed. if event.calculated_state != event.described_state @@ -725,8 +725,8 @@ public def index_of_last_described_event events = self.info_request_events events.each_index do |i| - revi = events.size - 1 - i - m = events[revi] + revi = events.size - 1 - i + m = events[revi] if not m.described_state.nil? return revi end @@ -737,7 +737,7 @@ public def last_event_id_needing_description last_event = events_needing_description[-1] last_event.nil? ? 0 : last_event.id - end + end # Returns all the events which the user hasn't described yet - an empty array if all described. def events_needing_description @@ -825,11 +825,11 @@ public track_thing.destroy end self.user_info_request_sent_alerts.each { |a| a.destroy } - self.info_request_events.each do |info_request_event| + self.info_request_events.each do |info_request_event| info_request_event.track_things_sent_emails.each { |a| a.destroy } info_request_event.destroy end - self.exim_logs.each do |exim_log| + self.exim_logs.each do |exim_log| exim_log.destroy end self.outgoing_messages.each { |a| a.destroy } @@ -844,8 +844,8 @@ public return InfoRequest.magic_email_for_id(prefix_part, self.id) end - def InfoRequest.magic_email_for_id(prefix_part, id) - magic_email = MySociety::Config.get("INCOMING_EMAIL_PREFIX", "") + def InfoRequest.magic_email_for_id(prefix_part, id) + magic_email = MySociety::Config.get("INCOMING_EMAIL_PREFIX", "") magic_email += prefix_part + id.to_s magic_email += "-" + InfoRequest.hash_from_id(id) magic_email += "@" + MySociety::Config.get("INCOMING_EMAIL_DOMAIN", "localhost") @@ -890,14 +890,14 @@ public def InfoRequest.find_old_unclassified(extra_params={}) 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'", - true, Time.now() - age], + params = {:select => "*, #{last_response_created_at} as last_response_time", + :conditions => ["awaiting_description = ? and #{last_response_created_at} < ? and url_title != 'holding_pen'", + true, Time.now() - age], :order => "last_response_time"} params[:limit] = extra_params[:limit] if extra_params[:limit] params[:include] = extra_params[:include] if extra_params[:include] if extra_params[:order] - params[:order] = extra_params[:order] + params[:order] = extra_params[:order] params.delete(:select) end if extra_params[:conditions] @@ -907,7 +907,7 @@ public end find(:all, params) end - + def is_old_unclassified? return false if !awaiting_description return false if url_title == 'holding_pen' @@ -926,7 +926,7 @@ public next end incoming_message.safe_mail_from - + email = OutgoingMailer.email_for_followup(self, incoming_message) name = OutgoingMailer.name_for_followup(self, incoming_message) @@ -955,7 +955,7 @@ public end end end - + def apply_censor_rules_to_binary!(binary) for censor_rule in self.censor_rules censor_rule.apply_to_binary!(binary) @@ -966,7 +966,7 @@ public end end end - + def is_owning_user?(user) !user.nil? && (user.id == user_id || user.owns_every_request?) end @@ -975,10 +975,10 @@ public end def user_can_view?(user) - if self.prominence == 'hidden' + if self.prominence == 'hidden' return User.view_hidden_requests?(user) end - if self.prominence == 'requester_only' + if self.prominence == 'requester_only' return self.is_owning_user?(user) end return true @@ -1019,7 +1019,7 @@ public end def json_for_api(deep) - ret = { + ret = { :id => self.id, :url_title => self.url_title, :title => self.title, -- cgit v1.2.3 From ce72f203bbeb6ee5ae356f12b007b9631cf0dd7a Mon Sep 17 00:00:00 2001 From: Seb Bacon Date: Tue, 15 May 2012 11:04:50 +0100 Subject: Merge from wombleton:feature/440_sparkly_admin_css Includes a couple of additional fixes: * Remember to HTML-quote things that could come from users * Fix form post action for editing users --- app/models/info_request.rb | 8 ++++++-- 1 file changed, 6 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 3b86f4cb3..1e55f92ae 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -1059,6 +1059,10 @@ public req.save() end end -end - + def for_admin_column + self.class.content_columns.map{|c| c unless %w(title url_title).include?(c.name) }.compact.each do |column| + yield(column.human_name, self.send(column.name), column.type.to_s, column.name) + end + end +end -- cgit v1.2.3 From 6b4e84909554224ea30f901146610d452d612a55 Mon Sep 17 00:00:00 2001 From: Seb Bacon Date: Thu, 24 May 2012 09:35:15 +0100 Subject: First stab at new feature. Adds new box in sidebar for reporting an issue, a new "needs admin attention"-type state to InfoRequests, a flag indicating that a request has ever been marked as needing admin attention, and a controller method and route for setting this state & flag. Also adds the reason something needs admin attention to the subject of the email that gets sent to administrators. Neeeds tests. --- app/models/info_request.rb | 10 ++++++++-- 1 file changed, 8 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 1e55f92ae..4a70e365d 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -88,7 +88,8 @@ class InfoRequest < ActiveRecord::Base 'internal_review', 'error_message', 'requires_admin', - 'user_withdrawn' + 'user_withdrawn', + 'attention_requested' ] if @@custom_states_loaded states += InfoRequest.theme_extra_states @@ -503,7 +504,7 @@ 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'] + return ['requires_admin', 'error_message', 'attention_requested'] end def requires_admin? @@ -511,6 +512,9 @@ public return false end + def can_have_attention_requested? + end + # change status, including for last event for later historical purposes def set_described_state(new_state) ActiveRecord::Base.transaction do @@ -803,6 +807,8 @@ public _("Delivery error") elsif status == 'requires_admin' _("Unusual response.") + elsif status == 'attention_requested' + _("Reported for administrator attention.") elsif status == 'user_withdrawn' _("Withdrawn by the requester.") else -- cgit v1.2.3 From 211d62c99dc70a2c259a2d00485f6f15c353ab2f Mon Sep 17 00:00:00 2001 From: Seb Bacon Date: Thu, 24 May 2012 10:14:05 +0100 Subject: Re-annotate models --- app/models/info_request.rb | 43 ++++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) (limited to 'app/models/info_request.rb') diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 4a70e365d..a6e108970 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -1,24 +1,3 @@ -# == Schema Information -# Schema version: 108 -# -# Table name: info_requests -# -# id :integer not null, primary key -# title :text not null -# user_id :integer not null -# public_body_id :integer not null -# created_at :datetime not null -# updated_at :datetime not null -# described_state :string(255) not null -# awaiting_description :boolean default(FALSE), not null -# prominence :string(255) default("normal"), not null -# url_title :text not null -# law_used :string(255) default("foi"), not null -# allow_new_responses_from :string(255) default("anybody"), not null -# handle_rejected_responses :string(255) default("bounce"), not null -# idhash :string(255) not null -# - require 'digest/sha1' @@ -1072,3 +1051,25 @@ public end end end + +# == Schema Information +# +# Table name: info_requests +# +# id :integer not null, primary key +# title :text not null +# user_id :integer not null +# public_body_id :integer not null +# created_at :datetime not null +# updated_at :datetime not null +# described_state :string(255) not null +# awaiting_description :boolean default(FALSE), not null +# prominence :string(255) default("normal"), not null +# url_title :text not null +# law_used :string(255) default("foi"), not null +# allow_new_responses_from :string(255) default("anybody"), not null +# handle_rejected_responses :string(255) default("bounce"), not null +# idhash :string(255) not null +# attention_requested :boolean default(FALSE) +# + -- cgit v1.2.3 From b89fe3a3b65cad4d6cf0c044b2569e0ea8e8e163 Mon Sep 17 00:00:00 2001 From: Seb Bacon Date: Thu, 24 May 2012 11:32:49 +0100 Subject: Further annotation corrections --- app/models/info_request.rb | 43 ++++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 21 deletions(-) (limited to 'app/models/info_request.rb') diff --git a/app/models/info_request.rb b/app/models/info_request.rb index a6e108970..47398dabb 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -1,3 +1,25 @@ +# == Schema Information +# Schema version: 114 +# +# Table name: info_requests +# +# id :integer not null, primary key +# title :text not null +# user_id :integer not null +# public_body_id :integer not null +# created_at :datetime not null +# updated_at :datetime not null +# described_state :string(255) not null +# awaiting_description :boolean default(FALSE), not null +# prominence :string(255) default("normal"), not null +# url_title :text not null +# law_used :string(255) default("foi"), not null +# allow_new_responses_from :string(255) default("anybody"), not null +# handle_rejected_responses :string(255) default("bounce"), not null +# idhash :string(255) not null +# attention_requested :boolean default(FALSE) +# + require 'digest/sha1' @@ -1052,24 +1074,3 @@ public end end -# == Schema Information -# -# Table name: info_requests -# -# id :integer not null, primary key -# title :text not null -# user_id :integer not null -# public_body_id :integer not null -# created_at :datetime not null -# updated_at :datetime not null -# described_state :string(255) not null -# awaiting_description :boolean default(FALSE), not null -# prominence :string(255) default("normal"), not null -# url_title :text not null -# law_used :string(255) default("foi"), not null -# allow_new_responses_from :string(255) default("anybody"), not null -# handle_rejected_responses :string(255) default("bounce"), not null -# idhash :string(255) not null -# attention_requested :boolean default(FALSE) -# - -- cgit v1.2.3