Audience-aware whisper unread + merge mod-note bell + badge targeting#9
Closed
Shalom-Karr wants to merge 5 commits into
Closed
Audience-aware whisper unread + merge mod-note bell + badge targeting#9Shalom-Karr wants to merge 5 commits into
Shalom-Karr wants to merge 5 commits into
Conversation
POST /discourse-mod-categories/notes-feed/seen.json was not idempotent
under concurrent requests (double-click, two tabs, retry). Both calls
read zero existing rows under READ COMMITTED, then both INSERT into
user_custom_fields for the same (user_id, name='mod_notes_seen_at')
pair. There is no DB-level uniqueness on that field, so both writes
succeed.
Once duplicates exist, HasCustomFields returns an Array for that key.
The :current_user serializer passed that Array straight into
.where("value > ?", seen_at), which Rails rendered as a Postgres tuple
comparison (AND (value > 'a', 'b')). Postgres rejects that with
PG::DatatypeMismatch, so every authenticated HTML bootstrap for the
affected user 500'd until a duplicate row was deleted by hand.
Two-part fix, defence in depth:
* sub_plugins/mod_categories.rb -- defensive serializer coercion.
Array(custom_fields[...]).compact.max collapses scalar, missing,
and duplicate-array shapes to a single newest ISO8601 string so
the serializer can never crash on this shape again. .max picks
the newest timestamp so seen-state stays correct rather than
being silently rewound to the older row.
* app/controllers/discourse_mod_categories/messages_controller.rb --
wrap the read-modify-write in current_user.with_lock so concurrent
callers serialize on the user row instead of racing. Cheaper and
less invasive than adding a partial unique index plus an upsert.
Plus plugin.rb: add Shalom_Karr and Ars18 to authors.
Pre-existing duplicate rows in production need a one-shot cleanup,
run separately against the database:
DELETE FROM user_custom_fields a
USING user_custom_fields b
WHERE a.name = 'mod_notes_seen_at'
AND b.name = 'mod_notes_seen_at'
AND a.user_id = b.user_id
AND a.id < b.id;
* Whisper unread badge — non-audience viewers no longer see a "+N unread" topic-list bump from whispers they can't read. on(:post_created) rolls back Topic#highest_post_number to the last non-whisper post; a new add_to_serializer(:listable_topic, :highest_post_number) adds the bump back per-user via WhisperQueryFilter so the audience still sees it. * Merge mod-note notifications into the standard avatar bell dot. The shield-tab inside the user-menu dropdown stays and its count derives from the same unread Notification rows as the bell, so reading a mod-note in either place decrements the other. Deleted the separate header-pip + (N) title-prefix indicators and the now-dead /mod-note-unread-count MessageBus channel. * Badge-member targeting on whispers (lazy membership via UserBadge join in WhisperQueryFilter and Guardian) and a PM-composer "Add badge group" button that resolves badge -> current holders via a new /discourse-mod-categories/badge-members/:badge_id endpoint and splices them into target_recipients. Specs: whisper_unread_badge, mod_note_unread_count, whisper_badge_targeting, badge_members_endpoint. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-race # Conflicts: # config/routes.rb # plugin.rb # sub_plugins/mod_categories.rb
…ystem specs * Run prettier --write on the changed/new .gjs and .js files so lint:prettier passes. * Adjust sub_plugins/mod_categories.rb to use `if exclude?` instead of `unless include?` to satisfy Style/InvertibleUnlessCondition. * Update mod_serialization_spec.rb to drive mod_note_unread_count via unread Notification rows (the new source of truth) rather than the retired `mod_topic_private_note_activity_at` timestamp. * Delete system specs for the removed header pip + (N) title prefix (mod_note_avatar_badge_visuals_spec.rb, mod_note_header_indicators_spec.rb) — the feature itself is gone. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
|
Re-targeting to fork main first per request — will reopen against upstream after CI confirms on Shalom-Karr/main. |
Merged
4 tasks
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
1 participant
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.
Summary
on(:post_created)rolls backTopic#highest_post_numberto the last non-whisper post, and a newadd_to_serializer(:listable_topic, :highest_post_number)adds the bump back per-user viaWhisperQueryFilter— so the audience still sees the badge.mod_note_unread_countnow derives from the same unreadNotificationrows that drive the avatar bell dot, so reading a mod-note from either the bell or the shield-tab decrements both. Removed the separate header pip +(N)title-prefix indicator and the now-dead/mod-note-unread-countMessageBus channel.UserBadgejoin inWhisperQueryFilterand Guardian). The PM composer gains an "Add badge group" button that resolves badge → current holders via a new/discourse-mod-categories/badge-members/:badge_idendpoint and splices them intotarget_recipients.Test plan
spec/requests/whisper_unread_badge_spec.rb— audience-aware highest_post_number + rollback.spec/requests/mod_note_unread_count_spec.rb— count derives fromNotificationrows; single bell mark-read decrements the shield-tab count.spec/requests/whisper_badge_targeting_spec.rb— badge holders see, non-holders don't; lazy membership.spec/requests/badge_members_endpoint_spec.rb— endpoint returns current holders, excludes self.whisper_serialization_spec,whisper_create_spec,whisper_guardian_spec) should still pass.🤖 Generated with Claude Code