Merge pull request #143 from alphagov/hide-non-actionable-feedback-in…

…-feedex

Track and hide non-actionable feedback in Feedex

db/migrate/20140228185036_add_actionable_flag_and_reason_to_anonymous_feedback.rb

@@ -0,0 +1,6 @@
+class AddActionableFlagAndReasonToAnonymousFeedback < ActiveRecord::Migration
+  def change
+    add_column :anonymous_contacts, :is_actionable, :boolean, default: true, null: false
+    add_column :anonymous_contacts, :reason_why_not_actionable, :string
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended to check this file into your version control system.
 
-ActiveRecord::Schema.define(:version => 20140221155725) do
+ActiveRecord::Schema.define(:version => 20140228185036) do
 
   create_table "anonymous_contacts", :force => true do |t|
     t.string   "type"
@@ -24,12 +24,14 @@
     t.string   "user_agent"
     t.string   "referrer"
     t.boolean  "javascript_enabled"
-    t.datetime "created_at",                  :null => false
-    t.datetime "updated_at",                  :null => false
+    t.datetime "created_at",                                    :null => false
+    t.datetime "updated_at",                                    :null => false
     t.string   "personal_information_status"
     t.string   "slug"
     t.integer  "service_satisfaction_rating"
     t.text     "user_specified_url"
+    t.boolean  "is_actionable",               :default => true, :null => false
+    t.string   "reason_why_not_actionable"
   end
 
 end

lib/support/requests/anonymous/anonymous_contact.rb

@@ -6,21 +6,23 @@ module Requests
     module Anonymous
       class AnonymousContact < ActiveRecord::Base
         attr_accessible :referrer, :javascript_enabled, :user_agent, :personal_information_status
-        validates :referrer, url: true, allow_nil: true
+        attr_accessible :is_actionable, :reason_why_not_actionable
 
         before_save :detect_personal_information
 
         def requester
           Requester.anonymous
         end
 
+        validates :referrer, url: true, allow_nil: true
         validates :details, length: { maximum: 2 ** 16 }
         validates_inclusion_of :javascript_enabled, in: [ true, false ]
         validates_inclusion_of :personal_information_status, in: [ "suspected", "absent" ], allow_nil: true
+        validates_inclusion_of :is_actionable, in: [ true, false ]
+        validates_presence_of :reason_why_not_actionable, unless: "is_actionable"
 
-        def self.free_of_personal_info
-          where(personal_information_status: "absent")
-        end
+        scope :free_of_personal_info, where(personal_information_status: "absent")
+        scope :only_actionable, where(is_actionable: true)
 
         def path
           URI(url).path
@@ -31,7 +33,11 @@ def path
         end
 
         def self.find_all_starting_with_path(path)
-          where("url is not null and url like ?", "%" + path + "%").free_of_personal_info.order("created_at desc").select { |pr| pr.path && pr.path.start_with?(path) }
+          where("url is not null and url like ?", "%" + path + "%").
+            free_of_personal_info.
+            only_actionable.
+            order("created_at desc").
+            select { |pr| pr.path && pr.path.start_with?(path) }
         end
 
         private

test/unit/support/requests/anonymous/anonymous_contact_test.rb

@@ -23,6 +23,12 @@ def path_for(url)
           URI(url).path
         end
 
+        should "enforce the presence of a reason why feedback isn't actionable" do
+          contact = new_contact(is_actionable: false, reason_why_not_actionable: "")
+          refute contact.valid?
+          refute contact.errors[:reason_why_not_actionable].empty?
+        end
+
         should "not detect personal info when none is present in free text fields" do
           assert_equal "absent", contact(details: "abc", what_wrong: "abc", what_doing: "abc").personal_information_status
         end
@@ -83,6 +89,13 @@ def path_for(url)
             assert_equal [a], TestContact.find_all_starting_with_path(path_for(DEFAULTS[:url]))
           end
 
+          should "only return actionable feedback" do
+            a = contact(is_actionable: true)
+            _ = contact(is_actionable: false, reason_why_not_actionable: "spam")
+
+            assert_equal [a], TestContact.find_all_starting_with_path(path_for(DEFAULTS[:url]))
+          end
+
           def teardown
             TestContact.delete_all
           end