Merge remote-tracking branch 'origin/release/0.19'0.19

12 files changed, 45 insertions, 71 deletions

diff --git a/app/models/comment.rb b/app/models/comment.rb
index b4c099123..a62c086d5 100644
--- a/app/models/comment.rb
+++ b/app/models/comment.rb
@@ -63,7 +63,7 @@ class Comment < ActiveRecord::Base
 
     # When posting a new comment, use this to check user hasn't double submitted.
     def Comment.find_existing(info_request_id, body)
-        # XXX can add other databases here which have regexp_replace
+        # TODO: can add other databases here which have regexp_replace
         if ActiveRecord::Base.connection.adapter_name == "PostgreSQL"
             # Exclude spaces from the body comparison using regexp_replace
             return Comment.find(:first, :conditions => [ "info_request_id = ? and regexp_replace(body, '[[:space:]]', '', 'g') = regexp_replace(?, '[[:space:]]', '', 'g')", info_request_id, body ])
diff --git a/app/models/foi_attachment.rb b/app/models/foi_attachment.rb
index 6f198249a..a8d105f52 100644
--- a/app/models/foi_attachment.rb
+++ b/app/models/foi_attachment.rb
@@ -178,7 +178,7 @@ class FoiAttachment < ActiveRecord::Base
         return filename
     end
 
-    # XXX changing this will break existing URLs, so have a care - maybe
+    # TODO: changing this will break existing URLs, so have a care - maybe
     # make another old_display_filename see above
     def display_filename
         filename = self.filename
diff --git a/app/models/incoming_message.rb b/app/models/incoming_message.rb
index 124db8d4a..135a6bdaf 100644
--- a/app/models/incoming_message.rb
+++ b/app/models/incoming_message.rb
@@ -150,7 +150,7 @@ class IncomingMessage < ActiveRecord::Base
     end
 
     # The cached fields mentioned in the previous comment
-    # XXX there must be a nicer way to do this without all that
+    # TODO: there must be a nicer way to do this without all that
     # repetition.  I tried overriding method_missing but got some
     # unpredictable results.
     def valid_to_reply_to
@@ -194,7 +194,7 @@ class IncomingMessage < ActiveRecord::Base
     end
 
     # And look up by URL part number and display filename to get an attachment
-    # XXX relies on extract_attachments calling MailHandler.ensure_parts_counted
+    # TODO: relies on extract_attachments calling MailHandler.ensure_parts_counted
     # The filename here is passed from the URL parameter, so it's the
     # display_filename rather than the real filename.
     def self.get_attachment_by_url_part_number_and_filename(attachments, found_url_part_number, display_filename)
@@ -220,7 +220,7 @@ class IncomingMessage < ActiveRecord::Base
 
     # Converts email addresses we know about into textual descriptions of them
     def mask_special_emails!(text)
-        # XXX can later display some of these special emails as actual emails,
+        # TODO: can later display some of these special emails as actual emails,
         # if they are public anyway.  For now just be precautionary and only
         # put in descriptions of them in square brackets.
         if self.info_request.public_body.is_followupable?
@@ -368,8 +368,8 @@ class IncomingMessage < ActiveRecord::Base
 
 
     # Remove quoted sections from emails (eventually the aim would be for this
-    # to do as good a job as GMail does) XXX bet it needs a proper parser
-    # XXX and this FOLDED_QUOTED_SECTION stuff is a mess
+    # to do as good a job as GMail does) TODO: bet it needs a proper parser
+    # TODO: and this FOLDED_QUOTED_SECTION stuff is a mess
     def self.remove_quoted_sections(text, replacement = "FOLDED_QUOTED_SECTION")
         text = text.dup
         replacement = "\n" + replacement + "\n"
@@ -399,7 +399,7 @@ class IncomingMessage < ActiveRecord::Base
                         (
                             \s*#{score}\n(?:(?!#{score}\n).)*? # top line
                             (disclaimer:\n|confidential|received\sthis\semail\sin\serror|virus|intended\s+recipient|monitored\s+centrally|intended\s+(for\s+|only\s+for\s+use\s+by\s+)the\s+addressee|routinely\s+monitored|MessageLabs|unauthorised\s+use)
-                            .*?(?:#{score}|\z) # bottom line OR end of whole string (for ones with no terminator XXX risky)
+                            .*?(?:#{score}|\z) # bottom line OR end of whole string (for ones with no terminator TODO: risky)
                         )
                        /imx, replacement)
         end
@@ -480,7 +480,7 @@ class IncomingMessage < ActiveRecord::Base
     # Returns body text from main text part of email, converted to UTF-8, with uudecode removed,
     # emails and privacy sensitive things remove, censored, and folded to remove excess quoted text
     # (marked with FOLDED_QUOTED_SECTION)
-    # XXX returns a .dup of the text, so calling functions can in place modify it
+    # TODO: returns a .dup of the text, so calling functions can in place modify it
     def get_main_body_text_folded
         if self.cached_main_body_text_folded.nil?
             self._cache_main_body_text
@@ -511,7 +511,7 @@ class IncomingMessage < ActiveRecord::Base
             source_charset = part.charset
             if part.content_type == 'text/html'
                 # e.g. http://www.whatdotheyknow.com/request/35/response/177
-                # XXX This is a bit of a hack as it is calling a
+                # TODO: This is a bit of a hack as it is calling a
                 # convert to text routine.  Could instead call a
                 # sanitize HTML one.
 
@@ -627,7 +627,7 @@ class IncomingMessage < ActiveRecord::Base
             return nil
         end
         # otherwise return it assuming it is text (sometimes you get things
-        # like binary/octet-stream, or the like, which are really text - XXX if
+        # like binary/octet-stream, or the like, which are really text - TODO: if
         # you find an example, put URL here - perhaps we should be always returning
         # nil in this case)
         return p
@@ -722,7 +722,7 @@ class IncomingMessage < ActiveRecord::Base
         text = get_main_body_text_unfolded
         folded_quoted_text = get_main_body_text_folded
 
-        # Remove quoted sections, adding HTML. XXX The FOLDED_QUOTED_SECTION is
+        # Remove quoted sections, adding HTML. TODO: The FOLDED_QUOTED_SECTION is
         # a nasty hack so we can escape other HTML before adding the unfold
         # links, without escaping them. Rather than using some proper parser
         # making a tree structure (I don't know of one that is to hand, that
diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index 47ad435cb..aed651ad3 100644
--- a/app/models/info_request.rb
+++ b/app/models/info_request.rb
@@ -115,6 +115,16 @@ class InfoRequest < ActiveRecord::Base
         states
     end
 
+    # Subset of states accepted via the API
+    def self.allowed_incoming_states
+        [
+            'waiting_response',
+            'rejected',
+            'successful',
+            'partially_successful'
+        ]
+    end
+
     # Possible reasons that a request could be reported for administrator attention
     def report_reasons
         [_("Contains defamatory material"),
@@ -387,16 +397,16 @@ public
 
 
     # When constructing a new request, use this to check user hasn't double submitted.
-    # XXX could have a date range here, so say only check last month's worth of new requests. If somebody is making
+    # TODO: could have a date range here, so say only check last month's worth of new requests. If somebody is making
     # repeated requests, say once a quarter for time information, then might need to do that.
-    # XXX this *should* also check outgoing message joined to is an initial
+    # TODO: this *should* also check outgoing message joined to is an initial
     # request (rather than follow up)
     def InfoRequest.find_existing(title, public_body_id, body)
         return InfoRequest.find(:first, :conditions => [ "title = ? and public_body_id = ? and outgoing_messages.body = ?", title, public_body_id, body ], :include => [ :outgoing_messages ] )
     end
 
     def find_existing_outgoing_message(body)
-        # XXX can add other databases here which have regexp_replace
+        # TODO: can add other databases here which have regexp_replace
         if ActiveRecord::Base.connection.adapter_name == "PostgreSQL"
             # Exclude spaces from the body comparison using regexp_replace
             return self.outgoing_messages.find(:first, :conditions => [ "regexp_replace(outgoing_messages.body, '[[:space:]]', '', 'g') = regexp_replace(?, '[[:space:]]', '', 'g')", body ])
@@ -658,7 +668,7 @@ public
                     event.last_described_at = Time.now()
                     event.save!
                 end
-                if event.last_described_at.nil? # XXX actually maybe this isn't needed
+                if event.last_described_at.nil? # TODO: actually maybe this isn't needed
                     event.last_described_at = Time.now()
                     event.save!
                 end
@@ -713,7 +723,7 @@ public
                 elsif event.event_type == 'resent'
                     last_sent = event
                 elsif expecting_clarification and event.event_type == 'followup_sent'
-                    # XXX this needs to cope with followup_resent, which it doesn't.
+                    # TODO: this needs to cope with followup_resent, which it doesn't.
                     # Not really easy to do, and only affects cases where followups
                     # were resent after a clarification.
                     last_sent = event
diff --git a/app/models/info_request_event.rb b/app/models/info_request_event.rb
index 5eed5ba83..9dde3ba80 100644
--- a/app/models/info_request_event.rb
+++ b/app/models/info_request_event.rb
@@ -75,7 +75,7 @@ class InfoRequestEvent < ActiveRecord::Base
         :values => [
                      [ :created_at, 0, "range_search", :date ], # for QueryParser range searches e.g. 01/01/2008..14/01/2008
                      [ :created_at_numeric, 1, "created_at", :number ], # for sorting
-                     [ :described_at_numeric, 2, "described_at", :number ], # XXX using :number for lack of :datetime support in Xapian values
+                     [ :described_at_numeric, 2, "described_at", :number ], # TODO: using :number for lack of :datetime support in Xapian values
                      [ :request, 3, "request_collapse", :string ],
                      [ :request_title_collapse, 4, "request_title_collapse", :string ],
                    ],
@@ -174,7 +174,7 @@ class InfoRequestEvent < ActiveRecord::Base
     end
 
     def get_clipped_response_efficiently
-        # XXX this ugly code is an attempt to not always load all the
+        # TODO: this ugly code is an attempt to not always load all the
         # columns for an incoming message, which can be *very* large
         # (due to all the cached text).  We care particularly in this
         # case because it's called for every search result on a page
@@ -266,7 +266,7 @@ class InfoRequestEvent < ActiveRecord::Base
 
     # We store YAML version of parameters in the database
     def params=(params)
-        # XXX should really set these explicitly, and stop storing them in
+        # TODO: should really set these explicitly, and stop storing them in
         # here, but keep it for compatibility with old way for now
         if not params[:incoming_message_id].nil?
             self.incoming_message_id = params[:incoming_message_id]
@@ -392,7 +392,7 @@ class InfoRequestEvent < ActiveRecord::Base
             :outgoing_message_id => self.outgoing_message_id,
             :comment_id => self.comment_id,
 
-            # XXX would be nice to add links here, but alas the
+            # TODO: would be nice to add links here, but alas the
             # code to make them is in views only. See views/request/details.html.erb
             # perhaps can call with @template somehow
         }
diff --git a/app/models/mail_server_log.rb b/app/models/mail_server_log.rb
index 0e5b60ff1..07d2fdac0 100644
--- a/app/models/mail_server_log.rb
+++ b/app/models/mail_server_log.rb
@@ -166,7 +166,7 @@ class MailServerLog < ActiveRecord::Base
     # lines. Writes any errors to STDERR. This check is really mainly to
     # check the envelope from is the request address, as Ruby is quite
     # flaky with regard to that, and it is important for anti-spam reasons.
-    # XXX does this really check that, as the log just wouldn't pick
+    # TODO: does this really check that, as the log just wouldn't pick
     # up at all if the requests weren't sent that way as there would be
     # no request- email in it?
     #
diff --git a/app/models/outgoing_message.rb b/app/models/outgoing_message.rb
index a435511d3..160f69d0b 100644
--- a/app/models/outgoing_message.rb
+++ b/app/models/outgoing_message.rb
@@ -125,7 +125,7 @@ class OutgoingMessage < ActiveRecord::Base
         get_salutation + "\n\n" + get_default_letter + "\n\n" + get_signoff + "\n\n"
     end
     def set_signature_name(name)
-        # XXX We use raw_body here to get unstripped one
+        # TODO: We use raw_body here to get unstripped one
         if self.raw_body == self.get_default_message
             self.body = self.raw_body + name
         end
diff --git a/app/models/post_redirect.rb b/app/models/post_redirect.rb
index 5da3d2742..6f288b471 100644
--- a/app/models/post_redirect.rb
+++ b/app/models/post_redirect.rb
@@ -65,7 +65,7 @@ class PostRedirect < ActiveRecord::Base
 
     # Used by (rspec) test code only
     def self.get_last_post_redirect
-        # XXX yeuch - no other easy way of getting the token so we can check
+        # TODO: yeuch - no other easy way of getting the token so we can check
         # the redirect URL, as it is by definition opaque to the controller
         # apart from in the place that it redirects to.
         post_redirects = PostRedirect.find_by_sql("select * from post_redirects order by id desc limit 1")
diff --git a/app/models/profile_photo.rb b/app/models/profile_photo.rb
index 6c3b2cfa0..3c0be222c 100644
--- a/app/models/profile_photo.rb
+++ b/app/models/profile_photo.rb
@@ -115,7 +115,7 @@ class ProfilePhoto < ActiveRecord::Base
             return
         end
 
-        self.image = image_list[0] # XXX perhaps take largest image or somesuch if there were multiple in the file?
+        self.image = image_list[0] # TODO: perhaps take largest image or somesuch if there were multiple in the file?
         self.convert_image
     end
 end
diff --git a/app/models/public_body.rb b/app/models/public_body.rb
index 03ec270ee..b22482541 100644
--- a/app/models/public_body.rb
+++ b/app/models/public_body.rb
@@ -93,7 +93,7 @@ class PublicBody < ActiveRecord::Base
         self.translations.find_by_locale(locale)
     end
 
-    # XXX - Don't like repeating this!
+    # TODO: - Don't like repeating this!
     def calculate_cached_fields(t)
         PublicBody.set_first_letter(t)
         short_long_name = t.name
@@ -329,7 +329,7 @@ class PublicBody < ActiveRecord::Base
                     first = false
                 end
                 if html
-                    # XXX this should call proper route helpers, but is in model sigh
+                    # TODO: this should call proper route helpers, but is in model sigh
                     desc = '<a href="/body/list/' + tag.name + '">' + desc + '</a>'
                 end
                 types.push(desc)
@@ -399,6 +399,9 @@ class PublicBody < ActiveRecord::Base
         end
     end
 
+    def site_administration?
+        has_tag?('site_administration')
+    end
 
     class ImportCSVDryRun < StandardError
     end
@@ -569,45 +572,6 @@ class PublicBody < ActiveRecord::Base
         return [errors, notes]
     end
 
-    # Returns all public bodies (except for the internal admin authority) as csv
-    def self.export_csv(output_filename)
-        CSV.open(output_filename, "w") do |csv|
-            csv << [
-                    'Name',
-                    'Short name',
-                    # deliberately not including 'Request email'
-                    'URL name',
-                    'Tags',
-                    'Home page',
-                    'Publication scheme',
-                    'Disclosure log',
-                    'Notes',
-                    'Created at',
-                    'Updated at',
-                    'Version',
-            ]
-            PublicBody.visible.find_each(:include => [:translations, :tags]) do |public_body|
-                # Skip bodies we use only for site admin
-                next if public_body.has_tag?('site_administration')
-                csv << [
-                    public_body.name,
-                    public_body.short_name,
-                    # DO NOT include request_email (we don't want to make it
-                    # easy to spam all authorities with requests)
-                    public_body.url_name,
-                    public_body.tag_string,
-                    public_body.calculated_home_page,
-                    public_body.publication_scheme,
-                    public_body.disclosure_log,
-                    public_body.notes,
-                    public_body.created_at,
-                    public_body.updated_at,
-                    public_body.version,
-                ]
-            end
-        end
-    end
-
     # Does this user have the power of FOI officer for this body?
     def is_foi_officer?(user)
         user_domain = user.email_domain
diff --git a/app/models/track_thing.rb b/app/models/track_thing.rb
index 13b6f78dd..10ba28f4a 100644
--- a/app/models/track_thing.rb
+++ b/app/models/track_thing.rb
@@ -149,7 +149,7 @@ class TrackThing < ActiveRecord::Base
             end
         end
         track_thing.track_query = query
-        # XXX should extract requested_by:, request:, requested_from:
+        # TODO: should extract requested_by:, request:, requested_from:
         # and stick their values into the respective relations.
         # Should also update "params" to make the list_description
         # nicer and more generic.  It will need to do some clever
@@ -271,7 +271,7 @@ class TrackThing < ActiveRecord::Base
                     :web => _("To follow requests and responses matching your search"),
                     :email => _("Then you will be notified whenever a new request or response matches your search."),
                     :email_subject => _("Confirm you want to follow new requests or responses matching your search"),
-                    # RSS sorting - XXX hmmm, we don't really know which to use
+                    # RSS sorting - TODO: hmmm, we don't really know which to use
                     # here for sorting. Might be a query term (e.g. 'cctv'), in
                     # which case newest is good, or might be something like
                     # all refused requests in which case want to sort by
diff --git a/app/models/user.rb b/app/models/user.rb
index d75622b37..4b83d8572 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -99,7 +99,7 @@ class User < ActiveRecord::Base
     end
 
     # Don't display any leading/trailing spaces
-    # XXX we have strip_attributes! now, so perhaps this can be removed (might
+    # TODO: we have strip_attributes! now, so perhaps this can be removed (might
     # be still needed for existing cases)
     def name
         name = read_attribute(:name)
@@ -222,7 +222,7 @@ class User < ActiveRecord::Base
 
     # Can the user make new requests, without having to describe state of (most) existing ones?
     def can_leave_requests_undescribed?
-        # XXX should be flag in database really
+        # TODO: should be flag in database really
         if self.url_name == "heather_brooke" || self.url_name == "heather_brooke_2"
             return true
         end
@@ -425,7 +425,7 @@ class User < ActiveRecord::Base
 
     ## Class methods
     def User.encrypted_password(password, salt)
-        string_to_hash = password + salt # XXX need to add a secret here too?
+        string_to_hash = password + salt # TODO: need to add a secret here too?
         Digest::SHA1.hexdigest(string_to_hash)
     end