From 86647f0a54611f7f3bb620417b9b52464b0ee77f Mon Sep 17 00:00:00 2001
From: Neil Lalonde <neillalonde@gmail.com>
Date: Wed, 14 Aug 2013 11:05:53 -0400
Subject: [PATCH] Add ScreenedUrl. Rename BlockedEmail to ScreenedEmail.

---
 .../javascripts/admin/models/admin_user.js    |  3 +-
 .../admin/blocked_emails_controller.rb        |  8 ---
 .../admin/screened_emails_controller.rb       |  8 +++
 app/controllers/admin/users_controller.rb     |  2 +-
 app/models/blocked_email.rb                   | 54 -------------------
 app/models/screened_email.rb                  | 25 +++++++++
 app/models/screened_url.rb                    | 26 +++++++++
 ...alizer.rb => screened_email_serializer.rb} |  6 +--
 config/routes.rb                              |  2 +-
 .../20130813204212_create_screened_urls.rb    | 14 +++++
 ...ename_blocked_emails_to_screened_emails.rb |  5 ++
 lib/oneboxer.rb                               |  4 ++
 lib/screening_model.rb                        | 31 +++++++++++
 lib/user_destroyer.rb                         |  9 +++-
 lib/validators/email_validator.rb             |  2 +-
 spec/components/user_destroyer_spec.rb        | 50 +++++++++++++++--
 .../validators/email_validator_spec.rb        |  4 +-
 ....rb => screened_emails_controller_spec.rb} |  4 +-
 spec/fabricators/blocked_email_fabricator.rb  |  4 --
 spec/fabricators/screened_email_fabricator.rb |  4 ++
 spec/fabricators/screened_url_fabricator.rb   |  5 ++
 ...d_email_spec.rb => screened_email_spec.rb} | 18 +++----
 spec/models/screened_url_spec.rb              | 53 ++++++++++++++++++
 23 files changed, 251 insertions(+), 90 deletions(-)
 delete mode 100644 app/controllers/admin/blocked_emails_controller.rb
 create mode 100644 app/controllers/admin/screened_emails_controller.rb
 delete mode 100644 app/models/blocked_email.rb
 create mode 100644 app/models/screened_email.rb
 create mode 100644 app/models/screened_url.rb
 rename app/serializers/{blocked_email_serializer.rb => screened_email_serializer.rb} (57%)
 create mode 100644 db/migrate/20130813204212_create_screened_urls.rb
 create mode 100644 db/migrate/20130813224817_rename_blocked_emails_to_screened_emails.rb
 create mode 100644 lib/screening_model.rb
 rename spec/controllers/admin/{blocked_emails_controller_spec.rb => screened_emails_controller_spec.rb} (72%)
 delete mode 100644 spec/fabricators/blocked_email_fabricator.rb
 create mode 100644 spec/fabricators/screened_email_fabricator.rb
 create mode 100644 spec/fabricators/screened_url_fabricator.rb
 rename spec/models/{blocked_email_spec.rb => screened_email_spec.rb} (73%)
 create mode 100644 spec/models/screened_url_spec.rb

diff --git a/app/assets/javascripts/admin/models/admin_user.js b/app/assets/javascripts/admin/models/admin_user.js
index c76df0099ec..853cf484a6f 100644
--- a/app/assets/javascripts/admin/models/admin_user.js
+++ b/app/assets/javascripts/admin/models/admin_user.js
@@ -235,6 +235,7 @@ Discourse.AdminUser = Discourse.User.extend({
       var formData = { context: window.location.pathname };
       if (block) {
         formData["block_email"] = true;
+        formData["block_urls"] = true;
       }
       Discourse.ajax("/admin/users/" + user.get('id') + '.json', {
         type: 'DELETE',
@@ -292,7 +293,7 @@ Discourse.AdminUser = Discourse.User.extend({
       "callback": function() {
         Discourse.ajax("/admin/users/" + user.get('id') + '.json', {
           type: 'DELETE',
-          data: {delete_posts: true, block_email: true, context: window.location.pathname}
+          data: {delete_posts: true, block_email: true, block_urls: true, context: window.location.pathname}
         }).then(function(data) {
           if (data.deleted) {
             bootbox.alert(I18n.t("admin.user.deleted"), function() {
diff --git a/app/controllers/admin/blocked_emails_controller.rb b/app/controllers/admin/blocked_emails_controller.rb
deleted file mode 100644
index 866672f3f64..00000000000
--- a/app/controllers/admin/blocked_emails_controller.rb
+++ /dev/null
@@ -1,8 +0,0 @@
-class Admin::BlockedEmailsController < Admin::AdminController
-
-  def index
-    blocked_emails = BlockedEmail.limit(200).order('last_match_at desc').to_a
-    render_serialized(blocked_emails, BlockedEmailSerializer)
-  end
-
-end
diff --git a/app/controllers/admin/screened_emails_controller.rb b/app/controllers/admin/screened_emails_controller.rb
new file mode 100644
index 00000000000..92847e39e7e
--- /dev/null
+++ b/app/controllers/admin/screened_emails_controller.rb
@@ -0,0 +1,8 @@
+class Admin::ScreenedEmailsController < Admin::AdminController
+
+  def index
+    screened_emails = ScreenedEmail.limit(200).order('last_match_at desc').to_a
+    render_serialized(screened_emails, ScreenedEmailSerializer)
+  end
+
+end
diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
index 486700a6f6f..d158ca42775 100644
--- a/app/controllers/admin/users_controller.rb
+++ b/app/controllers/admin/users_controller.rb
@@ -118,7 +118,7 @@ class Admin::UsersController < Admin::AdminController
     user = User.where(id: params[:id]).first
     guardian.ensure_can_delete_user!(user)
     begin
-      if UserDestroyer.new(current_user).destroy(user, params.slice(:delete_posts, :block_email, :context))
+      if UserDestroyer.new(current_user).destroy(user, params.slice(:delete_posts, :block_email, :block_urls, :context))
         render json: {deleted: true}
       else
         render json: {deleted: false, user: AdminDetailedUserSerializer.new(user, root: false).as_json}
diff --git a/app/models/blocked_email.rb b/app/models/blocked_email.rb
deleted file mode 100644
index 1aa85ec4345..00000000000
--- a/app/models/blocked_email.rb
+++ /dev/null
@@ -1,54 +0,0 @@
-# A BlockedEmail record represents an email address that is being watched,
-# typically when creating a new User account. If the email of the signup form
-# (or some other form) matches a BlockedEmail record, an action can be
-# performed based on the action_type.
-class BlockedEmail < ActiveRecord::Base
-
-  before_validation :set_defaults
-
-  validates :email, presence: true, uniqueness: true
-
-  def self.actions
-    @actions ||= Enum.new(:block, :do_nothing)
-  end
-
-  def self.block(email, opts={})
-    find_by_email(email) || create(opts.slice(:action_type).merge({email: email}))
-  end
-
-  def self.should_block?(email)
-    blocked_email = BlockedEmail.where(email: email).first
-    blocked_email.record_match! if blocked_email
-    blocked_email && blocked_email.action_type == actions[:block]
-  end
-
-  def set_defaults
-    self.action_type ||= BlockedEmail.actions[:block]
-  end
-
-  def record_match!
-    self.match_count += 1
-    self.last_match_at = Time.zone.now
-    save
-  end
-
-end
-
-# == Schema Information
-#
-# Table name: blocked_emails
-#
-#  id            :integer          not null, primary key
-#  email         :string(255)      not null
-#  action_type   :integer          not null
-#  match_count   :integer          default(0), not null
-#  last_match_at :datetime
-#  created_at    :datetime         not null
-#  updated_at    :datetime         not null
-#
-# Indexes
-#
-#  index_blocked_emails_on_email          (email) UNIQUE
-#  index_blocked_emails_on_last_match_at  (last_match_at)
-#
-
diff --git a/app/models/screened_email.rb b/app/models/screened_email.rb
new file mode 100644
index 00000000000..05a2c5e196b
--- /dev/null
+++ b/app/models/screened_email.rb
@@ -0,0 +1,25 @@
+require_dependency 'screening_model'
+
+# A ScreenedEmail record represents an email address that is being watched,
+# typically when creating a new User account. If the email of the signup form
+# (or some other form) matches a ScreenedEmail record, an action can be
+# performed based on the action_type.
+class ScreenedEmail < ActiveRecord::Base
+
+  include ScreeningModel
+
+  default_action :block
+
+  validates :email, presence: true, uniqueness: true
+
+  def self.block(email, opts={})
+    find_by_email(email) || create(opts.slice(:action_type).merge({email: email}))
+  end
+
+  def self.should_block?(email)
+    screened_email = ScreenedEmail.where(email: email).first
+    screened_email.record_match! if screened_email
+    screened_email && screened_email.action_type == actions[:block]
+  end
+
+end
diff --git a/app/models/screened_url.rb b/app/models/screened_url.rb
new file mode 100644
index 00000000000..c27f6820d17
--- /dev/null
+++ b/app/models/screened_url.rb
@@ -0,0 +1,26 @@
+require_dependency 'screening_model'
+
+# A ScreenedUrl record represents a URL that is being watched.
+# If the URL is found in a post, some action can be performed.
+
+# For now, nothing is done. We're just collecting the data and will decide
+# what to do with it later.
+class ScreenedUrl < ActiveRecord::Base
+
+  include ScreeningModel
+
+  default_action :do_nothing
+
+  before_validation :strip_http
+
+  validates :url, presence: true, uniqueness: true
+  validates :domain, presence: true
+
+  def strip_http
+    self.url.gsub!(/http(s?):\/\//i, '')
+  end
+
+  def self.watch(url, domain, opts={})
+    find_by_url(url) || create(opts.slice(:action_type).merge(url: url, domain: domain))
+  end
+end
diff --git a/app/serializers/blocked_email_serializer.rb b/app/serializers/screened_email_serializer.rb
similarity index 57%
rename from app/serializers/blocked_email_serializer.rb
rename to app/serializers/screened_email_serializer.rb
index 3d8686c0784..22d884f883f 100644
--- a/app/serializers/blocked_email_serializer.rb
+++ b/app/serializers/screened_email_serializer.rb
@@ -1,11 +1,11 @@
-class BlockedEmailSerializer < ApplicationSerializer
+class ScreenedEmailSerializer < ApplicationSerializer
   attributes :email,
              :action,
              :match_count,
              :last_match_at,
              :created_at
-  
+
   def action
-    BlockedEmail.actions.key(object.action_type).to_s
+    ScreenedEmail.actions.key(object.action_type).to_s
   end
 end
diff --git a/config/routes.rb b/config/routes.rb
index ad3374f706f..5c7e8bcd323 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -63,7 +63,7 @@ Discourse::Application.routes.draw do
     end
 
     scope '/logs' do
-      resources :blocked_emails,    only: [:index, :create, :update, :destroy]
+      resources :screened_emails,   only: [:index, :create, :update, :destroy]
       resources :staff_action_logs, only: [:index, :create, :update, :destroy]
     end
 
diff --git a/db/migrate/20130813204212_create_screened_urls.rb b/db/migrate/20130813204212_create_screened_urls.rb
new file mode 100644
index 00000000000..5e613c748e9
--- /dev/null
+++ b/db/migrate/20130813204212_create_screened_urls.rb
@@ -0,0 +1,14 @@
+class CreateScreenedUrls < ActiveRecord::Migration
+  def change
+    create_table :screened_urls do |t|
+      t.string :url, null: false
+      t.string :domain, null: false
+      t.integer :action_type, null: false
+      t.integer :match_count, null: false, default: 0
+      t.datetime :last_match_at
+      t.timestamps
+    end
+    add_index :screened_urls, :url, unique: true
+    add_index :screened_urls, :last_match_at
+  end
+end
diff --git a/db/migrate/20130813224817_rename_blocked_emails_to_screened_emails.rb b/db/migrate/20130813224817_rename_blocked_emails_to_screened_emails.rb
new file mode 100644
index 00000000000..5f593e445ad
--- /dev/null
+++ b/db/migrate/20130813224817_rename_blocked_emails_to_screened_emails.rb
@@ -0,0 +1,5 @@
+class RenameBlockedEmailsToScreenedEmails < ActiveRecord::Migration
+  def change
+    rename_table :blocked_emails, :screened_emails
+  end
+end
diff --git a/lib/oneboxer.rb b/lib/oneboxer.rb
index 046f1891af0..e821b014545 100644
--- a/lib/oneboxer.rb
+++ b/lib/oneboxer.rb
@@ -31,6 +31,10 @@ module Oneboxer
     1.day
   end
 
+  def self.oneboxer_exists_for_url?(url)
+    Whitelist.entry_for_url(url) || matchers.any? { |matcher| url =~ matcher.regexp }
+  end
+
   # Return a oneboxer for a given URL
   def self.onebox_for_url(url)
     matchers.each do |matcher|
diff --git a/lib/screening_model.rb b/lib/screening_model.rb
new file mode 100644
index 00000000000..5d9b86ad00c
--- /dev/null
+++ b/lib/screening_model.rb
@@ -0,0 +1,31 @@
+module ScreeningModel
+  extend ActiveSupport::Concern
+
+  module ClassMethods
+    def actions
+      @actions ||= Enum.new(:block, :do_nothing)
+    end
+
+    def default_action(action_key)
+      @default_action = action_key
+    end
+
+    def df_action
+      @default_action || :do_nothing
+    end
+  end
+
+  included do
+    before_validation :set_default_action
+  end
+
+  def set_default_action
+    self.action_type ||= self.class.actions[self.class.df_action]
+  end
+
+  def record_match!
+    self.match_count += 1
+    self.last_match_at = Time.zone.now
+    save
+  end
+end
diff --git a/lib/user_destroyer.rb b/lib/user_destroyer.rb
index 8a158e6eab1..b7e72dbd351 100644
--- a/lib/user_destroyer.rb
+++ b/lib/user_destroyer.rb
@@ -17,6 +17,13 @@ class UserDestroyer
     User.transaction do
       if opts[:delete_posts]
         user.posts.each do |post|
+          if opts[:block_urls]
+            post.topic_links.each do |link|
+              unless link.internal or Oneboxer.oneboxer_exists_for_url?(link.url)
+                ScreenedUrl.watch(link.url, link.domain).try(:record_match!)
+              end
+            end
+          end
           PostDestroyer.new(@staff, post).destroy
         end
         raise PostsExistError if user.reload.post_count != 0
@@ -24,7 +31,7 @@ class UserDestroyer
       user.destroy.tap do |u|
         if u
           if opts[:block_email]
-            b = BlockedEmail.block(u.email)
+            b = ScreenedEmail.block(u.email)
             b.record_match! if b
           end
           Post.with_deleted.where(user_id: user.id).update_all("nuked_user = true")
diff --git a/lib/validators/email_validator.rb b/lib/validators/email_validator.rb
index c2e4284b4ec..77749834841 100644
--- a/lib/validators/email_validator.rb
+++ b/lib/validators/email_validator.rb
@@ -10,7 +10,7 @@ class EmailValidator < ActiveModel::EachValidator
         record.errors.add(attribute, I18n.t(:'user.email.not_allowed'))
       end
     end
-    if record.errors[attribute].blank? and BlockedEmail.should_block?(value)
+    if record.errors[attribute].blank? and ScreenedEmail.should_block?(value)
       record.errors.add(attribute, I18n.t(:'user.email.blocked'))
     end
   end
diff --git a/spec/components/user_destroyer_spec.rb b/spec/components/user_destroyer_spec.rb
index 4b23a28474d..8e0d4e01494 100644
--- a/spec/components/user_destroyer_spec.rb
+++ b/spec/components/user_destroyer_spec.rb
@@ -74,13 +74,13 @@ describe UserDestroyer do
 
     shared_examples "email block list" do
       it "doesn't add email to block list by default" do
-        BlockedEmail.expects(:block).never
+        ScreenedEmail.expects(:block).never
         destroy
       end
 
       it "adds email to block list if block_email is true" do
-        b = Fabricate.build(:blocked_email, email: @user.email)
-        BlockedEmail.expects(:block).with(@user.email).returns(b)
+        b = Fabricate.build(:screened_email, email: @user.email)
+        ScreenedEmail.expects(:block).with(@user.email).returns(b)
         b.expects(:record_match!).once.returns(true)
         UserDestroyer.new(@admin).destroy(@user, destroy_opts.merge({block_email: true}))
       end
@@ -164,6 +164,50 @@ describe UserDestroyer do
         end
       end
     end
+
+    context 'user has posts with links' do
+      context 'external links' do
+        before do
+          @post = Fabricate(:post_with_external_links, user: @user)
+          TopicLink.extract_from(@post)
+        end
+
+        it "doesn't add ScreenedUrl records by default" do
+          ScreenedUrl.expects(:watch).never
+          UserDestroyer.new(@admin).destroy(@user, {delete_posts: true})
+        end
+
+        it "adds ScreenedUrl records when :block_urls is true" do
+          ScreenedUrl.expects(:watch).at_least_once
+          UserDestroyer.new(@admin).destroy(@user, {delete_posts: true, block_urls: true})
+        end
+      end
+
+      context 'internal links' do
+        before do
+          @post = Fabricate(:post_with_external_links, user: @user)
+          TopicLink.extract_from(@post)
+          TopicLink.any_instance.stubs(:internal).returns(true)
+        end
+
+        it "doesn't add ScreenedUrl records" do
+          ScreenedUrl.expects(:watch).never
+          UserDestroyer.new(@admin).destroy(@user, {delete_posts: true, block_urls: true})
+        end
+      end
+
+      context 'with oneboxed links' do
+        before do
+          @post = Fabricate(:post_with_youtube, user: @user)
+          TopicLink.extract_from(@post)
+        end
+
+        it "doesn't add ScreenedUrl records" do
+          ScreenedUrl.expects(:watch).never
+          UserDestroyer.new(@admin).destroy(@user, {delete_posts: true, block_urls: true})
+        end
+      end
+    end
   end
 
 end
diff --git a/spec/components/validators/email_validator_spec.rb b/spec/components/validators/email_validator_spec.rb
index 2591a1ec733..e310aab342b 100644
--- a/spec/components/validators/email_validator_spec.rb
+++ b/spec/components/validators/email_validator_spec.rb
@@ -8,13 +8,13 @@ describe EmailValidator do
 
   context "blocked email" do
     it "doesn't add an error when email doesn't match a blocked email" do
-      BlockedEmail.stubs(:should_block?).with(record.email).returns(false)
+      ScreenedEmail.stubs(:should_block?).with(record.email).returns(false)
       validate
       record.errors[:email].should_not be_present
     end
 
     it "adds an error when email matches a blocked email" do
-      BlockedEmail.stubs(:should_block?).with(record.email).returns(true)
+      ScreenedEmail.stubs(:should_block?).with(record.email).returns(true)
       validate
       record.errors[:email].should be_present
     end
diff --git a/spec/controllers/admin/blocked_emails_controller_spec.rb b/spec/controllers/admin/screened_emails_controller_spec.rb
similarity index 72%
rename from spec/controllers/admin/blocked_emails_controller_spec.rb
rename to spec/controllers/admin/screened_emails_controller_spec.rb
index 0ee48d423e6..e10553ea2dd 100644
--- a/spec/controllers/admin/blocked_emails_controller_spec.rb
+++ b/spec/controllers/admin/screened_emails_controller_spec.rb
@@ -1,8 +1,8 @@
 require 'spec_helper'
 
-describe Admin::BlockedEmailsController do
+describe Admin::ScreenedEmailsController do
   it "is a subclass of AdminController" do
-    (Admin::BlockedEmailsController < Admin::AdminController).should be_true
+    (Admin::ScreenedEmailsController < Admin::AdminController).should be_true
   end
 
   let!(:user) { log_in(:admin) }
diff --git a/spec/fabricators/blocked_email_fabricator.rb b/spec/fabricators/blocked_email_fabricator.rb
deleted file mode 100644
index 7e932502698..00000000000
--- a/spec/fabricators/blocked_email_fabricator.rb
+++ /dev/null
@@ -1,4 +0,0 @@
-Fabricator(:blocked_email) do
-  email { sequence(:email) { |n| "bad#{n}@spammers.org" } }
-  action_type BlockedEmail.actions[:block]
-end
diff --git a/spec/fabricators/screened_email_fabricator.rb b/spec/fabricators/screened_email_fabricator.rb
new file mode 100644
index 00000000000..5373e3370a3
--- /dev/null
+++ b/spec/fabricators/screened_email_fabricator.rb
@@ -0,0 +1,4 @@
+Fabricator(:screened_email) do
+  email { sequence(:email) { |n| "bad#{n}@spammers.org" } }
+  action_type ScreenedEmail.actions[:block]
+end
diff --git a/spec/fabricators/screened_url_fabricator.rb b/spec/fabricators/screened_url_fabricator.rb
new file mode 100644
index 00000000000..09b6eae4124
--- /dev/null
+++ b/spec/fabricators/screened_url_fabricator.rb
@@ -0,0 +1,5 @@
+Fabricator(:screened_url) do
+  url         { sequence(:url)    { |n| "spammers#{n}.org/buy/stuff" } }
+  domain      { sequence(:domain) { |n| "spammers#{n}.org" } }
+  action_type ScreenedEmail.actions[:do_nothing]
+end
diff --git a/spec/models/blocked_email_spec.rb b/spec/models/screened_email_spec.rb
similarity index 73%
rename from spec/models/blocked_email_spec.rb
rename to spec/models/screened_email_spec.rb
index ea473291a72..3e242034bf9 100644
--- a/spec/models/blocked_email_spec.rb
+++ b/spec/models/screened_email_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe BlockedEmail do
+describe ScreenedEmail do
 
   let(:email) { 'block@spamfromhome.org' }
 
@@ -34,7 +34,7 @@ describe BlockedEmail do
     end
 
     context 'email is already being blocked' do
-      let!(:existing) { Fabricate(:blocked_email, email: email) }
+      let!(:existing) { Fabricate(:screened_email, email: email) }
 
       it "doesn't create a new record" do
         expect { described_class.block(email) }.to_not change { described_class.count }
@@ -53,25 +53,25 @@ describe BlockedEmail do
       subject.should be_false
     end
 
-    shared_examples "when a BlockedEmail record matches" do
+    shared_examples "when a ScreenedEmail record matches" do
       it "updates statistics" do
         Timecop.freeze(Time.zone.now) do
-          expect { subject }.to change { blocked_email.reload.match_count }.by(1)
-          blocked_email.last_match_at.should be_within_one_second_of(Time.zone.now)
+          expect { subject }.to change { screened_email.reload.match_count }.by(1)
+          screened_email.last_match_at.should be_within_one_second_of(Time.zone.now)
         end
       end
     end
 
     context "action_type is :block" do
-      let!(:blocked_email) { Fabricate(:blocked_email, email: email, action_type: BlockedEmail.actions[:block]) }
+      let!(:screened_email) { Fabricate(:screened_email, email: email, action_type: described_class.actions[:block]) }
       it { should be_true }
-      include_examples "when a BlockedEmail record matches"
+      include_examples "when a ScreenedEmail record matches"
     end
 
     context "action_type is :do_nothing" do
-      let!(:blocked_email) { Fabricate(:blocked_email, email: email, action_type: BlockedEmail.actions[:do_nothing]) }
+      let!(:screened_email) { Fabricate(:screened_email, email: email, action_type: described_class.actions[:do_nothing]) }
       it { should be_false }
-      include_examples "when a BlockedEmail record matches"
+      include_examples "when a ScreenedEmail record matches"
     end
   end
 
diff --git a/spec/models/screened_url_spec.rb b/spec/models/screened_url_spec.rb
new file mode 100644
index 00000000000..14229be65b1
--- /dev/null
+++ b/spec/models/screened_url_spec.rb
@@ -0,0 +1,53 @@
+require 'spec_helper'
+
+describe ScreenedUrl do
+
+  let(:url)    { 'http://shopppping.com/bad/drugz' }
+  let(:domain) { 'shopppping.com' }
+
+  let(:valid_params) { {url: url, domain: domain} }
+
+  describe "new record" do
+    it "sets a default action_type" do
+      described_class.create(valid_params).action_type.should == described_class.actions[:do_nothing]
+    end
+
+    it "last_match_at is null" do
+      described_class.create(valid_params).last_match_at.should be_nil
+    end
+
+    ['http://', 'HTTP://', 'https://', 'HTTPS://'].each do |prefix|
+      it "strips #{prefix}" do
+        described_class.create(valid_params.merge(url: url.gsub('http://', prefix))).url.should == url.gsub('http://', '')
+      end
+    end
+  end
+
+  describe '#watch' do
+    context 'url is not being blocked' do
+      it 'creates a new record with default action of :do_nothing' do
+        record = described_class.watch(url, domain)
+        record.should_not be_new_record
+        record.action_type.should == described_class.actions[:do_nothing]
+      end
+
+      it 'lets action_type be overriden' do
+        record = described_class.watch(url, domain, action_type: described_class.actions[:block])
+        record.should_not be_new_record
+        record.action_type.should == described_class.actions[:block]
+      end
+    end
+
+    context 'url is already being blocked' do
+      let!(:existing) { Fabricate(:screened_url, url: url, domain: domain) }
+
+      it "doesn't create a new record" do
+        expect { described_class.watch(url, domain) }.to_not change { described_class.count }
+      end
+
+      it "returns the existing record" do
+        described_class.watch(url, domain).should == existing
+      end
+    end
+  end
+end