Merge 511c476 into 8f3d2b2

app/models/risk_trigger.rb

@@ -1,29 +1,18 @@
 class RiskTrigger < ActiveRecord::Base
-  def risky?(notice)
-    if notice.submitter.try(:email) == 'google@lumendatabase.org' &&
-       notice.try(:type) == 'Defamation'
-      false
-    else
-      begin
-        field_present?(notice) && condition_matches?(notice)
-      rescue NoMethodError => ex
-        Rails.logger.warn "Invalid risk trigger (#{id}): #{ex}"
-        false
-      end
-    end
-  end
+  ALLOWED_MATCHING_TYPES = %w[any all].freeze
 
-  private
+  has_many :risk_trigger_conditions, dependent: :destroy
 
-  def field_present?(notice)
-    notice.send(field).present?
-  end
+  validates_presence_of :name, :matching_type
+  validates_inclusion_of :matching_type, in: ALLOWED_MATCHING_TYPES
+
+  def risky?(notice)
+    return false unless risk_trigger_conditions.any?
 
-  def condition_matches?(notice)
-    if negated?
-      notice.send(condition_field) != condition_value
+    if matching_type == 'any'
+      risk_trigger_conditions.any? { |condition| condition.risky?(notice) }
     else
-      notice.send(condition_field) == condition_value
+      risk_trigger_conditions.all? { |condition| condition.risky?(notice) }
     end
   end
 end

app/models/risk_trigger_condition.rb

@@ -0,0 +1,69 @@
+class RiskTriggerCondition < ActiveRecord::Base
+  ALLOWED_FIELDS = %w[
+    title subject source tag_list language jurisdiction_list action_taken body
+    type
+    submitter.name submitter.kind submitter.address_line_1
+    submitter.address_line_2 submitter.city submitter.state submitter.zip
+    submitter.country_code submitter.phone submitter.email submitter.url
+    recipient.name recipient.kind recipient.address_line_1
+    recipient.address_line_2 recipient.city recipient.state recipient.zip
+    recipient.country_code recipient.phone recipient.email recipient.url
+    principal.name principal.kind principal.address_line_1
+    principal.address_line_2 principal.city principal.state principal.zip
+    principal.country_code principal.phone principal.email principal.url
+    sender.name sender.kind sender.address_line_1
+    sender.address_line_2 sender.city sender.state sender.zip
+    sender.country_code sender.phone sender.email sender.url
+  ].freeze
+
+  ALLOWED_MATCHING_TYPES = %w[exact broad].freeze
+
+  belongs_to :risk_trigger
+
+  validates_presence_of :field, :value
+  validates_inclusion_of :field, in: ALLOWED_FIELDS
+  validates_inclusion_of :matching_type, in: ALLOWED_MATCHING_TYPES
+
+  def risky?(notice)
+    field_present?(notice) && condition_matches?(notice)
+  rescue NoMethodError => ex
+    Rails.logger.warn "Invalid risk trigger condition (#{id}): #{ex}"
+    false
+  end
+
+  private
+
+  def field_present?(notice)
+    notice_field_value(notice).present?
+  end
+
+  def condition_matches?(notice)
+    notice_field_value = notice_field_value(notice)
+
+    if matching_type == 'exact'
+      match_exact(notice_field_value)
+    else
+      match_broad(notice_field_value)
+    end
+  end
+
+  def notice_field_value(notice)
+    field.split('.').inject(notice, :send)
+  end
+
+  def match_exact(notice_field_value)
+    if negated?
+      notice_field_value != value
+    else
+      notice_field_value == value
+    end
+  end
+
+  def match_broad(notice_field_value)
+    if negated?
+      !notice_field_value.include?(value)
+    else
+      notice_field_value.include?(value)
+    end
+  end
+end

config/initializers/rails_admin.rb

@@ -249,4 +249,29 @@ def custom_work_label
       end
     end
   end
+
+  config.model 'RiskTriggerCondition' do
+    edit do
+      configure :field, :enum do
+        enum do
+          RiskTriggerCondition::ALLOWED_FIELDS
+        end
+      end
+      configure :matching_type, :enum do
+        enum do
+          RiskTriggerCondition::ALLOWED_MATCHING_TYPES
+        end
+      end
+    end
+  end
+
+  config.model 'RiskTrigger' do
+    edit do
+      configure :matching_type, :enum do
+        enum do
+          RiskTrigger::ALLOWED_MATCHING_TYPES
+        end
+      end
+    end
+  end
 end

db/migrate/20190122150704_rename_risk_triggers.rb

@@ -0,0 +1,12 @@
+class RenameRiskTriggers < ActiveRecord::Migration
+  def change
+    rename_table :risk_triggers, :risk_trigger_conditions
+    remove_column :risk_trigger_conditions, :field
+    rename_column :risk_trigger_conditions, :condition_field, :field
+    rename_column :risk_trigger_conditions, :condition_value, :value
+    add_column :risk_trigger_conditions, :type, :string
+    add_column :risk_trigger_conditions, :matching_type, :string
+    change_column_null :risk_trigger_conditions, :field, false
+    change_column_null :risk_trigger_conditions, :value, false
+  end
+end

db/migrate/20190122151538_create_new_risk_triggers.rb

@@ -0,0 +1,38 @@
+class CreateNewRiskTriggers < ActiveRecord::Migration
+  def change
+    # Create a new table
+    create_table :risk_triggers do |t|
+      t.string :name, null: false
+      t.string :matching_type, null: false
+      t.string :comment
+    end
+
+    # Add a reference field to the risk_trigger_conditions
+    add_reference :risk_trigger_conditions, :risk_trigger, index: true
+
+    # Create a default risk trigger
+    default_risk_trigger = RiskTrigger.create(
+      name: 'Default',
+      matching_type: 'all'
+    )
+
+    # Assign all the exisiting conditions to the default trigger
+    RiskTriggerCondition.update_all(risk_trigger_id: default_risk_trigger)
+
+    # Create a risk trigger conditions from the ones that were hard-coded in the RiskTrigger model
+    RiskTriggerCondition.create(
+      field: 'type',
+      value: 'Defamation',
+      negated: true,
+      risk_trigger_id: default_risk_trigger,
+      matching_type: 'exact'
+    )
+    RiskTriggerCondition.create(
+      field: 'submitter.email',
+      value: 'google@lumendatabase.org',
+      negated: true,
+      risk_trigger_id: default_risk_trigger,
+      matching_type: 'exact'
+    )
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20180815162606) do
+ActiveRecord::Schema.define(version: 20190122151538) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -229,11 +229,21 @@
   add_index "relevant_questions_topics", ["relevant_question_id"], name: "index_relevant_questions_topics_on_relevant_question_id", using: :btree
   add_index "relevant_questions_topics", ["topic_id"], name: "index_relevant_questions_topics_on_topic_id", using: :btree
 
-  create_table "risk_triggers", force: :cascade do |t|
-    t.string  "field"
-    t.string  "condition_field"
-    t.string  "condition_value"
+  create_table "risk_trigger_conditions", force: :cascade do |t|
+    t.string  "field",           null: false
+    t.string  "value",           null: false
     t.boolean "negated"
+    t.string  "type"
+    t.string  "matching_type"
+    t.integer "risk_trigger_id"
+  end
+
+  add_index "risk_trigger_conditions", ["risk_trigger_id"], name: "index_risk_trigger_conditions_on_risk_trigger_id", using: :btree
+
+  create_table "risk_triggers", force: :cascade do |t|
+    t.string "name",          null: false
+    t.string "matching_type", null: false
+    t.string "comment"
   end
 
   create_table "roles", force: :cascade do |t|

spec/integration/reviewable_notice_spec.rb

@@ -5,7 +5,7 @@
 
   let(:harmless_text) { 'Some harmless text' }
 
-  scenario 'A non-US notice with Body text' do
+  scenario 'A non-US notice with a non-us trigger condition is risky' do
     create_non_us_risk_trigger
 
     submit_notice_from('Spain') do
@@ -18,7 +18,7 @@
     end
   end
 
-  scenario 'A US notice with Body text' do
+  scenario 'A US notice with a non-us trigger condition is not risky' do
     create_non_us_risk_trigger
 
     submit_notice_from('United States') do
@@ -31,14 +31,105 @@
     end
   end
 
+  scenario 'A notice and a trigger that should match all with two conditions and matching all of them is risky' do
+    condition1 = create_condition('title', 'risky', false, 'broad')
+    condition2 = create_condition('body', 'risky', false, 'broad')
+    create_trigger('Risky stuff', 'all', [condition1, condition2])
+
+    submit_notice_from('United States') do
+      fill_in 'Title', with: 'I\'m so risky title, woooo!'
+      fill_in 'Body', with: 'I\'m so risky body, watch out!'
+    end
+
+    open_recent_notice
+    within('.notice-body') do
+      expect(page).to have_words Notice::UNDER_REVIEW_VALUE
+    end
+  end
+
+  scenario 'A notice and a trigger that should match all with two conditions and matching just one of them is not risky' do
+    condition1 = create_condition('title', 'risky', false, 'broad')
+    condition2 = create_condition('body', 'risky', false, 'broad')
+    create_trigger('Risky stuff', 'all', [condition1, condition2])
+
+    submit_notice_from('United States') do
+      fill_in 'Title', with: 'I\'m so risky title, woooo!'
+      fill_in 'Body', with: harmless_text
+    end
+
+    open_recent_notice
+    within('.notice-body') do
+      expect(page).to have_words harmless_text
+    end
+  end
+
+  scenario 'A notice and a trigger that should match any with two conditions and matching just one of them is risky' do
+    condition1 = create_condition('title', 'risky', false, 'broad')
+    condition2 = create_condition('body', 'risky', false, 'broad')
+    create_trigger('Risky stuff', 'any', [condition1, condition2])
+
+    submit_notice_from('United States') do
+      fill_in 'Title', with: 'I\'m so risky title, woooo!'
+      fill_in 'Body', with: harmless_text
+    end
+
+    open_recent_notice
+    within('.notice-body') do
+      expect(page).to have_words Notice::UNDER_REVIEW_VALUE
+    end
+  end
+
+  scenario 'A notice and a trigger with exact conditions match and is risky' do
+    condition1 = create_condition('title', 'risky1', false, 'exact')
+    condition2 = create_condition('body', 'risky2', false, 'exact')
+    create_trigger('Risky stuff', 'all', [condition1, condition2])
+
+    submit_notice_from('United States') do
+      fill_in 'Title', with: 'risky1'
+      fill_in 'Body', with: 'risky2'
+    end
+
+    open_recent_notice
+    within('.notice-body') do
+      expect(page).to have_words Notice::UNDER_REVIEW_VALUE
+    end
+  end
+
+  scenario 'A notice and a trigger with no conditions is not risky' do
+    create_trigger('Risky stuff', 'all', [])
+
+    submit_notice_from('United States') do
+      fill_in 'Title', with: 'risky1'
+      fill_in 'Body', with: 'risky2'
+    end
+
+    open_recent_notice
+    within('.notice-body') do
+      expect(page).not_to have_words Notice::UNDER_REVIEW_VALUE
+    end
+  end
+
   private
 
   def create_non_us_risk_trigger
+    risk_trigger_condition = create_condition('recipient.country_code', 'us', true, 'exact')
+    create_trigger('Non US risk trigger', 'all', [risk_trigger_condition])
+  end
+
+  def create_condition(field, value, negated, matching_type)
+    RiskTriggerCondition.create!(
+      field: field,
+      value: value,
+      negated: negated,
+      matching_type: matching_type
+    )
+  end
+
+  def create_trigger(name, matching_type, conditions)
     RiskTrigger.create!(
-      field: 'body',
-      condition_field: 'country_code',
-      condition_value: 'us',
-      negated: true
+      name: name,
+      matching_type: matching_type,
+      risk_trigger_conditions: conditions
     )
   end