diff options
| -rw-r--r-- | app/controllers/request_controller.rb | 12 | ||||
| -rw-r--r-- | app/models/info_request.rb | 17 | ||||
| -rw-r--r-- | app/models/info_request_event.rb | 5 | ||||
| -rw-r--r-- | app/models/outgoing_message.rb | 10 | ||||
| -rw-r--r-- | app/models/public_body.rb | 3 | ||||
| -rw-r--r-- | app/views/admin_request/edit.rhtml | 1 | ||||
| -rw-r--r-- | app/views/general/search.rhtml | 1 | ||||
| -rw-r--r-- | app/views/request/_followup.rhtml | 39 | ||||
| -rw-r--r-- | app/views/request/followup_preview.rhtml | 7 | ||||
| -rw-r--r-- | app/views/request/show.rhtml | 5 | ||||
| -rw-r--r-- | db/migrate/069_add_what_doing.rb | 12 | ||||
| -rw-r--r-- | db/schema.rb | 5 | ||||
| -rw-r--r-- | spec/fixtures/outgoing_messages.yml | 2 | ||||
| -rw-r--r-- | todo.txt | 60 |
14 files changed, 139 insertions, 40 deletions
diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index c71c2f804..d123e0757 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -4,7 +4,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: request_controller.rb,v 1.132 2008-10-28 18:23:31 francis Exp $ +# $Id: request_controller.rb,v 1.133 2008-11-05 18:19:46 francis Exp $ class RequestController < ApplicationController @@ -280,6 +280,9 @@ class RequestController < ApplicationController redirect_to show_response_url(:id => @info_request.id, :incoming_message_id => @events_needing_description[-1].params[:incoming_message_id]) elsif @info_request.calculate_status == 'gone_postal' redirect_to respond_to_last_url(@info_request) + "?gone_postal=1" + elsif @info_request.calculate_status == 'internal_review' + flash[:notice] = 'XXX message to give people entering internal review state to be done' + redirect_to request_url(@info_request) elsif @info_request.calculate_status == 'requires_admin' flash[:notice] = "Please use the form below if you would like to tell us what is unusual about the response." redirect_to help_general_url(:action => 'contact') @@ -379,10 +382,15 @@ class RequestController < ApplicationController render :action => 'followup_preview' return end + # Send a follow up message @outgoing_message.send_message @outgoing_message.save! - flash[:notice] = "Your follow up message has been created and sent on its way." + if @outgoing_message.what_doing == 'internal_review' + flash[:notice] = "Your internal review request has been created and sent on its way." + else + flash[:notice] = "Your follow up message has been created and sent on its way." + end redirect_to request_url(@info_request) end else diff --git a/app/models/info_request.rb b/app/models/info_request.rb index 231f8dc91..2360a5b8b 100644 --- a/app/models/info_request.rb +++ b/app/models/info_request.rb @@ -23,7 +23,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: info_request.rb,v 1.150 2008-10-28 13:29:21 francis Exp $ +# $Id: info_request.rb,v 1.151 2008-11-05 18:19:46 francis Exp $ require 'digest/sha1' require File.join(File.dirname(__FILE__),'../../vendor/plugins/acts_as_xapian/lib/acts_as_xapian') @@ -55,6 +55,7 @@ class InfoRequest < ActiveRecord::Base 'rejected', 'successful', 'partially_successful', + 'internal_review', 'requires_admin' ] @@ -304,7 +305,8 @@ 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' }) ir.outgoing_messages << om @@ -386,10 +388,11 @@ public event.save! end curr_state = nil - elsif !curr_state.nil? && event.event_type == 'followup_sent' && !event.described_state.nil? && event.described_state == 'waiting_response' - # followups can set the status to waiting response, which we don't - # want to propogate to the response itself, as that might already be - # set to waiting_clarification, which we want to know about. + elsif !curr_state.nil? && event.event_type == 'followup_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, which we don't want to propogate to the response + # itself, as that might already be set to waiting_clarification + # / a success status, which we want to know about. curr_state = nil end end @@ -630,6 +633,8 @@ public "Waiting clarification." elsif status == 'gone_postal' "Handled by post." + elsif status == 'internal review' + "Awaiting internal review." elsif status == 'requires_admin' "Unusual response." else diff --git a/app/models/info_request_event.rb b/app/models/info_request_event.rb index 3a882e0c6..45fb1c48e 100644 --- a/app/models/info_request_event.rb +++ b/app/models/info_request_event.rb @@ -21,7 +21,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: info_request_event.rb,v 1.65 2008-10-28 13:04:20 francis Exp $ +# $Id: info_request_event.rb,v 1.66 2008-11-05 18:19:46 francis Exp $ class InfoRequestEvent < ActiveRecord::Base belongs_to :info_request @@ -60,6 +60,7 @@ class InfoRequestEvent < ActiveRecord::Base 'rejected', 'successful', 'partially_successful', + 'internal_review', 'requires_admin' ] @@ -211,6 +212,8 @@ class InfoRequestEvent < ActiveRecord::Base "Some information sent" elsif status == 'successful' "All information sent" + elsif status == 'internal review' + "Internal review acknowledgement" elsif status == 'requires_admin' "Unusual response" else diff --git a/app/models/outgoing_message.rb b/app/models/outgoing_message.rb index 4ca0d391d..8560f70bd 100644 --- a/app/models/outgoing_message.rb +++ b/app/models/outgoing_message.rb @@ -21,7 +21,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: outgoing_message.rb,v 1.71 2008-10-28 13:04:20 francis Exp $ +# $Id: outgoing_message.rb,v 1.72 2008-11-05 18:19:46 francis Exp $ class OutgoingMessage < ActiveRecord::Base belongs_to :info_request @@ -32,6 +32,8 @@ class OutgoingMessage < ActiveRecord::Base belongs_to :incoming_message_followup, :foreign_key => 'incoming_message_followup_id', :class_name => 'IncomingMessage' + validates_inclusion_of :what_doing, :in => ['new_information', 'internal_review', 'normal_sort'] + # can have many events, for items which were resent by site admin e.g. if # contact address changed has_many :info_request_events @@ -117,6 +119,9 @@ class OutgoingMessage < ActiveRecord::Base if self.body =~ /#{get_signoff}\s*\Z/ms errors.add(:body, '^Please sign at the bottom with your name, or alter the "' + get_signoff + '" signature') end + if self.what_doing.nil? + errors.add(:what_doing_dummy, '^Please choose what sort of reply you are making.') + end end # Deliver outgoing message @@ -139,6 +144,9 @@ class OutgoingMessage < ActiveRecord::Base if self.info_request.described_state == 'waiting_clarification' self.info_request.set_described_state('waiting_response') end + if self.what_doing == 'internal_review' + self.info_request.set_described_state('internal_review') + end else raise "Message id #{self.id} has type '#{self.message_type}' which send_message can't handle" end diff --git a/app/models/public_body.rb b/app/models/public_body.rb index 032af9820..e27496172 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -24,7 +24,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: public_body.rb,v 1.120 2008-11-03 21:48:01 francis Exp $ +# $Id: public_body.rb,v 1.121 2008-11-05 18:19:46 francis Exp $ require 'csv' require 'set' @@ -350,6 +350,7 @@ class PublicBody < ActiveRecord::Base set_of_importing = Set.new() line = 0 + CSV::Reader.parse(csv) do |row| line = line + 1 diff --git a/app/views/admin_request/edit.rhtml b/app/views/admin_request/edit.rhtml index b5f31e403..646d643fe 100644 --- a/app/views/admin_request/edit.rhtml +++ b/app/views/admin_request/edit.rhtml @@ -24,6 +24,7 @@ 'rejected', 'successful', 'partially_successful', + 'internal_review', 'requires_admin', ]) %> </p> diff --git a/app/views/general/search.rhtml b/app/views/general/search.rhtml index cd63bc12d..54c680ade 100644 --- a/app/views/general/search.rhtml +++ b/app/views/general/search.rhtml @@ -130,6 +130,7 @@ <tr><td><strong><%=search_link('status:successful')%></strong></td><td> All of the information requested has been received </td></tr> <tr><td><strong><%=search_link('status:waiting_clarification')%></strong></td><td> The public authority would like part of the request explained </td></tr> <tr><td><strong><%=search_link('status:gone_postal')%></strong></td><td> The public authority would like to / has responded by post </td></tr> + <tr><td><strong><%=search_link('status:internal_review')%></strong></td><td> Waiting for the public authority to complete an internal review of the request</td></tr> <tr><td><strong><%=search_link('status:requires_admin')%></strong></td><td> A strange reponse, required attention by the WhatDoTheyKnow team </td></tr> </table> <% end %> diff --git a/app/views/request/_followup.rhtml b/app/views/request/_followup.rhtml index 87b1bde82..fe16ff33b 100644 --- a/app/views/request/_followup.rhtml +++ b/app/views/request/_followup.rhtml @@ -1,11 +1,11 @@ <div id="followup"> <% if incoming_message.nil? || !incoming_message.valid_to_reply_to? %> - <h2>Send a follow up message + <h2>Send a public follow up message to '<%=h RequestMailer.name_for_followup(@info_request, nil) %>' </h2> <% else %> - <h2>Send a reply to <%=h RequestMailer.name_for_followup(@info_request, incoming_message) %> + <h2>Send a public reply to <%=h RequestMailer.name_for_followup(@info_request, incoming_message) %> </h2> <% end %> @@ -14,14 +14,13 @@ <a href="/help/contact">contact us</a> if you are <%= user_link(@info_request.user) %> and need to send a follow up.</p> <% else %> - <p>Use this to tell the public authority something, such as to clarify - your request, or if they are late responding.</p> - <p>Please do <strong>not</strong> make new requests here. +<!-- <p>Please do <strong>not</strong> make new requests here. If you are asking for information which was not in your original request, then <%= link_to "file a new request", new_request_to_body_url(:public_body_id => @info_request.public_body.id.to_s) %> instead. </p> + --> <% if @info_request.calculate_status == 'waiting_response_overdue' %> <p>This request is currently <strong>overdue a response</strong> from <%= @@ -39,11 +38,31 @@ <%= o.text_area :body, :rows => 10, :cols => 55 %> <br><script type="text/javascript">document.write('<input name="doSpell" type="button" value="Check spelling" onClick="openSpellChecker(document.getElementById(\'followup_form\').body);"/> (optional)')</script> </p> - - <p> - <strong>Privacy warning:</strong> Your follow up message, and any response - to it, will also be displayed publicly on this website. - </p> + + <h3>What are you doing?</h3> + + <% if !@outgoing_message.errors[:what_doing_dummy].nil? %> + <div class="fieldWithErrors"> + <% else %> + <div> + <% end %> + <!-- + <div> + <%= radio_button "outgoing_message", "what_doing", "new_information", :id => "new_information" %> + <label for="new_information">I am asking for <strong>new information</strong> </label> + </div> + --> + <div> + <%= radio_button "outgoing_message", "what_doing", "internal_review", :id => "internal_review" %> + <label for="internal_review">I am requesting an <strong>internal review</strong> + (<a href="/help/unhappy">what's that?</a>) + </label> + </div> + <div> + <%= radio_button "outgoing_message", "what_doing", "normal_sort", :id => "sort_normal" %> + <label for="sort_normal"><strong>Anything else</strong>, such as clarifying, prompting, thanking</label> + </div> + </div> <p> <%= hidden_field_tag 'submitted_followup', 1 %> diff --git a/app/views/request/followup_preview.rhtml b/app/views/request/followup_preview.rhtml index 543924ed2..a924b46ae 100644 --- a/app/views/request/followup_preview.rhtml +++ b/app/views/request/followup_preview.rhtml @@ -29,6 +29,13 @@ <% end %> <p> + <strong>Privacy warning:</strong> Your message, and any response + to it, will be displayed publicly on this website. + + <%= o.hidden_field(:what_doing) %> + </p> + + <p> <%= hidden_field_tag(:submitted_followup, 1) %> <%= hidden_field_tag(:preview, 0 ) %> <%= submit_tag "Re-edit this follow up", :name => 'reedit' %> diff --git a/app/views/request/show.rhtml b/app/views/request/show.rhtml index 00cd27086..eb09bfa43 100644 --- a/app/views/request/show.rhtml +++ b/app/views/request/show.rhtml @@ -94,6 +94,8 @@ <% end %> <% elsif @status == 'gone_postal' %> The authority would like to / has <strong>responded by post</strong> to this request. + <% elsif @status == 'internal_review' %> + Waiting for an <strong>internal review</strong> by <%= public_body_link(@info_request.public_body) %> of their handling of this request. <% elsif @status == 'requires_admin' %> This request has had an unusual response, and <strong>requires attention</strong> from the WhatDoTheyKnow team. <% else %> @@ -125,6 +127,9 @@ <% end %> (<%=h @info_request.user.name %> only) <br> + <!-- <%= link_to "Request an internal review", show_response_no_followup_url(:id => @info_request.id, :incoming_message_id => nil) + "#followup" %> + (<%=h @info_request.user.name %> only) + <br> --> <%= link_to "Respond to request", upload_response_url(:url_title => @info_request.url_title) %> (<%=h @info_request.public_body.name %> only) </div> diff --git a/db/migrate/069_add_what_doing.rb b/db/migrate/069_add_what_doing.rb new file mode 100644 index 000000000..be8039fec --- /dev/null +++ b/db/migrate/069_add_what_doing.rb @@ -0,0 +1,12 @@ +class AddWhatDoing < ActiveRecord::Migration + def self.up + add_column :outgoing_messages, :what_doing, :string + add_index :outgoing_messages, :what_doing + OutgoingMessage.update_all "what_doing = 'normal_sort'" + change_column :outgoing_messages, :what_doing, :string, :null => false + end + + def self.down + remove_column :outgoing_messages, :what_doing + end +end diff --git a/db/schema.rb b/db/schema.rb index e33f36c38..dd0a70937 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -9,7 +9,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 68) do +ActiveRecord::Schema.define(:version => 69) do create_table "acts_as_xapian_jobs", :force => true do |t| t.string "model", :null => false @@ -93,8 +93,11 @@ ActiveRecord::Schema.define(:version => 68) do t.datetime "updated_at", :null => false t.datetime "last_sent_at" t.integer "incoming_message_followup_id" + t.string "what_doing", :null => false end + add_index "outgoing_messages", ["what_doing"], :name => "index_outgoing_messages_on_what_doing" + create_table "post_redirects", :force => true do |t| t.text "token", :null => false t.text "uri", :null => false diff --git a/spec/fixtures/outgoing_messages.yml b/spec/fixtures/outgoing_messages.yml index d4e6bdc13..b89492aa5 100644 --- a/spec/fixtures/outgoing_messages.yml +++ b/spec/fixtures/outgoing_messages.yml @@ -13,6 +13,7 @@ useless_outgoing_message: Bob\r\n" last_sent_at: 2007-10-25 10:41:12.686264 created_at: 2007-10-12 01:56:58.586598 + what_doing: normal_sort silly_outgoing_message: id: 2 info_request_id: 103 @@ -31,4 +32,5 @@ silly_outgoing_message: Bob\r\n" last_sent_at: 2007-10-14 10:41:12.686264 created_at: 2007-10-14 01:56:58.586598 + what_doing: normal_sort @@ -1,48 +1,65 @@ Test data for Tony - Internal review =============== +Change wording of the status change dialog when coming out of internal +review + +Fix up controllers/request_controller internal_review state redirect +in that case. + +When you select "waiting_response" might want to convert to +internal_review in case where current state is already internal_review. + +Clarification of internal review in outgoing message send_message function +might want to go back to internal review, not to general awaiting response? + models/outgoing_message.rb line 144 + +Send internal reviews to original main request address + Where to offer button to enter internal review state: - always offer option on request to have internal review, except when in waiting_response earlier than 20 days, and except when in waiting_clarification (call button "make clarification"!) - offer optin more forcefully when in waiting_response and lots of days have gone by - when entering not_held, rejected, partially_successful offer internal review button clearly -If you've already conducted an internal review, at all those places above offer -link through to ICO submitting help page instead. - -Ask Julian which address is best to send internal reviews to? +Make the internal review option default the radio button on the followup form -Search for text "internal review" in followups and add warning -Let people change status at any point +Search for text "internal review" in followups and add warning if they aren't +using the internal review mode. -Fix up controllers/request_controller internal_review state +Change preview when asking for internal review -Change wording of the status change dialog when coming out of itnernal -review +If you've already conducted an internal review, at all those places above offer +link through to ICO submitting help page instead. -Clarification of internal review in outgoing message send_message runction -might want to go back to internal review, not to general awaiting response? +Clock for internal review -When you select "waiting_response" might want to convert to -internal_review in case where current state is already internal_review. +Add the "passing on" text from Julian's requests as default for internal review Next ==== +Finish "new information" option when writing followup, so makes new request +Let people change status at any point + Make green box more visible when click reminder email link Check new wording in reminder email looks good +Make the second and third reminder email for new responses go to the green bit +Remove login link from first email Make it so you definitely don't get alert for the annotation that you just made The Issue document here doesn't load - need to decect word docs from file content. -http://www.whatdotheyknow.com/request/monitoring_of_foi_internal_revie - -Do something about <title> of views + http://www.whatdotheyknow.com/request/monitoring_of_foi_internal_revie +Maybe use mahoro-ruby - add that to config/packages + require 'mahoro' + @m = Mahoro.new + @m.flags = Mahoro::MIME + @m.buffer(File.read('mahoro.c')) Performance: Remove loading of public body tags from every info request load @@ -98,11 +115,16 @@ CSS things Write code to make sure the Return-Path is never foi@sandwich.org.uk (even if the Rails code breaks for Sendmail case in future botched Rails upgrades :) -Consider removing login links from notifications of new responses, now we have remember me +Make the search link to the top of the page for requests + -- or at least for successful responses Make HTML attachments have view as HTML :) http://www.whatdotheyknow.com/request/enforced_medication#incoming-7395 +This one has wrong title when viewed as HTML "private patients". Perhaps +should use attachment filename for title instead? +http://www.whatdotheyknow.com/request/recycling_levels_in_the_winchest + Include log files from exim in admin interface for each request Later @@ -301,6 +323,8 @@ Quoting fixing TODO: Render HTML alternative rather than text (so tables look good) e.g.: http://www.whatdotheyknow.com/request/parking_policy +And indeed so links work: + http://www.whatdotheyknow.com/request/recycling_levels_in_the_winchest Larger new features ------------------- |