diff --git a/app/assets/javascripts/discourse/app/models/tag.js b/app/assets/javascripts/discourse/app/models/tag.js index 5f514f8db1..54f3733065 100644 --- a/app/assets/javascripts/discourse/app/models/tag.js +++ b/app/assets/javascripts/discourse/app/models/tag.js @@ -1,15 +1,13 @@ import RestModel from "discourse/models/rest"; import discourseComputed from "discourse-common/utils/decorators"; +import { readOnly } from "@ember/object/computed"; export default RestModel.extend({ - @discourseComputed("count", "pm_count") - totalCount(count, pmCount) { - return count + pmCount; - }, + pmOnly: readOnly("pm_only"), @discourseComputed("count", "pm_count") - pmOnly(count, pmCount) { - return count === 0 && pmCount > 0; + totalCount(count, pmCount) { + return pmCount ? count + pmCount : count; }, @discourseComputed("id") diff --git a/app/assets/javascripts/discourse/tests/helpers/create-pretender.js b/app/assets/javascripts/discourse/tests/helpers/create-pretender.js index d694826cf5..25364f0ff0 100644 --- a/app/assets/javascripts/discourse/tests/helpers/create-pretender.js +++ b/app/assets/javascripts/discourse/tests/helpers/create-pretender.js @@ -100,8 +100,8 @@ export function applyDefaultHandlers(pretender) { return response({ tags: [ { id: "eviltrout", count: 1 }, - { id: "planned", text: "planned", count: 7, pm_count: 0 }, - { id: "private", text: "private", count: 0, pm_count: 7 }, + { id: "planned", text: "planned", count: 7, pm_only: false }, + { id: "private", text: "private", count: 0, pm_only: true }, ], extras: { tag_groups: [ @@ -109,24 +109,24 @@ export function applyDefaultHandlers(pretender) { id: 2, name: "Ford Cars", tags: [ - { id: "Escort", text: "Escort", count: 1, pm_count: 0 }, - { id: "focus", text: "focus", count: 3, pm_count: 0 }, + { id: "Escort", text: "Escort", count: 1, pm_only: false }, + { id: "focus", text: "focus", count: 3, pm_only: false }, ], }, { id: 1, name: "Honda Cars", tags: [ - { id: "civic", text: "civic", count: 4, pm_count: 0 }, - { id: "accord", text: "accord", count: 2, pm_count: 0 }, + { id: "civic", text: "civic", count: 4, pm_only: false }, + { id: "accord", text: "accord", count: 2, pm_only: false }, ], }, { id: 1, name: "Makes", tags: [ - { id: "ford", text: "ford", count: 5, pm_count: 0 }, - { id: "honda", text: "honda", count: 6, pm_count: 0 }, + { id: "ford", text: "ford", count: 5, pm_only: false }, + { id: "honda", text: "honda", count: 6, pm_only: false }, ], }, ], diff --git a/app/assets/javascripts/discourse/tests/integration/components/select-kit/tag-drop-test.js b/app/assets/javascripts/discourse/tests/integration/components/select-kit/tag-drop-test.js index 385c94991e..e8ff49de4c 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/select-kit/tag-drop-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/select-kit/tag-drop-test.js @@ -28,7 +28,7 @@ module("Integration | Component | select-kit/tag-drop", function (hooks) { pretender.get("/tags/filter/search", (params) => { if (params.queryParams.q === "dav") { return response({ - results: [{ id: "David", name: "David", count: 2, pm_count: 0 }], + results: [{ id: "David", name: "David", count: 2, pm_only: false }], }); } }); diff --git a/app/assets/javascripts/discourse/tests/unit/models/tag-test.js b/app/assets/javascripts/discourse/tests/unit/models/tag-test.js new file mode 100644 index 0000000000..c42364fa07 --- /dev/null +++ b/app/assets/javascripts/discourse/tests/unit/models/tag-test.js @@ -0,0 +1,31 @@ +import { module, test } from "qunit"; +import { getOwner } from "discourse-common/lib/get-owner"; +import { setupTest } from "ember-qunit"; + +module("Unit | Model | tag", function (hooks) { + setupTest(hooks); + + hooks.beforeEach(function () { + this.store = getOwner(this).lookup("service:store"); + }); + + test("totalCount when pm_count is not present", function (assert) { + const tag = this.store.createRecord("tag", { count: 5 }); + assert.strictEqual(tag.totalCount, 5); + }); + + test("totalCount when pm_count is present", function (assert) { + const tag = this.store.createRecord("tag", { count: 5, pm_count: 8 }); + assert.strictEqual(tag.totalCount, 13); + }); + + test("pmOnly", function (assert) { + const tag = this.store.createRecord("tag", { pm_only: false }); + + assert.notOk(tag.pmOnly); + + tag.set("pm_only", true); + + assert.ok(tag.pmOnly); + }); +}); diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index d8b7a2fb76..5bf4a45958 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -396,16 +396,22 @@ class TagsController < ::ApplicationController next if topic_count == 0 && t.pm_topic_count > 0 && !show_pm_tags - { + attrs = { id: t.name, text: t.name, name: t.name, description: t.description, count: topic_count, - pm_count: show_pm_tags ? t.pm_topic_count : 0, + pm_only: topic_count == 0 && t.pm_topic_count > 0, target_tag: t.target_tag_id ? target_tags.find { |x| x.id == t.target_tag_id }&.name : nil, } + + if show_pm_tags && SiteSetting.display_personal_messages_tag_counts + attrs[:pm_count] = t.pm_topic_count + end + + attrs end .compact end diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 85b1523ed1..1661e9544e 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1652,6 +1652,7 @@ en: content_security_policy_script_src: "Additional allowlisted script sources. The current host and CDN are included by default. See Mitigate XSS Attacks with Content Security Policy." invalidate_inactive_admin_email_after_days: "Admin accounts that have not visited the site in this number of days will need to re-validate their email address before logging in. Set to 0 to disable." include_secure_categories_in_tag_counts: "When enabled, count of topics for a tag will include topics that are in read restricted categories for all users. When disabled, normal users are only shown a count of topics for a tag where all the topics are in public categories." + display_personal_messages_tag_counts: "When enabled, count of personal messages tagged with a given tag will be displayed." top_menu: "Determine which items appear in the homepage navigation, and in what order. Example latest|new|unread|categories|top|read|posted|bookmarks" post_menu: "Determine which items appear on the post menu, and in what order. Example like|edit|flag|delete|share|bookmark|reply" post_menu_hidden_items: "The menu items to hide by default in the post menu unless an expansion ellipsis is clicked on." diff --git a/config/site_settings.yml b/config/site_settings.yml index c7361ed2fb..e2764ab579 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -1788,6 +1788,8 @@ security: hidden: true include_secure_categories_in_tag_counts: default: false + display_personal_messages_tag_counts: + default: false onebox: post_onebox_maxlength: diff --git a/spec/requests/tags_controller_spec.rb b/spec/requests/tags_controller_spec.rb index b7dc5bfe4a..7513902ecd 100644 --- a/spec/requests/tags_controller_spec.rb +++ b/spec/requests/tags_controller_spec.rb @@ -11,10 +11,22 @@ RSpec.describe TagsController do before { SiteSetting.tagging_enabled = true } describe "#index" do - fab!(:test_tag) { Fabricate(:tag, name: "test") } + fab!(:test_tag) { Fabricate(:tag, name: "test", description: "some description") } + fab!(:topic_tag) do - Fabricate(:tag, name: "topic-test", public_topic_count: 1, staff_topic_count: 1) + Fabricate( + :tag, + name: "topic-test", + public_topic_count: 1, + staff_topic_count: 1, + pm_topic_count: 5, + ) end + + fab!(:pm_only_tag) do + Fabricate(:tag, public_topic_count: 0, staff_topic_count: 0, pm_topic_count: 1) + end + fab!(:synonym) { Fabricate(:tag, name: "synonym", target_tag: topic_tag) } shared_examples "retrieves the right tags" do @@ -27,13 +39,63 @@ RSpec.describe TagsController do tags = response.parsed_body["tags"] + serialized_tag = tags.find { |t| t["id"] == test_tag.name } + + expect(serialized_tag["count"]).to eq(0) + expect(serialized_tag["pm_count"]).to eq(nil) + expect(serialized_tag["pm_only"]).to eq(false) + + serialized_tag = tags.find { |t| t["id"] == topic_tag.name } + + expect(serialized_tag["count"]).to eq(1) + expect(serialized_tag["pm_count"]).to eq(nil) + expect(serialized_tag["pm_only"]).to eq(false) + end + + it "does not include pm_count attribute when user cannot tag PM topics even if display_personal_messages_tag_counts site setting has been enabled" do + SiteSetting.display_personal_messages_tag_counts = true + + sign_in(admin) + + get "/tags.json" + + expect(response.status).to eq(200) + + tags = response.parsed_body["tags"] + expect(tags[0]["name"]).to eq(test_tag.name) - expect(tags[0]["count"]).to eq(0) - expect(tags[0]["pm_count"]).to eq(0) + expect(tags[0]["pm_count"]).to eq(nil) expect(tags[1]["name"]).to eq(topic_tag.name) - expect(tags[1]["count"]).to eq(1) - expect(tags[1]["pm_count"]).to eq(0) + expect(tags[1]["pm_count"]).to eq(nil) + end + + it "includes pm_count attribute when user can tag PM topics and display_personal_messages_tag_counts site setting has been enabled" do + SiteSetting.display_personal_messages_tag_counts = true + SiteSetting.pm_tags_allowed_for_groups = Group::AUTO_GROUPS[:admins] + + sign_in(admin) + + get "/tags.json" + + expect(response.status).to eq(200) + + tags = response.parsed_body["tags"] + + serialized_tag = tags.find { |t| t["id"] == test_tag.name } + + expect(serialized_tag["pm_count"]).to eq(0) + expect(serialized_tag["pm_only"]).to eq(false) + + serialized_tag = tags.find { |t| t["id"] == topic_tag.name } + + expect(serialized_tag["pm_count"]).to eq(5) + expect(serialized_tag["pm_only"]).to eq(false) + + serialized_tag = tags.find { |t| t["id"] == pm_only_tag.name } + + expect(serialized_tag["pm_count"]).to eq(1) + expect(serialized_tag["pm_only"]).to eq(true) end it "only retrieve tags that have been used in public topics for non-staff user" do @@ -48,7 +110,7 @@ RSpec.describe TagsController do expect(tags[0]["name"]).to eq(topic_tag.name) expect(tags[0]["count"]).to eq(1) - expect(tags[0]["pm_count"]).to eq(0) + expect(tags[0]["pm_count"]).to eq(nil) end end @@ -66,17 +128,17 @@ RSpec.describe TagsController do context "when enabled" do before do SiteSetting.pm_tags_allowed_for_groups = "1|2|3" + SiteSetting.display_personal_messages_tag_counts = true sign_in(admin) end it "shows topic tags and pm tags" do get "/tags.json" tags = response.parsed_body["tags"] - expect(tags.length).to eq(2) serialized_tag = tags.find { |t| t["id"] == topic_tag.name } expect(serialized_tag["count"]).to eq(2) - expect(serialized_tag["pm_count"]).to eq(0) + expect(serialized_tag["pm_count"]).to eq(5) serialized_tag = tags.find { |t| t["id"] == test_tag.name } expect(serialized_tag["count"]).to eq(0)