From e9bc763440f90a589c2616754e4ca5eeb848be04 Mon Sep 17 00:00:00 2001
From: Arpit Jalan <arpit@techapj.com>
Date: Thu, 15 Mar 2018 18:03:08 +0530
Subject: [PATCH] FIX: show only allowed tags on PM tags page and display
correct count
FIX: tags page should link to user profile we are browsing
---
.../user-private-messages-tags.js.es6 | 1 +
.../controllers/user-topics-list.js.es6 | 1 +
.../discourse/lib/render-tag.js.es6 | 8 ++-
.../discourse/lib/render-tags.js.es6 | 12 +++--
.../build-private-messages-route.js.es6 | 1 +
.../routes/user-private-messages-tags.js.es6 | 6 ++-
.../templates/components/basic-topic-list.hbs | 3 +-
.../templates/components/tag-list.hbs | 2 +-
.../templates/components/topic-list.hbs | 3 +-
.../templates/list/topic-list-item.raw.hbs | 2 +-
.../templates/user-private-messages-tags.hbs | 2 +-
.../discourse/templates/user-topics-list.hbs | 3 +-
app/controllers/tags_controller.rb | 5 +-
app/models/tag.rb | 16 ++++--
config/routes.rb | 2 +-
spec/models/tag_spec.rb | 16 ++++--
spec/requests/tags_controller_spec.rb | 53 ++++++++++++++++---
17 files changed, 107 insertions(+), 29 deletions(-)
diff --git a/app/assets/javascripts/discourse/controllers/user-private-messages-tags.js.es6 b/app/assets/javascripts/discourse/controllers/user-private-messages-tags.js.es6
index 8f2ee7ce79b..6fc3485357c 100644
--- a/app/assets/javascripts/discourse/controllers/user-private-messages-tags.js.es6
+++ b/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() {
diff --git a/app/assets/javascripts/discourse/controllers/user-topics-list.js.es6 b/app/assets/javascripts/discourse/controllers/user-topics-list.js.es6
index 9f877a1bb38..0d119f6d331 100644
--- a/app/assets/javascripts/discourse/controllers/user-topics-list.js.es6
+++ b/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"));
diff --git a/app/assets/javascripts/discourse/lib/render-tag.js.es6 b/app/assets/javascripts/discourse/lib/render-tag.js.es6
index ea34505e5bd..b9763676f40 100644
--- a/app/assets/javascripts/discourse/lib/render-tag.js.es6
+++ b/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();
- path = params.isPrivateMessage ? `/u/${current_user.username}/messages/tags/${tag}` : `/tags/${tag}`;
+ if (params.isPrivateMessage && Discourse.User.current()) {
+ 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)}' ` : "";
diff --git a/app/assets/javascripts/discourse/lib/render-tags.js.es6 b/app/assets/javascripts/discourse/lib/render-tags.js.es6
index ef86b502d31..f6bb19b92e4 100644
--- a/app/assets/javascripts/discourse/lib/render-tags.js.es6
+++ b/app/assets/javascripts/discourse/lib/render-tags.js.es6
@@ -20,10 +20,16 @@ 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") {
- tags = topic.get("visibleListTags");
+ if (params) {
+ if (params.mode === "list") {
+ tags = topic.get("visibleListTags");
+ }
+ if (params.tagsForUser) {
+ tagsForUser = params.tagsForUser;
+ }
}
let customHtml = null;
@@ -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 }) + ' ';
}
}
diff --git a/app/assets/javascripts/discourse/routes/build-private-messages-route.js.es6 b/app/assets/javascripts/discourse/routes/build-private-messages-route.js.es6
index bb50a91634d..4be8889dea9 100644
--- a/app/assets/javascripts/discourse/routes/build-private-messages-route.js.es6
+++ b/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: []
});
diff --git a/app/assets/javascripts/discourse/routes/user-private-messages-tags.js.es6 b/app/assets/javascripts/discourse/routes/user-private-messages-tags.js.es6
index b57002e7e3a..d2297f23842 100644
--- a/app/assets/javascripts/discourse/routes/user-private-messages-tags.js.es6
+++ b/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,
diff --git a/app/assets/javascripts/discourse/templates/components/basic-topic-list.hbs b/app/assets/javascripts/discourse/templates/components/basic-topic-list.hbs
index fbcad14b57a..1fe3c6eaa3b 100644
--- a/app/assets/javascripts/discourse/templates/components/basic-topic-list.hbs
+++ b/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'>
diff --git a/app/assets/javascripts/discourse/templates/components/tag-list.hbs b/app/assets/javascripts/discourse/templates/components/tag-list.hbs
index 5d4deec2c2d..cd09db7bc35 100644
--- a/app/assets/javascripts/discourse/templates/components/tag-list.hbs
+++ b/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}}
diff --git a/app/assets/javascripts/discourse/templates/components/topic-list.hbs b/app/assets/javascripts/discourse/templates/components/topic-list.hbs
index b0d2ae5b115..dcb3f4c307b 100644
--- a/app/assets/javascripts/discourse/templates/components/topic-list.hbs
+++ b/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>
diff --git a/app/assets/javascripts/discourse/templates/list/topic-list-item.raw.hbs b/app/assets/javascripts/discourse/templates/list/topic-list-item.raw.hbs
index 020e32ecf4b..942e3f8d820 100644
--- a/app/assets/javascripts/discourse/templates/list/topic-list-item.raw.hbs
+++ b/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}}
diff --git a/app/assets/javascripts/discourse/templates/user-private-messages-tags.hbs b/app/assets/javascripts/discourse/templates/user-private-messages-tags.hbs
index 0682a895765..2310f0be8df 100644
--- a/app/assets/javascripts/discourse/templates/user-private-messages-tags.hbs
+++ b/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}}
diff --git a/app/assets/javascripts/discourse/templates/user-topics-list.hbs b/app/assets/javascripts/discourse/templates/user-topics-list.hbs
index 0749c7c837c..933e3dfe168 100644
--- a/app/assets/javascripts/discourse/templates/user-topics-list.hbs
+++ b/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}}
diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb
index 6cf3b813ffb..3a6d5f0d674 100644
--- a/app/controllers/tags_controller.rb
+++ b/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
diff --git a/app/models/tag.rb b/app/models/tag.rb
index c7c67f2625b..f110e677ce6 100644
--- a/app/models/tag.rb
+++ b/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)
- return [] unless (guardian || Guardian.new).can_tag_pms?
+ def self.pm_tags(limit_arg: nil, guardian: nil, allowed_user: nil)
+ 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
diff --git a/config/routes.rb b/config/routes.rb
index f1dfc801bb6..23b2bebef95 100644
--- a/config/routes.rb
+++ b/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'
diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb
index d642d96f70f..23602878b26 100644
--- a/spec/models/tag_spec.rb
+++ b/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")
diff --git a/spec/requests/tags_controller_spec.rb b/spec/requests/tags_controller_spec.rb
index 58928384cf5..f77f8ee312c 100644
--- a/spec/requests/tags_controller_spec.rb
+++ b/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
- it "should return the right response" do
- get "/tags/personal_messages.json"
+ context "as a regular user" do
+ it "can't see pm tags" do
+ 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
- get "/tags/personal_messages.json"
+ it "can see pm tags for regular user" do
+ 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