diff --git a/app/assets/javascripts/discourse/app/components/sidebar/tags-section.js b/app/assets/javascripts/discourse/app/components/sidebar/tags-section.js index f2722244c4f..55dc9a376e5 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/tags-section.js +++ b/app/assets/javascripts/discourse/app/components/sidebar/tags-section.js @@ -1,15 +1,17 @@ import I18n from "I18n"; import { cached } from "@glimmer/tracking"; +import Component from "@glimmer/component"; import { inject as service } from "@ember/service"; import { action } from "@ember/object"; -import Component from "@glimmer/component"; import TagSectionLink from "discourse/lib/sidebar/tags-section/tag-section-link"; +import PMTagSectionLink from "discourse/lib/sidebar/tags-section/pm-tag-section-link"; export default class SidebarTagsSection extends Component { @service router; @service topicTrackingState; + @service pmTopicTrackingState; @service currentUser; constructor() { @@ -17,7 +19,9 @@ export default class SidebarTagsSection extends Component { this.callbackId = this.topicTrackingState.onStateChange(() => { this.sectionLinks.forEach((sectionLink) => { - sectionLink.refreshCounts(); + if (sectionLink.refreshCounts) { + sectionLink.refreshCounts(); + } }); }); } @@ -30,13 +34,22 @@ export default class SidebarTagsSection extends Component { get sectionLinks() { const links = []; - for (const tagName of this.currentUser.sidebarTagNames) { - links.push( - new TagSectionLink({ - tagName, - topicTrackingState: this.topicTrackingState, - }) - ); + for (const tag of this.currentUser.sidebarTags) { + if (tag.pm_only) { + links.push( + new PMTagSectionLink({ + tag, + currentUser: this.currentUser, + }) + ); + } else { + links.push( + new TagSectionLink({ + tag, + topicTrackingState: this.topicTrackingState, + }) + ); + } } return links; diff --git a/app/assets/javascripts/discourse/app/controllers/preferences/sidebar.js b/app/assets/javascripts/discourse/app/controllers/preferences/sidebar.js index 73c840b008a..2b712f0290e 100644 --- a/app/assets/javascripts/discourse/app/controllers/preferences/sidebar.js +++ b/app/assets/javascripts/discourse/app/controllers/preferences/sidebar.js @@ -12,24 +12,29 @@ export default class extends Controller { @action save() { const initialSidebarCategoryIds = this.model.sidebarCategoryIds; - const initialSidebarTagNames = this.model.sidebarTagNames; - - this.model.set("sidebar_tag_names", this.selectedSidebarTagNames); this.model.set( "sidebarCategoryIds", this.selectedSidebarCategories.mapBy("id") ); + this.model.set("sidebar_tag_names", this.selectedSidebarTagNames); + this.model .save() - .then(() => { + .then((result) => { + if (result.user.sidebar_tags) { + this.model.set("sidebar_tags", result.user.sidebar_tags); + } + this.saved = true; }) .catch((error) => { this.model.set("sidebarCategoryIds", initialSidebarCategoryIds); - this.model.set("sidebar_tag_names", initialSidebarTagNames); popupAjaxError(error); + }) + .finally(() => { + this.model.set("sidebar_tag_names", []); }); } } diff --git a/app/assets/javascripts/discourse/app/lib/sidebar/tags-section/pm-tag-section-link.js b/app/assets/javascripts/discourse/app/lib/sidebar/tags-section/pm-tag-section-link.js new file mode 100644 index 00000000000..aec926b6013 --- /dev/null +++ b/app/assets/javascripts/discourse/app/lib/sidebar/tags-section/pm-tag-section-link.js @@ -0,0 +1,22 @@ +export default class PMTagSectionLink { + constructor({ tag, currentUser }) { + this.tag = tag; + this.currentUser = currentUser; + } + + get name() { + return this.tag.name; + } + + get models() { + return [this.currentUser, this.tag.name]; + } + + get route() { + return "userPrivateMessages.tagsShow"; + } + + get text() { + return this.tag.name; + } +} diff --git a/app/assets/javascripts/discourse/app/lib/sidebar/tags-section/tag-section-link.js b/app/assets/javascripts/discourse/app/lib/sidebar/tags-section/tag-section-link.js index fda4b8fa290..dd24165a494 100644 --- a/app/assets/javascripts/discourse/app/lib/sidebar/tags-section/tag-section-link.js +++ b/app/assets/javascripts/discourse/app/lib/sidebar/tags-section/tag-section-link.js @@ -8,8 +8,8 @@ export default class TagSectionLink { @tracked totalUnread = 0; @tracked totalNew = 0; - constructor({ tagName, topicTrackingState }) { - this.tagName = tagName; + constructor({ tag, topicTrackingState }) { + this.tagName = tag.name; this.topicTrackingState = topicTrackingState; this.refreshCounts(); } @@ -31,18 +31,24 @@ export default class TagSectionLink { return this.tagName; } - get model() { - return this.tagName; + get models() { + return [this.tagName]; + } + + get route() { + if (this.totalUnread > 0) { + return "tag.showUnread"; + } else if (this.totalNew > 0) { + return "tag.showNew"; + } else { + return "tag.show"; + } } get currentWhen() { return "tag.show tag.showNew tag.showUnread tag.showTop"; } - get route() { - return "tag.show"; - } - get text() { return this.tagName; } @@ -58,14 +64,4 @@ export default class TagSectionLink { }); } } - - get route() { - if (this.totalUnread > 0) { - return "tag.showUnread"; - } else if (this.totalNew > 0) { - return "tag.showNew"; - } else { - return "tag.show"; - } - } } diff --git a/app/assets/javascripts/discourse/app/models/user.js b/app/assets/javascripts/discourse/app/models/user.js index 99e9eb4d8ca..76c4d828697 100644 --- a/app/assets/javascripts/discourse/app/models/user.js +++ b/app/assets/javascripts/discourse/app/models/user.js @@ -1,7 +1,7 @@ import EmberObject, { computed, get, getProperties } from "@ember/object"; import cookie, { removeCookie } from "discourse/lib/cookie"; import { defaultHomepage, escapeExpression } from "discourse/lib/utilities"; -import { alias, equal, filterBy, gt, or } from "@ember/object/computed"; +import { alias, equal, filterBy, gt, mapBy, or } from "@ember/object/computed"; import getURL, { getURLWithCDN } from "discourse-common/lib/get-url"; import { A } from "@ember/array"; import Badge from "discourse/models/badge"; @@ -314,15 +314,23 @@ const User = RestModel.extend({ sidebarCategoryIds: alias("sidebar_category_ids"), - @discourseComputed("sidebar_tag_names.[]") - sidebarTagNames(sidebarTagNames) { - if (!sidebarTagNames || sidebarTagNames.length === 0) { + @discourseComputed("sidebar_tags.[]") + sidebarTags(sidebarTags) { + if (!sidebarTags || sidebarTags.length === 0) { return []; } - return sidebarTagNames; + if (this.siteSettings.tags_sort_alphabetically) { + return sidebarTags.sort((a, b) => { + return a.name.localeCompare(b); + }); + } else { + return sidebarTags; + } }, + sidebarTagNames: mapBy("sidebarTags", "name"), + @discourseComputed("sidebar_category_ids.[]") sidebarCategories(sidebarCategoryIds) { if (!sidebarCategoryIds || sidebarCategoryIds.length === 0) { @@ -446,6 +454,7 @@ const User = RestModel.extend({ ); User.current().setProperties(userProps); this.setProperties(updatedState); + return result; }) .finally(() => { this.set("isSaving", false); diff --git a/app/assets/javascripts/discourse/app/templates/components/sidebar/tags-section.hbs b/app/assets/javascripts/discourse/app/templates/components/sidebar/tags-section.hbs index 1f94d16fc82..5e9e4193897 100644 --- a/app/assets/javascripts/discourse/app/templates/components/sidebar/tags-section.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/sidebar/tags-section.hbs @@ -16,7 +16,7 @@ @content={{sectionLink.text}} @currentWhen={{sectionLink.currentWhen}} @badgeText={{sectionLink.badgeText}} - @model={{sectionLink.model}}> + @models={{sectionLink.models}} > {{/each}} {{else}} diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-categories-section-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-categories-section-test.js index 7abc849dbb1..9c7c2390b1a 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-categories-section-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sidebar-categories-section-test.js @@ -57,7 +57,7 @@ acceptance( acceptance("Sidebar - Categories Section", function (needs) { needs.user({ sidebar_category_ids: [], - sidebar_tag_names: [], + sidebar_tags: [], }); needs.settings({ diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-tags-section-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-tags-section-test.js index 4d370ea55af..c70e71b0f47 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-tags-section-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sidebar-tags-section-test.js @@ -42,7 +42,18 @@ acceptance("Sidebar - Tags section", function (needs) { tracked_tags: ["tag1"], watched_tags: ["tag2", "tag3"], watching_first_post_tags: [], - sidebar_tag_names: ["tag1", "tag2", "tag3"], + sidebar_tags: [ + { name: "tag1", pm_only: false }, + { name: "tag2", pm_only: false }, + { + name: "tag3", + pm_only: false, + }, + { + name: "tag4", + pm_only: true, + }, + ], }); needs.pretender((server, helper) => { @@ -52,6 +63,20 @@ acceptance("Sidebar - Tags section", function (needs) { }); }); + server.get("/topics/private-messages-tags/:username/:tagId", () => { + const topics = [ + { id: 1, posters: [] }, + { id: 2, posters: [] }, + { id: 3, posters: [] }, + ]; + + return helper.response({ + topic_list: { + topics, + }, + }); + }); + ["latest", "top", "new", "unread"].forEach((type) => { server.get(`/tag/:tagId/l/${type}.json`, () => { return helper.response( @@ -85,7 +110,7 @@ acceptance("Sidebar - Tags section", function (needs) { test("section content when user has not added any tags", async function (assert) { updateCurrentUser({ - sidebar_tag_names: [], + sidebar_tags: [], }); await visit("/"); @@ -106,8 +131,8 @@ acceptance("Sidebar - Tags section", function (needs) { assert.strictEqual( count(".sidebar-section-tags .sidebar-section-link"), - 3, - "3 section links under the section" + 4, + "4 section links under the section" ); assert.strictEqual( @@ -167,6 +192,29 @@ acceptance("Sidebar - Tags section", function (needs) { ); }); + test("private message tag section links for user", async function (assert) { + await visit("/"); + + await click(".sidebar-section-link-tag4"); + + assert.strictEqual( + currentURL(), + "/u/eviltrout/messages/tags/tag4", + "it should transition to user's private message tag4 tag page" + ); + + assert.strictEqual( + count(".sidebar-section-tags .sidebar-section-link.active"), + 1, + "only one link is marked as active" + ); + + assert.ok( + exists(`.sidebar-section-link-tag4.active`), + "the tag4 section link is marked as active" + ); + }); + test("visiting tag discovery top route", async function (assert) { await visit(`/tag/tag1/l/top`); diff --git a/app/assets/javascripts/discourse/tests/acceptance/user-preferences-sidebar-test.js b/app/assets/javascripts/discourse/tests/acceptance/user-preferences-sidebar-test.js index 6a2d3d429e8..c5c2b3aeb4c 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/user-preferences-sidebar-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/user-preferences-sidebar-test.js @@ -12,7 +12,7 @@ import selectKit from "discourse/tests/helpers/select-kit-helper"; acceptance("User Preferences - Sidebar", function (needs) { needs.user({ sidebar_category_ids: [], - sidebar_tag_names: [], + sidebar_tags: [], }); needs.settings({ @@ -39,7 +39,14 @@ acceptance("User Preferences - Sidebar", function (needs) { // This request format will cause an error return helper.response(400, {}); } else { - return helper.response({ user: {} }); + return helper.response({ + user: { + sidebar_tags: [ + { name: "monkey", pm_only: false }, + { name: "gazelle", pm_only: false }, + ], + }, + }); } }); }); @@ -121,7 +128,7 @@ acceptance("User Preferences - Sidebar", function (needs) { }); test("user encountering error when adding tags to sidebar", async function (assert) { - updateCurrentUser({ sidebar_tag_names: ["monkey"] }); + updateCurrentUser({ sidebar_tags: [{ name: "monkey", pm_only: false }] }); await visit("/"); diff --git a/app/serializers/concerns/user_sidebar_tags_mixin.rb b/app/serializers/concerns/user_sidebar_tags_mixin.rb new file mode 100644 index 00000000000..f9ec79fbeea --- /dev/null +++ b/app/serializers/concerns/user_sidebar_tags_mixin.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module UserSidebarTagsMixin + def self.included(base) + base.has_many :sidebar_tags, serializer: Sidebar::TagSerializer, embed: :objects + end + + def include_sidebar_tags? + SiteSetting.enable_experimental_sidebar_hamburger && SiteSetting.tagging_enabled + end +end diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb index a922e4fcaaa..cea7bda1857 100644 --- a/app/serializers/current_user_serializer.rb +++ b/app/serializers/current_user_serializer.rb @@ -2,6 +2,7 @@ class CurrentUserSerializer < BasicUserSerializer include UserTagNotificationsMixin + include UserSidebarTagsMixin attributes :name, :unread_notifications, @@ -75,7 +76,7 @@ class CurrentUserSerializer < BasicUserSerializer :pending_posts_count, :status, :sidebar_category_ids, - :sidebar_tag_names, + :sidebar_tags, :likes_notifications_disabled, :grouped_unread_high_priority_notifications, :redesigned_user_menu_enabled @@ -315,14 +316,6 @@ class CurrentUserSerializer < BasicUserSerializer SiteSetting.enable_experimental_sidebar_hamburger end - def sidebar_tag_names - object.sidebar_tags.pluck(:name) - end - - def include_sidebar_tag_names? - include_sidebar_category_ids? && SiteSetting.tagging_enabled - end - def include_status? SiteSetting.enable_user_status && object.has_status? end diff --git a/app/serializers/sidebar/tag_serializer.rb b/app/serializers/sidebar/tag_serializer.rb new file mode 100644 index 00000000000..3e576cd2474 --- /dev/null +++ b/app/serializers/sidebar/tag_serializer.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Sidebar + class TagSerializer < ::ApplicationSerializer + attributes :name, :pm_only + + def pm_only + object.topic_count == 0 && object.pm_topic_count > 0 + end + end +end diff --git a/app/serializers/user_card_serializer.rb b/app/serializers/user_card_serializer.rb index 3170936174d..a109e9ebbfb 100644 --- a/app/serializers/user_card_serializer.rb +++ b/app/serializers/user_card_serializer.rb @@ -16,7 +16,11 @@ class UserCardSerializer < BasicUserSerializer attributes(*attrs) attrs.each do |attr| define_method "include_#{attr}?" do - can_edit + if defined?(super) + super() && can_edit + else + can_edit + end end end end diff --git a/app/serializers/user_serializer.rb b/app/serializers/user_serializer.rb index 7c0a97378c5..0e9f975032e 100644 --- a/app/serializers/user_serializer.rb +++ b/app/serializers/user_serializer.rb @@ -2,6 +2,7 @@ class UserSerializer < UserCardSerializer include UserTagNotificationsMixin + include UserSidebarTagsMixin attributes :bio_raw, :bio_cooked, @@ -62,7 +63,8 @@ class UserSerializer < UserCardSerializer :user_api_keys, :user_auth_tokens, :user_notification_schedule, - :use_logo_small_as_avatar + :use_logo_small_as_avatar, + :sidebar_tags untrusted_attributes :bio_raw, :bio_cooked, diff --git a/spec/serializers/current_user_serializer_spec.rb b/spec/serializers/current_user_serializer_spec.rb index 7063db3fdce..85ede7e1344 100644 --- a/spec/serializers/current_user_serializer_spec.rb +++ b/spec/serializers/current_user_serializer_spec.rb @@ -220,45 +220,39 @@ RSpec.describe CurrentUserSerializer do end end - describe '#sidebar_tag_names' do + describe '#sidebar_tags' do fab!(:tag_sidebar_section_link) { Fabricate(:tag_sidebar_section_link, user: user) } fab!(:tag_sidebar_section_link_2) { Fabricate(:tag_sidebar_section_link, user: user) } - it "is not included when SiteSeting.enable_experimental_sidebar_hamburger is false" do - SiteSetting.enable_experimental_sidebar_hamburger = false - - json = serializer.as_json - - expect(json[:sidebar_tag_names]).to eq(nil) - end - - it "is not included when SiteSeting.tagging_enabled is false" do - SiteSetting.enable_experimental_sidebar_hamburger = true - SiteSetting.tagging_enabled = false - - json = serializer.as_json - - expect(json[:sidebar_tag_names]).to eq(nil) - end - it "is not included when experimental sidebar has not been enabled" do SiteSetting.enable_experimental_sidebar_hamburger = false SiteSetting.tagging_enabled = true json = serializer.as_json - expect(json[:sidebar_tag_names]).to eq(nil) + expect(json[:sidebar_tags]).to eq(nil) end - it "is present when experimental sidebar has been enabled" do + it "is not included when tagging has not been enabled" do SiteSetting.enable_experimental_sidebar_hamburger = true - SiteSetting.tagging_enabled = true + SiteSetting.tagging_enabled = false json = serializer.as_json - expect(json[:sidebar_tag_names]).to contain_exactly( - tag_sidebar_section_link.linkable.name, - tag_sidebar_section_link_2.linkable.name + expect(json[:sidebar_tags]).to eq(nil) + end + + it "is present when experimental sidebar and tagging has been enabled" do + SiteSetting.enable_experimental_sidebar_hamburger = true + SiteSetting.tagging_enabled = true + + tag_sidebar_section_link_2.linkable.update!(pm_topic_count: 5, topic_count: 0) + + json = serializer.as_json + + expect(json[:sidebar_tags]).to contain_exactly( + { name: tag_sidebar_section_link.linkable.name, pm_only: false }, + { name: tag_sidebar_section_link_2.linkable.name, pm_only: true } ) end end diff --git a/spec/serializers/user_serializer_spec.rb b/spec/serializers/user_serializer_spec.rb index bfe37e15bf0..966d5f225fb 100644 --- a/spec/serializers/user_serializer_spec.rb +++ b/spec/serializers/user_serializer_spec.rb @@ -376,4 +376,52 @@ RSpec.describe UserSerializer do expect(json[:user_api_keys][2][:id]).to eq(user_api_key_2.id) end end + + describe '#sidebar_tags' do + fab!(:tag_sidebar_section_link) { Fabricate(:tag_sidebar_section_link, user: user) } + fab!(:tag_sidebar_section_link_2) { Fabricate(:tag_sidebar_section_link, user: user) } + + context 'when viewing self' do + subject(:json) { UserSerializer.new(user, scope: Guardian.new(user), root: false).as_json } + + it "is not included when SiteSeting.enable_experimental_sidebar_hamburger is false" do + SiteSetting.enable_experimental_sidebar_hamburger = false + SiteSetting.tagging_enabled = true + + expect(json[:sidebar_tags]).to eq(nil) + end + + it "is not included when SiteSeting.tagging_enabled is false" do + SiteSetting.enable_experimental_sidebar_hamburger = true + SiteSetting.tagging_enabled = false + + expect(json[:sidebar_tags]).to eq(nil) + end + + it "is present when experimental sidebar and tagging has been enabled" do + SiteSetting.enable_experimental_sidebar_hamburger = true + SiteSetting.tagging_enabled = true + + tag_sidebar_section_link_2.linkable.update!(pm_topic_count: 5, topic_count: 0) + + expect(json[:sidebar_tags]).to contain_exactly( + { name: tag_sidebar_section_link.linkable.name, pm_only: false }, + { name: tag_sidebar_section_link_2.linkable.name, pm_only: true } + ) + end + end + + context 'when viewing another user' do + fab!(:user2) { Fabricate(:user) } + + subject(:json) { UserSerializer.new(user, scope: Guardian.new(user2), root: false).as_json } + + it "is not present even when experimental sidebar and tagging has been enabled" do + SiteSetting.enable_experimental_sidebar_hamburger = true + SiteSetting.tagging_enabled = true + + expect(json[:sidebar_tags]).to eq(nil) + end + end + end end