Whisper notification clearing + dedupe + viewers pill + audience-aware bumped_at + whisper edit endpoint#13
Merged
TripleU613 merged 12 commits intoMay 28, 2026
Conversation
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/<slug>/<id> 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
Two new captures for the post-PR-JTech-Forums#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) <noreply@anthropic.com>
(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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
…flow
(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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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 <branch> No org approval needed — it runs in the fork's own Actions environment. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…on tests 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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
Shalom-Karr
force-pushed
the
fix/mod-note-anchor-and-whisper-bump
branch
from
1c1e8d0 to
cd5976c
Compare
Member
Author
Summary of changes in this PR12 commits on top of PR #12's merge, grouped by feature area. The fork's full Discourse Plugin CI matrix is green on the latest tip ( 🔔 Bell + notification hygiene
👁 "Viewed by" mod-note panel feature
⏫ Audience-aware whisper bumping on /latest
✏️ Whisper toggle on edit
🔧 CI infra
What's proven by spec
Visual proofScreenshot scenarios 7–22 captured in
Maintenance flag (the one risk)The 🤖 Generated with Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft for CI validation — not for review/merge until I say go.
Seven commits stacked on top of PR #12's merge, grouped by feature area.
Bell + notification hygiene
19fba70— Discourse's auto-mark-read on topic view skipsNotification.types[:custom](which every plugin notification uses), so whisper + mod-note bell rows stayed unread after opening the topic. AddsPOST .../topic/:id/notifications/seen+ anonPageChangeinitializer that fires it on every/t/...navigation, scoped via data-column markers (mod_note: true,mod_whisper: true, the legacy whisper-message i18n key) so unrelated custom notifications another plugin attaches to the same topic are untouched. Also addsJobs::DedupeModWhisperNotifications(5s delayed) that removes the duplicate:repliedrows PostAlerter creates for users who also got our custom whisper."Viewed by" mod-note pill
d1fd79b— Topic custom fieldmod_topic_note_viewers(JSON array, staff-only serializer),POST .../topic/:id/note-viewendpoint (idempotent re-views updateviewed_at), component records on insert, popover lists viewers with avatars + names + relative-time.8388628— Replaces the "Viewed by N staff" text label with a row of small (20px) avatars stacked with a slight overlap + ring outline, plus+Noverflow indicator. Click still opens the popover for the detailed list.Audience-aware bumped_at on /latest
cf0b14f— The Activity column was reading rawtopics.bumped_at, so non-audience viewers saw "5m" on a topic whose latest visible activity was actually 1+ hour old. Addsadd_to_serializer(:listable_topic, :bumped_at)mirroring the existinghighest_post_numberaudience check pattern.444a04c— Fix the previous commit's regression:HasCustomFields::PreloadedProxyrejected the field access inside the serializer withNotPreloadedError, 500ing/latest. Registers both fields viaadd_preloaded_topic_list_custom_fieldso they're batch-loaded with the topic list, plus wraps the access in a defensive rescue that falls through to raw on any error.Whisper edit endpoint
dc4a209—PUT .../post/:id/whispertoggles whisper state on existing posts. Discourse's PostsController#update drops whisper params (noserializeOnUpdate), so the composer's edit flow couldn't change whisper state. Staff-only — non-staff (including the post's own author) get 403. Frontend wiresmodel:composer#saveto chain the PUT after a staff edit save when the modal marked the state dirty; arming merges new audience into the topic's cumulative participants; disarming hard-deletes thePostCustomFieldrows somod_is_whisperflips back to false.Screenshot scenario updates
4a68b6f— Initial viewers-pill captures.What this PR proves vs. doesn't prove
CI-side this draft exists to run:
backend_testsagainstupdate_post_whisper_spec(15 cases),topic_notifications_seen_spec,dedupe_mod_whisper_notifications_spec,record_note_view_spec, and the extendedwhisper_unread_badge_spec.system_testsandfeature_screenshotsfor the visible UI changes.What this draft does NOT exercise:
model:composer#savepatch — it's purely behavioral, no spec coverage. Manual eyeball test is the only way to verify the whisper toggle on edit actually propagates from the composer to the new endpoint.🤖 Generated with Claude Code