Merge pull request #5682 from techAPJ/allowed-tags-page

FIX: show only allowed tags on PM tags page and display correct count

app/assets/javascripts/discourse/controllers/user-private-messages-tags.js.es6

@@ -1,5 +1,6 @@
 export default Ember.Controller.extend({
   sortProperties: ['count:desc', 'id'],
+  tagsForUser: null,
 
   actions: {
     sortByCount() {

app/assets/javascripts/discourse/controllers/user-topics-list.js.es6

@@ -9,6 +9,7 @@ export default Ember.Controller.extend({
   newIncoming: [],
   incomingCount: 0,
   channel: null,
+  tagsForUser: null,
 
   _showFooter: function() {
     this.set("application.showFooter", !this.get("model.canLoadMore"));

app/assets/javascripts/discourse/lib/render-tag.js.es6

@@ -5,8 +5,12 @@ export default function renderTag(tag, params) {
   const tagName = params.tagName || "a";
   let path;
   if (tagName === "a" && !params.noHref) {
-    const current_user = Discourse.User.current();
+    if (params.isPrivateMessage && Discourse.User.current()) {
-    path = params.isPrivateMessage ? `/u/${current_user.username}/messages/tags/${tag}` : `/tags/${tag}`;
+      const username = params.tagsForUser ? params.tagsForUser : Discourse.User.current().username;
+      path = `/u/${username}/messages/tags/${tag}`;
+    } else {
+      path = `/tags/${tag}`;
+    }
   }
   const href = path ? ` href='${Discourse.getURL(path)}' ` : "";
 

app/assets/javascripts/discourse/lib/render-tags.js.es6

@@ -20,11 +20,17 @@ export function addTagsHtmlCallback(callback, options) {
 export default function(topic, params){
   let tags = topic.tags;
   let buffer = "";
+  let tagsForUser = null;
   const isPrivateMessage = topic.get('isPrivateMessage');
 
-  if (params && params.mode === "list") {
+  if (params) {
+    if (params.mode === "list") {
       tags = topic.get("visibleListTags");
     }
+    if (params.tagsForUser) {
+      tagsForUser = params.tagsForUser;
+    }
+  }
 
   let customHtml = null;
   if (callbacks) {
@@ -44,7 +50,7 @@ export default function(topic, params){
     buffer = "<div class='discourse-tags'>";
     if (tags) {
       for(let i=0; i<tags.length; i++){
-        buffer += renderTag(tags[i], { isPrivateMessage }) + ' ';
+        buffer += renderTag(tags[i], { isPrivateMessage, tagsForUser }) + ' ';
       }
     }
 

app/assets/javascripts/discourse/routes/build-private-messages-route.js.es6

@@ -32,6 +32,7 @@ export default (viewName, path, channel) => {
         hideCategory: true,
         showPosters: true,
         canBulkSelect: true,
+        tagsForUser: this.modelFor("user").get("username_lower"),
         selected: []
       });
 

app/assets/javascripts/discourse/routes/user-private-messages-tags.js.es6

@@ -3,7 +3,8 @@ import { popupAjaxError } from 'discourse/lib/ajax-error';
 
 export default Discourse.Route.extend({
   model() {
-    return ajax('/tags/personal_messages').then(result => {
+    const username = this.modelFor("user").get("username_lower");
+    return ajax(`/tags/personal_messages/${username}`).then(result => {
       return result.tags.map(tag => Ember.Object.create(tag));
     }).catch(popupAjaxError);
   },
@@ -15,7 +16,8 @@ export default Discourse.Route.extend({
   setupController(controller, model) {
     this.controllerFor('user-private-messages-tags').setProperties({
       model,
-      sortProperties: this.siteSettings.tags_sort_alphabetically ? ['id'] : ['count:desc', 'id']
+      sortProperties: this.siteSettings.tags_sort_alphabetically ? ['id'] : ['count:desc', 'id'],
+      tagsForUser: this.modelFor("user").get("username_lower")
     });
     this.controllerFor('user-private-messages').setProperties({
       showToggleBulkSelect: false,

app/assets/javascripts/discourse/templates/components/basic-topic-list.hbs

@@ -17,7 +17,8 @@
                  bulkSelectEnabled=bulkSelectEnabled
                  canBulkSelect=canBulkSelect
                  selected=selected
-                 skipHeader=skipHeader}}
+                 skipHeader=skipHeader
+                 tagsForUser=tagsForUser}}
   {{else}}
     {{#unless loadingMore}}
     <div class='alert alert-info'>

app/assets/javascripts/discourse/templates/components/tag-list.hbs

@@ -10,7 +10,7 @@
 {{#each sortedTags as |tag|}}
   <div class='tag-box'>
     {{#if tag.count}}
-      {{discourse-tag tag.id isPrivateMessage=isPrivateMessage}} <span class='tag-count'>x {{tag.count}}</span>
+      {{discourse-tag tag.id isPrivateMessage=isPrivateMessage tagsForUser=tagsForUser}} <span class='tag-count'>x {{tag.count}}</span>
     {{else}}
       {{discourse-tag tag.id}}
     {{/if}}

app/assets/javascripts/discourse/templates/components/topic-list.hbs

@@ -40,7 +40,8 @@
                       expandGloballyPinned=expandGloballyPinned
                       expandAllPinned=expandAllPinned
                       lastVisitedTopic=lastVisitedTopic
-                      selected=selected}}
+                      selected=selected
+                      tagsForUser=tagsForUser}}
     {{raw "list/visited-line" lastVisitedTopic=lastVisitedTopic topic=topic}}
   {{/each}}
 </tbody>

app/assets/javascripts/discourse/templates/list/topic-list-item.raw.hbs

@@ -18,7 +18,7 @@
     {{/if}}
   </span>
 
-  {{discourse-tags topic mode="list"}}
+  {{discourse-tags topic mode="list" tagsForUser=tagsForUser}}
   {{#if expandPinned}}
     {{raw "list/topic-excerpt" topic=topic}}
   {{/if}}

app/assets/javascripts/discourse/templates/user-private-messages-tags.hbs

@@ -13,5 +13,5 @@
 <hr/>
 
 {{#if model}}
-  {{tag-list tags=model sortProperties=sortProperties titleKey="tagging.all_tags" isPrivateMessage=true}}
+  {{tag-list tags=model sortProperties=sortProperties titleKey="tagging.all_tags" isPrivateMessage=true tagsForUser=tagsForUser}}
 {{/if}}

app/assets/javascripts/discourse/templates/user-topics-list.hbs

@@ -7,7 +7,8 @@
                      selected=selected
                      hasIncoming=hasIncoming
                      incomingCount=incomingCount
-                     showInserted="showInserted"}}
+                     showInserted="showInserted"
+                     tagsForUser=tagsForUser}}
 
   {{conditional-loading-spinner condition=model.loadingMore}}
 {{/load-more}}

app/controllers/tags_controller.rb

@@ -193,7 +193,10 @@ class TagsController < ::ApplicationController
 
   def personal_messages
     guardian.ensure_can_tag_pms!
-    pm_tags = Tag.pm_tags(guardian: guardian)
+    allowed_user = fetch_user_from_params
+    raise Discourse::NotFound if allowed_user.blank?
+    raise Discourse::NotFound if current_user.id != allowed_user.id && !@guardian.is_admin?
+    pm_tags = Tag.pm_tags(guardian: guardian, allowed_user: allowed_user)
 
     render json: { tags: pm_tags }
   end

app/models/tag.rb

@@ -59,17 +59,27 @@ class Tag < ActiveRecord::Base
     tag_names_with_counts.map { |row| row['tag_name'] }
   end
 
-  def self.pm_tags(limit_arg: nil, guardian: nil)
+  def self.pm_tags(limit_arg: nil, guardian: nil, allowed_user: nil)
-    return [] unless (guardian || Guardian.new).can_tag_pms?
+    return [] if allowed_user.blank? || !(guardian || Guardian.new).can_tag_pms?
     limit = limit_arg || SiteSetting.max_tags_in_filter_list
+    user_id = allowed_user.id
 
     tag_names_with_counts = Tag.exec_sql <<~SQL
-      SELECT tags.name, COUNT(topics.id) AS topic_count
+      SELECT tags.name,
+          COUNT(topics.id) AS topic_count
       FROM tags
       INNER JOIN topic_tags ON tags.id = topic_tags.tag_id
       INNER JOIN topics ON topics.id = topic_tags.topic_id
       AND topics.deleted_at IS NULL
       AND topics.archetype = 'private_message'
+      WHERE topic_tags.topic_id IN
+      (SELECT topic_id
+        FROM topic_allowed_users
+        WHERE user_id = #{user_id}
+        UNION ALL SELECT tg.topic_id
+        FROM topic_allowed_groups tg
+        JOIN group_users gu ON gu.user_id = #{user_id}
+        AND gu.group_id = tg.group_id)
       GROUP BY tags.name
       LIMIT #{limit}
     SQL

config/routes.rb

@@ -739,7 +739,7 @@ Discourse::Application.routes.draw do
     get '/filter/list' => 'tags#index'
     get '/filter/search' => 'tags#search'
     get '/check' => 'tags#check_hashtag'
-    get '/personal_messages' => 'tags#personal_messages'
+    get '/personal_messages/:username' => 'tags#personal_messages'
     constraints(tag_id: /[^\/]+?/, format: /json|rss/) do
       get '/:tag_id.rss' => 'tags#tag_feed'
       get '/:tag_id' => 'tags#show', as: 'tag_show'

spec/models/tag_spec.rb

@@ -97,23 +97,31 @@ describe Tag do
   end
 
   describe '#pm_tags' do
+    let(:regular_user) { Fabricate(:trust_level_4) }
+    let(:admin) { Fabricate(:admin) }
+    let(:personal_message) do
+      Fabricate(:private_message_topic, user: regular_user, topic_allowed_users: [
+        Fabricate.build(:topic_allowed_user, user: regular_user),
+        Fabricate.build(:topic_allowed_user, user: admin)
+      ])
+    end
+
     before do
-      personal_message = Fabricate(:private_message_topic)
       2.times { |i| Fabricate(:tag, topics: [personal_message], name: "tag-#{i}") }
     end
 
     it "returns nothing if user is not a staff" do
-      expect(described_class.pm_tags.sort).to be_empty
+      expect(described_class.pm_tags(guardian: Guardian.new(regular_user))).to be_empty
     end
 
     it "returns nothing if allow_staff_to_tag_pms setting is disabled" do
       SiteSetting.allow_staff_to_tag_pms = false
-      expect(described_class.pm_tags(guardian: Guardian.new(Fabricate(:admin))).sort).to be_empty
+      expect(described_class.pm_tags(guardian: Guardian.new(admin)).sort).to be_empty
     end
 
     it "returns all pm tags if user is a staff and pm tagging is enabled" do
       SiteSetting.allow_staff_to_tag_pms = true
-      tags = described_class.pm_tags(guardian: Guardian.new(Fabricate(:admin)))
+      tags = described_class.pm_tags(guardian: Guardian.new(admin), allowed_user: regular_user)
       expect(tags.length).to eq(2)
       expect(tags[0][:id]).to eq("tag-0")
       expect(tags[1][:text]).to eq("tag-1")

spec/requests/tags_controller_spec.rb

@@ -55,28 +55,67 @@ describe TagsController do
   end
 
   describe '#personal_messages' do
+    let(:regular_user) { Fabricate(:trust_level_4) }
+    let(:moderator) { Fabricate(:moderator) }
+    let(:admin) { Fabricate(:admin) }
+    let(:personal_message) do
+      Fabricate(:private_message_topic, user: regular_user, topic_allowed_users: [
+        Fabricate.build(:topic_allowed_user, user: regular_user),
+        Fabricate.build(:topic_allowed_user, user: moderator),
+        Fabricate.build(:topic_allowed_user, user: admin)
+      ])
+    end
+
     before do
       SiteSetting.allow_staff_to_tag_pms = true
-      personal_message = Fabricate(:private_message_topic)
       Fabricate(:tag, topics: [personal_message], name: 'test')
     end
 
-    context "as a normal user" do
+    context "as a regular user" do
-      it "should return the right response" do
+      it "can't see pm tags" do
-        get "/tags/personal_messages.json"
+        get "/tags/personal_messages/#{regular_user.username}.json"
 
         expect(response).not_to be_success
       end
     end
 
+    context "as an moderator" do
+      before do
+        sign_in(moderator)
+      end
+
+      it "can't see pm tags for regular user" do
+        get "/tags/personal_messages/#{regular_user.username}.json"
+
+        expect(response).not_to be_success
+      end
+
+      it "can see their own pm tags" do
+        get "/tags/personal_messages/#{moderator.username}.json"
+
+        expect(response).to be_success
+
+        tag = JSON.parse(response.body)['tags']
+        expect(tag[0]["id"]).to eq('test')
+      end
+    end
+
     context "as an admin" do
       before do
-        admin = Fabricate(:admin)
         sign_in(admin)
       end
 
-      it "should return the right response" do
+      it "can see pm tags for regular user" do
-        get "/tags/personal_messages.json"
+        get "/tags/personal_messages/#{regular_user.username}.json"
+
+        expect(response).to be_success
+
+        tag = JSON.parse(response.body)['tags']
+        expect(tag[0]["id"]).to eq('test')
+      end
+
+      it "can see their own pm tags" do
+        get "/tags/personal_messages/#{admin.username}.json"
 
         expect(response).to be_success