Fix duplicate by by (see http://www.whatdotheyknow.com/track/feed/5)

Added more privacy warnings to preview
Fix escaping of HTML in titles in RSS
Improved CSV import function a bit

8 files changed, 109 insertions, 70 deletions

diff --git a/app/models/info_request_event.rb b/app/models/info_request_event.rb
index 7682aff11..6e8ce0231 100644
--- a/app/models/info_request_event.rb
+++ b/app/models/info_request_event.rb
@@ -18,7 +18,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.34 2008-04-04 01:44:41 francis Exp $
+# $Id: info_request_event.rb,v 1.35 2008-04-09 16:53:59 francis Exp $
 
 class InfoRequestEvent < ActiveRecord::Base
     belongs_to :info_request
@@ -152,7 +152,7 @@ class InfoRequestEvent < ActiveRecord::Base
         elsif status == 'not_held'
             "Information not held"
         elsif status == 'rejected'
-            "Rejection by"
+            "Rejection"
         elsif status == 'partially_successful'
             "Some information sent"
         elsif status == 'successful'
diff --git a/app/models/public_body.rb b/app/models/public_body.rb
index 178b6f23b..68129d051 100644
--- a/app/models/public_body.rb
+++ b/app/models/public_body.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: public_body.rb,v 1.53 2008-04-09 02:51:46 francis Exp $
+# $Id: public_body.rb,v 1.54 2008-04-09 16:53:59 francis Exp $
 
 require 'csv'
 require 'set'
@@ -145,62 +145,85 @@ class PublicBody < ActiveRecord::Base
         end
     end
 
-    # Import from CSV 
-    def self.import_csv(csv, tag)
-        ActiveRecord::Base.transaction do
-            existing_bodies = PublicBody.find_by_tag(tag)
-
-            bodies_by_name = {}
-            set_of_existing = Set.new()
-            for existing_body in existing_bodies
-                bodies_by_name[existing_body.name] = existing_body
-                set_of_existing.add(existing_body.name)
-            end
+    class ImportCSVDryRun < StandardError
+    end
 
-            set_of_importing = Set.new()
-            CSV::Reader.parse(csv) do |row|
-                name = row[1]
-                email = row[2]
-                next if name.nil?
-                #or email.nil?
-                if email.nil?
-                    email = '' # unknown/bad contact is empty string
+    # Import from CSV 
+    def self.import_csv(csv, tag, dry_run = false)
+        errors = []
+        notes = []
+
+        begin
+            ActiveRecord::Base.transaction do
+                existing_bodies = PublicBody.find_by_tag(tag)
+
+                bodies_by_name = {}
+                set_of_existing = Set.new()
+                for existing_body in existing_bodies
+                    bodies_by_name[existing_body.name] = existing_body
+                    set_of_existing.add(existing_body.name)
                 end
 
-                name.strip!
-                email.strip!
+                set_of_importing = Set.new()
+                line = 0
+                CSV::Reader.parse(csv) do |row|
+                    line = line + 1
+
+                    name = row[1]
+                    email = row[2]
+                    next if name.nil?
+                    #or email.nil?
+                    if email.nil?
+                        email = '' # unknown/bad contact is empty string
+                    end
 
-                if email != "" && !MySociety::Validate.is_valid_email(email)
-                    raise "invalid email:" + name + " " + email
-                end
+                    name.strip!
+                    email.strip!
 
-                if bodies_by_name[name]
-                    # Already have the public body, just update email
-                    public_body = bodies_by_name[name]
-                    if public_body.request_email != email
-                        public_body.request_email = email
-                        public_body.last_edit_editor = 'import_csv'
-                        public_body.last_edit_comment = 'Updated from spreadsheet'
-                        public_body.save
+                    if email != "" && !MySociety::Validate.is_valid_email(email)
+                        errors.push "error: line " + line.to_s + ": invalid email " + email + " for authority '" + name + "'"
+                        next
                     end
-                else
-                    # New public body
-                    public_body = PublicBody.new(:name => name, :request_email => email, :short_name => "", :last_edit_editor => "import_csv", :last_edit_comment => 'Created from spreadsheet')
-                    public_body.tag_string = tag
-                    public_body.save!
 
+                    if bodies_by_name[name]
+                        # Already have the public body, just update email
+                        public_body = bodies_by_name[name]
+                        if public_body.request_email != email
+                            notes.push "line " + line.to_s + ": updating email for '" + name + "' from " + public_body.request_email + " to " + email
+                            public_body.request_email = email
+                            public_body.last_edit_editor = 'import_csv'
+                            public_body.last_edit_comment = 'Updated from spreadsheet'
+                            public_body.save!
+                        end
+                    else
+                        # New public body
+                        notes.push "line " + line.to_s + ": new authority '" + name + "' with email " + email
+                        public_body = PublicBody.new(:name => name, :request_email => email, :short_name => "", :last_edit_editor => "import_csv", :last_edit_comment => 'Created from spreadsheet')
+                        public_body.tag_string = tag
+                        public_body.save!
+                    end
+
+                    set_of_importing.add(name)
                 end
 
-                set_of_importing.add(name)
-            end
+                # Give an error listing ones that are to be deleted 
+                deleted_ones = set_of_existing - set_of_importing
+                if deleted_ones.size > 0
+                    errors.push "error: Some " + tag + " bodies are in database, but not in CSV file: " + Array(deleted_ones).join(", ")
+                end
 
-            # Give an error listing ones that are to be deleted 
-            deleted_ones = set_of_existing - set_of_importing
-            if deleted_ones.size > 0
-                raise "Some " + tag + " bodies are in database, but not in CSV file:\n" + Array(deleted_ones).join(", ")
+                # Rollback if a dry run, or we had errors
+                if dry_run or errors.size > 0
+                    raise ImportCSVDryRun
+                end
             end
+        rescue ImportCSVDryRun
+            # Ignore
         end
+
+        return errors.join("\n") + notes.join("\n")
     end
+
 end
 
 
diff --git a/app/models/track_thing.rb b/app/models/track_thing.rb
index 2ee494b9d..71ecedb82 100644
--- a/app/models/track_thing.rb
+++ b/app/models/track_thing.rb
@@ -21,7 +21,7 @@
 # Copyright (c) 2008 UK Citizens Online Democracy. All rights reserved.
 # Email: francis@mysociety.org; WWW: http://www.mysociety.org/
 #
-# $Id: track_thing.rb,v 1.8 2008-04-09 01:32:53 francis Exp $
+# $Id: track_thing.rb,v 1.9 2008-04-09 16:53:59 francis Exp $
 
 class TrackThing < ActiveRecord::Base
     belongs_to :tracking_user, :class_name => 'User'
@@ -62,7 +62,7 @@ class TrackThing < ActiveRecord::Base
                     :list_description => "'<a href=\"/request/" + CGI.escapeHTML(self.info_request.url_title) + "\">" + CGI.escapeHTML(self.info_request.title) + "</a>', a request", # XXX yeuch, sometimes I just want to call view helpers from the model, sorry! can't work out how 
                     # Email
                     :title_in_email => "New updates for the request '" + self.info_request.title + "'",
-                    :title_in_rss => "New updates for the request '" + CGI.escapeHTML(self.info_request.title) + "'",
+                    :title_in_rss => "New updates for the request '" + self.info_request.title + "'",
                     # Authentication
                     :web => "To follow updates to the request '" + CGI.escapeHTML(self.info_request.title) + "'",
                     :email => "Then you will be emailed whenever the request '" + CGI.escapeHTML(self.info_request.title) + "' is updated.",
diff --git a/app/views/help/about.rhtml b/app/views/help/about.rhtml
index 1f4e1df13..377703620 100644
--- a/app/views/help/about.rhtml
+++ b/app/views/help/about.rhtml
@@ -76,7 +76,7 @@ pages on the Information Commisioner's website.
 
 <dt id="data_protection">Can I request information about myself?</dt>
 
-<dd>No. Requests made using WhatDoTheyKnow are public, made under the
+<dd>Not using this site. Requests made using WhatDoTheyKnow are public, made under the
 Freedom of Information Act, and cannot help you find information about a
 private individual. If you would like to know what information a public
 authority holds about yourself, you should make a "Subject Access Request" in
@@ -96,7 +96,7 @@ you manage FOI requests in secret, then <a href="/help/contact">contact us</a>.
 
 </dl>
 
-<h1 id="privacy">Privacy Questions</h1>
+<h1 id="privacy">Privacy policy</h1>
 
 <dl>
 
@@ -115,14 +115,14 @@ about the site or related mySociety services. We will never give or sell
 your email addresses to anyone else, unless we are obliged to by law, or you
 ask us to.</dd>
 
-<dt>Why will my name appear publically on the site?</dt>
+<dt id="public_request">Why will my name appear publically on the site?</dt>
 
 <dd>It means that someone researching the same area can get in touch with you,
 and maybe give you more information or ideas relating to your request.  Also,
 we're going to publish the response with the name of the civil servant who
 wrote it, so it seems only fair that your name should be public too!
 Therefore, we encourage you to use your real name, but you may use a pseudonym
-if you would like to be anonymous.
+if you would like to be anonymous. 
 </dd>
 
 <dt id="tracking">Why does the list of things that I'm tracking appear publically on the site?</dt>
diff --git a/app/views/request/new.rhtml b/app/views/request/new.rhtml
index 5d2b85ddb..d36f1142a 100644
--- a/app/views/request/new.rhtml
+++ b/app/views/request/new.rhtml
@@ -72,7 +72,7 @@
     </p>
 
     <p class="form_note">
-        <Strong>Can I request information about myself?</strong>
+        <strong>Can I request information about myself?</strong>
         <a href="/help/about#data_protection">No! (Click here for details)</a> 
     </p>
 
diff --git a/app/views/request/preview.rhtml b/app/views/request/preview.rhtml
index ef12f89ce..bac3a0a64 100644
--- a/app/views/request/preview.rhtml
+++ b/app/views/request/preview.rhtml
@@ -2,7 +2,14 @@
 
 <% form_for(:info_request, @info_request, :html => { :id => 'preview_form' }  ) do |f| %>
     
+
     <h1>Now preview your request</h1>
+    <ul>
+    <li>Check you haven't included any <strong>personal information</strong>.</li>
+    <li>Your name, request and any responses will appear in <strong>search engines</strong>
+        (<a href="/help/about#public_request">details</a>).
+    </li>
+    </ul>
 
     <% fields_for :outgoing_message do |o| %>
     <p class="outgoing_message_preview">
@@ -14,8 +21,8 @@
     </p>
     <% end %>
 
-    <p><strong>Note:</strong> Your request and any response will be
-    displayed publically on this website.</p>
+    <p><strong>Privacy note:</strong> If you want to request private information about
+    yourself then <a href="/help/about#data_protection">click here</a>.
 
     <p>
     <%= f.hidden_field(:title) %>
diff --git a/script/import-public-body-csv b/script/import-public-body-csv
index d29e52770..269b7cee6 100755
--- a/script/import-public-body-csv
+++ b/script/import-public-body-csv
@@ -5,6 +5,6 @@ LOC=`dirname $0`
 # Arguments are:
 # - tag to use for public bodies from this spreadsheet
 
-$LOC/runner 'PublicBody.import_csv(STDIN.read, "'$1'")'
+$LOC/runner 'STDERR.write(PublicBody.import_csv(STDIN.read, "'$1'"))'
 
 
diff --git a/todo.txt b/todo.txt
index 196ff64a9..2fa2b7823 100644
--- a/todo.txt
+++ b/todo.txt
@@ -1,3 +1,10 @@
+CVS commits:
+
+Fix duplicate by by (see http://www.whatdotheyknow.com/track/feed/5)
+Added more privacy warnings to preview
+Fix escaping of HTML in titles in RSS
+Improved CSV import function a bit
+
 FOI requests to use to test it
 ==============================
 
@@ -28,10 +35,15 @@ BAILII - relationship with law courts, robots.txt ?
 Next
 ====
 
+Check date order here
+http://www.whatdotheyknow.com/track/feed/5
+
 Fix search to deal with overdue message query that Tomski asks for
 
 Don't let people track their own requests?
 
+Let you change feeds from email to RSS
+
 "Some other requests..." should have "more" link now
 
 If you edit needs anim to waiting_response it doesn't edit the value in the event history, so screws up search stuff?
@@ -40,6 +52,11 @@ https://secure.mysociety.org/admin/foi/request/show/137
 
 GUI for uploading changed CSV files
 
+Things to track:
+    - new requests
+    - new requests with keyword
+    - successful responses
+
 Later
 =====
 
@@ -61,6 +78,12 @@ Also sort user.info_requests for user pages
 Also see in request_controller.rb: @info_request_events.sort! { |a,b| a.created_at <=> b.created_at }
 Go through all has_many and pick something sane
 
+Add credits for everyone
+    Heather Brook, Chris Lightfoot, other original suggester in Call for Proposals
+    Julian Todd, Francis Davey, Etienne Pollard
+    Alex Skene, John Cross, Adam McGregor
+    Angie Ahl, Matthew Somerville, Tom Steinberg, Tommy, Francis Irving
+
 Offer search on 404s
 
 Search and replace text "FOI" and "Freedom of Information" out the way more
@@ -110,8 +133,6 @@ Overdue response events, so track alerts / RSS picks up when response is late
 Overdue response alert email click through should show how many days overdue
     it is near where you write your reply
 
-Get deploying of Lucene search working
-
 Consider on staging sites making follow ups go to dummy address also
 
 Synthesise these tips into our handful of snappy snappy bullet points
@@ -120,18 +141,6 @@ Synthesise these tips into our handful of snappy snappy bullet points
 
 Show public body email address on their public page, with a link to say "this isn't right!"
 
-Email me updates on this request
-Generic alerting/tracking system including
-    - email alerts and RSS
-Things to track:
-    - updates to a particular request
-    - new requests
-    - new requests with keyword
-    - successful responses
-
-    - new requests, new good responses etc.
-    - new responses to a particular request
-
 Requests with related content (via Lucene compare document search somehow?)
 Blog posts / Wikipedia articles about this request