FIX: Check whether group is mentionable by user when cooking post

FIX: Check whether group is mentionable by user when cooking post.

From 3f636b2d19647025f5646acc2087187b37d9870a Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Thu, 22 Nov 2018 16:01:03 +0800
Subject: [PATCH] FIX: Check whether group is mentionable by user when cooking
 post.


diff --git a/app/models/group.rb b/app/models/group.rb
index a3c467e..45615dd 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -132,16 +132,13 @@ class Group < ActiveRecord::Base
   }
 
   scope :mentionable, lambda { |user|
-
-    where("mentionable_level in (:levels) OR
-          (
-            mentionable_level = #{ALIAS_LEVELS[:members_mods_and_admins]} AND id in (
-            SELECT group_id FROM group_users WHERE user_id = :user_id)
-          )", levels: alias_levels(user), user_id: user && user.id)
+    where(self.mentionable_sql_clause,
+      levels: alias_levels(user),
+      user_id: user&.id
+    )
   }
 
   scope :messageable, lambda { |user|
-
     where("messageable_level in (:levels) OR
           (
             messageable_level = #{ALIAS_LEVELS[:members_mods_and_admins]} AND id in (
@@ -149,6 +146,17 @@ class Group < ActiveRecord::Base
           )", levels: alias_levels(user), user_id: user && user.id)
   }
 
+  def self.mentionable_sql_clause
+    <<~SQL
+    mentionable_level in (:levels)
+    OR (
+      mentionable_level = #{ALIAS_LEVELS[:members_mods_and_admins]}
+      AND id in (
+        SELECT group_id FROM group_users WHERE user_id = :user_id)
+      )
+    SQL
+  end
+
   def self.alias_levels(user)
     levels = [ALIAS_LEVELS[:everyone]]
 
diff --git a/lib/pretty_text.rb b/lib/pretty_text.rb
index e7682f9..a22d269 100644
--- a/lib/pretty_text.rb
+++ b/lib/pretty_text.rb
@@ -264,7 +264,9 @@ module PrettyText
       add_s3_cdn(doc)
     end
 
-    add_mentions(doc) if SiteSetting.enable_mentions
+    if SiteSetting.enable_mentions
+      add_mentions(doc, user_id: opts[:user_id])
+    end
 
     doc.to_html
   end
@@ -425,11 +427,11 @@ module PrettyText
   USER_TYPE ||= 'user'
   GROUP_TYPE ||= 'group'
 
-  def self.add_mentions(doc)
+  def self.add_mentions(doc, user_id: nil)
     elements = doc.css("span.mention")
     names = elements.map { |element| element.text[1..-1] }
 
-    mentions = lookup_mentions(names)
+    mentions = lookup_mentions(names, user_id: user_id)
 
     doc.css("span.mention").each do |element|
       name = element.text[1..-1]
@@ -453,7 +455,7 @@ module PrettyText
     end
   end
 
-  def self.lookup_mentions(names)
+  def self.lookup_mentions(names, user_id: nil)
     sql = <<~SQL
     (
       SELECT
@@ -468,14 +470,18 @@ module PrettyText
         :group_type AS type,
         name
       FROM groups
-      WHERE name IN (:names)
+      WHERE name IN (:names) AND (#{Group.mentionable_sql_clause})
     )
     SQL
 
+    user = User.find_by(id: user_id)
+
     results = DB.query(sql,
       names: names,
       user_type: USER_TYPE,
-      group_type: GROUP_TYPE
+      group_type: GROUP_TYPE,
+      levels: Group.alias_levels(user),
+      user_id: user_id
     )
 
     mentions = {}
diff --git a/spec/components/pretty_text_spec.rb b/spec/components/pretty_text_spec.rb
index 6655e08..7b58eec 100644
--- a/spec/components/pretty_text_spec.rb
+++ b/spec/components/pretty_text_spec.rb
@@ -225,7 +225,9 @@ describe PrettyText do
         Fabricate(:user, username: username)
       end
 
-      group = Fabricate(:group)
+      group = Fabricate(:group,
+        mentionable_level: Group::ALIAS_LEVELS[:everyone]
+      )
 
       [
         [
@@ -241,6 +243,14 @@ describe PrettyText do
       end
     end
 
+    it "does not create mention for a non mentionable group" do
+      group = Fabricate(:group, mentionable_level: Group::ALIAS_LEVELS[:nobody])
+
+      expect(PrettyText.cook("test @#{group.name} test")).to eq(
+        %Q|<p>test <span class="mention">@#{group.name}</span> test</p>|
+      )
+    end
+
     it 'does not mention staged users' do
       user = Fabricate(:user, staged: true)
 
diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb
index 6ac88df..532781d 100644
--- a/spec/models/post_spec.rb
+++ b/spec/models/post_spec.rb
@@ -969,6 +969,38 @@ describe Post do
       post.save
       expect(post.cooked).to match(/nofollow noopener/)
     end
+
+    describe 'mentions' do
+      let(:group) do
+        Fabricate(:group,
+          mentionable_level: Group::ALIAS_LEVELS[:members_mods_and_admins]
+        )
+      end
+
+      describe 'when user can not mention a group' do
+        it "should not create the mention" do
+          post = Fabricate(:post, raw: "hello @#{group.name}")
+
+          expect(post.cooked).to eq(
+            %Q|<p>hello <span class="mention">@#{group.name}</span></p>|
+          )
+        end
+      end
+
+      describe 'when user can mention a group' do
+        before do
+          group.add(post.user)
+        end
+
+        it 'should create the mention' do
+          post.update!(raw: "hello @#{group.name}")
+
+          expect(post.cooked).to eq(
+            %Q|<p>hello <a class="mention-group" href="/groups/#{group.name}">@#{group.name}</a></p>|
+          )
+        end
+      end
+    end
   end
 
   describe "calculate_avg_time" do

GitHub

1 Like