FEATURE: Prompt users when they are entering duplicate links

app/assets/javascripts/discourse/components/composer-editor.js.es6

@@ -459,6 +459,7 @@ export default Ember.Component.extend({
       // Paint oneboxes
       $('a.onebox', $preview).each((i, e) => Discourse.Onebox.load(e, refresh));
       this.trigger('previewRefreshed', $preview);
+      this.sendAction('afterRefresh', $preview);
     },
   }
 });

app/assets/javascripts/discourse/components/composer-message.js.es6

@@ -5,7 +5,7 @@ export default Ember.Component.extend({
 
   @computed('message.templateName')
   defaultLayout(templateName) {
-    return this.container.lookup(`template:${templateName}`)
+    return this.container.lookup(`template:composer/${templateName}`)
   },
 
   didInsertElement() {

app/assets/javascripts/discourse/components/composer-messages.js.es6

@@ -1,3 +1,5 @@
+import LinkLookup from 'discourse/lib/link-lookup';
+
 export default Ember.Component.extend({
   classNameBindings: [':composer-popup-container', 'hidden'],
   checkedMessages: false,
@@ -17,6 +19,7 @@ export default Ember.Component.extend({
     this.appEvents.on('composer:opened', this, this._findMessages);
     this.appEvents.on('composer:find-similar', this, this._findSimilar);
     this.appEvents.on('composer-messages:close', this, this._closeTop);
+    this.appEvents.on('composer-messages:create', this, this._create);
   },
 
   willDestroyElement() {
@@ -24,6 +27,7 @@ export default Ember.Component.extend({
     this.appEvents.off('composer:opened', this, this._findMessages);
     this.appEvents.off('composer:find-similar', this, this._findSimilar);
     this.appEvents.off('composer-messages:close', this, this._closeTop);
+    this.appEvents.off('composer-messages:create', this, this._create);
   },
 
   _closeTop() {
@@ -82,20 +86,21 @@ export default Ember.Component.extend({
     this.get('queuedForTyping').forEach(msg => this.send("popup", msg));
   },
 
+  _create(info) {
+    this.reset();
+    this.send('popup', Ember.Object.create(info));
+  },
+
   groupsMentioned(groups) {
     // reset existing messages, this should always win it is critical
     this.reset();
     groups.forEach(group => {
-      const msg = I18n.t('composer.group_mentioned', {
+      const body = I18n.t('composer.group_mentioned', {
         group: "@" + group.name,
         count: group.user_count,
         group_link: Discourse.getURL(`/group/${group.name}/members`)
       });
-      this.send("popup",
+      this.send("popup", Ember.Object.create({ templateName: 'custom-body', body }));
-        Em.Object.create({
-          templateName: 'composer/group-mentioned',
-          body: msg})
-        );
     });
   },
 
@@ -123,7 +128,7 @@ export default Ember.Component.extend({
     const similarTopics = this.get('similarTopics');
     const message = this._similarTopicsMessage || composer.store.createRecord('composer-message', {
       id: 'similar_topics',
-      templateName: 'composer/similar-topics',
+      templateName: 'similar-topics',
       extraClass: 'similar-topics'
     });
 
@@ -147,7 +152,7 @@ export default Ember.Component.extend({
     if (this.get('checkedMessages')) { return; }
 
     const composer = this.get('composer');
-    const args = { composerAction: composer.get('action') };
+    const args = { composer_action: composer.get('action') };
     const topicId = composer.get('topic.id');
     const postId = composer.get('post.id');
 
@@ -156,6 +161,13 @@ export default Ember.Component.extend({
 
     const queuedForTyping = this.get('queuedForTyping');
     composer.store.find('composer-message', args).then(messages => {
+
+      // Checking composer messages on replies can give us a list of links to check for
+      // duplicates
+      if (messages.extras && messages.extras.duplicate_lookup) {
+        this.sendAction('addLinkLookup', new LinkLookup(messages.extras.duplicate_lookup));
+      }
+
       this.set('checkedMessages', true);
       messages.forEach(msg => msg.wait_for_typing ? queuedForTyping.addObject(msg) : this.send('popup', msg));
     });

app/assets/javascripts/discourse/components/d-editor.js.es6

@@ -561,5 +561,4 @@ export default Ember.Component.extend({
       });
     }
   }
-
 });

app/assets/javascripts/discourse/controllers/composer.js.es6

@@ -3,6 +3,7 @@ import Quote from 'discourse/lib/quote';
 import Draft from 'discourse/models/draft';
 import Composer from 'discourse/models/composer';
 import { default as computed, observes } from 'ember-addons/ember-computed-decorators';
+import { relativeAge } from 'discourse/lib/formatter';
 
 function loadDraft(store, opts) {
   opts = opts || {};
@@ -46,7 +47,6 @@ export default Ember.Controller.extend({
   replyAsNewTopicDraft: Em.computed.equal('model.draftKey', Composer.REPLY_AS_NEW_TOPIC_KEY),
   checkedMessages: false,
   messageCount: null,
-
   showEditReason: false,
   editReason: null,
   scopedCategoryId: null,
@@ -54,6 +54,8 @@ export default Ember.Controller.extend({
   lastValidatedAt: null,
   isUploading: false,
   topic: null,
+  linkLookup: null,
+
   showToolbar: Em.computed({
     get(){
       const keyValueStore = this.container.lookup('key-value-store:main');
@@ -104,6 +106,39 @@ export default Ember.Controller.extend({
   }.property('model.creatingPrivateMessage', 'model.targetUsernames'),
 
   actions: {
+    addLinkLookup(linkLookup) {
+      this.set('linkLookup', linkLookup);
+    },
+
+    afterRefresh($preview) {
+      const linkLookup = this.get('linkLookup');
+      if (linkLookup) {
+        const $links = $('a[href]', $preview);
+
+        $links.each((idx, l) => {
+          const href = $(l).prop('href');
+          if (href && href.length) {
+            const [warn, info] = linkLookup.check(href);
+
+            if (warn) {
+              const body = I18n.t('composer.duplicate_link', {
+                domain: info.domain,
+                username: info.username,
+                ago: relativeAge(new Date(info.posted_at), { format: 'medium' }),
+                href
+              });
+              this.appEvents.trigger('composer-messages:create', {
+                extraClass: 'custom-body',
+                templateName: 'custom-body',
+                body
+              });
+              return false;
+            }
+          }
+          return true;
+        });
+      }
+    },
 
     toggleWhisper() {
       this.toggleProperty('model.whisper');
@@ -435,6 +470,8 @@ export default Ember.Controller.extend({
 
   // Given a potential instance and options, set the model for this composer.
   _setModel(composerModel, opts) {
+    this.set('linkList', null);
+
     if (opts.draft) {
       composerModel = loadDraft(this.store, opts);
       if (composerModel) {

app/assets/javascripts/discourse/lib/link-lookup.js.es6

@@ -0,0 +1,24 @@
+const _warned = {};
+
+export default class LinkLookup {
+
+  constructor(links) {
+    this._links = links;
+  }
+
+  check(href) {
+    if (_warned[href]) { return [false, null]; }
+
+    const normalized = href.replace(/^https?:\/\//, '');
+    if (_warned[normalized]) { return [false, null]; }
+
+    const linkInfo = this._links[normalized];
+    if (linkInfo) {
+      _warned[href] = true;
+      _warned[normalized] = true;
+      return [true, linkInfo];
+    }
+
+    return [false, null];
+  }
+};

app/assets/javascripts/discourse/templates/composer.hbs

@@ -9,7 +9,7 @@
     {{/popup-menu}}
   {{/if}}
 
-  {{composer-messages composer=model messageCount=messageCount}}
+  {{composer-messages composer=model messageCount=messageCount addLinkLookup="addLinkLookup"}}
 
   <div class='control'>
     {{#if site.mobileView}}
@@ -92,7 +92,8 @@
                           importQuote="importQuote"
                           showOptions="showOptions"
                           showToolbar=showToolbar
-                          showUploadSelector="showUploadSelector"}}
+                          showUploadSelector="showUploadSelector"
+                          afterRefresh="afterRefresh"}}
 
         {{#if currentUser}}
           <div class='submit-panel'>

app/assets/javascripts/discourse/templates/composer/custom-body.hbs

@@ -0,0 +1,2 @@
+<a href {{action "closeMessage"}} class='close'>{{fa-icon "close"}}</a>
+<p>{{{message.body}}}</p>

app/assets/stylesheets/desktop/compose.scss

@@ -78,6 +78,13 @@
   }
 }
 
+.custom-body {
+  background-color: dark-light-diff($tertiary, $secondary, 85%, -65%);
+  p {
+    max-width: 98%;
+  }
+}
+
 .similar-topics {
   background-color: dark-light-diff($tertiary, $secondary, 85%, -65%);
 

app/controllers/composer_messages_controller.rb

@@ -5,9 +5,16 @@ class ComposerMessagesController < ApplicationController
   before_filter :ensure_logged_in
 
   def index
-    finder = ComposerMessagesFinder.new(current_user, params.slice(:composerAction, :topic_id, :post_id))
+    finder = ComposerMessagesFinder.new(current_user, params.slice(:composer_action, :topic_id, :post_id))
     json = { composer_messages: [finder.find].compact }
 
+    if params[:composer_action] == "reply" && params[:topic_id].present?
+      topic = Topic.where(id: params[:topic_id]).first
+      if guardian.can_see?(topic)
+        json[:extras] = {duplicate_lookup: TopicLink.duplicate_lookup(topic)}
+      end
+    end
+
     render_json_dump(json, rest_serializer: true)
   end
 end

app/models/topic_link.rb

@@ -214,6 +214,27 @@ class TopicLink < ActiveRecord::Base
   def crawl_link_title
     Jobs.enqueue(:crawl_topic_link, topic_link_id: id)
   end
+
+  def self.duplicate_lookup(topic)
+    builder = SqlBuilder.new("SELECT tl.url, tl.domain, u.username_lower, p.created_at
+                              FROM topic_links AS tl
+                              INNER JOIN posts AS p ON p.id = tl.post_id
+                              INNER JOIN users AS u ON p.user_id = u.id
+                              /*where*/
+                              ORDER BY p.created_at DESC
+                              LIMIT 200")
+
+    builder.where('tl.topic_id = :topic_id', topic_id: topic.id)
+
+    lookup = {}
+
+    builder.exec.to_a.each do |row|
+      normalized = row['url'].downcase.sub(/^https?:\/\//, '')
+      lookup[normalized] = {domain: row['domain'], username: row['username_lower'], posted_at: row['created_at']}
+    end
+
+    lookup
+  end
 end
 
 # == Schema Information

config/locales/client.en.yml

@@ -989,6 +989,7 @@ en:
       drafts_offline: "drafts offline"
 
       group_mentioned: "By mentioning {{group}}, you are about to notify <a href='{{group_link}}'>{{count}} people</a>."
+      duplicate_link: "It looks like your link to <b>{{domain}}</b> was already posted in the topic by <b>@{{username}}</b> in <a href='{{href}}'>an earlier reply on {{ago}}</a>"
 
       error:
         title_missing: "Title is required"

lib/composer_messages_finder.rb

@@ -29,7 +29,7 @@ class ComposerMessagesFinder
       education_posts_text = I18n.t('education.until_posts', count: SiteSetting.educate_until_posts)
       return {
         id: 'education',
-        templateName: 'composer/education',
+        templateName: 'education',
         wait_for_typing: true,
         body: PrettyText.cook(I18n.t(education_key, education_posts_text: education_posts_text, site_name: SiteSetting.title))
       }
@@ -44,7 +44,7 @@ class ComposerMessagesFinder
 
     {
       id: 'too_many_replies',
-      templateName: 'composer/education',
+      templateName: 'education',
       body: PrettyText.cook(I18n.t('education.too_many_replies', newuser_max_replies_per_topic: SiteSetting.newuser_max_replies_per_topic))
     }
   end
@@ -70,7 +70,7 @@ class ComposerMessagesFinder
     # Return the message
     {
       id: 'avatar',
-      templateName: 'composer/education',
+      templateName: 'education',
       body: PrettyText.cook(I18n.t('education.avatar', profile_path: "/users/#{@user.username_lower}"))
     }
   end
@@ -108,7 +108,7 @@ class ComposerMessagesFinder
 
     {
       id: 'sequential_replies',
-      templateName: 'composer/education',
+      templateName: 'education',
       wait_for_typing: true,
       extraClass: 'education-message',
       body: PrettyText.cook(I18n.t('education.sequential_replies'))
@@ -140,7 +140,7 @@ class ComposerMessagesFinder
 
     {
       id: 'dominating_topic',
-      templateName: 'composer/education',
+      templateName: 'education',
       wait_for_typing: true,
       extraClass: 'education-message',
       body: PrettyText.cook(I18n.t('education.dominating_topic', percent: (ratio * 100).round))
@@ -156,7 +156,7 @@ class ComposerMessagesFinder
 
     {
       id: 'reviving_old',
-      templateName: 'composer/education',
+      templateName: 'education',
       wait_for_typing: false,
       extraClass: 'education-message',
       body: PrettyText.cook(I18n.t('education.reviving_old_topic', days: (Time.zone.now - @topic.last_posted_at).round / 1.day))
@@ -166,11 +166,11 @@ class ComposerMessagesFinder
   private
 
     def creating_topic?
-      @details[:composerAction] == "createTopic"
+      @details[:composer_action] == "createTopic"
     end
 
     def replying?
-      @details[:composerAction] == "reply"
+      @details[:composer_action] == "reply"
     end
 
 end

spec/components/composer_messages_finder_spec.rb

@@ -6,7 +6,7 @@ describe ComposerMessagesFinder do
 
   context "delegates work" do
     let(:user) { Fabricate.build(:user) }
-    let(:finder) { ComposerMessagesFinder.new(user, composerAction: 'createTopic') }
+    let(:finder) { ComposerMessagesFinder.new(user, composer_action: 'createTopic') }
 
     it "calls all the message finders" do
       finder.expects(:check_education_message).once
@@ -24,7 +24,7 @@ describe ComposerMessagesFinder do
     let(:user) { Fabricate.build(:user) }
 
     context 'creating topic' do
-      let(:finder) { ComposerMessagesFinder.new(user, composerAction: 'createTopic') }
+      let(:finder) { ComposerMessagesFinder.new(user, composer_action: 'createTopic') }
 
       before do
         SiteSetting.stubs(:educate_until_posts).returns(10)
@@ -42,7 +42,7 @@ describe ComposerMessagesFinder do
     end
 
     context 'creating reply' do
-      let(:finder) { ComposerMessagesFinder.new(user, composerAction: 'reply') }
+      let(:finder) { ComposerMessagesFinder.new(user, composer_action: 'reply') }
 
       before do
         SiteSetting.stubs(:educate_until_posts).returns(10)
@@ -64,7 +64,7 @@ describe ComposerMessagesFinder do
     let(:user) { Fabricate.build(:user) }
 
     context 'replying' do
-      let(:finder) { ComposerMessagesFinder.new(user, composerAction: 'reply') }
+      let(:finder) { ComposerMessagesFinder.new(user, composer_action: 'reply') }
 
       it "has no message when `posted_too_much_in_topic?` is false" do
         user.expects(:posted_too_much_in_topic?).returns(false)
@@ -80,7 +80,7 @@ describe ComposerMessagesFinder do
   end
 
   context '.check_avatar_notification' do
-    let(:finder) { ComposerMessagesFinder.new(user, composerAction: 'createTopic') }
+    let(:finder) { ComposerMessagesFinder.new(user, composer_action: 'createTopic') }
     let(:user) { Fabricate(:user) }
 
     context "success" do
@@ -141,16 +141,16 @@ describe ComposerMessagesFinder do
     end
 
     it "does not give a message for new topics" do
-      finder = ComposerMessagesFinder.new(user, composerAction: 'createTopic')
+      finder = ComposerMessagesFinder.new(user, composer_action: 'createTopic')
       expect(finder.check_sequential_replies).to be_blank
     end
 
     it "does not give a message without a topic id" do
-      expect(ComposerMessagesFinder.new(user, composerAction: 'reply').check_sequential_replies).to be_blank
+      expect(ComposerMessagesFinder.new(user, composer_action: 'reply').check_sequential_replies).to be_blank
     end
 
     context "reply" do
-      let(:finder) { ComposerMessagesFinder.new(user, composerAction: 'reply', topic_id: topic.id) }
+      let(:finder) { ComposerMessagesFinder.new(user, composer_action: 'reply', topic_id: topic.id) }
 
       it "does not give a message to users who are still in the 'education' phase" do
         user.stubs(:post_count).returns(9)
@@ -216,16 +216,16 @@ describe ComposerMessagesFinder do
     end
 
     it "does not give a message for new topics" do
-      finder = ComposerMessagesFinder.new(user, composerAction: 'createTopic')
+      finder = ComposerMessagesFinder.new(user, composer_action: 'createTopic')
       expect(finder.check_dominating_topic).to be_blank
     end
 
     it "does not give a message without a topic id" do
-      expect(ComposerMessagesFinder.new(user, composerAction: 'reply').check_dominating_topic).to be_blank
+      expect(ComposerMessagesFinder.new(user, composer_action: 'reply').check_dominating_topic).to be_blank
     end
 
     context "reply" do
-      let(:finder) { ComposerMessagesFinder.new(user, composerAction: 'reply', topic_id: topic.id) }
+      let(:finder) { ComposerMessagesFinder.new(user, composer_action: 'reply', topic_id: topic.id) }
 
       it "does not give a message to users who are still in the 'education' phase" do
         user.stubs(:post_count).returns(9)
@@ -288,8 +288,8 @@ describe ComposerMessagesFinder do
     let(:topic) { Fabricate(:topic) }
 
     it "does not give a message without a topic id" do
-      expect(described_class.new(user, composerAction: 'createTopic').check_reviving_old_topic).to be_blank
+      expect(described_class.new(user, composer_action: 'createTopic').check_reviving_old_topic).to be_blank
-      expect(described_class.new(user, composerAction: 'reply').check_reviving_old_topic).to be_blank
+      expect(described_class.new(user, composer_action: 'reply').check_reviving_old_topic).to be_blank
     end
 
     context "a reply" do
@@ -300,12 +300,12 @@ describe ComposerMessagesFinder do
 
         it "does not notify if last post is recent" do
           topic = Fabricate(:topic, last_posted_at: 1.hour.ago)
-          expect(described_class.new(user, composerAction: 'reply', topic_id: topic.id).check_reviving_old_topic).to be_blank
+          expect(described_class.new(user, composer_action: 'reply', topic_id: topic.id).check_reviving_old_topic).to be_blank
         end
 
         it "notifies if last post is old" do
           topic = Fabricate(:topic, last_posted_at: 181.days.ago)
-          expect(described_class.new(user, composerAction: 'reply', topic_id: topic.id).check_reviving_old_topic).not_to be_blank
+          expect(described_class.new(user, composer_action: 'reply', topic_id: topic.id).check_reviving_old_topic).not_to be_blank
         end
       end
 
@@ -316,12 +316,12 @@ describe ComposerMessagesFinder do
 
         it "does not notify if last post is new" do
           topic = Fabricate(:topic, last_posted_at: 1.hour.ago)
-          expect(described_class.new(user, composerAction: 'reply', topic_id: topic.id).check_reviving_old_topic).to be_blank
+          expect(described_class.new(user, composer_action: 'reply', topic_id: topic.id).check_reviving_old_topic).to be_blank
         end
 
         it "does not notify if last post is old" do
           topic = Fabricate(:topic, last_posted_at: 365.days.ago)
-          expect(described_class.new(user, composerAction: 'reply', topic_id: topic.id).check_reviving_old_topic).to be_blank
+          expect(described_class.new(user, composer_action: 'reply', topic_id: topic.id).check_reviving_old_topic).to be_blank
         end
       end
     end

spec/controllers/composer_messages_controller_spec.rb

@@ -10,7 +10,7 @@ describe ComposerMessagesController do
 
     context 'when logged in' do
       let!(:user) { log_in }
-      let(:args) { {'topic_id' => '123', 'post_id' => '333', 'composerAction' => 'reply'} }
+      let(:args) { {'topic_id' => '123', 'post_id' => '333', 'composer_action' => 'reply'} }
 
       it 'redirects to your user preferences' do
         xhr :get, :index

spec/fabricators/post_fabricator.rb

@@ -110,6 +110,7 @@ Fabricator(:post_with_external_links, from: :post) do
   raw "
 Here's a link to twitter: http://twitter.com
 And a link to google: http://google.com
+And a secure link to google: https://google.com
 And a markdown link: [forumwarz](http://forumwarz.com)
 And a markdown link with a period after it [codinghorror](http://www.codinghorror.com/blog).
   "

spec/models/topic_link_spec.rb

@@ -258,7 +258,7 @@ http://b.com/#{'a'*500}
     end
   end
 
-  describe 'counts_for and topic_map' do
+  describe 'query methods' do
     it 'returns blank without posts' do
       expect(TopicLink.counts_for(Guardian.new, nil, nil)).to be_blank
     end
@@ -274,6 +274,19 @@ http://b.com/#{'a'*500}
         TopicLink.counts_for(Guardian.new, post.topic, [post])
       end
 
+      it 'creates a valid topic lookup' do
+        TopicLink.extract_from(post)
+
+        lookup = TopicLink.duplicate_lookup(post.topic)
+        expect(lookup).to be_present
+        expect(lookup['google.com']).to be_present
+
+        ch = lookup['www.codinghorror.com/blog']
+        expect(ch).to be_present
+        expect(ch[:domain]).to eq('www.codinghorror.com')
+        expect(ch[:username]).to eq(post.username)
+        expect(ch[:posted_at]).to be_present
+      end
 
       it 'has the correct results' do
         TopicLink.extract_from(post)
@@ -285,7 +298,7 @@ http://b.com/#{'a'*500}
         expect(counts_for[post.id].first[:clicks]).to eq(1)
 
         array = TopicLink.topic_map(Guardian.new, post.topic_id)
-        expect(array.length).to eq(4)
+        expect(array.length).to eq(5)
         expect(array[0]["clicks"]).to eq("1")
       end