diff --git a/.github/workflows/discourse-plugin.yml b/.github/workflows/discourse-plugin.yml index f5cf62e..3f57035 100644 --- a/.github/workflows/discourse-plugin.yml +++ b/.github/workflows/discourse-plugin.yml @@ -5,7 +5,23 @@ 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: 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/app/controllers/discourse_mod_categories/messages_controller.rb b/app/controllers/discourse_mod_categories/messages_controller.rb index d4046c6..2493745 100644 --- a/app/controllers/discourse_mod_categories/messages_controller.rb +++ b/app/controllers/discourse_mod_categories/messages_controller.rb @@ -206,6 +206,155 @@ 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 + # `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 + # 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 @@ -555,5 +704,48 @@ 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. + 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/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/components/mod-private-note.gjs b/assets/javascripts/discourse/components/mod-private-note.gjs index d0cb0a9..2c962e1 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,64 @@ 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), + })); + } + + // 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() { @@ -535,6 +604,66 @@ 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/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-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/assets/javascripts/discourse/initializers/mod-whisper.js b/assets/javascripts/discourse/initializers/mod-whisper.js index 63ff28f..e4ead92 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"; @@ -7,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", @@ -26,9 +23,78 @@ 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) => { + // 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", @@ -38,33 +104,13 @@ export default { perform: () => { const composerService = api.container.lookup("service:composer"); const model = composerService?.model; - if (!model || !currentUser) { - return; - } - - if (currentUser.staff) { - const modal = api.container.lookup("service:modal"); - modal?.show(ModWhisperTargetModal, { - model: { composer: model }, - }); + if (!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/assets/stylesheets/topic-footer-message.scss b/assets/stylesheets/topic-footer-message.scss index 45c92bf..569bc67 100644 --- a/assets/stylesheets/topic-footer-message.scss +++ b/assets/stylesheets/topic-footer-message.scss @@ -241,6 +241,107 @@ 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.4em; + padding: 0.2em 0.5em 0.2em 0.35em; + 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); + } + } + + .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; + 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 ed74c15..2892803 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -35,6 +35,9 @@ 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" + 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/jobs/dedupe_mod_whisper_notifications_spec.rb b/spec/jobs/dedupe_mod_whisper_notifications_spec.rb new file mode 100644 index 0000000..a8d38cf --- /dev/null +++ b/spec/jobs/dedupe_mod_whisper_notifications_spec.rb @@ -0,0 +1,84 @@ +# 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:, 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_record.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/record_note_view_spec.rb b/spec/requests/record_note_view_spec.rb new file mode 100644 index 0000000..acafc19 --- /dev/null +++ b/spec/requests/record_note_view_spec.rb @@ -0,0 +1,100 @@ +# 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 + # `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) + + 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) + 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/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/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 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 dcfcc8b..fd43c5b 100644 --- a/spec/system/feature_screenshots_spec.rb +++ b/spec/system/feature_screenshots_spec.rb @@ -396,4 +396,246 @@ 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 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 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: 5, wait: 5) + 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 diff --git a/spec/system/whisper_edit_toggle_spec.rb b/spec/system/whisper_edit_toggle_spec.rb new file mode 100644 index 0000000..a58839c --- /dev/null +++ b/spec/system/whisper_edit_toggle_spec.rb @@ -0,0 +1,177 @@ +# 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. + # 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 + # 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) + + # 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 + + 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 + + # 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 + + # 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 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 diff --git a/sub_plugins/mod_categories.rb b/sub_plugins/mod_categories.rb index 11b40cc..08ecd5a 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,16 @@ class Engine < ::Rails::Engine DiscourseModCategories::TOPIC_NON_WHISPER_BUMPED_AT_FIELD, :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) @@ -442,6 +458,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 @@ -688,6 +724,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 +741,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 @@ -901,4 +959,41 @@ 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? + + # 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? + + parsed = ::Time.zone.parse(nwba.to_s) + parsed || raw + rescue StandardError + raw + end + end end