From 9d61c19edd4b4fafb0ef62eae148d991079805d8 Mon Sep 17 00:00:00 2001 From: Shalom Karr Date: Wed, 27 May 2026 04:22:18 -0400 Subject: [PATCH 01/46] Merge pull request #1 from Shalom-Karr/fix/notes-feed-seen-race Audience-aware whisper unread + merge mod-note bell + badge targeting --- .../messages_controller.rb | 53 +++--- .../components/mod-note-header-pip.gjs | 178 ------------------ .../components/mod-pm-badge-picker.gjs | 107 +++++++++++ .../components/mod-whisper-target-modal.gjs | 90 ++++++++- .../mod-note-header-pip.gjs | 8 - .../composer-fields/mod-pm-badge-group.gjs | 47 +++++ .../mod-whisper-armed-pill.gjs | 27 ++- .../mod-note-header-indicators.js | 114 ----------- .../discourse/initializers/mod-whisper.js | 40 +++- .../discourse/lib/mod-note-unread-title.js | 25 --- assets/stylesheets/mod-note-header-pip.scss | 34 ---- config/locales/client.en.yml | 15 +- config/routes.rb | 1 + .../guardian_extensions.rb | 13 ++ .../whisper_query_filter.rb | 13 ++ spec/requests/badge_members_endpoint_spec.rb | 54 ++++++ .../mod_note_header_indicators_spec.rb | 149 --------------- spec/requests/mod_note_unread_count_spec.rb | 88 +++++++++ spec/requests/mod_serialization_spec.rb | 23 ++- spec/requests/whisper_badge_targeting_spec.rb | 127 +++++++++++++ spec/requests/whisper_unread_badge_spec.rb | 137 ++++++++++++++ .../mod_note_avatar_badge_visuals_spec.rb | 158 ---------------- .../system/mod_note_header_indicators_spec.rb | 124 ------------ sub_plugins/mod_categories.rb | 168 +++++++++++++++-- 24 files changed, 944 insertions(+), 849 deletions(-) delete mode 100644 assets/javascripts/discourse/components/mod-note-header-pip.gjs create mode 100644 assets/javascripts/discourse/components/mod-pm-badge-picker.gjs delete mode 100644 assets/javascripts/discourse/connectors/before-header-panel/mod-note-header-pip.gjs create mode 100644 assets/javascripts/discourse/connectors/composer-fields/mod-pm-badge-group.gjs delete mode 100644 assets/javascripts/discourse/initializers/mod-note-header-indicators.js delete mode 100644 assets/javascripts/discourse/lib/mod-note-unread-title.js delete mode 100644 assets/stylesheets/mod-note-header-pip.scss create mode 100644 spec/requests/badge_members_endpoint_spec.rb delete mode 100644 spec/requests/mod_note_header_indicators_spec.rb create mode 100644 spec/requests/mod_note_unread_count_spec.rb create mode 100644 spec/requests/whisper_badge_targeting_spec.rb create mode 100644 spec/requests/whisper_unread_badge_spec.rb delete mode 100644 spec/system/mod_note_avatar_badge_visuals_spec.rb delete mode 100644 spec/system/mod_note_header_indicators_spec.rb diff --git a/app/controllers/discourse_mod_categories/messages_controller.rb b/app/controllers/discourse_mod_categories/messages_controller.rb index 34120b8..0c51787 100644 --- a/app/controllers/discourse_mod_categories/messages_controller.rb +++ b/app/controllers/discourse_mod_categories/messages_controller.rb @@ -271,21 +271,37 @@ def notes_feed_seen .update_all(read: true) # Push the recalculated bell counts to every open tab so they refresh - # in lockstep with the shield tab being opened. + # in lockstep with the shield tab being opened. This also drops the + # in-dropdown shield-tab pip, which now derives from unread Notification + # rows (the same source the bell uses), so it stays in lockstep with + # the bell badge without a dedicated MessageBus channel. current_user.publish_notifications_state if marked > 0 - # Reset any other browser tabs/devices the staff member has open so - # their header pip + title prefix clears in lockstep, not on the next - # page load. - MessageBus.publish( - "/mod-note-unread-count/#{current_user.id}", - { reset: true }, - user_ids: [current_user.id], - ) - render json: success_json end + # Resolves a badge id to the current set of usernames who hold it. + # Used by the PM composer "Add badge group" button to splice badge + # holders into the standard `target_recipients` field — the PM is then + # sent through the normal PostCreator path with no further plugin code. + # Self is excluded (no point messaging yourself); the list is deduped. + def badge_members + guardian.ensure_can_send_private_messages! + badge = Badge.find_by(id: params[:badge_id]) + raise Discourse::NotFound unless badge + + usernames = + User + .joins(:user_badges) + .where(user_badges: { badge_id: badge.id }) + .where(active: true) + .where.not(id: current_user.id) + .distinct + .pluck(:username) + + render json: { usernames: usernames, badge: { id: badge.id, name: badge.display_name } } + end + # Adds a user to a topic's cumulative whisper conversation. From then on # that user sees every whisper in the topic (both Guardian#can_see_post? # and the topic-stream SQL filter grant visibility to participants). @@ -387,22 +403,13 @@ def notify_staff_of_note(topic) ) publish_note_alert(staff_user, topic, note, note_url) - publish_unread_count_bump(staff_user) + # The standard /notifications poll picks up the new unread row so + # both the bell dot and the in-dropdown shield-tab pip refresh + # together. No separate /mod-note-unread-count channel is needed. + staff_user.publish_notifications_state end end - # Publishes a small "+1" payload on a dedicated MessageBus channel the - # header pip / title-prefix subscriber listens on. A separate channel - # (independent of `/notification-alert/`) keeps the client-side reactivity - # focused on the moderator-notes counter rather than the bell badge. - def publish_unread_count_bump(staff_user) - MessageBus.publish( - "/mod-note-unread-count/#{staff_user.id}", - { delta: 1 }, - user_ids: [staff_user.id], - ) - end - # Fires the small live notification pop-up for one staff member. The # payload mirrors `PostAlerter.create_notification_alert`, but carries an # explicit `translated_title` so the pop-up text clearly names a diff --git a/assets/javascripts/discourse/components/mod-note-header-pip.gjs b/assets/javascripts/discourse/components/mod-note-header-pip.gjs deleted file mode 100644 index 99caeb0..0000000 --- a/assets/javascripts/discourse/components/mod-note-header-pip.gjs +++ /dev/null @@ -1,178 +0,0 @@ -import Component from "@glimmer/component"; -import { getOwner } from "@ember/owner"; -import { action } from "@ember/object"; -import { service } from "@ember/service"; -import didInsert from "@ember/render-modifiers/modifiers/did-insert"; -import willDestroy from "@ember/render-modifiers/modifiers/will-destroy"; - -// Avatar-overlay indicator of the moderator-notes shield-tab's own unread -// count. Injects a small badge directly onto the current-user avatar in -// the page header, mirroring Discourse's native reviewables badge. -// -// The component itself renders an invisible placeholder; on insert it -// finds the avatar element and appends a `.mod-note-avatar-pip` span to -// it. A `MutationObserver` on `document.body` re-attaches if Discourse -// re-renders the header. The count is held in sync via: -// 1. A classic Ember property observer on the current user. -// 2. A MessageBus subscription on `/mod-note-unread-count/{user_id}`. -// -// `pointer-events: none` on the badge lets clicks pass through to the -// avatar — the user menu opens normally and exposes the shield tab. -export default class ModNoteHeaderPip extends Component { - @service currentUser; - - #onUserChange; - #unsubscribe; - #observer; - #badge; - #unreadCount = 0; - - get #avatarSelectors() { - return [ - ".header-dropdown-toggle.current-user button", - ".header-dropdown-toggle.current-user", - ".header-dropdown-toggle__current-user button", - ".header-dropdown-toggle__current-user", - ]; - } - - #findAvatar() { - for (const sel of this.#avatarSelectors) { - const el = document.querySelector(sel); - if (el) { - return el; - } - } - return null; - } - - #ensureBadge() { - const avatar = this.#findAvatar(); - if (!avatar) { - return null; - } - - // If the existing badge is still in the same avatar, reuse it. - if (this.#badge && this.#badge.parentNode === avatar) { - return this.#badge; - } - - // Clean up any stale badge inside any avatar (e.g. after re-render). - document - .querySelectorAll(".mod-note-avatar-pip") - .forEach((n) => n.remove()); - - const span = document.createElement("span"); - span.className = "mod-note-avatar-pip"; - span.setAttribute("aria-hidden", "true"); - - // Ensure the avatar can host an absolutely-positioned child. - const cs = window.getComputedStyle(avatar); - if (cs.position === "static") { - avatar.style.position = "relative"; - } - - avatar.appendChild(span); - this.#badge = span; - return span; - } - - #renderCount(n) { - this.#unreadCount = Math.max(0, n | 0); - const badge = this.#ensureBadge(); - if (!badge) { - return; - } - if (this.#unreadCount > 0) { - const label = this.#unreadCount > 9 ? "9+" : String(this.#unreadCount); - badge.setAttribute("data-count", label); - badge.classList.add("visible"); - } else { - badge.removeAttribute("data-count"); - badge.classList.remove("visible"); - } - } - - @action - attach() { - if (!this.currentUser?.staff) { - return; - } - - const initial = this.currentUser.mod_note_unread_count || 0; - this.#renderCount(initial); - - this.#onUserChange = () => { - this.#renderCount(this.currentUser?.mod_note_unread_count || 0); - }; - if (typeof this.currentUser.addObserver === "function") { - this.currentUser.addObserver("mod_note_unread_count", this.#onUserChange); - } - - const messageBus = getOwner(this)?.lookup?.("service:message-bus"); - if (messageBus && typeof messageBus.subscribe === "function") { - const channel = `/mod-note-unread-count/${this.currentUser.id}`; - const handler = (payload) => { - if (!payload) { - return; - } - if (payload.reset) { - this.currentUser?.set?.("mod_note_unread_count", 0); - this.#renderCount(0); - return; - } - if (typeof payload.delta === "number") { - const next = Math.max(0, this.#unreadCount + payload.delta); - this.currentUser?.set?.("mod_note_unread_count", next); - this.#renderCount(next); - } - }; - messageBus.subscribe(channel, handler); - this.#unsubscribe = () => { - if (typeof messageBus.unsubscribe === "function") { - messageBus.unsubscribe(channel, handler); - } - }; - } - - // Re-attach the badge if Discourse re-renders the header avatar. - this.#observer = new MutationObserver(() => { - const avatar = this.#findAvatar(); - if (!avatar) { - return; - } - if (!this.#badge || this.#badge.parentNode !== avatar) { - this.#renderCount(this.#unreadCount); - } - }); - this.#observer.observe(document.body, { - childList: true, - subtree: true, - }); - } - - @action - detach() { - if ( - this.#onUserChange && - typeof this.currentUser?.removeObserver === "function" - ) { - this.currentUser.removeObserver( - "mod_note_unread_count", - this.#onUserChange - ); - } - this.#unsubscribe?.(); - this.#observer?.disconnect(); - this.#badge?.remove(); - this.#badge = null; - } - - -} diff --git a/assets/javascripts/discourse/components/mod-pm-badge-picker.gjs b/assets/javascripts/discourse/components/mod-pm-badge-picker.gjs new file mode 100644 index 0000000..ec233da --- /dev/null +++ b/assets/javascripts/discourse/components/mod-pm-badge-picker.gjs @@ -0,0 +1,107 @@ +import Component from "@glimmer/component"; +import { tracked } from "@glimmer/tracking"; +import { hash } from "@ember/helper"; +import { action } from "@ember/object"; +import { service } from "@ember/service"; +import DButton from "discourse/components/d-button"; +import DModal from "discourse/components/d-modal"; +import { ajax } from "discourse/lib/ajax"; +import { popupAjaxError } from "discourse/lib/ajax-error"; +import ComboBox from "select-kit/components/combo-box"; +import { i18n } from "discourse-i18n"; + +// Picks a single badge, fetches the current holders' usernames, and +// splices them into the PM composer's targetRecipients string (deduped, +// comma-joined per Discourse convention). The PM is then sent through the +// normal PostCreator path with that union as recipients — badge-grant +// changes after send do NOT propagate, by design (a PM is a fixed-recipient +// conversation). +export default class ModPmBadgePicker extends Component { + @service store; + + @tracked badgeChoices = []; + @tracked selectedBadgeId = null; + @tracked saving = false; + + constructor() { + super(...arguments); + this.#loadBadges(); + } + + async #loadBadges() { + try { + const list = await this.store.findAll("badge"); + this.badgeChoices = (list?.content || list || []) + .filter((b) => b?.enabled !== false) + .map((b) => ({ id: b.id, name: b.display_name || b.name })); + } catch (_e) { + this.badgeChoices = []; + } + } + + @action + updateBadge(id) { + this.selectedBadgeId = id ? Number(id) : null; + } + + @action + async confirm() { + const composer = this.args.model?.composer; + if (!composer || !this.selectedBadgeId) { + this.args.closeModal(); + return; + } + + this.saving = true; + try { + const data = await ajax( + `/discourse-mod-categories/badge-members/${this.selectedBadgeId}.json` + ); + const newUsernames = Array.isArray(data?.usernames) ? data.usernames : []; + + const current = (composer.targetRecipients || "") + .split(",") + .map((s) => s.trim()) + .filter(Boolean); + const merged = [...new Set([...current, ...newUsernames])]; + composer.set("targetRecipients", merged.join(",")); + + this.args.closeModal(); + } catch (e) { + popupAjaxError(e); + } finally { + this.saving = false; + } + } + + +} diff --git a/assets/javascripts/discourse/components/mod-whisper-target-modal.gjs b/assets/javascripts/discourse/components/mod-whisper-target-modal.gjs index 8cb710e..f31f8d9 100644 --- a/assets/javascripts/discourse/components/mod-whisper-target-modal.gjs +++ b/assets/javascripts/discourse/components/mod-whisper-target-modal.gjs @@ -2,22 +2,34 @@ import Component from "@glimmer/component"; import { tracked } from "@glimmer/tracking"; import { hash } from "@ember/helper"; import { action } from "@ember/object"; +import { service } from "@ember/service"; import DButton from "discourse/components/d-button"; import DModal from "discourse/components/d-modal"; import { ajax } from "discourse/lib/ajax"; import { popupAjaxError } from "discourse/lib/ajax-error"; import EmailGroupUserChooser from "discourse/select-kit/components/email-group-user-chooser"; +import MultiSelect from "select-kit/components/multi-select"; import { i18n } from "discourse-i18n"; // Staff-facing modal (opened from the composer toolbar eye button) for -// picking the users AND groups a whisper reply should be visible to. Writes -// the chosen ids/usernames/group ids/group names/objects onto the composer -// model. The chooser returns a flat array mixing usernames and group names; -// `confirm` resolves each entry to either a user id or a group id. +// picking the users, groups, AND badges a whisper reply should be visible +// to. Writes the chosen ids/usernames/group ids/group names/badge ids/ +// badge names onto the composer model. The user+group chooser returns a +// flat array mixing usernames and group names; `confirm` resolves each +// entry to either a user id or a group id. Badge selection is independent. export default class ModWhisperTargetModal extends Component { + @service store; + @tracked selection = this.#initialSelection(); + @tracked selectedBadgeIds = this.#initialBadgeIds(); + @tracked badgeChoices = []; @tracked saving = false; + constructor() { + super(...arguments); + this.#loadBadges(); + } + #initialSelection() { const composer = this.args.model?.composer; const usernames = Array.isArray(composer?.modWhisperTargetUsernames) @@ -29,11 +41,35 @@ export default class ModWhisperTargetModal extends Component { return [...usernames, ...groupNames]; } + #initialBadgeIds() { + const composer = this.args.model?.composer; + const ids = Array.isArray(composer?.modWhisperTargetBadgeIds) + ? composer.modWhisperTargetBadgeIds + : []; + return ids.map((n) => Number(n)).filter((n) => Number.isInteger(n)); + } + + async #loadBadges() { + try { + const list = await this.store.findAll("badge"); + this.badgeChoices = (list?.content || list || []) + .filter((b) => b?.enabled !== false) + .map((b) => ({ id: b.id, name: b.display_name || b.name })); + } catch (_e) { + this.badgeChoices = []; + } + } + @action updateSelection(names) { this.selection = names; } + @action + updateBadgeSelection(ids) { + this.selectedBadgeIds = (ids || []).map((n) => Number(n)); + } + @action async confirm() { const composer = this.args.model?.composer; @@ -42,7 +78,10 @@ export default class ModWhisperTargetModal extends Component { return; } - if (!this.selection.length) { + const badgeIds = this.selectedBadgeIds.slice(); + const badges = this.badgeChoices.filter((b) => badgeIds.includes(b.id)); + + if (!this.selection.length && !badgeIds.length) { // An empty selection still ARMS a whisper — a staff-only whisper-back. composer.set("modWhisperArmed", true); composer.set("modWhisperTargetUserIds", []); @@ -51,6 +90,23 @@ export default class ModWhisperTargetModal extends Component { composer.set("modWhisperTargetGroupIds", []); composer.set("modWhisperTargetGroupNames", []); composer.set("modWhisperTargetGroups", []); + composer.set("modWhisperTargetBadgeIds", []); + composer.set("modWhisperTargetBadges", []); + this.args.closeModal(); + return; + } + + if (!this.selection.length && badgeIds.length) { + // Badge-only audience — no user or group lookups needed. + composer.set("modWhisperArmed", true); + composer.set("modWhisperTargetUserIds", []); + composer.set("modWhisperTargetUsernames", []); + composer.set("modWhisperTargets", []); + composer.set("modWhisperTargetGroupIds", []); + composer.set("modWhisperTargetGroupNames", []); + composer.set("modWhisperTargetGroups", []); + composer.set("modWhisperTargetBadgeIds", badgeIds); + composer.set("modWhisperTargetBadges", badges); this.args.closeModal(); return; } @@ -119,6 +175,9 @@ export default class ModWhisperTargetModal extends Component { groups.map((g) => ({ id: g.id, name: g.name })) ); + composer.set("modWhisperTargetBadgeIds", badgeIds); + composer.set("modWhisperTargetBadges", badges); + this.args.closeModal(); } catch (e) { popupAjaxError(e); @@ -138,6 +197,8 @@ export default class ModWhisperTargetModal extends Component { composer.set("modWhisperTargetGroupIds", null); composer.set("modWhisperTargetGroupNames", null); composer.set("modWhisperTargetGroups", null); + composer.set("modWhisperTargetBadgeIds", null); + composer.set("modWhisperTargetBadges", null); } this.args.closeModal(); } @@ -161,6 +222,25 @@ export default class ModWhisperTargetModal extends Component { filterPlaceholder="discourse_mod_categories.whisper.search_placeholder" }} /> + + {{#if this.badgeChoices.length}} +

+ {{i18n "discourse_mod_categories.whisper.modal_badge_instructions"}} +

+ + {{/if}} <:footer> 0`, regardless of -// whether the user menu is open. The `before-header-panel-outlet` outlet -// sits inside the header just before the user-menu panel, so the pip lines -// up alongside the existing notification icons. - diff --git a/assets/javascripts/discourse/connectors/composer-fields/mod-pm-badge-group.gjs b/assets/javascripts/discourse/connectors/composer-fields/mod-pm-badge-group.gjs new file mode 100644 index 0000000..48100e6 --- /dev/null +++ b/assets/javascripts/discourse/connectors/composer-fields/mod-pm-badge-group.gjs @@ -0,0 +1,47 @@ +import Component from "@glimmer/component"; +import { action } from "@ember/object"; +import { service } from "@ember/service"; +import DButton from "discourse/components/d-button"; +import ModPmBadgePicker from "../../components/mod-pm-badge-picker"; + +// "Add badge group" button rendered inside the composer-fields outlet +// whenever the composer is in private-message mode. Opens a modal that +// resolves a chosen badge to its current holders' usernames and splices +// them into the standard target_recipients field. From that point the PM +// is sent through Discourse's normal PostCreator path with no further +// plugin code — the audience is the snapshot of holders at send time. +export default class ModPmBadgeGroup extends Component { + @service modal; + + get composer() { + return this.args.outletArgs?.model; + } + + get show() { + const composer = this.composer; + if (!composer) { + return false; + } + return !!composer.privateMessage; + } + + @action + open() { + const composer = this.composer; + if (!composer) { + return; + } + this.modal.show(ModPmBadgePicker, { model: { composer } }); + } + + +} diff --git a/assets/javascripts/discourse/connectors/composer-fields/mod-whisper-armed-pill.gjs b/assets/javascripts/discourse/connectors/composer-fields/mod-whisper-armed-pill.gjs index 1a5d86d..b2434e6 100644 --- a/assets/javascripts/discourse/connectors/composer-fields/mod-whisper-armed-pill.gjs +++ b/assets/javascripts/discourse/connectors/composer-fields/mod-whisper-armed-pill.gjs @@ -31,7 +31,11 @@ export default class ModWhisperArmedPill extends Component { } get staffOnly() { - return this.usernames.length === 0 && this.groupNames.length === 0; + return ( + this.usernames.length === 0 && + this.groupNames.length === 0 && + this.badges.length === 0 + ); } get usernames() { @@ -51,6 +55,18 @@ export default class ModWhisperArmedPill extends Component { return this.usernames.length > 0 || groupIndex > 0; } + get badges() { + const composer = this.composer; + return composer ? get(composer, "modWhisperTargetBadges") || [] : []; + } + + @action + needsBadgeSep(badgeIndex) { + return ( + this.usernames.length > 0 || this.groupNames.length > 0 || badgeIndex > 0 + ); + } + @action clearArmed() { const composer = this.composer; @@ -64,6 +80,8 @@ export default class ModWhisperArmedPill extends Component { composer.set("modWhisperTargetGroupIds", null); composer.set("modWhisperTargetGroupNames", null); composer.set("modWhisperTargetGroups", null); + composer.set("modWhisperTargetBadgeIds", null); + composer.set("modWhisperTargetBadges", null); }