From 81075f7e004511eb9924c81692fddf2357a09e32 Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Wed, 27 May 2026 19:28:12 -0400 Subject: [PATCH 01/12] Clear topic-notifications on open + dedupe whisper/reply duplicates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three related fixes to the bell behavior around whispers and mod notes: 1. Discourse's built-in auto-mark-read covers a hardcoded set of notification types and skips `Notification.types[:custom]`, so the plugin's whisper + mod-note notifications stayed unread in the bell even after the user opened the topic they were about. Adds a new endpoint `POST /discourse-mod-categories/topic/:id/notifications/seen` that marks the current user's custom notifications for that topic read, scoped by data-column markers (`mod_note: true`, `mod_whisper: true`, and the legacy whisper_notification i18n key) so unrelated custom notifications another plugin might attach to the same topic are untouched. A new initializer wires `onPageChange` to ping the endpoint whenever the user navigates to a /t// URL. 2. Mod-note notifications get the same clearing behavior — same endpoint, same trigger — so opening the topic where the mod note lives clears the bell row. 3. When a whisper is posted, PostAlerter (running async in its own sidekiq job) still creates standard :replied / :posted / :quoted / :mentioned notifications for the topic author, watchers, and mentioned users. If any of those are also in our whisper audience, they see two bell rows for the same post. Adds a new Sidekiq job `Jobs::DedupeModWhisperNotifications` that runs 5s after the whisper :post_created — by then PostAlerter has had time to create the duplicates, which we delete for users on our recipient list. The custom whisper notification stays. Also adds a `mod_whisper: true` marker to the on(:post_created) data JSON so the new endpoint can identify these notifications. Specs: topic_notifications_seen_spec covers the endpoint shape + scoping; dedupe_mod_whisper_notifications_spec covers the job's delete-but-keep-our-row behavior. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../messages_controller.rb | 35 ++++++ .../dedupe_mod_whisper_notifications.rb | 46 +++++++ .../mod-topic-notifications-clear.js | 45 +++++++ config/routes.rb | 1 + .../dedupe_mod_whisper_notifications_spec.rb | 81 ++++++++++++ .../requests/topic_notifications_seen_spec.rb | 115 ++++++++++++++++++ sub_plugins/mod_categories.rb | 22 ++++ 7 files changed, 345 insertions(+) create mode 100644 app/jobs/regular/dedupe_mod_whisper_notifications.rb create mode 100644 assets/javascripts/discourse/initializers/mod-topic-notifications-clear.js create mode 100644 spec/jobs/dedupe_mod_whisper_notifications_spec.rb create mode 100644 spec/requests/topic_notifications_seen_spec.rb diff --git a/app/controllers/discourse_mod_categories/messages_controller.rb b/app/controllers/discourse_mod_categories/messages_controller.rb index d4046c6..7fa1698 100644 --- a/app/controllers/discourse_mod_categories/messages_controller.rb +++ b/app/controllers/discourse_mod_categories/messages_controller.rb @@ -206,6 +206,41 @@ def delete_note render json: note_thread_json(topic) end + # Marks the current user's custom mod-note + whisper notifications for + # the given topic as read. Called by the frontend whenever the user + # navigates to a topic page — Discourse's built-in auto-mark-read only + # covers a hardcoded list of notification types and skips + # `Notification.types[:custom]`, so plugin notifications about a topic + # would sit unread in the bell forever even after the user opened the + # topic. The data-column LIKE filter pins this to our notifications + # only (mod_note, mod_whisper, and the legacy whisper_notification + # message key) so unrelated custom notifications another plugin might + # attach to the same topic are left alone. + def mark_topic_notifications_seen + topic = Topic.find_by(id: params[:topic_id]) + raise Discourse::NotFound unless topic + + marked = + ::Notification + .where( + user_id: current_user.id, + topic_id: topic.id, + notification_type: ::Notification.types[:custom], + read: false, + ) + .where( + "data LIKE ? OR data LIKE ? OR data LIKE ?", + '%"mod_note":true%', + '%"mod_whisper":true%', + '%"discourse_mod_categories.whisper.whisper_notification"%', + ) + .update_all(read: true) + + current_user.publish_notifications_state if marked > 0 + + render json: { marked: marked } + end + # Lists recent moderator notes across topics, for the staff user-menu # tab, newest first. def notes_feed diff --git a/app/jobs/regular/dedupe_mod_whisper_notifications.rb b/app/jobs/regular/dedupe_mod_whisper_notifications.rb new file mode 100644 index 0000000..8f51a7f --- /dev/null +++ b/app/jobs/regular/dedupe_mod_whisper_notifications.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +module ::Jobs + # Removes core Discourse notifications (:replied, :posted, :quoted, + # :mentioned) for users who also received a custom mod_whisper + # notification for the same post. Scheduled with a small delay from + # the on(:post_created) hook so PostAlerter has had time to create + # the duplicates — PostAlerter runs in its own Sidekiq job after + # :post_created, so an inline cleanup races it. + # + # Failure mode: if the job runs before PostAlerter (rare — would mean + # PostAlerter is slower than 5s), the duplicates haven't been + # created yet and the delete is a no-op. The duplicates would then + # stay in the bell. Acceptable degradation; the worst case matches + # the pre-fix behavior. + class DedupeModWhisperNotifications < ::Jobs::Base + def execute(args) + post_id = args[:post_id] + recipient_ids = Array(args[:recipient_ids]).map(&:to_i).reject(&:zero?) + return if post_id.blank? || recipient_ids.empty? + + post = ::Post.find_by(id: post_id) + return unless post + + removed = + ::Notification.where( + user_id: recipient_ids, + topic_id: post.topic_id, + post_number: post.post_number, + notification_type: [ + ::Notification.types[:replied], + ::Notification.types[:posted], + ::Notification.types[:quoted], + ::Notification.types[:mentioned], + ], + ).delete_all + + return if removed.zero? + + # Refresh the bell counts for each affected user so the badge in + # the header reflects the decreased unread total without waiting + # for the next /session/current poll. + ::User.where(id: recipient_ids).find_each(&:publish_notifications_state) + end + end +end diff --git a/assets/javascripts/discourse/initializers/mod-topic-notifications-clear.js b/assets/javascripts/discourse/initializers/mod-topic-notifications-clear.js new file mode 100644 index 0000000..242aee9 --- /dev/null +++ b/assets/javascripts/discourse/initializers/mod-topic-notifications-clear.js @@ -0,0 +1,45 @@ +import { withPluginApi } from "discourse/lib/plugin-api"; +import { ajax } from "discourse/lib/ajax"; + +const TOPIC_URL_RE = /^\/t\/[^/]+\/(\d+)(?:\/\d+)?\/?$/; + +// Marks the current user's custom mod-note + mod-whisper notifications +// for the topic they just opened as read. Discourse's built-in +// auto-mark-read only covers a hardcoded list of notification types +// and skips `custom`, so plugin notifications about a topic would sit +// unread in the bell forever even after the user opened the topic. +// +// Triggered on every page-change to a /t//[/] +// URL. The backend filter pins this to OUR custom notifications only, +// so we don't touch notifications another plugin might attach to the +// same topic. +export default { + name: "discourse-mod-topic-notifications-clear", + + initialize() { + withPluginApi("1.0", (api) => { + const currentUser = api.getCurrentUser(); + if (!currentUser) { + return; + } + + api.onPageChange((url) => { + if (typeof url !== "string") { + return; + } + const match = url.match(TOPIC_URL_RE); + if (!match) { + return; + } + const topicId = match[1]; + ajax(`/discourse-mod-categories/topic/${topicId}/notifications/seen`, { + type: "POST", + }).catch(() => { + // Silent — marking-read on topic open is best-effort. A + // failure here just means the notification stays unread, + // which is the pre-fix state. + }); + }); + }); + }, +}; diff --git a/config/routes.rb b/config/routes.rb index ed74c15..573352d 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -35,6 +35,7 @@ get "/badge-members/:badge_id" => "messages#badge_members" get "/notes-feed" => "messages#notes_feed" post "/notes-feed/seen" => "messages#notes_feed_seen" + post "/topic/:topic_id/notifications/seen" => "messages#mark_topic_notifications_seen" get "/checklist" => "checklist#show" get "/checklist/owed" => "checklist#owed" put "/checklist" => "checklist#update" diff --git a/spec/jobs/dedupe_mod_whisper_notifications_spec.rb b/spec/jobs/dedupe_mod_whisper_notifications_spec.rb new file mode 100644 index 0000000..17d9de3 --- /dev/null +++ b/spec/jobs/dedupe_mod_whisper_notifications_spec.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +require "rails_helper" + +# Verifies the dedupe job removes core notifications (:replied, +# :posted, :quoted, :mentioned) for users who also got a custom +# mod_whisper notification for the same post — so a whisper-audience +# member who is also the topic author / a watcher doesn't see two +# bell rows for the same post. +RSpec.describe Jobs::DedupeModWhisperNotifications do + fab!(:topic) + fab!(:post_record) { Fabricate(:post, topic: topic) } + fab!(:audience_user, :user) + fab!(:other_user, :user) + + def make_notification(type:, user:, topic: topic, post_number: post_record.post_number) + Notification.create!( + notification_type: Notification.types[type], + user_id: user.id, + topic_id: topic.id, + post_number: post_number, + data: { topic_title: topic.title }.to_json, + ) + end + + it "deletes :replied / :posted / :quoted / :mentioned for audience recipients" do + replied = make_notification(type: :replied, user: audience_user) + posted = make_notification(type: :posted, user: audience_user) + quoted = make_notification(type: :quoted, user: audience_user) + mentioned = make_notification(type: :mentioned, user: audience_user) + # Sentinel: our custom whisper notification stays. + whisper = + Notification.create!( + notification_type: Notification.types[:custom], + user_id: audience_user.id, + topic_id: topic.id, + post_number: post_record.post_number, + data: { mod_whisper: true }.to_json, + ) + + described_class.new.execute(post_id: post_record.id, recipient_ids: [audience_user.id]) + + expect(Notification.where(id: replied.id)).to be_empty + expect(Notification.where(id: posted.id)).to be_empty + expect(Notification.where(id: quoted.id)).to be_empty + expect(Notification.where(id: mentioned.id)).to be_empty + expect(Notification.where(id: whisper.id)).to exist + end + + it "does not touch notifications for users not in the recipient list" do + other_replied = make_notification(type: :replied, user: other_user) + + described_class.new.execute(post_id: post_record.id, recipient_ids: [audience_user.id]) + + expect(Notification.where(id: other_replied.id)).to exist + end + + it "does not touch notifications for other posts in the topic" do + other_post = Fabricate(:post, topic: topic) + other_post_replied = + make_notification(type: :replied, user: audience_user, post_number: other_post.post_number) + + described_class.new.execute(post_id: post_record.id, recipient_ids: [audience_user.id]) + + expect(Notification.where(id: other_post_replied.id)).to exist + end + + it "is a no-op when the post no longer exists" do + expect { + described_class.new.execute(post_id: 9_999_999, recipient_ids: [audience_user.id]) + }.not_to raise_error + end + + it "is a no-op for an empty recipient list" do + replied = make_notification(type: :replied, user: audience_user) + + described_class.new.execute(post_id: post_record.id, recipient_ids: []) + + expect(Notification.where(id: replied.id)).to exist + end +end diff --git a/spec/requests/topic_notifications_seen_spec.rb b/spec/requests/topic_notifications_seen_spec.rb new file mode 100644 index 0000000..2680d95 --- /dev/null +++ b/spec/requests/topic_notifications_seen_spec.rb @@ -0,0 +1,115 @@ +# frozen_string_literal: true + +require "rails_helper" + +# Verifies the POST /discourse-mod-categories/topic/:topic_id/notifications/seen +# endpoint marks the current user's custom mod-note and mod-whisper +# notifications as read for that topic — Discourse's built-in +# auto-mark-read skips `Notification.types[:custom]`, so without this +# endpoint plugin notifications would stay unread in the bell after the +# user opens the topic. +RSpec.describe "Mark topic notifications seen" do + fab!(:admin) + fab!(:moderator) + fab!(:user) + fab!(:other_topic, :topic) + fab!(:topic) + fab!(:op) { Fabricate(:post, topic: topic, user: user) } + + before do + SiteSetting.mod_categories_enabled = true + SiteSetting.mod_whisper_enabled = true + end + + def make_notification(user:, topic:, data_marker:) + Notification.create!( + notification_type: Notification.types[:custom], + user_id: user.id, + topic_id: topic.id, + post_number: topic.highest_post_number, + data: { topic_title: topic.title }.merge(data_marker).to_json, + ) + end + + it "marks mod-note notifications for the topic as read" do + notif = make_notification(user: admin, topic: topic, data_marker: { mod_note: true }) + sign_in(admin) + + post "/discourse-mod-categories/topic/#{topic.id}/notifications/seen.json" + + expect(response.status).to eq(200) + expect(response.parsed_body["marked"]).to eq(1) + expect(notif.reload.read).to eq(true) + end + + it "marks mod-whisper notifications for the topic as read" do + notif = make_notification(user: admin, topic: topic, data_marker: { mod_whisper: true }) + sign_in(admin) + + post "/discourse-mod-categories/topic/#{topic.id}/notifications/seen.json" + + expect(notif.reload.read).to eq(true) + end + + it "marks legacy whisper_notification rows by the i18n message key" do + notif = + make_notification( + user: admin, + topic: topic, + data_marker: { + message: "discourse_mod_categories.whisper.whisper_notification", + }, + ) + sign_in(admin) + + post "/discourse-mod-categories/topic/#{topic.id}/notifications/seen.json" + + expect(notif.reload.read).to eq(true) + end + + it "does not touch notifications for other topics" do + target_notif = make_notification(user: admin, topic: topic, data_marker: { mod_note: true }) + untouched = make_notification(user: admin, topic: other_topic, data_marker: { mod_note: true }) + sign_in(admin) + + post "/discourse-mod-categories/topic/#{topic.id}/notifications/seen.json" + + expect(target_notif.reload.read).to eq(true) + expect(untouched.reload.read).to eq(false) + end + + it "does not touch other users' notifications for the same topic" do + target_notif = make_notification(user: admin, topic: topic, data_marker: { mod_note: true }) + other_user_notif = + make_notification(user: moderator, topic: topic, data_marker: { mod_note: true }) + sign_in(admin) + + post "/discourse-mod-categories/topic/#{topic.id}/notifications/seen.json" + + expect(target_notif.reload.read).to eq(true) + expect(other_user_notif.reload.read).to eq(false) + end + + it "does not touch unrelated custom notifications attached to the same topic" do + target_notif = make_notification(user: admin, topic: topic, data_marker: { mod_note: true }) + third_party = + make_notification(user: admin, topic: topic, data_marker: { some_other_plugin: true }) + sign_in(admin) + + post "/discourse-mod-categories/topic/#{topic.id}/notifications/seen.json" + + expect(target_notif.reload.read).to eq(true) + expect(third_party.reload.read).to eq(false) + end + + it "returns 404 for a non-existent topic" do + sign_in(admin) + post "/discourse-mod-categories/topic/9999999/notifications/seen.json" + expect(response.status).to eq(404) + end + + it "requires login" do + post "/discourse-mod-categories/topic/#{topic.id}/notifications/seen.json" + expect(response.status).to eq(403).or eq(404) + end +end diff --git a/sub_plugins/mod_categories.rb b/sub_plugins/mod_categories.rb index 11b40cc..97d4718 100644 --- a/sub_plugins/mod_categories.rb +++ b/sub_plugins/mod_categories.rb @@ -688,6 +688,10 @@ class Engine < ::Rails::Engine data = { topic_title: topic&.title, display_username: user&.username, + # Stable marker so MessagesController#mark_topic_notifications_seen + # can scope its read-flip to OUR notifications without touching + # other plugins' custom notifications attached to the same topic. + mod_whisper: true, original_post_id: post.id, original_post_type: post.post_type, }.to_json @@ -701,6 +705,24 @@ class Engine < ::Rails::Engine data: data, ) end + + # Dedupe: PostAlerter runs asynchronously and creates standard + # :replied / :posted / :quoted / :mentioned notifications for the + # topic author, watchers, and mentioned users. If any of those users + # are also in our whisper audience, they see TWO bell rows for the + # same post — one custom whisper from us, one core reply from + # PostAlerter. We schedule a 5-second delayed cleanup that removes + # the core duplicates only for users who got our custom whisper. + # Done as a delayed job because PostAlerter runs in its own Sidekiq + # job after :post_created, so we'd race it if we cleaned up inline. + if topic && post.persisted? + ::Jobs.enqueue_in( + 5.seconds, + :dedupe_mod_whisper_notifications, + post_id: post.id, + recipient_ids: recipient_ids, + ) + end end # Audience-aware ordering on the topic list. The DB column Topic#bumped_at From 8029775382d44b8258f753f778748be83dc30fab Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Wed, 27 May 2026 19:33:50 -0400 Subject: [PATCH 02/12] Add "Viewed by N" pill to mod-note panel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New tracking layer for the mod-note panel. Every staff member who renders the panel on a topic is recorded into a topic custom field `mod_topic_note_viewers` (a JSON array of `{user_id, username, name, avatar_template, viewed_at}` rows). Re-viewing updates `viewed_at` on the existing entry — one row per staff user, no duplicates. Frontend: the component fires a single POST to `/discourse-mod-categories/topic/:id/note-view` from `refreshOnNavigation` (the same hook the scroll-on-hash uses) when the panel mounts on a topic. A small "👁 Viewed by N" pill at the bottom of the panel toggles a popover listing each viewer with their avatar, name, and a relative-time label. The panel pulls the initial viewers list from the topic_view serializer (staff-only `:mod_topic_note_viewers`), then swaps it for the response of the record-view call so the current viewer appears in the pill on first paint without a topic reload. Endpoint: - 404s if the topic has no mod-note set (so a stray ping from a panel-less navigation doesn't seed viewer rows). - Gated on `guardian.ensure_can_manage_mod_messages!` — non-staff hits 403 and the viewers field is left alone. Spec: record_note_view_spec covers idempotent re-view, multi-viewer, non-staff rejection, empty-topic 404. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../messages_controller.rb | 59 ++++++++++ .../discourse/components/mod-private-note.gjs | 102 ++++++++++++++++++ assets/stylesheets/topic-footer-message.scss | 75 +++++++++++++ config/locales/client.en.yml | 3 + config/routes.rb | 1 + spec/requests/record_note_view_spec.rb | 95 ++++++++++++++++ sub_plugins/mod_categories.rb | 27 +++++ 7 files changed, 362 insertions(+) create mode 100644 spec/requests/record_note_view_spec.rb diff --git a/app/controllers/discourse_mod_categories/messages_controller.rb b/app/controllers/discourse_mod_categories/messages_controller.rb index 7fa1698..bc91f0d 100644 --- a/app/controllers/discourse_mod_categories/messages_controller.rb +++ b/app/controllers/discourse_mod_categories/messages_controller.rb @@ -206,6 +206,50 @@ def delete_note render json: note_thread_json(topic) end + # Records the current staff user as a viewer of the mod-note panel on + # the given topic. Idempotent — re-viewing updates `viewed_at` on the + # existing entry rather than appending a duplicate row. The returned + # `viewers` array drives the "👁 Viewed by N" pill at the bottom of + # the panel, refreshed inline without a topic reload. + def record_note_view + topic = Topic.find_by(id: params[:topic_id]) + raise Discourse::NotFound unless topic + + guardian.ensure_can_manage_mod_messages! + + # No-op if there's no note to view — keeps stray refresh-on-mount + # pings from creating viewer rows on topics that never had a note. + note = topic.custom_fields[TOPIC_PRIVATE_NOTE_FIELD].to_s + raise Discourse::NotFound if note.strip.empty? + + now = Time.zone.now.iso8601 + raw = topic.custom_fields[DiscourseModCategories::TOPIC_NOTE_VIEWERS_FIELD] + viewers = raw.is_a?(Array) ? raw.deep_dup : [] + + existing = viewers.find { |v| v["user_id"].to_i == current_user.id } + if existing + existing["viewed_at"] = now + # Refresh denormalized identity fields in case the user renamed / + # changed their avatar since their last view. + existing["username"] = current_user.username + existing["name"] = current_user.name + existing["avatar_template"] = current_user.avatar_template + else + viewers << { + "user_id" => current_user.id, + "username" => current_user.username, + "name" => current_user.name, + "avatar_template" => current_user.avatar_template, + "viewed_at" => now, + } + end + + topic.custom_fields[DiscourseModCategories::TOPIC_NOTE_VIEWERS_FIELD] = viewers + topic.save_custom_fields(true) + + render json: { viewers: serialized_note_viewers(viewers) } + end + # Marks the current user's custom mod-note + whisper notifications for # the given topic as read. Called by the frontend whenever the user # navigates to a topic page — Discourse's built-in auto-mark-read only @@ -590,5 +634,20 @@ def serialized_note_replies(topic) } end end + + # Shape returned by record_note_view — mirrors the :topic_view + # serializer's `mod_topic_note_viewers` so the frontend can swap the + # one for the other without a topic reload. + def serialized_note_viewers(viewers) + Array(viewers).map do |entry| + { + user_id: entry["user_id"], + username: entry["username"], + name: entry["name"], + avatar_template: entry["avatar_template"], + viewed_at: entry["viewed_at"], + } + end + end end end diff --git a/assets/javascripts/discourse/components/mod-private-note.gjs b/assets/javascripts/discourse/components/mod-private-note.gjs index d0cb0a9..67cc226 100644 --- a/assets/javascripts/discourse/components/mod-private-note.gjs +++ b/assets/javascripts/discourse/components/mod-private-note.gjs @@ -77,6 +77,12 @@ export default class ModPrivateNote extends Component { // The id of the reply currently being edited inline, or null. @tracked editingReplyId = null; @tracked editText = ""; + // Staff who have viewed this mod-note panel (each is + // `{user_id, username, name, avatar_template, viewed_at}`). + @tracked viewers = this.args.topic?.mod_topic_note_viewers || []; + // Whether the "👁 Viewed by N" popover is open. Single popover at a + // time per panel — clicking the pill toggles it. + @tracked viewersPopoverOpen = false; constructor() { super(...arguments); @@ -130,9 +136,12 @@ export default class ModPrivateNote extends Component { // Re-read all per-topic state from the current topic. Called on initial // insert and whenever the connector is reused for a different topic. + // Also records the current staff user as a viewer of this panel so + // the "👁 Viewed by N" pill reflects them on the next paint. @action refreshOnNavigation() { this.readTopicState(this.args.topic); + this.recordNoteView(); } readTopicState(topic) { @@ -141,6 +150,8 @@ export default class ModPrivateNote extends Component { this.author = topic?.mod_topic_private_note_author || null; this.createdAt = topic?.mod_topic_private_note_created_at || null; this.replies = topic?.mod_topic_private_note_replies || []; + this.viewers = topic?.mod_topic_note_viewers || []; + this.viewersPopoverOpen = false; this.replying = false; this.replyText = ""; this.editingReplyId = null; @@ -148,6 +159,53 @@ export default class ModPrivateNote extends Component { this.cookContent(); } + // Pings the server to record the current user as a viewer of this + // mod-note panel. Idempotent — re-views update `viewed_at` on the + // existing entry. Fires once per topic navigation via the same + // `didInsert` modifier the scroll-on-hash uses. + @action + async recordNoteView() { + if (!this.visible) { + return; + } + try { + const result = await ajax( + `/discourse-mod-categories/topic/${this.args.topic.id}/note-view`, + { type: "POST" } + ); + this.viewers = result?.viewers || []; + this.args.topic.set("mod_topic_note_viewers", this.viewers); + } catch { + // Best-effort — failing to record a view shouldn't block the + // panel from rendering. The pill just won't update to include + // the current user; the next render will pick it up. + } + } + + @action + toggleViewersPopover() { + this.viewersPopoverOpen = !this.viewersPopoverOpen; + } + + get sortedViewers() { + // Most recent first. + return [...(this.viewers || [])].sort((a, b) => { + const aTime = a?.viewed_at ? Date.parse(a.viewed_at) : 0; + const bTime = b?.viewed_at ? Date.parse(b.viewed_at) : 0; + return bTime - aTime; + }); + } + + get decoratedViewers() { + return this.sortedViewers.map((v) => ({ + userId: v.user_id, + username: v.username, + name: v.name || v.username, + avatarUrl: avatarUrl(v), + agoLabel: timeAgo(v.viewed_at), + })); + } + // Cooks the raw note markdown and each reply body asynchronously. The // stored/edited values stay raw — only the display is cooked. async cookContent() { @@ -535,6 +593,50 @@ export default class ModPrivateNote extends Component { class="btn-flat btn-small mod-private-note-reply-button" /> {{/if}} + + {{#if this.decoratedViewers.length}} +
+ + {{#if this.viewersPopoverOpen}} +
    + {{#each this.decoratedViewers as |viewer|}} +
  • + {{#if viewer.avatarUrl}} + + {{/if}} + + {{viewer.name}} + + {{#if viewer.agoLabel}} + + {{viewer.agoLabel}} + + {{/if}} +
    • + {{/each}} +
+ {{/if}} +
+ {{/if}} {{/if}} diff --git a/assets/stylesheets/topic-footer-message.scss b/assets/stylesheets/topic-footer-message.scss index 45c92bf..ad96837 100644 --- a/assets/stylesheets/topic-footer-message.scss +++ b/assets/stylesheets/topic-footer-message.scss @@ -241,6 +241,81 @@ gap: 0.4em; margin-top: 0.4em; } + + .mod-private-note-viewers { + position: relative; + margin-top: 0.75em; + padding-top: 0.5em; + border-top: 1px solid var(--primary-low); + } + + .mod-private-note-viewers-pill { + display: inline-flex; + align-items: center; + gap: 0.35em; + padding: 0.2em 0.6em; + background: transparent; + border: 1px solid var(--primary-low); + border-radius: 999px; + color: var(--primary-medium); + font-size: var(--font-down-1); + cursor: pointer; + + &:hover { + background: var(--primary-very-low); + color: var(--primary); + } + + .d-icon { + flex: none; + } + } + + .mod-private-note-viewers-list { + position: absolute; + z-index: 5; + bottom: calc(100% + 4px); + left: 0; + margin: 0; + padding: 0.4em 0; + list-style: none; + background: var(--secondary); + border: 1px solid var(--primary-low); + border-radius: var(--d-border-radius, 0.25em); + box-shadow: 0 4px 12px rgba(0, 0, 0, 0.1); + max-height: 14em; + overflow-y: auto; + min-width: 14em; + } + + .mod-private-note-viewers-list-item { + display: flex; + align-items: center; + gap: 0.5em; + padding: 0.3em 0.75em; + font-size: var(--font-down-1); + color: var(--primary); + } + + .mod-private-note-viewers-avatar { + width: 24px; + height: 24px; + border-radius: 50%; + flex-shrink: 0; + } + + .mod-private-note-viewers-name { + flex: 1; + font-weight: 500; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + } + + .mod-private-note-viewers-time { + color: var(--primary-medium); + font-size: var(--font-down-2); + } } .mod-private-note-control { diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index b448a4b..8344b18 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -75,6 +75,9 @@ en: edit_note: Edit note delete_note: Delete note delete_note_confirm: Delete this moderator note and all its replies? + viewed_by: + one: Viewed by %{count} staff + other: Viewed by %{count} staff notes_tab: title: Moderator notes loading: Loading… diff --git a/config/routes.rb b/config/routes.rb index 573352d..42ceff5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -36,6 +36,7 @@ get "/notes-feed" => "messages#notes_feed" post "/notes-feed/seen" => "messages#notes_feed_seen" post "/topic/:topic_id/notifications/seen" => "messages#mark_topic_notifications_seen" + post "/topic/:topic_id/note-view" => "messages#record_note_view" get "/checklist" => "checklist#show" get "/checklist/owed" => "checklist#owed" put "/checklist" => "checklist#update" diff --git a/spec/requests/record_note_view_spec.rb b/spec/requests/record_note_view_spec.rb new file mode 100644 index 0000000..3f4c7ee --- /dev/null +++ b/spec/requests/record_note_view_spec.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +require "rails_helper" + +# Verifies the POST /discourse-mod-categories/topic/:topic_id/note-view +# endpoint: records the current staff user as a viewer of the mod-note +# panel, idempotently (re-views update viewed_at on the existing entry +# rather than appending a duplicate), and gates non-staff out. +RSpec.describe "Record mod-note view" do + fab!(:admin) + fab!(:moderator) + fab!(:other_moderator, :moderator) + fab!(:user) + fab!(:topic) + + before do + SiteSetting.mod_categories_enabled = true + topic.custom_fields[DiscourseModCategories::TOPIC_PRIVATE_NOTE_FIELD] = "Triage in progress." + topic.save_custom_fields(true) + end + + def viewers + Array(topic.reload.custom_fields[DiscourseModCategories::TOPIC_NOTE_VIEWERS_FIELD]) + end + + it "appends the current user when they have not viewed yet" do + sign_in(admin) + + post "/discourse-mod-categories/topic/#{topic.id}/note-view.json" + + expect(response.status).to eq(200) + expect(viewers.map { |v| v["user_id"] }).to contain_exactly(admin.id) + expect(viewers.first["username"]).to eq(admin.username) + expect(viewers.first["viewed_at"]).to be_present + end + + it "updates viewed_at on re-view without duplicating the entry" do + sign_in(admin) + + post "/discourse-mod-categories/topic/#{topic.id}/note-view.json" + first_time = viewers.first["viewed_at"] + + travel 5.seconds do + post "/discourse-mod-categories/topic/#{topic.id}/note-view.json" + end + + expect(viewers.size).to eq(1) + expect(viewers.first["user_id"]).to eq(admin.id) + expect(Time.zone.parse(viewers.first["viewed_at"])).to be > Time.zone.parse(first_time) + end + + it "keeps separate entries per viewer" do + sign_in(admin) + post "/discourse-mod-categories/topic/#{topic.id}/note-view.json" + + sign_in(other_moderator) + post "/discourse-mod-categories/topic/#{topic.id}/note-view.json" + + expect(viewers.map { |v| v["user_id"] }).to contain_exactly(admin.id, other_moderator.id) + end + + it "returns the viewers array in the response" do + sign_in(admin) + + post "/discourse-mod-categories/topic/#{topic.id}/note-view.json" + + expect(response.parsed_body["viewers"]).to be_an(Array) + expect(response.parsed_body["viewers"].first["username"]).to eq(admin.username) + expect(response.parsed_body["viewers"].first["user_id"]).to eq(admin.id) + end + + it "404s when the topic has no mod-note set" do + no_note_topic = Fabricate(:topic) + sign_in(admin) + + post "/discourse-mod-categories/topic/#{no_note_topic.id}/note-view.json" + + expect(response.status).to eq(404) + end + + it "forbids non-staff users from recording a view" do + sign_in(user) + + post "/discourse-mod-categories/topic/#{topic.id}/note-view.json" + + expect(response.status).to eq(403) + expect(viewers).to be_empty + end + + it "404s for a non-existent topic" do + sign_in(admin) + post "/discourse-mod-categories/topic/9999999/note-view.json" + expect(response.status).to eq(404) + end +end diff --git a/sub_plugins/mod_categories.rb b/sub_plugins/mod_categories.rb index 97d4718..fd22baa 100644 --- a/sub_plugins/mod_categories.rb +++ b/sub_plugins/mod_categories.rb @@ -14,6 +14,7 @@ register_svg_icon "pencil" register_svg_icon "trash-can" register_svg_icon "certificate" +register_svg_icon "eye" module ::DiscourseModCategories # Custom-field keys for the moderator-set messages. @@ -246,6 +247,11 @@ def self.targeted_checklists # modifier can sort non-audience users by this value instead of the live # Topic#bumped_at, while audience members keep the actual bump time. TOPIC_NON_WHISPER_BUMPED_AT_FIELD = "mod_non_whisper_bumped_at" + # JSON array of `{user_id, username, name, avatar_template, viewed_at}` + # entries — staff who have rendered the mod-note panel on the topic. + # Used by the "👁 Viewed by N" pill at the bottom of the panel. Re-view + # updates the entry's `viewed_at` in place (one row per user). + TOPIC_NOTE_VIEWERS_FIELD = "mod_topic_note_viewers" MAX_WHISPER_TARGETS = 10 # Explicit boolean armed flag sent by the composer. A boolean survives # form-encoding even when the target id array is empty, so it — not the @@ -320,6 +326,7 @@ class Engine < ::Rails::Engine DiscourseModCategories::TOPIC_NON_WHISPER_BUMPED_AT_FIELD, :string, ) + register_topic_custom_field_type(DiscourseModCategories::TOPIC_NOTE_VIEWERS_FIELD, :json) register_user_custom_field_type(DiscourseModCategories::USER_NOTES_SEEN_FIELD, :string) register_user_custom_field_type(DiscourseModCategories::USER_CHECKLIST_VERSION_FIELD, :integer) register_user_custom_field_type(DiscourseModCategories::USER_TARGETED_CHECKLIST_FIELD, :json) @@ -442,6 +449,26 @@ class Engine < ::Rails::Engine end end + # Staff who have rendered the mod-note panel on this topic — used by the + # "👁 Viewed by N" pill at the bottom of the panel. Newest viewer last, + # so the UI can show the most recent at the top when reversed. + add_to_serializer( + :topic_view, + :mod_topic_note_viewers, + include_condition: -> { scope.is_staff? }, + ) do + raw = object.topic.custom_fields[DiscourseModCategories::TOPIC_NOTE_VIEWERS_FIELD] + Array(raw).map do |entry| + { + user_id: entry["user_id"], + username: entry["username"], + name: entry["name"], + avatar_template: entry["avatar_template"], + viewed_at: entry["viewed_at"], + } + end + end + # Unread moderator-note count, for the staff member's user-menu tab. Derived # from the same unread Notification rows that drive the standard avatar # bell dot, so reading a mod-note from the bell decrements this count and From d9590f4e7c16919da3065c1e2d6af98a95215bea Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Wed, 27 May 2026 19:35:25 -0400 Subject: [PATCH 03/12] Show viewer avatars in the mod-note pill (not just a count) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the "👁 Viewed by N staff" text label with an inline stack of small (20px) avatars — up to 5 shown with a slight horizontal overlap and a ring outline for separation, then a "+N" overflow indicator if more staff have viewed. Each avatar carries `title={{viewer.name}}` so hovering still surfaces the name without opening the popover. The popover (click-to-toggle) still exists for the full list with names + relative-time labels — the pill is now the at-a-glance summary, the popover the drill-down. The `viewed_by` locale key stays — moved to the pill's `aria-label` so screen readers still get the count. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../discourse/components/mod-private-note.gjs | 39 ++++++++++++++++--- assets/stylesheets/topic-footer-message.scss | 31 +++++++++++++-- 2 files changed, 60 insertions(+), 10 deletions(-) diff --git a/assets/javascripts/discourse/components/mod-private-note.gjs b/assets/javascripts/discourse/components/mod-private-note.gjs index 67cc226..2c962e1 100644 --- a/assets/javascripts/discourse/components/mod-private-note.gjs +++ b/assets/javascripts/discourse/components/mod-private-note.gjs @@ -206,6 +206,17 @@ export default class ModPrivateNote extends Component { })); } + // Up to MAX_PILL_AVATARS small avatars rendered inline in the pill. + // The rest go into the "+N" overflow indicator and remain accessible + // via the popover. + get pillViewers() { + return this.decoratedViewers.slice(0, 5); + } + + get overflowCount() { + return Math.max(0, this.decoratedViewers.length - 5); + } + // Cooks the raw note markdown and each reply body asynchronously. The // stored/edited values stay raw — only the display is cooked. async cookContent() { @@ -600,15 +611,31 @@ export default class ModPrivateNote extends Component { type="button" class="mod-private-note-viewers-pill" aria-expanded={{if this.viewersPopoverOpen "true" "false"}} + aria-label={{i18n + "discourse_mod_categories.private_note.viewed_by" + count=this.decoratedViewers.length + }} {{on "click" this.toggleViewersPopover}} > - {{icon "eye"}} - - {{i18n - "discourse_mod_categories.private_note.viewed_by" - count=this.decoratedViewers.length - }} + + {{#each this.pillViewers as |viewer|}} + {{#if viewer.avatarUrl}} + {{viewer.name}} + {{/if}} + {{/each}} + {{#if this.overflowCount}} + + +{{this.overflowCount}} + + {{/if}} {{#if this.viewersPopoverOpen}}
    diff --git a/assets/stylesheets/topic-footer-message.scss b/assets/stylesheets/topic-footer-message.scss index ad96837..0c9b6b0 100644 --- a/assets/stylesheets/topic-footer-message.scss +++ b/assets/stylesheets/topic-footer-message.scss @@ -252,8 +252,8 @@ .mod-private-note-viewers-pill { display: inline-flex; align-items: center; - gap: 0.35em; - padding: 0.2em 0.6em; + gap: 0.4em; + padding: 0.2em 0.5em 0.2em 0.35em; background: transparent; border: 1px solid var(--primary-low); border-radius: 999px; @@ -265,12 +265,35 @@ background: var(--primary-very-low); color: var(--primary); } + } - .d-icon { - flex: none; + .mod-private-note-viewers-pill-avatars { + display: inline-flex; + align-items: center; + // Negative left margin on each avatar past the first stacks them + // with a slight overlap — the classic "presence avatars" pattern. + > .mod-private-note-viewers-pill-avatar + .mod-private-note-viewers-pill-avatar { + margin-left: -0.45em; } } + .mod-private-note-viewers-pill-avatar { + width: 20px; + height: 20px; + border-radius: 50%; + // Ring outline so overlapping avatars are visually separated against + // both the panel background and each other. + box-shadow: 0 0 0 1.5px var(--primary-very-low); + background: var(--primary-very-low); + flex: none; + } + + .mod-private-note-viewers-pill-more { + font-size: var(--font-down-2); + font-weight: 600; + color: var(--primary-medium); + } + .mod-private-note-viewers-list { position: absolute; z-index: 5; From 59fe5fc23695817a3f9e5925e8998f40d8970814 Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Wed, 27 May 2026 19:41:28 -0400 Subject: [PATCH 04/12] Add screenshot scenarios for the "Viewed by" pill + popover MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two new captures for the post-PR-#12 viewer-tracking UI: 16. Mod-note panel rendered with the avatar pill at the bottom — three prior viewers' avatars stacked, plus the signed-in admin's avatar after the record-on-mount POST lands. 17. Same panel with the popover open — full list of viewers with avatars, names, and relative-time labels. Seeded via a helper that pre-fills `mod_topic_note_viewers` with randomized `viewed_at` timestamps so the popover shows a realistic spread of "12m / 23m / 38m ago" labels rather than all the same time. Co-Authored-By: Claude Opus 4.7 (1M context) --- spec/system/feature_screenshots_spec.rb | 57 +++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/spec/system/feature_screenshots_spec.rb b/spec/system/feature_screenshots_spec.rb index dcfcc8b..329f814 100644 --- a/spec/system/feature_screenshots_spec.rb +++ b/spec/system/feature_screenshots_spec.rb @@ -396,4 +396,61 @@ def seed_audience_aware_bump_scenario sleep 0.3 shot("15_whisper_banner_css_sanity") end + + # ────────────────────────────────────────────────────────────────────── + # Post-PR-#12 additions: "Viewed by N" avatar pill at the bottom of + # the mod-note panel + the click-to-open popover with full viewer + # details (avatar, name, relative-time). + # ────────────────────────────────────────────────────────────────────── + + # Seeds a panel with prior viewers (other than the signed-in user) so + # the pill renders multiple avatars on first paint, before the current + # user's own POST-on-mount lands. + def seed_panel_with_viewers(topic, viewers) + topic.custom_fields[DiscourseModCategories::TOPIC_NOTE_VIEWERS_FIELD] = viewers.map do |user| + { + "user_id" => user.id, + "username" => user.username, + "name" => user.name || user.username, + "avatar_template" => user.avatar_template, + "viewed_at" => rand(1..40).minutes.ago.iso8601, + } + end + topic.save_custom_fields(true) + end + + it "16. captures the mod-note panel with the 'Viewed by' avatar pill" do + topic = + seed_topic_with_note( + title: "Mod note viewers pill demo", + note: "Pinned at the bottom — staff who view this panel are stacked below.", + ) + seed_panel_with_viewers(topic, [moderator, other_moderator, author]) + + sign_in(admin) + visit("/t/#{topic.slug}/#{topic.id}") + expect(page).to have_css(".mod-private-note-viewers-pill", wait: 15) + # Each prior viewer's avatar + the current user's after the + # record-on-mount POST resolves. + expect(page).to have_css(".mod-private-note-viewers-pill-avatar", minimum: 3, wait: 10) + sleep 0.3 + shot("16_mod_note_viewers_pill_closed") + end + + it "17. captures the mod-note viewers popover open with the full list" do + topic = + seed_topic_with_note( + title: "Mod note viewers popover demo", + note: "Pinned at the bottom — click the avatar stack to see who viewed.", + ) + seed_panel_with_viewers(topic, [moderator, other_moderator, author, audience_user]) + + sign_in(admin) + visit("/t/#{topic.slug}/#{topic.id}") + expect(page).to have_css(".mod-private-note-viewers-pill", wait: 15) + find(".mod-private-note-viewers-pill").click + expect(page).to have_css(".mod-private-note-viewers-list-item", minimum: 4, wait: 5) + sleep 0.3 + shot("17_mod_note_viewers_popover_open") + end end From 21ab08fd62dd28f48a4ea26d8a1a3e1082e81ccb Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Wed, 27 May 2026 19:51:32 -0400 Subject: [PATCH 05/12] Audience-aware bumped_at on /latest + realistic mod-note screenshots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (1) The /latest Activity column was reading raw `topics.bumped_at`, so a non-audience viewer saw "5m" on a topic whose latest visible activity was actually 1+ hour old (the whisper they can't see was what produced the "5m"). Sort order was already audience-aware via the :topic_query_create_list_topics modifier; the displayed time wasn't. Adds `add_to_serializer(:listable_topic, :bumped_at)` that mirrors the same audience check (staff OR topic participants → raw bumped_at, otherwise the non-whisper bump time from the custom field). Staff and audience members keep the live whisper bump; non-audience users now see a displayed Activity that matches what they can actually see. Regression spec assertion added to whisper_unread_badge_spec under "audience-aware /latest ordering": stranger's bumped_at on /latest.json equals regular_reply.created_at; target's equals topic.bumped_at. (2) Screenshot scenarios 17 and 18 rewritten to show the realistic mod-note panel — 3 staff replies in the thread AND a row of viewer avatars at the bottom — with the popover closed (17) and open (18). The previous scenario 17 only showed the popover without any replies, which doesn't reflect what production looks like. Co-Authored-By: Claude Opus 4.7 (1M context) --- spec/requests/whisper_unread_badge_spec.rb | 17 ++++++ spec/system/feature_screenshots_spec.rb | 71 ++++++++++++++++++++-- sub_plugins/mod_categories.rb | 30 +++++++++ 3 files changed, 113 insertions(+), 5 deletions(-) diff --git a/spec/requests/whisper_unread_badge_spec.rb b/spec/requests/whisper_unread_badge_spec.rb index ff48b7b..c7c97a6 100644 --- a/spec/requests/whisper_unread_badge_spec.rb +++ b/spec/requests/whisper_unread_badge_spec.rb @@ -209,6 +209,23 @@ def latest_topic_ids(as_user) expect(ids.index(public_topic.id)).to be < ids.index(topic.id) end + it "serializes audience-aware bumped_at on /latest (audience sees actual, stranger sees non-whisper)" do + sign_in(stranger) + get "/latest.json" + stranger_view = response.parsed_body["topic_list"]["topics"].find { |t| t["id"] == topic.id } + expect(Time.zone.parse(stranger_view["bumped_at"])).to be_within(2.seconds).of( + regular_reply.reload.created_at, + ) + + sign_in(target) + get "/latest.json" + audience_view = response.parsed_body["topic_list"]["topics"].find { |t| t["id"] == topic.id } + # Audience members still see the live bump (5 min ago via the outer before). + expect(Time.zone.parse(audience_view["bumped_at"])).to be_within(2.seconds).of( + topic.reload.bumped_at, + ) + end + it "skips the timestamp cast when the non_whisper_bumped_at value is malformed" do # The custom field is normally written by on(:post_created) as an # iso8601 string, but a corrupted, hand-edited, or legacy value diff --git a/spec/system/feature_screenshots_spec.rb b/spec/system/feature_screenshots_spec.rb index 329f814..c112b71 100644 --- a/spec/system/feature_screenshots_spec.rb +++ b/spec/system/feature_screenshots_spec.rb @@ -437,20 +437,81 @@ def seed_panel_with_viewers(topic, viewers) shot("16_mod_note_viewers_pill_closed") end - it "17. captures the mod-note viewers popover open with the full list" do + it "17. captures the mod-note panel with both replies AND viewer avatars (realistic)" do + # The realistic case — a thread with multiple staff replies AND a + # row of viewer avatars at the bottom. This is what the production + # forum looks like once a mod note has been triaged: 2-3 replies in + # the conversation, several staff who have laid eyes on it. topic = seed_topic_with_note( - title: "Mod note viewers popover demo", - note: "Pinned at the bottom — click the avatar stack to see who viewed.", + title: "Mod note replies + viewers demo", + note: "Triage on this reported user.", + replies: [ + { + "id" => "viewers-rep-001", + "user_id" => moderator.id, + "raw" => "DM'd them, asking for context on the original post.", + "created_at" => 90.minutes.ago.iso8601, + }, + { + "id" => "viewers-rep-002", + "user_id" => other_moderator.id, + "raw" => "Thanks. I'll watch the next reply they make.", + "created_at" => 60.minutes.ago.iso8601, + }, + { + "id" => "viewers-rep-003", + "user_id" => admin.id, + "raw" => "Looks resolved on my end — closing the loop.", + "created_at" => 25.minutes.ago.iso8601, + }, + ], ) seed_panel_with_viewers(topic, [moderator, other_moderator, author, audience_user]) sign_in(admin) visit("/t/#{topic.slug}/#{topic.id}") + expect(page).to have_css(".mod-private-note-reply", count: 3, wait: 15) + expect(page).to have_css(".mod-private-note-viewers-pill-avatar", minimum: 4, wait: 10) + sleep 0.3 + shot("17_mod_note_replies_and_viewers_closed") + end + + it "18. captures the same panel with replies AND the viewers popover open" do + topic = + seed_topic_with_note( + title: "Mod note replies + viewers popover demo", + note: "Triage on this reported user.", + replies: [ + { + "id" => "viewers-rep-a01", + "user_id" => moderator.id, + "raw" => "DM'd them, asking for context on the original post.", + "created_at" => 90.minutes.ago.iso8601, + }, + { + "id" => "viewers-rep-a02", + "user_id" => other_moderator.id, + "raw" => "Thanks. I'll watch the next reply they make.", + "created_at" => 60.minutes.ago.iso8601, + }, + { + "id" => "viewers-rep-a03", + "user_id" => admin.id, + "raw" => "Looks resolved on my end — closing the loop.", + "created_at" => 25.minutes.ago.iso8601, + }, + ], + ) + seed_panel_with_viewers(topic, [moderator, other_moderator, author, audience_user, stranger]) + + sign_in(admin) + visit("/t/#{topic.slug}/#{topic.id}") + expect(page).to have_css(".mod-private-note-reply", count: 3, wait: 15) expect(page).to have_css(".mod-private-note-viewers-pill", wait: 15) find(".mod-private-note-viewers-pill").click - expect(page).to have_css(".mod-private-note-viewers-list-item", minimum: 4, wait: 5) + expect(page).to have_css(".mod-private-note-viewers-list-item", minimum: 5, wait: 5) sleep 0.3 - shot("17_mod_note_viewers_popover_open") + shot("18_mod_note_replies_and_viewers_popover_open") end end diff --git a/sub_plugins/mod_categories.rb b/sub_plugins/mod_categories.rb index fd22baa..2925c15 100644 --- a/sub_plugins/mod_categories.rb +++ b/sub_plugins/mod_categories.rb @@ -950,4 +950,34 @@ class Engine < ::Rails::Engine visible_max = DiscourseModCategories.whisper_audience_max_post_number(object, scope&.user) visible_max || raw end + + # Audience-aware bumped_at for the topic list's "Activity" column. The + # topic-list query modifier already SORTS non-audience viewers by the + # non-whisper bump time, but the displayed Activity column read the raw + # `topics.bumped_at` and showed e.g. "5m" for a whisper they can't see. + # Mirror the same audience check here so the displayed time matches the + # sort position: audience members (staff + topic participants) see the + # actual bump time; non-audience viewers see the non-whisper bump time + # stored in the custom field. Falls through to raw on missing/malformed + # field values so an upgrade to a topic without the stamp still works. + add_to_serializer(:listable_topic, :bumped_at) do + raw = object.bumped_at + next raw unless SiteSetting.mod_whisper_enabled + + user = scope&.user + next raw if user&.staff? + + participants = object.custom_fields[DiscourseModCategories::TOPIC_WHISPER_PARTICIPANTS_FIELD] + next raw if user && participants.is_a?(Array) && participants.map(&:to_i).include?(user.id) + + nwba = object.custom_fields[DiscourseModCategories::TOPIC_NON_WHISPER_BUMPED_AT_FIELD] + next raw if nwba.blank? + + begin + parsed = ::Time.zone.parse(nwba.to_s) + parsed || raw + rescue StandardError + raw + end + end end From 6931dc7438fecf23f3e6d7b8bc0c1f46df4de9d8 Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Wed, 27 May 2026 20:00:03 -0400 Subject: [PATCH 06/12] Preload custom fields for the audience-aware bumped_at serializer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit's :listable_topic bumped_at override raised HasCustomFields::NotPreloadedError on every /latest request, 500-ing the topic list: Attempted to access the non preloaded custom field 'mod_whisper_participant_ids' on the 'Topic' class. Discourse's PreloadedProxy guard rejects custom-field reads in serializer context unless the field is explicitly registered for preloading on topic lists — the guard exists to prevent N+1 queries. The existing :highest_post_number serializer sidesteps this by querying Post directly (whisper_audience_max_post_number runs a ::Post.where, no custom_fields access), so it never triggered the proxy. The new bumped_at code does need the participants field and the non-whisper bump time, so both fields are now registered with `add_preloaded_topic_list_custom_field`. Also wraps the field accesses in a single rescue that falls through to the raw bumped_at on any error — defense against future Discourse changes that might reshape the preloader or rename the proxy. The worst-case degradation is the pre-fix "stranger sees the whisper time" display, recoverable on the next request. Co-Authored-By: Claude Opus 4.7 (1M context) --- sub_plugins/mod_categories.rb | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/sub_plugins/mod_categories.rb b/sub_plugins/mod_categories.rb index 2925c15..08ecd5a 100644 --- a/sub_plugins/mod_categories.rb +++ b/sub_plugins/mod_categories.rb @@ -327,6 +327,15 @@ class Engine < ::Rails::Engine :string, ) register_topic_custom_field_type(DiscourseModCategories::TOPIC_NOTE_VIEWERS_FIELD, :json) + + # Preload the two custom fields the audience-aware bumped_at serializer + # below reads. Without these, Discourse's HasCustomFields::PreloadedProxy + # raises NotPreloadedError when the serializer touches the fields on a + # topic-list row (the guard exists to prevent N+1 queries — preloading + # is the documented way to declare you intend to use the field for + # every topic on the list). + add_preloaded_topic_list_custom_field(DiscourseModCategories::TOPIC_WHISPER_PARTICIPANTS_FIELD) + add_preloaded_topic_list_custom_field(DiscourseModCategories::TOPIC_NON_WHISPER_BUMPED_AT_FIELD) register_user_custom_field_type(DiscourseModCategories::USER_NOTES_SEEN_FIELD, :string) register_user_custom_field_type(DiscourseModCategories::USER_CHECKLIST_VERSION_FIELD, :integer) register_user_custom_field_type(DiscourseModCategories::USER_TARGETED_CHECKLIST_FIELD, :json) @@ -967,13 +976,20 @@ class Engine < ::Rails::Engine user = scope&.user next raw if user&.staff? - participants = object.custom_fields[DiscourseModCategories::TOPIC_WHISPER_PARTICIPANTS_FIELD] - next raw if user && participants.is_a?(Array) && participants.map(&:to_i).include?(user.id) + # All custom-field access wrapped together: HasCustomFields::PreloadedProxy + # raises NotPreloadedError if `add_preloaded_topic_list_custom_field` + # registrations above haven't taken effect (e.g. early in boot, or + # after a Discourse release reshapes the preloader). Falling through + # to `raw` keeps /latest responsive in that case — the worst outcome + # is the pre-fix "stranger sees the whisper time" display, which is + # recoverable on the next request. + begin + participants = object.custom_fields[DiscourseModCategories::TOPIC_WHISPER_PARTICIPANTS_FIELD] + next raw if user && participants.is_a?(Array) && participants.map(&:to_i).include?(user.id) - nwba = object.custom_fields[DiscourseModCategories::TOPIC_NON_WHISPER_BUMPED_AT_FIELD] - next raw if nwba.blank? + nwba = object.custom_fields[DiscourseModCategories::TOPIC_NON_WHISPER_BUMPED_AT_FIELD] + next raw if nwba.blank? - begin parsed = ::Time.zone.parse(nwba.to_s) parsed || raw rescue StandardError From 5e2c48edf2a08ce34f042effe7acdca474bcb01d Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Wed, 27 May 2026 20:13:37 -0400 Subject: [PATCH 07/12] Allow staff to toggle whisper state on existing posts via PUT endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Discourse's PostsController#update drops whisper params — the plugin's `add_permitted_post_create_param` whitelist is create-only and there's no `serializeOnUpdate` for these fields — so editing a post in the composer and toggling the whisper modal had no effect: the raw saved, the whisper state stayed whatever it was. Adds a dedicated endpoint `PUT /discourse-mod-categories/post/:id/whisper` that takes the same shape as the create-time params: mod_whisper: bool mod_whisper_target_user_ids: [int] mod_whisper_target_group_ids: [int] mod_whisper_target_badge_ids: [int] Arming writes the three custom fields onto the post and merges the new audience members into the topic's cumulative participants list (mirrors what on(:post_created) does so a freshly-targeted user sees all PRIOR whispers in the topic too). Disarming hard-deletes the PostCustomField rows — the `mod_is_whisper` serializer keys off `custom_fields.key?(targets_field)`, so an empty array isn't enough. Authorization: staff-only. A regular user editing their own post gets 403, including the post's own author. A non-staff user couldn't arm a whisper on create — they shouldn't be able to arm/disarm one on edit. Frontend wiring: - model:composer#save is patched to chain a PUT to the new endpoint after a staff edit-save resolves, IF the whisper state was changed in the modal (tracked via a `modWhisperDirty` flag on the composer). - The modal's `confirm` and `clear` actions set the dirty flag at the top so any state change is detected; non-dirty edits skip the PUT. - On success, the response is swapped into the post model so the cooked-element decorator re-evaluates the banner without a reload. Spec coverage (update_post_whisper_spec): * arm + disarm + audience merge into participants * 403 for regular users (including post author) and anonymous * 404 for missing post + when SiteSetting.mod_whisper_enabled is off * empty-audience arm (staff-only whisper-back) Co-Authored-By: Claude Opus 4.7 (1M context) --- .../messages_controller.rb | 98 ++++++++ .../components/mod-whisper-target-modal.gjs | 9 + .../discourse/initializers/mod-whisper.js | 61 +++++ config/routes.rb | 1 + spec/requests/update_post_whisper_spec.rb | 225 ++++++++++++++++++ 5 files changed, 394 insertions(+) create mode 100644 spec/requests/update_post_whisper_spec.rb diff --git a/app/controllers/discourse_mod_categories/messages_controller.rb b/app/controllers/discourse_mod_categories/messages_controller.rb index bc91f0d..2493745 100644 --- a/app/controllers/discourse_mod_categories/messages_controller.rb +++ b/app/controllers/discourse_mod_categories/messages_controller.rb @@ -206,6 +206,76 @@ def delete_note render json: note_thread_json(topic) end + # Toggles or updates a post's whisper state (and audience) after the + # post already exists. Discourse's PostsController#update path drops + # whisper params (`add_permitted_post_create_param` is create-only and + # there's no `serializeOnUpdate` for these fields), so editing a post + # in the composer and saving doesn't propagate whisper toggles. + # The frontend calls this endpoint as a follow-up to any staff edit + # where the whisper state was changed. + # + # Staff-only: non-staff users get 403, including the post's own + # author. A user who didn't have permission to arm a whisper on + # create shouldn't be able to add or remove one on edit. + def update_post_whisper + raise Discourse::NotFound unless SiteSetting.mod_whisper_enabled + + post = ::Post.find_by(id: params[:id]) + raise Discourse::NotFound unless post + raise Discourse::InvalidAccess.new("staff_only") unless current_user.staff? + raise Discourse::InvalidAccess.new("cannot_edit") unless guardian.can_edit?(post) + + armed = ActiveModel::Type::Boolean.new.cast(params[:mod_whisper]) + + targets_field = DiscourseModCategories::POST_WHISPER_TARGETS_FIELD + groups_field = DiscourseModCategories::POST_WHISPER_TARGET_GROUPS_FIELD + badges_field = DiscourseModCategories::POST_WHISPER_TARGET_BADGES_FIELD + participants_field = DiscourseModCategories::TOPIC_WHISPER_PARTICIPANTS_FIELD + + if armed + user_ids = sanitize_ids(params[:mod_whisper_target_user_ids]) + group_ids = sanitize_ids(params[:mod_whisper_target_group_ids]) + badge_ids = sanitize_ids(params[:mod_whisper_target_badge_ids]) + + # Validate IDs against the DB so a typo / stale ID doesn't end up + # in the custom_fields. on(:post_created) does the same shape. + user_ids = ::User.where(id: user_ids).pluck(:id) if user_ids.any? + group_ids = ::Group.where(id: group_ids).pluck(:id) if group_ids.any? + badge_ids = ::Badge.where(id: badge_ids, enabled: true).pluck(:id) if badge_ids.any? + + post.custom_fields[targets_field] = user_ids + post.custom_fields[groups_field] = group_ids + post.custom_fields[badges_field] = badge_ids + post.save_custom_fields(true) + + # Cumulative topic-participants update — mirrors what + # on(:post_created) does so a freshly-targeted user starts seeing + # ALL whispers in the topic, not just future ones. + if post.topic + existing = Array(post.topic.custom_fields[participants_field]).map(&:to_i) + additions = user_ids.dup + additions += ::GroupUser.where(group_id: group_ids).pluck(:user_id) if group_ids.any? + additions += ::UserBadge.where(badge_id: badge_ids).pluck(:user_id) if badge_ids.any? + merged = (existing + additions).uniq + if merged.sort != existing.sort + post.topic.custom_fields[participants_field] = merged + post.topic.save_custom_fields(true) + end + end + else + # Disarming: the `mod_is_whisper` serializer keys off + # `custom_fields.key?(targets_field)`, so an empty array is NOT + # enough — the rows have to be physically removed. + ::PostCustomField.where( + post_id: post.id, + name: [targets_field, groups_field, badges_field], + ).destroy_all + post.reload + end + + render json: serialized_post_whisper_state(post.reload) + end + # Records the current staff user as a viewer of the mod-note panel on # the given topic. Idempotent — re-viewing updates `viewed_at` on the # existing entry rather than appending a duplicate row. The returned @@ -635,6 +705,34 @@ def serialized_note_replies(topic) end end + # Strips/dedupes ID arrays sent by the composer for whisper target + # update. Casts to ints, drops zero/nil, dedupes. Both endpoints + # (update_post_whisper) and the on(:post_created) hook share this + # shape so they normalize identically. + def sanitize_ids(raw) + Array(raw).map(&:to_i).reject(&:zero?).uniq + end + + # Mirrors the post serializer's whisper fields so the frontend can + # swap the response in for the post's local state without a topic + # reload. The four ids* fields match the existing :post serializer + # overrides in sub_plugins/mod_categories.rb. + def serialized_post_whisper_state(post) + targets_field = DiscourseModCategories::POST_WHISPER_TARGETS_FIELD + { + mod_is_whisper: post.custom_fields.key?(targets_field), + mod_whisper_target_user_ids: Array(post.custom_fields[targets_field]).map(&:to_i), + mod_whisper_target_group_ids: + Array(post.custom_fields[DiscourseModCategories::POST_WHISPER_TARGET_GROUPS_FIELD]).map( + &:to_i + ), + mod_whisper_target_badge_ids: + Array(post.custom_fields[DiscourseModCategories::POST_WHISPER_TARGET_BADGES_FIELD]).map( + &:to_i + ), + } + end + # Shape returned by record_note_view — mirrors the :topic_view # serializer's `mod_topic_note_viewers` so the frontend can swap the # one for the other without a topic reload. diff --git a/assets/javascripts/discourse/components/mod-whisper-target-modal.gjs b/assets/javascripts/discourse/components/mod-whisper-target-modal.gjs index f31f8d9..f95a76e 100644 --- a/assets/javascripts/discourse/components/mod-whisper-target-modal.gjs +++ b/assets/javascripts/discourse/components/mod-whisper-target-modal.gjs @@ -78,6 +78,12 @@ export default class ModWhisperTargetModal extends Component { return; } + // Mark the whisper state as dirty so model:composer#save knows to + // chain the update_post_whisper endpoint after an edit-save resolves + // (Discourse's PostsController#update drops whisper params, so an + // edit-only path would otherwise silently fail to toggle state). + composer.set("modWhisperDirty", true); + const badgeIds = this.selectedBadgeIds.slice(); const badges = this.badgeChoices.filter((b) => badgeIds.includes(b.id)); @@ -190,6 +196,9 @@ export default class ModWhisperTargetModal extends Component { clear() { const composer = this.args.model?.composer; if (composer) { + // Disarm is also a whisper-state change — mark dirty so an edit + // save propagates the "remove whisper" intent to the server. + composer.set("modWhisperDirty", true); composer.set("modWhisperArmed", false); composer.set("modWhisperTargetUserIds", null); composer.set("modWhisperTargetUsernames", null); diff --git a/assets/javascripts/discourse/initializers/mod-whisper.js b/assets/javascripts/discourse/initializers/mod-whisper.js index 63ff28f..be7127d 100644 --- a/assets/javascripts/discourse/initializers/mod-whisper.js +++ b/assets/javascripts/discourse/initializers/mod-whisper.js @@ -1,4 +1,6 @@ import { withPluginApi } from "discourse/lib/plugin-api"; +import { ajax } from "discourse/lib/ajax"; +import { popupAjaxError } from "discourse/lib/ajax-error"; import { i18n } from "discourse-i18n"; import ModWhisperTargetModal from "../components/mod-whisper-target-modal"; import { computeReplyAudience } from "../lib/mod-whisper-reply-audience"; @@ -26,6 +28,65 @@ export default { api.modifyClass("model:composer", { pluginId: "discourse-mod-whisper", + + // Discourse's PostsController#update drops whisper params (the + // plugin's `add_permitted_post_create_param` whitelist is + // create-only, and there's no `serializeOnUpdate`), so editing + // a post and changing the whisper state in the modal saves the + // raw but the whisper state stays whatever it was. Hooks the + // composer's save: if this is a STAFF edit AND the whisper + // state was touched in the modal (modWhisperDirty), chain a + // call to the dedicated update_post_whisper endpoint after the + // edit save resolves. Non-staff users never hit this path — + // the modal isn't opened for them in the first place — and the + // server endpoint 403s defensively even if they did. + save() { + const editingPost = this.editingPost; + const post = this.post; + const dirty = this.modWhisperDirty; + const user = api.getCurrentUser(); + const result = this._super(...arguments); + + if (editingPost && dirty && post && user?.staff) { + const state = { + mod_whisper: this.modWhisperArmed, + mod_whisper_target_user_ids: this.modWhisperTargetUserIds || [], + mod_whisper_target_group_ids: this.modWhisperTargetGroupIds || [], + mod_whisper_target_badge_ids: this.modWhisperTargetBadgeIds || [], + }; + Promise.resolve(result) + .then(() => + ajax(`/discourse-mod-categories/post/${post.id}/whisper`, { + type: "PUT", + data: state, + }) + ) + .then((res) => { + this.set("modWhisperDirty", false); + // Push the new state onto the post so the cooked-element + // decorator and the post serializer's mod_is_whisper read + // the same source as the response. + if (post.set) { + post.set("mod_is_whisper", res?.mod_is_whisper); + post.set( + "mod_whisper_target_user_ids", + res?.mod_whisper_target_user_ids || [] + ); + post.set( + "mod_whisper_target_group_ids", + res?.mod_whisper_target_group_ids || [] + ); + post.set( + "mod_whisper_target_badge_ids", + res?.mod_whisper_target_badge_ids || [] + ); + } + }) + .catch(popupAjaxError); + } + + return result; + }, }); api.onToolbarCreate((toolbar) => { diff --git a/config/routes.rb b/config/routes.rb index 42ceff5..2892803 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -37,6 +37,7 @@ post "/notes-feed/seen" => "messages#notes_feed_seen" post "/topic/:topic_id/notifications/seen" => "messages#mark_topic_notifications_seen" post "/topic/:topic_id/note-view" => "messages#record_note_view" + put "/post/:id/whisper" => "messages#update_post_whisper" get "/checklist" => "checklist#show" get "/checklist/owed" => "checklist#owed" put "/checklist" => "checklist#update" diff --git a/spec/requests/update_post_whisper_spec.rb b/spec/requests/update_post_whisper_spec.rb new file mode 100644 index 0000000..93cc349 --- /dev/null +++ b/spec/requests/update_post_whisper_spec.rb @@ -0,0 +1,225 @@ +# frozen_string_literal: true + +require "rails_helper" + +# Verifies the PUT /discourse-mod-categories/post/:id/whisper endpoint — +# the dedicated path for toggling/changing whisper state on an EXISTING +# post (Discourse's PostsController#update drops whisper params because +# `add_permitted_post_create_param` is create-only). +# +# Critical gate: staff-only. A user editing their own post can change the +# raw via the normal update flow, but cannot arm/disarm a whisper — that +# would let a non-staff author whisper-back to add an audience after the +# fact, bypassing the same staff check that gates whisper creation. +RSpec.describe "Update post whisper" do + fab!(:admin) + fab!(:moderator) + fab!(:author, :user) + fab!(:other_user, :user) + fab!(:target, :user) + fab!(:group_member, :user) + fab!(:whisper_group) { Fabricate(:group, name: "whisper_squad") } + fab!(:badge) { Fabricate(:badge, name: "WhisperEditBadge") } + fab!(:badge_holder, :user) + fab!(:topic) + fab!(:post_record) { Fabricate(:post, topic: topic, user: author) } + + let(:targets_field) { DiscourseModCategories::POST_WHISPER_TARGETS_FIELD } + let(:groups_field) { DiscourseModCategories::POST_WHISPER_TARGET_GROUPS_FIELD } + let(:badges_field) { DiscourseModCategories::POST_WHISPER_TARGET_BADGES_FIELD } + let(:participants_field) { DiscourseModCategories::TOPIC_WHISPER_PARTICIPANTS_FIELD } + + before do + SiteSetting.mod_categories_enabled = true + SiteSetting.mod_whisper_enabled = true + whisper_group.add(group_member) + BadgeGranter.grant(badge, badge_holder) + end + + describe "arming a regular post into a whisper" do + it "writes the target user ids onto the post" do + sign_in(moderator) + + put "/discourse-mod-categories/post/#{post_record.id}/whisper.json", + params: { + mod_whisper: true, + mod_whisper_target_user_ids: [target.id], + } + + expect(response.status).to eq(200) + expect(post_record.reload.custom_fields[targets_field]).to eq([target.id]) + end + + it "drops invalid user ids before saving" do + sign_in(moderator) + + put "/discourse-mod-categories/post/#{post_record.id}/whisper.json", + params: { + mod_whisper: true, + mod_whisper_target_user_ids: [target.id, 9_999_999], + } + + expect(post_record.reload.custom_fields[targets_field]).to eq([target.id]) + end + + it "writes group and badge targets onto the post" do + sign_in(moderator) + + put "/discourse-mod-categories/post/#{post_record.id}/whisper.json", + params: { + mod_whisper: true, + mod_whisper_target_group_ids: [whisper_group.id], + mod_whisper_target_badge_ids: [badge.id], + } + + expect(post_record.reload.custom_fields[groups_field]).to eq([whisper_group.id]) + expect(post_record.reload.custom_fields[badges_field]).to eq([badge.id]) + end + + it "adds new audience members to the topic's cumulative participants list" do + topic.custom_fields[participants_field] = [admin.id] + topic.save_custom_fields(true) + sign_in(moderator) + + put "/discourse-mod-categories/post/#{post_record.id}/whisper.json", + params: { + mod_whisper: true, + mod_whisper_target_user_ids: [target.id], + mod_whisper_target_group_ids: [whisper_group.id], + mod_whisper_target_badge_ids: [badge.id], + } + + participants = Array(topic.reload.custom_fields[participants_field]).map(&:to_i) + expect(participants).to include(admin.id, target.id, group_member.id, badge_holder.id) + end + + it "returns the new whisper state in the response body" do + sign_in(moderator) + + put "/discourse-mod-categories/post/#{post_record.id}/whisper.json", + params: { + mod_whisper: true, + mod_whisper_target_user_ids: [target.id], + } + + body = response.parsed_body + expect(body["mod_is_whisper"]).to eq(true) + expect(body["mod_whisper_target_user_ids"]).to eq([target.id]) + end + end + + describe "disarming a whisper back into a regular post" do + before do + post_record.custom_fields[targets_field] = [target.id] + post_record.custom_fields[groups_field] = [whisper_group.id] + post_record.custom_fields[badges_field] = [badge.id] + post_record.save_custom_fields(true) + end + + it "removes all three whisper custom fields" do + sign_in(moderator) + + put "/discourse-mod-categories/post/#{post_record.id}/whisper.json", + params: { + mod_whisper: false, + } + + expect(response.status).to eq(200) + reloaded = post_record.reload + expect(reloaded.custom_fields).not_to have_key(targets_field) + expect(reloaded.custom_fields).not_to have_key(groups_field) + expect(reloaded.custom_fields).not_to have_key(badges_field) + end + + it "reports the post as no longer a whisper in the response" do + sign_in(moderator) + + put "/discourse-mod-categories/post/#{post_record.id}/whisper.json", + params: { + mod_whisper: false, + } + + expect(response.parsed_body["mod_is_whisper"]).to eq(false) + end + end + + describe "authorization" do + it "403s a regular user (even the post's own author)" do + sign_in(author) + + put "/discourse-mod-categories/post/#{post_record.id}/whisper.json", + params: { + mod_whisper: true, + mod_whisper_target_user_ids: [target.id], + } + + expect(response.status).to eq(403) + expect(post_record.reload.custom_fields).not_to have_key(targets_field) + end + + it "403s a different regular user" do + sign_in(other_user) + + put "/discourse-mod-categories/post/#{post_record.id}/whisper.json", + params: { + mod_whisper: true, + } + + expect(response.status).to eq(403) + end + + it "redirects anonymous users to login" do + put "/discourse-mod-categories/post/#{post_record.id}/whisper.json", + params: { + mod_whisper: true, + } + + expect(response.status).to eq(403).or eq(404) + end + + it "lets admins through" do + sign_in(admin) + + put "/discourse-mod-categories/post/#{post_record.id}/whisper.json", + params: { + mod_whisper: true, + mod_whisper_target_user_ids: [target.id], + } + + expect(response.status).to eq(200) + end + end + + describe "edge cases" do + it "404s for a non-existent post" do + sign_in(admin) + put "/discourse-mod-categories/post/9999999/whisper.json", params: { mod_whisper: true } + expect(response.status).to eq(404) + end + + it "404s when mod_whisper_enabled is off" do + SiteSetting.mod_whisper_enabled = false + sign_in(admin) + + put "/discourse-mod-categories/post/#{post_record.id}/whisper.json", + params: { + mod_whisper: true, + } + + expect(response.status).to eq(404) + end + + it "arms with an empty audience (staff-only whisper-back)" do + sign_in(moderator) + + put "/discourse-mod-categories/post/#{post_record.id}/whisper.json", + params: { + mod_whisper: true, + } + + expect(response.status).to eq(200) + expect(post_record.reload.custom_fields[targets_field]).to eq([]) + expect(response.parsed_body["mod_is_whisper"]).to eq(true) + end + end +end From 15452473a1c852f3334a03248a954630d63bba6c Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Thu, 28 May 2026 02:34:20 -0400 Subject: [PATCH 08/12] Hide whisper toolbar button for non-staff + screenshots for the edit flow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (1) Non-staff users used to see the whisper eye button in the composer toolbar but it only had a working behavior for topic-whisper participants — non-participants got a no-op click. The button now short-circuits in `api.onToolbarCreate` when the current user isn't staff, so non-staff toolbars never get the row at all. The auto-arm behavior for non-staff replying to an existing whisper post (the `composer:opened` handler) is unchanged — they still get their reply automatically whispered staff-only, they just don't get a manual UI toggle. Drops the now-dead participant-special-case from the perform handler and the now-unused `whisperParticipantIds` helper. (2) Four screenshot scenarios around the staff edit-to-whisper flow and the non-staff confirmation: 19. Staff editing a regular post — composer open, eye button visible in the toolbar (the "before" state of a regular → whisper switch). 20. Whisper modal open mid-edit (the "during switch" state, ready to confirm a target audience). 21. Post rendered as a whisper after the toggle saved — banner + audience pill visible to the audience member. Seeded directly so the screenshot reliably captures the rendered outcome without a flaky multi-step Capybara confirm/save chain (the modal-open half is proven by scenario 20). 22. Non-staff composer — explicit `have_no_css` assertion that the whisper toolbar button is absent, plus a screenshot for the reviewer artifact. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../discourse/initializers/mod-whisper.js | 45 +++---- spec/system/feature_screenshots_spec.rb | 124 ++++++++++++++++++ 2 files changed, 139 insertions(+), 30 deletions(-) diff --git a/assets/javascripts/discourse/initializers/mod-whisper.js b/assets/javascripts/discourse/initializers/mod-whisper.js index be7127d..e4ead92 100644 --- a/assets/javascripts/discourse/initializers/mod-whisper.js +++ b/assets/javascripts/discourse/initializers/mod-whisper.js @@ -9,11 +9,6 @@ import { computeReplyAudience } from "../lib/mod-whisper-reply-audience"; const EYE_PATH = "M12 5c-7 0-10 7-10 7s3 7 10 7 10-7 10-7-3-7-10-7zm0 11a4 4 0 110-8 4 4 0 010 8z"; -function whisperParticipantIds(topic) { - const ids = topic?.mod_whisper_participant_ids; - return Array.isArray(ids) ? ids : []; -} - export default { name: "discourse-mod-whisper", @@ -90,6 +85,16 @@ export default { }); api.onToolbarCreate((toolbar) => { + // Whisper-arming is staff-only. Non-staff users replying to an + // existing whisper post still get their reply auto-armed as a + // staff-only whisper by the `composer:opened` handler below, so + // they don't lose the ability to whisper-back — they just don't + // get a manual UI toggle. Hiding the toolbar button entirely + // avoids the confusing "eye button that does nothing for me" + // state that non-staff non-participants used to see. + if (!currentUser?.staff) { + return; + } toolbar.addButton({ id: "mod-whisper-target", className: "mod-whisper-target", @@ -99,33 +104,13 @@ export default { perform: () => { const composerService = api.container.lookup("service:composer"); const model = composerService?.model; - if (!model || !currentUser) { + if (!model) { return; } - - if (currentUser.staff) { - const modal = api.container.lookup("service:modal"); - modal?.show(ModWhisperTargetModal, { - model: { composer: model }, - }); - return; - } - - // Non-staff: only an existing topic whisper participant may - // whisper, and only ever staff-only. Arm it directly. - const participantIds = whisperParticipantIds(model.topic); - if (participantIds.includes(currentUser.id)) { - model.set("modWhisperArmed", true); - model.set("modWhisperTargetUserIds", []); - model.set("modWhisperTargetUsernames", []); - model.set("modWhisperTargets", []); - model.set("modWhisperTargetGroupIds", []); - model.set("modWhisperTargetGroupNames", []); - model.set("modWhisperTargetGroups", []); - model.set("modWhisperTargetBadgeIds", []); - model.set("modWhisperTargetBadges", []); - } - // Non-participant: no-op. + const modal = api.container.lookup("service:modal"); + modal?.show(ModWhisperTargetModal, { + model: { composer: model }, + }); }, }); }); diff --git a/spec/system/feature_screenshots_spec.rb b/spec/system/feature_screenshots_spec.rb index c112b71..fd43c5b 100644 --- a/spec/system/feature_screenshots_spec.rb +++ b/spec/system/feature_screenshots_spec.rb @@ -514,4 +514,128 @@ def seed_panel_with_viewers(topic, viewers) sleep 0.3 shot("18_mod_note_replies_and_viewers_popover_open") end + + # ────────────────────────────────────────────────────────────────────── + # Whisper toggle on edit + non-staff toolbar visibility. + # Three paired captures around the staff "switch regular → whisper" + # flow (before / during / after), plus the non-staff confirmation that + # the eye-button is hidden entirely. + # ────────────────────────────────────────────────────────────────────── + + it "19. captures the staff edit composer on a regular post — eye button visible" do + topic = Fabricate(:topic, category: category, title: "Whisper edit toggle demo") + target_post = + Fabricate( + :post, + topic: topic, + user: author, + raw: "Regular public post, about to be toggled to a whisper.", + ) + + sign_in(moderator) + visit("/t/#{topic.slug}/#{topic.id}") + expect(page).to have_css(".topic-post", wait: 15) + + # Open the post's edit composer via the "..." menu → pencil icon. + # Falls back to the keyboard shortcut path if the menu layout shifts + # across Discourse versions. + post_article = find("#post_#{target_post.post_number}") + begin + post_article.find(".show-more-actions", match: :first).click + rescue StandardError + nil + end + post_article.find(".edit", match: :first).click + + expect(page).to have_css(".d-editor-input", wait: 15) + expect(page).to have_css(".d-editor-button-bar button.mod-whisper-target", wait: 10) + sleep 0.3 + shot("19_staff_edit_composer_eye_button_visible") + end + + it "20. captures the whisper modal mid-edit with a target selected (the switch)" do + topic = Fabricate(:topic, category: category, title: "Whisper modal during edit demo") + Fabricate( + :post, + topic: topic, + user: author, + raw: "Public post that's getting whispered to a single staff member.", + ) + + sign_in(moderator) + visit("/t/#{topic.slug}/#{topic.id}") + expect(page).to have_css(".topic-post", wait: 15) + + # Use the topic-footer "Reply" composer to trigger the SAME toolbar + # the edit path uses (simpler than navigating the post menu, and the + # whisper modal is identical either way). + find("#topic-footer-buttons .create", match: :first).click + expect(page).to have_css(".d-editor-input", wait: 15) + + find( + ".d-editor-button-bar button.mod-whisper-target, " \ + ".d-editor-button-bar button[title='" \ + "#{I18n.t("js.discourse_mod_categories.whisper.toolbar_title")}']", + match: :first, + ).click + + expect(page).to have_css(".mod-whisper-target-modal", wait: 15) + shot("20_whisper_modal_during_edit_switch") + end + + it "21. captures a post rendered as a whisper after the switch is saved" do + # The end state — what the post looks like once the staff member + # confirms the modal and the PUT /post/:id/whisper writes the + # custom_fields. Seeding directly bypasses the modal interaction + # (covered by scenario 20) so the screenshot captures the rendered + # outcome reliably without a fragile multi-step Capybara flow. + topic = Fabricate(:topic, category: category, title: "Post rendered as whisper after save") + Fabricate(:post, topic: topic, user: author, raw: "Public OP context.") + whispered = + Fabricate( + :post, + topic: topic, + user: moderator, + raw: "This used to be a regular reply — now it's a staff-only whisper.", + ) + whispered.custom_fields[targets_field] = [audience_user.id] + whispered.save_custom_fields(true) + topic.custom_fields[participants_field] = [audience_user.id] + topic.save_custom_fields(true) + # Mirror the on(:post_created) rollback so the topic's highest_post + # also matches the post-save audience-aware state for the viewer. + non_whisper_max = + Post + .where(topic_id: topic.id, deleted_at: nil) + .where.not(id: PostCustomField.where(name: targets_field).select(:post_id)) + .maximum(:post_number) + Topic.where(id: topic.id).update_all(highest_post_number: non_whisper_max) if non_whisper_max + + sign_in(audience_user) + visit("/t/#{topic.slug}/#{topic.id}") + expect(page).to have_css(".mod-whisper-banner", wait: 15) + sleep 0.3 + shot("21_post_rendered_as_whisper_after_save") + end + + it "22. captures the non-staff composer with NO whisper eye button" do + topic = Fabricate(:topic, category: category, title: "Non-staff has no whisper button") + Fabricate( + :post, + topic: topic, + user: author, + raw: "Public reply chain — non-staff users shouldn't see the whisper toggle.", + ) + + sign_in(stranger) + visit("/t/#{topic.slug}/#{topic.id}") + expect(page).to have_css(".topic-post", wait: 15) + find("#topic-footer-buttons .create", match: :first).click + expect(page).to have_css(".d-editor-input", wait: 15) + # The button is registered conditionally on staff in + # mod-whisper.js — non-staff toolbars never get the row. + expect(page).to have_no_css(".d-editor-button-bar button.mod-whisper-target") + sleep 0.3 + shot("22_non_staff_composer_no_whisper_button") + end end From 0d02b95f7361f397ff90676196499a0821344dce Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Thu, 28 May 2026 03:10:35 -0400 Subject: [PATCH 09/12] Add end-to-end Capybara coverage for the whisper edit toggle chain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The frontend `model:composer#save` patch in mod-whisper.js was the single unproven piece — no Ruby spec could catch a regression where the toolbar click → modal confirm → save edit flow fails to fire `PUT /post/:id/whisper` (the patched override is the only thing that chains the call after the composer's save resolves). Three scenarios: 1. Regular post → confirm modal (empty audience, staff-only whisper-back) → save → assert post now has the whisper custom fields. Proves the arm chain. 2. Whisper post → Clear modal → save → assert the three whisper custom_fields are gone. Proves the disarm chain. 3. Edit raw WITHOUT opening the modal → save → assert no whisper fields written. Proves the `if (dirty)` guard works — non-toggle edits don't accidentally fire the PUT. System specs are flakier than request specs but this is the only shape that exercises the actual browser interaction with the modal, the dirty-flag tracking, and the save promise chain together. Co-Authored-By: Claude Opus 4.7 (1M context) --- spec/system/whisper_edit_toggle_spec.rb | 165 ++++++++++++++++++++++++ 1 file changed, 165 insertions(+) create mode 100644 spec/system/whisper_edit_toggle_spec.rb diff --git a/spec/system/whisper_edit_toggle_spec.rb b/spec/system/whisper_edit_toggle_spec.rb new file mode 100644 index 0000000..7914cea --- /dev/null +++ b/spec/system/whisper_edit_toggle_spec.rb @@ -0,0 +1,165 @@ +# frozen_string_literal: true + +require "rails_helper" + +# End-to-end Capybara coverage for the staff whisper-toggle-on-edit +# flow. The unit pieces are covered by other specs: +# +# * `update_post_whisper_spec` covers the server endpoint's contract +# (arm/disarm/authz/edge cases). +# * `feature_screenshots_spec` captures the visual states (composer +# open, modal open, post-rendered-as-whisper, non-staff no-button). +# +# What was missing — and what this spec covers — is the FRONTEND +# CHAIN: that the `model:composer#save` patch in mod-whisper.js +# actually fires the PUT after a staff edit save when the modal +# marked the state dirty. Without this, the modal could open, the +# composer could save, and the whisper toggle would silently drop on +# the floor with no Ruby spec able to catch it. +RSpec.describe "Whisper edit toggle (frontend save chain)" do + fab!(:moderator) + fab!(:author, :user) + fab!(:target, :user) + fab!(:topic) { Fabricate(:topic, title: "Edit whisper toggle e2e demo") } + + let(:targets_field) { DiscourseModCategories::POST_WHISPER_TARGETS_FIELD } + let(:groups_field) { DiscourseModCategories::POST_WHISPER_TARGET_GROUPS_FIELD } + let(:badges_field) { DiscourseModCategories::POST_WHISPER_TARGET_BADGES_FIELD } + let(:armed_param) { DiscourseModCategories::POST_WHISPER_ARMED_PARAM } + + before do + SiteSetting.mod_categories_enabled = true + SiteSetting.mod_whisper_enabled = true + SiteSetting.min_post_length = 5 + SiteSetting.body_min_entropy = 1 + Group.refresh_automatic_groups! + end + + # Click the post's edit pencil. The "..." menu may need to be opened + # first on some Discourse releases; both paths are tried. + def open_edit_composer(post) + article = find("#post_#{post.post_number}") + begin + article.find(".show-more-actions", match: :first).click + rescue Capybara::ElementNotFound + # Already-expanded post action row; no need to open the "..." menu. + end + article.find(".edit", match: :first).click + expect(page).to have_css(".d-editor-input", wait: 15) + end + + def click_whisper_toolbar_button + find( + ".d-editor-button-bar button.mod-whisper-target, " \ + ".d-editor-button-bar button[title='" \ + "#{I18n.t("js.discourse_mod_categories.whisper.toolbar_title")}']", + match: :first, + ).click + expect(page).to have_css(".mod-whisper-target-modal", wait: 10) + end + + it "staff edit on a regular post → confirm whisper modal → save → post becomes a whisper" do + regular_post = + Fabricate( + :post, + topic: topic, + user: author, + raw: "Regular public post, about to be toggled to a whisper.", + ) + expect(regular_post.custom_fields).not_to have_key(targets_field) + + sign_in(moderator) + visit("/t/#{topic.slug}/#{topic.id}") + expect(page).to have_css(".topic-post", wait: 15) + + open_edit_composer(regular_post) + click_whisper_toolbar_button + + # Confirm with an empty audience — staff-only whisper-back. The + # modal's `confirm()` sets `modWhisperDirty = true` and arms the + # composer, which is the state the save patch keys off. + within(".mod-whisper-target-modal") { find(".btn-primary.mod-whisper-confirm").click } + expect(page).to have_no_css(".mod-whisper-target-modal", wait: 5) + + # Save the edit. The composer's `save()` resolves, then the + # patched override chains the PUT to update_post_whisper. + find(".save-edits", match: :first).click + expect(page).to have_no_css(".d-editor-input", wait: 15) + + # Wait for the chained PUT to land. The composer's save promise + # resolves before the PUT, so a fixed delay covers the race + # without relying on a DOM signal that may not exist. + sleep 2 + + reloaded = regular_post.reload + expect(reloaded.custom_fields).to have_key(targets_field) + end + + it "staff edit on a whisper → clear modal → save → post becomes regular" do + whispered = + Fabricate( + :post, + topic: topic, + user: author, + raw: "Whispered post, about to be toggled BACK to regular.", + ) + whispered.custom_fields[targets_field] = [target.id] + whispered.save_custom_fields(true) + expect(whispered.reload.custom_fields).to have_key(targets_field) + + sign_in(moderator) + visit("/t/#{topic.slug}/#{topic.id}") + expect(page).to have_css(".topic-post", wait: 15) + + open_edit_composer(whispered) + click_whisper_toolbar_button + + # The modal's Clear button — disarms and closes. Same dirty-flag + # mechanism as confirm, but with `modWhisperArmed = false`. + within(".mod-whisper-target-modal") do + find(".btn-flat", text: I18n.t("js.discourse_mod_categories.whisper.clear")).click + end + expect(page).to have_no_css(".mod-whisper-target-modal", wait: 5) + + find(".save-edits", match: :first).click + expect(page).to have_no_css(".d-editor-input", wait: 15) + sleep 2 + + reloaded = whispered.reload + expect(reloaded.custom_fields).not_to have_key(targets_field) + expect(reloaded.custom_fields).not_to have_key(groups_field) + expect(reloaded.custom_fields).not_to have_key(badges_field) + end + + it "edit without opening the whisper modal does NOT fire the PUT (dirty flag stays false)" do + regular_post = + Fabricate( + :post, + topic: topic, + user: author, + raw: "Regular public post — staff is just fixing a typo.", + ) + + sign_in(moderator) + visit("/t/#{topic.slug}/#{topic.id}") + expect(page).to have_css(".topic-post", wait: 15) + + open_edit_composer(regular_post) + # Just edit the raw, don't touch the whisper modal. + page.execute_script(<<~JS) + const editor = document.querySelector('.d-editor-input'); + if (editor) { + editor.value = 'Fixed a typo in the regular post.'; + editor.dispatchEvent(new Event('input', { bubbles: true })); + } + JS + + find(".save-edits", match: :first).click + expect(page).to have_no_css(".d-editor-input", wait: 15) + sleep 2 + + # The save chain's `if (editingPost && dirty && ...)` skipped — the + # post should still be a regular post, no whisper rows. + expect(regular_post.reload.custom_fields).not_to have_key(targets_field) + end +end From 6f89c7e2d02b239c3115dc32bdb9e16a72262754 Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Thu, 28 May 2026 03:22:50 -0400 Subject: [PATCH 10/12] Add workflow_dispatch trigger to Discourse Plugin workflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The upstream PR's checks (`backend_tests`, `system_tests`, `linting`, `annotations_tests`) are gated on a manual workflow approval for fork-based PRs at JTech-Forums — so the request specs and the new end-to-end whisper-edit-toggle system spec can't validate themselves until an org admin clicks "Approve and run workflows" on the PR's Actions tab. Adding workflow_dispatch to the fork's caller workflow lets us fire the same reusable workflow manually against any branch with: gh workflow run "Discourse Plugin" --ref No org approval needed — it runs in the fork's own Actions environment. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/discourse-plugin.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/discourse-plugin.yml b/.github/workflows/discourse-plugin.yml index f5cf62e..92501f5 100644 --- a/.github/workflows/discourse-plugin.yml +++ b/.github/workflows/discourse-plugin.yml @@ -5,6 +5,12 @@ on: branches: - main pull_request: + # Manual trigger so we can run the full Discourse plugin matrix + # (linting, backend_tests, system_tests, annotations_tests) against + # an arbitrary branch on this fork without needing an upstream PR + # (which is gated on a manual workflow approval for fork + # contributors). `gh workflow run "Discourse Plugin" --ref `. + workflow_dispatch: jobs: ci: From 454d0f3499bcd686b54a42dfc0ecfc3776a0f950 Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Thu, 28 May 2026 03:41:24 -0400 Subject: [PATCH 11/12] Fix CI failures: lowercase plugin dir, circular defaults, hidden button tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five issues from the first Discourse Plugin workflow run on the fork: 1. system_tests / "Refused to apply style from JtechTools_*.css": Same SCSS-route bug `b284c8d` fixed for local dev. The reusable workflow defaults the plugin dir name to the repo name (uppercase "JtechTools"), Discourse's stylesheet route only matches lowercase `[-a-z0-9_]+`, so every CSS request 404 → text/html → browser rejected. Pass `name: jtech-tools` to the reusable workflow. 2. linting + backend_tests / `topic: topic` circular default arg in `make_notification` in dedupe_mod_whisper_notifications_spec.rb: Ruby 3.3 strict parser rejects, and at runtime the param defaulted to nil → `undefined method 'id' for nil`. Removed the redundant keyword arg — the outer `topic` let is in scope inside the method. 3. backend_tests / record_note_view_spec "updates viewed_at" — used `travel` (ActiveSupport::Testing::TimeHelpers) which isn't included by Discourse's rails_helper. Switched to `freeze_time` which is. 4. system_tests / whisper_edit_toggle_spec couldn't find `.save-edits` button. Discourse uses `.create.btn-primary` for both reply and edit composers (label differs via i18n); the legacy `.save-edits` class is version-dependent. Now matches either. 5. system_tests / whisper_spec "arms whisper-back from toolbar" and "eye button is a no-op for a non-participant" — both relied on the non-staff toolbar button. That button is now hidden for ALL non- staff per the design ask. Rewrote both tests to assert the button is absent rather than that clicking it does something specific. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/discourse-plugin.yml | 10 +++++ .../dedupe_mod_whisper_notifications_spec.rb | 7 +++- spec/requests/record_note_view_spec.rb | 13 ++++-- spec/system/whisper_edit_toggle_spec.rb | 18 ++++++-- spec/system/whisper_spec.rb | 41 +++++++------------ 5 files changed, 54 insertions(+), 35 deletions(-) diff --git a/.github/workflows/discourse-plugin.yml b/.github/workflows/discourse-plugin.yml index 92501f5..3f57035 100644 --- a/.github/workflows/discourse-plugin.yml +++ b/.github/workflows/discourse-plugin.yml @@ -15,3 +15,13 @@ on: jobs: ci: uses: discourse/.github/.github/workflows/discourse-plugin.yml@v1 + # Force lowercase plugin directory. Discourse derives the compiled + # stylesheet bundle slug from the on-disk dir, and the route that + # serves /stylesheets/_.css is constrained to + # `[-a-z0-9_]+`. The default (`github.event.repository.name`) + # evaluates to "JtechTools" (uppercase) which broke every CSS + # request → "Refused to apply style" → unstyled plugin everywhere. + # Same fix b284c8d applied for local dev; missing here is why the + # SCSS bug came back in this PR's system_tests. + with: + name: jtech-tools diff --git a/spec/jobs/dedupe_mod_whisper_notifications_spec.rb b/spec/jobs/dedupe_mod_whisper_notifications_spec.rb index 17d9de3..a8d38cf 100644 --- a/spec/jobs/dedupe_mod_whisper_notifications_spec.rb +++ b/spec/jobs/dedupe_mod_whisper_notifications_spec.rb @@ -13,12 +13,15 @@ fab!(:audience_user, :user) fab!(:other_user, :user) - def make_notification(type:, user:, topic: topic, post_number: post_record.post_number) + def make_notification(type:, user:, post_number: nil) + # The outer `topic` let is in scope inside this method via RSpec's + # method-dispatch — no circular keyword-arg default needed (and + # `topic: topic` is a Ruby 3.3 syntax error besides). Notification.create!( notification_type: Notification.types[type], user_id: user.id, topic_id: topic.id, - post_number: post_number, + post_number: post_number || post_record.post_number, data: { topic_title: topic.title }.to_json, ) end diff --git a/spec/requests/record_note_view_spec.rb b/spec/requests/record_note_view_spec.rb index 3f4c7ee..acafc19 100644 --- a/spec/requests/record_note_view_spec.rb +++ b/spec/requests/record_note_view_spec.rb @@ -35,14 +35,19 @@ def viewers end it "updates viewed_at on re-view without duplicating the entry" do + # `travel` (ActiveSupport::Testing::TimeHelpers) isn't auto-included + # by Discourse's rails_helper, so we use freeze_time which IS + # available there. Two POSTs at deterministically-different times + # let us assert both the no-duplicate semantic AND the timestamp + # progression. sign_in(admin) - post "/discourse-mod-categories/topic/#{topic.id}/note-view.json" - first_time = viewers.first["viewed_at"] - - travel 5.seconds do + freeze_time(30.minutes.ago) do post "/discourse-mod-categories/topic/#{topic.id}/note-view.json" end + first_time = viewers.first["viewed_at"] + + freeze_time(Time.zone.now) { post "/discourse-mod-categories/topic/#{topic.id}/note-view.json" } expect(viewers.size).to eq(1) expect(viewers.first["user_id"]).to eq(admin.id) diff --git a/spec/system/whisper_edit_toggle_spec.rb b/spec/system/whisper_edit_toggle_spec.rb index 7914cea..a58839c 100644 --- a/spec/system/whisper_edit_toggle_spec.rb +++ b/spec/system/whisper_edit_toggle_spec.rb @@ -83,7 +83,11 @@ def click_whisper_toolbar_button # Save the edit. The composer's `save()` resolves, then the # patched override chains the PUT to update_post_whisper. - find(".save-edits", match: :first).click + # Discourse's edit composer uses the same `.create.btn-primary` as the + # reply composer with a different label; the legacy `.save-edits` + # class may or may not be present depending on Discourse version, so + # match either to stay version-tolerant. + find(".save-edits, #reply-control .create.btn-primary", match: :first).click expect(page).to have_no_css(".d-editor-input", wait: 15) # Wait for the chained PUT to land. The composer's save promise @@ -121,7 +125,11 @@ def click_whisper_toolbar_button end expect(page).to have_no_css(".mod-whisper-target-modal", wait: 5) - find(".save-edits", match: :first).click + # Discourse's edit composer uses the same `.create.btn-primary` as the + # reply composer with a different label; the legacy `.save-edits` + # class may or may not be present depending on Discourse version, so + # match either to stay version-tolerant. + find(".save-edits, #reply-control .create.btn-primary", match: :first).click expect(page).to have_no_css(".d-editor-input", wait: 15) sleep 2 @@ -154,7 +162,11 @@ def click_whisper_toolbar_button } JS - find(".save-edits", match: :first).click + # Discourse's edit composer uses the same `.create.btn-primary` as the + # reply composer with a different label; the legacy `.save-edits` + # class may or may not be present depending on Discourse version, so + # match either to stay version-tolerant. + find(".save-edits, #reply-control .create.btn-primary", match: :first).click expect(page).to have_no_css(".d-editor-input", wait: 15) sleep 2 diff --git a/spec/system/whisper_spec.rb b/spec/system/whisper_spec.rb index a671e40..cc71755 100644 --- a/spec/system/whisper_spec.rb +++ b/spec/system/whisper_spec.rb @@ -259,48 +259,37 @@ def make_group_whisper_post(group_ids, raw: "A staff whisper for a group.") context "a topic participant whispers back" do before { make_whisper_post([recipient.id]) } - it "arms a staff-only whisper-back from the toolbar" do + it "does NOT show the whisper toolbar button to a non-staff participant" do + # Previously the participant could click the eye button to arm a + # staff-only whisper-back from the topic-level reply composer. + # That UI was removed: whisper-arming is staff-only at the toolbar + # level. Non-staff who hit Reply directly on a whisper POST still + # get their reply auto-armed by the `composer:opened` handler in + # mod-whisper.js — they just don't get a manual toggle from the + # topic-level reply composer. sign_in(recipient) visit("/t/#{topic.slug}/#{topic.id}") expect(page).to have_css(".mod-whisper-banner", wait: 15) open_reply_composer - whisper_toolbar_button.click - - expect(page).to have_css(".mod-whisper-armed-pill", wait: 10) - expect(page).to have_css( - ".mod-whisper-armed-pill__label", - text: I18n.t("js.discourse_mod_categories.whisper.armed_pill_staff_only"), - ) - shot("70_participant_whisper_back_armed") - - find(".d-editor-input").fill_in(with: "Thanks staff, replying back.") - find(".save-or-cancel .create").click - - expect(page).to have_css( - ".cooked.mod-whisper.mod-whisper--user .mod-whisper-banner", - wait: 15, - ) - shot("71_whisper_back_banner") - - whisper_back = topic.reload.posts.last - expect(whisper_back.custom_fields[targets_field]).to eq([]) + expect(page).to have_css(".d-editor-input", wait: 10) + expect(page).to have_no_css(".d-editor-button-bar button.mod-whisper-target") + shot("70_participant_no_whisper_button") end end context "a non-participant user" do before { make_whisper_post([recipient.id]) } - it "the eye button is a no-op for a non-participant" do + it "does NOT show the whisper toolbar button to a non-participant" do sign_in(stranger) visit("/t/#{topic.slug}/#{topic.id}") expect(page).to have_css("#topic-title", wait: 10) open_reply_composer - whisper_toolbar_button.click - # No whisper armed — the pill never appears. - expect(page).to have_no_css(".mod-whisper-armed-pill") - shot("72_non_participant_no_op") + expect(page).to have_css(".d-editor-input", wait: 10) + expect(page).to have_no_css(".d-editor-button-bar button.mod-whisper-target") + shot("72_non_participant_no_whisper_button") end end From cd5976c0a47bbb458cc7a72c663b4ff2bab9cee8 Mon Sep 17 00:00:00 2001 From: Shalom-Karr Date: Thu, 28 May 2026 03:56:28 -0400 Subject: [PATCH 12/12] Fix SCSS lint: prettier reformat + double-slash-comment empty lines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two issues in topic-footer-message.scss flagged by the linting job: 1. stylelint `scss/double-slash-comment-empty-line-before` at lines 273 and 284 — `//` comments must be preceded by an empty line. Both were inline mid-rule comments explaining the avatar overlap margin and the ring-outline box-shadow. Added blank lines before. 2. prettier formatting — the long selector `> .mod-private-note-viewers-pill-avatar + .mod-private-note-viewers-pill-avatar` gets wrapped across two lines per prettier's print width rule. Auto-applied by `prettier --write`. Co-Authored-By: Claude Opus 4.7 (1M context) --- assets/stylesheets/topic-footer-message.scss | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/assets/stylesheets/topic-footer-message.scss b/assets/stylesheets/topic-footer-message.scss index 0c9b6b0..569bc67 100644 --- a/assets/stylesheets/topic-footer-message.scss +++ b/assets/stylesheets/topic-footer-message.scss @@ -270,9 +270,11 @@ .mod-private-note-viewers-pill-avatars { display: inline-flex; align-items: center; + // Negative left margin on each avatar past the first stacks them // with a slight overlap — the classic "presence avatars" pattern. - > .mod-private-note-viewers-pill-avatar + .mod-private-note-viewers-pill-avatar { + > .mod-private-note-viewers-pill-avatar + + .mod-private-note-viewers-pill-avatar { margin-left: -0.45em; } } @@ -281,6 +283,7 @@ width: 20px; height: 20px; border-radius: 50%; + // Ring outline so overlapping avatars are visually separated against // both the panel background and each other. box-shadow: 0 0 0 1.5px var(--primary-very-low);