diff --git a/admins/pageflow/entry.rb b/admins/pageflow/entry.rb index faccd6d9da..1f7dd6f8a3 100644 --- a/admins/pageflow/entry.rb +++ b/admins/pageflow/entry.rb @@ -9,7 +9,8 @@ module Pageflow entry_publication_state_indicator(entry) end column :title, sortable: 'title' do |entry| - link_to(entry.title, admin_entry_path(entry)) + safe_join([link_to(entry.title, admin_entry_path(entry)), + entry_comments_indicator(entry)].compact) end column I18n.t('pageflow.admin.entries.members'), class: 'members' do |entry| entry_user_badge_list(entry) diff --git a/app/assets/images/pageflow/admin/icons/comment.svg b/app/assets/images/pageflow/admin/icons/comment.svg new file mode 100644 index 0000000000..ae5ba7b3a9 --- /dev/null +++ b/app/assets/images/pageflow/admin/icons/comment.svg @@ -0,0 +1 @@ + diff --git a/app/assets/stylesheets/pageflow/admin.scss b/app/assets/stylesheets/pageflow/admin.scss index 14cb08cc3f..4ce83be388 100644 --- a/app/assets/stylesheets/pageflow/admin.scss +++ b/app/assets/stylesheets/pageflow/admin.scss @@ -25,6 +25,7 @@ $pageflow-hint-color: #666 !default; @import "pageflow/admin/embed_code"; @import "pageflow/admin/embedded_index_table"; @import "pageflow/admin/entries"; +@import "pageflow/admin/entry_comments_indicator"; @import "pageflow/admin/features"; @import "pageflow/admin/filters"; @import "pageflow/admin/forms"; diff --git a/app/assets/stylesheets/pageflow/admin/entry_comments_indicator.scss b/app/assets/stylesheets/pageflow/admin/entry_comments_indicator.scss new file mode 100644 index 0000000000..415915feb8 --- /dev/null +++ b/app/assets/stylesheets/pageflow/admin/entry_comments_indicator.scss @@ -0,0 +1,45 @@ +$pageflow-entry-comments-indicator-color: #8a5a00 !default; + +$pageflow-entry-comments-indicator-background-color: #fdf1dc !default; + +$pageflow-entry-comments-indicator-dot-color: #ff7400 !default; + +$pageflow-entry-comments-indicator-icon-directory: "pageflow/admin/icons" !default; + +.entry_comments_indicator { + $dir: $pageflow-entry-comments-indicator-icon-directory; + + position: relative; + display: inline-block; + margin-left: 8px; + padding: 1px 6px 1px 22px; + vertical-align: middle; + border-radius: 10px; + background: $pageflow-entry-comments-indicator-background-color + image-url("#{$dir}/comment.svg") no-repeat 5px center / 13px 13px; + color: $pageflow-entry-comments-indicator-color; + font-size: 0.7rem; + line-height: 1.6; + + // The tooltip claims both pseudo elements of the indicator, so the + // dot has to be an element of its own. + // + // Straddles the corner like the unread dot inside the editor, so both + // read as the same signal. + .unread_dot { + position: absolute; + top: -2px; + right: -2px; + width: 6px; + height: 6px; + border-radius: 50%; + background-color: $pageflow-entry-comments-indicator-dot-color; + } + + // The bubble is placed by its static position, which on an inline + // element sits at the far end of the content and thus away from the + // arrow anchored to the left. + &::after { + left: 0; + } +} diff --git a/app/assets/stylesheets/pageflow/editor/menu.scss b/app/assets/stylesheets/pageflow/editor/menu.scss index 394895758a..392036aeee 100644 --- a/app/assets/stylesheets/pageflow/editor/menu.scss +++ b/app/assets/stylesheets/pageflow/editor/menu.scss @@ -15,4 +15,18 @@ ul.menu { display: block; padding: space(2.5); } + + // Set right after the label rather than at the edge of the item, so + // it reads as belonging to the label it marks. + a.indicator::after { + content: ""; + display: inline-block; + position: relative; + top: space(-2); + margin-left: space(0.5); + width: space(1); + height: space(1); + border-radius: 50%; + background: var(--ui-warning-color); + } } diff --git a/app/controllers/pageflow/review/comment_thread_reads_controller.rb b/app/controllers/pageflow/review/comment_thread_reads_controller.rb new file mode 100644 index 0000000000..2c0c29dd99 --- /dev/null +++ b/app/controllers/pageflow/review/comment_thread_reads_controller.rb @@ -0,0 +1,30 @@ +module Pageflow + module Review + # @api private + class CommentThreadReadsController < Pageflow::ApplicationController + respond_to :json + before_action :authenticate_user! + + def create + entry = DraftEntry.find(params[:entry_id]) + authorize!(:read, entry.to_model) + + CommentThreadRead.mark(entry: entry.to_model, + user: current_user, + comment_thread_perma_ids: known_perma_ids(entry)) + + head :no_content + end + + private + + # Guards against read records piling up for comment threads that + # do not exist in the entry. + def known_perma_ids(entry) + entry.comment_threads + .where(perma_id: params.fetch(:comment_thread_perma_ids, [])) + .pluck(:perma_id) + end + end + end +end diff --git a/app/controllers/pageflow/review/comment_threads_controller.rb b/app/controllers/pageflow/review/comment_threads_controller.rb index b579f968ee..59e0d62f2f 100644 --- a/app/controllers/pageflow/review/comment_threads_controller.rb +++ b/app/controllers/pageflow/review/comment_threads_controller.rb @@ -10,6 +10,8 @@ def index authorize!(:read, entry.to_model) @comment_threads = entry.comment_threads.includes(comments: :creator) + @read_at_by_perma_id = + CommentThreadRead.read_at_by_perma_id(entry: entry.to_model, user: current_user) end def create diff --git a/app/helpers/pageflow/admin/entries_helper.rb b/app/helpers/pageflow/admin/entries_helper.rb index 4a54acf3b4..9b85aa1851 100644 --- a/app/helpers/pageflow/admin/entries_helper.rb +++ b/app/helpers/pageflow/admin/entries_helper.rb @@ -19,6 +19,42 @@ def collection_for_entry_publication_states end end + def entry_comments_indicator(entry, summaries: entry_comment_summaries) + summary = summaries[entry.id] + return unless summary&.any? + + content_tag(:span, + class: 'entry_comments_indicator', + data: {tooltip: entry_comments_tooltip(summary)}) do + safe_join([summary.topic_count.to_s, + (content_tag(:span, '', class: 'unread_dot') if summary.new?)].compact) + end + end + + # Built for the whole page at once, so that rendering a row does + # not query. Views without an index table collection pass their own + # summaries instead. + def entry_comment_summaries + @entry_comment_summaries ||= + EntryCommentSummary.for_entries(collection, user: current_user) + end + + def entry_comments_tooltip(summary) + scope = 'pageflow.admin.entries.comments' + + parts = [t("#{scope}.topic_count", count: summary.topic_count)] + + if summary.new_topic_count.positive? + parts << t("#{scope}.new_topic_count", count: summary.new_topic_count) + end + + if summary.new_reply_count.positive? + parts << t("#{scope}.new_reply_count", count: summary.new_reply_count) + end + + t("#{scope}.tooltip", summary: parts.join(', ')) + end + def entry_type_collection(entry_types = Pageflow.config.entry_types) entry_types.map(&:name).index_by do |type| I18n.t(type, scope: 'activerecord.values.pageflow/entry.type_names') diff --git a/app/models/pageflow/comment_thread_read.rb b/app/models/pageflow/comment_thread_read.rb new file mode 100644 index 0000000000..a414fbf4ba --- /dev/null +++ b/app/models/pageflow/comment_thread_read.rb @@ -0,0 +1,22 @@ +module Pageflow + # Records when a user last read a comment thread. Keyed by perma id + # so read state survives comment threads being copied to a new + # revision. + # + # @api private + class CommentThreadRead < ApplicationRecord + belongs_to :entry + belongs_to :user + + def self.read_at_by_perma_id(entry:, user:) + where(entry:, user:).pluck(:comment_thread_perma_id, :read_at).to_h + end + + def self.mark(entry:, user:, comment_thread_perma_ids:, read_at: Time.current) + comment_thread_perma_ids.each do |perma_id| + find_or_initialize_by(entry:, user:, comment_thread_perma_id: perma_id) + .update!(read_at:) + end + end + end +end diff --git a/app/models/pageflow/entry.rb b/app/models/pageflow/entry.rb index 551146ed1b..b7e7a3f28a 100644 --- a/app/models/pageflow/entry.rb +++ b/app/models/pageflow/entry.rb @@ -33,6 +33,8 @@ class PasswordMissingError < StandardError has_many :imports, class_name: 'Pageflow::FileImport', dependent: :destroy + has_many :comment_thread_reads, dependent: :destroy + has_one :draft, -> { editable }, class_name: 'Revision', inverse_of: :entry has_one :published_revision, -> { published }, class_name: 'Revision', inverse_of: :entry diff --git a/app/models/pageflow/entry_comment_summary.rb b/app/models/pageflow/entry_comment_summary.rb new file mode 100644 index 0000000000..10e1674f85 --- /dev/null +++ b/app/models/pageflow/entry_comment_summary.rb @@ -0,0 +1,88 @@ +module Pageflow + # Counts of comment topics and comments the user has not seen, for + # displaying an indicator next to an entry in lists of entries. + # + # Built for a whole page of entries at once: rendering a list must not + # query per row. + # + # @api private + class EntryCommentSummary + attr_reader :topic_count, :new_topic_count, :new_reply_count + + def self.for_entries(entries, user:) + entries = entries.to_a + return {} if entries.empty? + + threads = unresolved_threads_by_entry_id(entries) + read_at = read_at_by_entry_id(entries, user) + + entries.to_h do |entry| + [entry.id, build(threads.fetch(entry.id, []), + read_at: read_at.fetch(entry.id, {}), + user:)] + end + end + + def initialize(topic_count:, new_topic_count:, new_reply_count:) + @topic_count = topic_count + @new_topic_count = new_topic_count + @new_reply_count = new_reply_count + end + + def any? + topic_count.positive? + end + + def new? + new_topic_count.positive? || new_reply_count.positive? + end + + # Comment threads live on the draft revision, so entries are reached + # through their editable revision rather than directly. + def self.unresolved_threads_by_entry_id(entries) + entry_id_by_revision_id = + Revision.editable.where(entry_id: entries.map(&:id)).pluck(:id, :entry_id).to_h + + CommentThread + .where(revision_id: entry_id_by_revision_id.keys, resolved_at: nil) + .includes(:comments) + .group_by { |thread| entry_id_by_revision_id[thread.revision_id] } + end + private_class_method :unresolved_threads_by_entry_id + + def self.read_at_by_entry_id(entries, user) + CommentThreadRead + .where(user:, entry_id: entries.map(&:id)) + .pluck(:entry_id, :comment_thread_perma_id, :read_at) + .group_by(&:first) + .transform_values do |rows| + rows.to_h { |(_entry_id, perma_id, read_at)| [perma_id, read_at] } + end + end + private_class_method :read_at_by_entry_id + + def self.build(threads, read_at:, user:) + new_topics = 0 + new_replies = 0 + + threads.each do |thread| + first, *replies = thread.comments.sort_by(&:id) + seen_up_to = [read_at[thread.perma_id], user.unread_comments_since_at].compact.max + + new_topics += 1 if first && unread?(first, seen_up_to, user) + new_replies += replies.count { |reply| unread?(reply, seen_up_to, user) } + end + + new(topic_count: threads.size, new_topic_count: new_topics, new_reply_count: new_replies) + end + private_class_method :build + + # Mirrors the unread rule of the review interface: own comments never + # count, and neither do comments from before the user's baseline. + def self.unread?(comment, seen_up_to, user) + comment.creator_id != user.id && + (seen_up_to.nil? || comment.created_at > seen_up_to) + end + private_class_method :unread? + end +end diff --git a/app/views/pageflow/review/comment_threads/index.json.jbuilder b/app/views/pageflow/review/comment_threads/index.json.jbuilder index bdd65bcfe4..ad38046c37 100644 --- a/app/views/pageflow/review/comment_threads/index.json.jbuilder +++ b/app/views/pageflow/review/comment_threads/index.json.jbuilder @@ -3,8 +3,11 @@ json.key_format!(camelize: :lower) json.current_user do json.id current_user.id json.name current_user.full_name + json.unread_comments_since_at current_user.unread_comments_since_at end json.comment_threads(@comment_threads) do |comment_thread| json.partial!('pageflow/review/comment_threads/comment_thread', comment_thread:) end + +json.comment_thread_reads(@read_at_by_perma_id.transform_keys(&:to_s)) diff --git a/config/locales/de.yml b/config/locales/de.yml index a093ab93c1..c8c77702ff 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -828,6 +828,17 @@ de: site_defaults_inline_help: Die folgenden Einstellungen werden als Standard für neue Beiträge des Kontos verwendet. Änderungen wirken sich nicht auf existierende Beiträge aus. entries: add_folder: Ordner hinzufügen + comments: + new_reply_count: + one: 1 neue Antwort + other: "%{count} neue Antworten" + new_topic_count: + one: 1 neues Thema + other: "%{count} neue Themen" + tooltip: "Kommentare: %{summary}" + topic_count: + one: 1 ungelöstes Thema + other: "%{count} ungelöste Themen" confirm_depublish: Soll der Beitrag wirklich depubliziert werden? confirm_duplicate: Beitrag wirklich duplizieren? confirm_restore: Soll der Beitrag wirklich auf den Stand dieser Revision zurückgesetzt werden? Vor dem Zurücksetzen wird eine automatische Sicherung des aktuellen Standes erstellt. diff --git a/config/locales/en.yml b/config/locales/en.yml index 530f2b7861..14672e825a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -827,6 +827,17 @@ en: site_defaults_inline_help: The following settings will be used as defaults for new stories in this account. Changes do not affect existing stories. entries: add_folder: Add folder + comments: + new_reply_count: + one: 1 new reply + other: "%{count} new replies" + new_topic_count: + one: 1 new topic + other: "%{count} new topics" + tooltip: "Comments: %{summary}" + topic_count: + one: 1 unresolved topic + other: "%{count} unresolved topics" confirm_depublish: Depublish this story? confirm_duplicate: Duplicate this story? confirm_restore: Restore story to the selected version? A snapshot will be created, so that you can roll back later. diff --git a/config/routes.rb b/config/routes.rb index 9215836b88..a4a4e998e6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -84,6 +84,8 @@ resources :comment_threads, only: [:index, :create, :update] do resources :comments, only: [:create, :update] end + + resources :comment_thread_reads, only: [:create] end end diff --git a/db/migrate/20260817000000_create_comment_thread_reads.rb b/db/migrate/20260817000000_create_comment_thread_reads.rb new file mode 100644 index 0000000000..6acb6b79e5 --- /dev/null +++ b/db/migrate/20260817000000_create_comment_thread_reads.rb @@ -0,0 +1,16 @@ +class CreateCommentThreadReads < ActiveRecord::Migration[6.0] + def change + create_table :pageflow_comment_thread_reads do |t| + t.integer :entry_id, null: false + t.integer :user_id, null: false + t.integer :comment_thread_perma_id, null: false + t.datetime :read_at, null: false + end + + add_index :pageflow_comment_thread_reads, + [:user_id, :entry_id, :comment_thread_perma_id], + unique: true, + name: 'index_comment_thread_reads_on_user_and_entry_and_thread' + add_index :pageflow_comment_thread_reads, :entry_id + end +end diff --git a/db/migrate/20260819000000_add_unread_comments_since_at_to_users.rb b/db/migrate/20260819000000_add_unread_comments_since_at_to_users.rb new file mode 100644 index 0000000000..dfba5a9491 --- /dev/null +++ b/db/migrate/20260819000000_add_unread_comments_since_at_to_users.rb @@ -0,0 +1,18 @@ +class AddUnreadCommentsSinceAtToUsers < ActiveRecord::Migration[7.1] + class MigratedUser < ActiveRecord::Base + self.table_name = 'users' + end + + def up + add_column :users, :unread_comments_since_at, :datetime + + # Comments written before the feature existed have not gone unread: + # without a baseline, every one of them would turn up as unread for + # every user at once. + MigratedUser.update_all(unread_comments_since_at: Time.current) + end + + def down + remove_column :users, :unread_comments_since_at + end +end diff --git a/entry_types/scrolled/config/locales/de.yml b/entry_types/scrolled/config/locales/de.yml index ab86ffdf4d..314888e115 100644 --- a/entry_types/scrolled/config/locales/de.yml +++ b/entry_types/scrolled/config/locales/de.yml @@ -1984,6 +1984,9 @@ de: cancel_add_comment: Abbrechen hide_comments: Kommentare ausblenden show_comments: Kommentare einblenden + show_comments_with_unread: + one: Kommentare einblenden (1 ungelesener Kommentar) + other: Kommentare einblenden (%{count} ungelesene Kommentare) comment_toolbar: Kommentare filter: label: Kommentare filtern @@ -1995,6 +1998,13 @@ de: zero: Keine Kommentare one: 1 Kommentar other: '%{count} Kommentare' + unread_comment_count: + one: 1 ungelesener Kommentar + other: '%{count} ungelesene Kommentare' + new_reply_count: + one: 1 neu + other: '%{count} neu' + new_replies: Neue Antworten select_content_element: Zum Kommentieren auswählen select_section: Abschnitt zum Kommentieren auswählen select_text_to_comment: Text zum Kommentieren auswählen diff --git a/entry_types/scrolled/config/locales/en.yml b/entry_types/scrolled/config/locales/en.yml index 8d4bca3d63..99c00d6ae1 100644 --- a/entry_types/scrolled/config/locales/en.yml +++ b/entry_types/scrolled/config/locales/en.yml @@ -1812,6 +1812,9 @@ en: cancel_add_comment: Cancel hide_comments: Hide comments show_comments: Show comments + show_comments_with_unread: + one: Show comments (1 unread comment) + other: Show comments (%{count} unread comments) comment_toolbar: Comments filter: label: Filter comments @@ -1823,6 +1826,13 @@ en: zero: No comments one: 1 comment other: '%{count} comments' + unread_comment_count: + one: 1 unread comment + other: '%{count} unread comments' + new_reply_count: + one: 1 new + other: '%{count} new' + new_replies: New replies select_content_element: Select to comment select_section: Select section to comment select_text_to_comment: Select text to comment diff --git a/entry_types/scrolled/package/.eslintrc.js b/entry_types/scrolled/package/.eslintrc.js index 52e75ca2cb..18d41ca91b 100644 --- a/entry_types/scrolled/package/.eslintrc.js +++ b/entry_types/scrolled/package/.eslintrc.js @@ -34,10 +34,10 @@ module.exports = { { // Content elements, widgets, the editor and the review interface // are bundled separately and need to import from - // 'pageflow-scrolled/frontend' and 'pageflow-scrolled/entryState' - // to keep that code external. Importing relatively would inline a - // second copy of the module, giving the bundle its own React - // contexts. + // 'pageflow-scrolled/frontend', 'pageflow-scrolled/entryState' and + // 'pageflow-scrolled/review' to keep that code external. Importing + // relatively would inline a second copy of the module, giving the + // bundle its own React contexts. // Stories are excluded because they legitimately import the // content element's sibling './frontend' aggregator via // '../frontend'. @@ -52,7 +52,8 @@ module.exports = { "no-restricted-imports": ["error", { "patterns": [ "**/frontend/**", "../**/frontend", - "**/entryState/**", "../**/entryState" + "**/entryState/**", "../**/entryState", + "**/review/**", "../**/review" ] }] } diff --git a/entry_types/scrolled/package/spec/frontend/commenting/features/unreadBadges-spec.js b/entry_types/scrolled/package/spec/frontend/commenting/features/unreadBadges-spec.js new file mode 100644 index 0000000000..772afc1217 --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/commenting/features/unreadBadges-spec.js @@ -0,0 +1,73 @@ +import '@testing-library/jest-dom/extend-expect'; +import {act, fireEvent} from '@testing-library/react'; +import {useFakeTranslations} from 'pageflow/testHelpers'; + +import {postReviewStateReadsChangeMessage} from 'review/postMessage'; + +import {renderEntry, useCommentingPageObjects} from 'support/pageObjects/commenting'; + +describe('unread badges', () => { + useCommentingPageObjects(); + + useFakeTranslations({ + 'pageflow_scrolled.review.reply_placeholder': 'Reply...', + 'pageflow_scrolled.review.unread_comment_count.one': '1 unread comment', + 'pageflow_scrolled.review.unread_comment_count.other': '%{count} unread comments' + }); + + function renderEntryWithUnreadThread() { + return renderEntry({ + seed: { + contentElements: [{typeName: 'withTestId', configuration: {testId: 5}}] + }, + commenting: { + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [ + { + id: 1, permaId: 7, subjectType: 'ContentElement', subjectId: 1, + comments: [{ + id: 10, body: 'Nice work', creatorName: 'Bob', creatorId: 2, + createdAt: '2026-08-17T11:00:00.000Z' + }] + } + ], + commentThreadReads: {} + } + }); + } + + // Delivered in a later task, so posting has to be flushed before the + // rendered output reflects it. + async function markThreadRead() { + await act(async () => { + postReviewStateReadsChangeMessage(window, {7: '2026-08-17T12:00:00.000Z'}); + await new Promise(resolve => setTimeout(resolve, 0)); + }); + } + + it('marks badge of subject with unseen comments', () => { + const entry = renderEntryWithUnreadThread(); + + expect(entry.queryAllUnreadCommentBadges()).toHaveLength(1); + }); + + it('keeps badge marked while the thread list is open', async () => { + const entry = renderEntryWithUnreadThread(); + + fireEvent.click(entry.getAllCommentBadges()[0]); + await markThreadRead(); + + expect(entry.getByText('Nice work')).toBeInTheDocument(); + expect(entry.queryAllUnreadCommentBadges()).toHaveLength(1); + }); + + it('clears badge once the thread list is closed', async () => { + const entry = renderEntryWithUnreadThread(); + + fireEvent.click(entry.getAllCommentBadges()[0]); + await markThreadRead(); + fireEvent.click(entry.getAllCommentBadges()[0]); + + expect(entry.queryAllUnreadCommentBadges()).toEqual([]); + }); +}); diff --git a/entry_types/scrolled/package/spec/frontend/commenting/features/unreadToolbar-spec.js b/entry_types/scrolled/package/spec/frontend/commenting/features/unreadToolbar-spec.js new file mode 100644 index 0000000000..3d120d752d --- /dev/null +++ b/entry_types/scrolled/package/spec/frontend/commenting/features/unreadToolbar-spec.js @@ -0,0 +1,75 @@ +import '@testing-library/jest-dom/extend-expect'; +import {fireEvent} from '@testing-library/react'; +import {useFakeTranslations} from 'pageflow/testHelpers'; + +import toolbarStyles from 'frontend/commenting/FloatingToolbar.module.css'; + +import {renderEntry, useCommentingPageObjects} from 'support/pageObjects/commenting'; + +describe('unread comments on the collapsed toolbar', () => { + useCommentingPageObjects(); + + useFakeTranslations({ + 'pageflow_scrolled.review.show_comments': 'Show comments', + 'pageflow_scrolled.review.show_comments_with_unread.one': + 'Show comments (1 unread comment)', + 'pageflow_scrolled.review.show_comments_with_unread.other': + 'Show comments (%{count} unread comments)' + }); + + const currentUser = {id: 42, name: 'Alice'}; + + function renderCollapsedEntry({comments}) { + const entry = renderEntry({ + seed: {contentElements: [{typeName: 'withTestId', configuration: {testId: 5}}]}, + commenting: { + currentUser, + commentThreads: [ + {id: 1, permaId: 7, subjectType: 'ContentElement', subjectId: 1, comments} + ], + commentThreadReads: {} + } + }); + + fireEvent.click(entry.getHideCommentsButton()); + + return entry; + } + + function comment(attributes) { + return { + id: 10, body: 'Nice work', creatorName: 'Bob', creatorId: 43, + createdAt: '2026-08-17T11:00:00.000Z', + ...attributes + }; + } + + function unreadDot(entry) { + return entry.getShowCommentsButton().querySelector(`.${toolbarStyles.unreadDot}`); + } + + it('marks the show button while comments are unseen', () => { + const entry = renderCollapsedEntry({comments: [comment()]}); + + expect(unreadDot(entry)).not.toBeNull(); + }); + + it('names the unseen comments on the show button', () => { + const entry = renderCollapsedEntry({ + comments: [comment(), comment({id: 11, creatorId: 44})] + }); + + expect(entry.getShowCommentsButton()) + .toHaveAttribute('aria-label', 'Show comments (2 unread comments)'); + }); + + it('leaves the show button unmarked without unseen comments', () => { + const entry = renderCollapsedEntry({ + comments: [comment({creatorId: currentUser.id, creatorName: 'Alice'})] + }); + + expect(unreadDot(entry)).toBeNull(); + expect(entry.getShowCommentsButton()) + .toHaveAttribute('aria-label', 'Show comments'); + }); +}); diff --git a/entry_types/scrolled/package/spec/review/ReviewMessageHandler-spec.js b/entry_types/scrolled/package/spec/review/ReviewMessageHandler-spec.js index a46c80d338..f5d9c0dd72 100644 --- a/entry_types/scrolled/package/spec/review/ReviewMessageHandler-spec.js +++ b/entry_types/scrolled/package/spec/review/ReviewMessageHandler-spec.js @@ -8,7 +8,8 @@ function fakeReviewSession() { createComment: jest.fn().mockResolvedValue(), updateThread: jest.fn().mockResolvedValue(), updateComment: jest.fn().mockResolvedValue(), - setDraft: jest.fn() + setDraft: jest.fn(), + markThreadsRead: jest.fn() }; Object.assign(session, BackboneEvents); @@ -98,6 +99,43 @@ describe('ReviewMessageHandler', () => { window.postMessage.mockRestore(); }); + it('calls session.markThreadsRead on MARK_THREADS_READ message from targetWindow', async () => { + const session = fakeReviewSession(); + + ReviewMessageHandler.create({session, targetWindow: window}); + + window.dispatchEvent(new MessageEvent('message', { + data: { + type: 'MARK_THREADS_READ', + payload: {permaIds: [5, 6]} + }, + origin: window.location.origin, + source: window + })); + + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(session.markThreadsRead).toHaveBeenCalledWith([5, 6]); + }); + + it('posts REVIEW_STATE_READS_CHANGE to target window on session change:reads', () => { + const session = fakeReviewSession(); + const postMessage = jest.fn(); + jest.spyOn(window, 'postMessage').mockImplementation(postMessage); + + ReviewMessageHandler.create({session, targetWindow: window}); + + const reads = {5: '2026-08-17T10:00:00.000Z'}; + session.trigger('change:reads', reads); + + expect(postMessage).toHaveBeenCalledWith( + {type: 'REVIEW_STATE_READS_CHANGE', payload: reads}, + window.location.origin + ); + + window.postMessage.mockRestore(); + }); + it('posts REVIEW_STATE_THREAD_CHANGE to target window on session change:thread', () => { const session = fakeReviewSession(); const postMessage = jest.fn(); diff --git a/entry_types/scrolled/package/spec/review/ReviewStateProvider-spec.js b/entry_types/scrolled/package/spec/review/ReviewStateProvider-spec.js index 08282207cc..e58461c7f7 100644 --- a/entry_types/scrolled/package/spec/review/ReviewStateProvider-spec.js +++ b/entry_types/scrolled/package/spec/review/ReviewStateProvider-spec.js @@ -7,16 +7,19 @@ import { ReviewStateProvider, useCommentDraft, useCommentThread, + useCommentThreadReads, useCommentThreads, useCreateComment, useCreateCommentThread, useCurrentUser, + useMarkThreadRead, useUpdateComment } from 'review/ReviewStateProvider'; import { postReviewStateResetMessage, postReviewStateThreadChangeMessage, - postReviewStateDraftsChangeMessage + postReviewStateDraftsChangeMessage, + postReviewStateReadsChangeMessage } from 'review/postMessage'; import {renderHookWithReviewState} from 'support/renderWithReviewState'; @@ -549,4 +552,90 @@ describe('ReviewStateProvider', () => { expect(result.current.other).toMatchObject({body: 'Elsewhere'}); }); }); + + describe('comment thread reads', () => { + function postReadsChange(payload) { + act(() => { + postReviewStateReadsChangeMessage(window, payload); + }); + } + + it('provides no read timestamp initially', () => { + const {result} = renderHook(() => useCommentThreadReads()[5], {wrapper}); + + expect(result.current).toBeUndefined(); + }); + + it('provides read timestamp from reset message', async () => { + const {result, waitForNextUpdate} = renderHook( + () => useCommentThreadReads()[5], + {wrapper} + ); + + postReset({ + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [], + commentThreadReads: {5: '2026-08-17T10:00:00.000Z'} + }); + await waitForNextUpdate(); + + expect(result.current).toEqual('2026-08-17T10:00:00.000Z'); + }); + + it('updates read timestamp on reads change message', async () => { + const {result, waitForNextUpdate} = renderHook( + () => useCommentThreadReads()[5], + {wrapper} + ); + + postReadsChange({5: '2026-08-17T12:00:00.000Z'}); + await waitForNextUpdate(); + + expect(result.current).toEqual('2026-08-17T12:00:00.000Z'); + }); + + it('marks thread read by posting a message', () => { + const postMessage = jest.spyOn(window.top, 'postMessage').mockImplementation(() => {}); + + const {result} = renderHook(() => useMarkThreadRead(), {wrapper}); + + result.current(5); + + expect(postMessage).toHaveBeenCalledWith( + {type: 'MARK_THREADS_READ', payload: {permaIds: [5]}}, + window.location.origin + ); + + postMessage.mockRestore(); + }); + + it('keeps mark callback stable when reads change', async () => { + const {result, waitForNextUpdate} = renderHook(() => useMarkThreadRead(), {wrapper}); + + const markThreadRead = result.current; + postReadsChange({5: '2026-08-17T12:00:00.000Z'}); + await waitForNextUpdate(); + + expect(result.current).toBe(markThreadRead); + }); + + it('does not invalidate thread state when reads change', async () => { + const {result, waitForNextUpdate} = renderHook( + () => useCommentThreads(), + {wrapper} + ); + + postReset({ + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [{id: 1, permaId: 5, subjectType: 'CE', subjectId: 10, comments: []}], + commentThreadReads: {} + }); + await waitForNextUpdate(); + + const threadsBefore = result.current; + postReadsChange({5: '2026-08-17T12:00:00.000Z'}); + + expect(result.current).toBe(threadsBefore); + }); + }); }); diff --git a/entry_types/scrolled/package/spec/review/Thread-spec.js b/entry_types/scrolled/package/spec/review/Thread-spec.js deleted file mode 100644 index dfa048419b..0000000000 --- a/entry_types/scrolled/package/spec/review/Thread-spec.js +++ /dev/null @@ -1,267 +0,0 @@ -import React from 'react'; -import '@testing-library/jest-dom/extend-expect'; -import userEvent from '@testing-library/user-event'; -import {useFakeTranslations} from 'pageflow/testHelpers'; - -import {Thread} from 'review/Thread'; -import {review} from 'review/api'; -import {renderWithReviewState} from 'support/renderWithReviewState'; - -describe('Thread', () => { - useFakeTranslations({ - 'pageflow_scrolled.review.refers_to_deleted_element': 'Refers to a deleted element', - 'pageflow_scrolled.review.reply_placeholder': 'Reply...', - 'pageflow_scrolled.review.reply_count.one': '1 reply', - 'pageflow_scrolled.review.reply_count.other': '%{count} replies', - 'pageflow_scrolled.review.toggle_replies': 'Toggle replies' - }); - - const thread = { - id: 1, - comments: [{id: 10, body: 'On the pull quote', creatorName: 'Bob', creatorId: 2}] - }; - - // The reply form is hidden while a thread with replies is collapsed, - // which would leave a drafted reply out of reach. - it('expands a collapsed thread that has a drafted reply', () => { - const threadWithReply = { - ...thread, - comments: [ - ...thread.comments, - {id: 11, body: 'A first reply', creatorName: 'Alice', creatorId: 1} - ] - }; - - const {getByPlaceholderText} = renderWithReviewState( - , - { - drafts: { - 'Thread:1': {threadId: 1, body: 'Half a reply', pending: false} - } - } - ); - - expect(getByPlaceholderText('Reply...')).toHaveValue('Half a reply'); - }); - - // Two textareas in one card leave it ambiguous which one typing lands in. - describe('while editing a comment', () => { - useFakeTranslations({ - 'pageflow_scrolled.review.comment_actions': 'Comment actions', - 'pageflow_scrolled.review.edit_comment': 'Edit', - 'pageflow_scrolled.review.cancel': 'Cancel' - }); - - const ownThread = { - id: 1, - comments: [ - {id: 10, body: 'On the pull quote', creatorName: 'Bob', creatorId: 2}, - {id: 11, body: 'A first reply', creatorName: 'Bob', creatorId: 2} - ] - }; - - async function startEditing(user, getAllByRole, index) { - await user.click(getAllByRole('button', {name: 'Comment actions'})[index]); - await user.click(getAllByRole('menuitem', {name: 'Edit'})[0]); - } - - it('hides the reply form', async () => { - const user = userEvent.setup(); - - const {getAllByRole, queryByPlaceholderText} = renderWithReviewState( - , - {currentUser: {id: 2, name: 'Bob'}} - ); - - expect(queryByPlaceholderText('Reply...')).toBeInTheDocument(); - - await startEditing(user, getAllByRole, 0); - - expect(queryByPlaceholderText('Reply...')).toBeNull(); - }); - - it('restores the reply form when editing ends', async () => { - const user = userEvent.setup(); - - const {getAllByRole, getByRole, queryByPlaceholderText} = renderWithReviewState( - , - {currentUser: {id: 2, name: 'Bob'}} - ); - - await startEditing(user, getAllByRole, 0); - await user.click(getByRole('button', {name: 'Cancel'})); - - expect(queryByPlaceholderText('Reply...')).toBeInTheDocument(); - }); - - it('keeps only one comment in edit mode', async () => { - const user = userEvent.setup(); - - const {getAllByRole} = renderWithReviewState( - , - {currentUser: {id: 2, name: 'Bob'}} - ); - - await startEditing(user, getAllByRole, 0); - await startEditing(user, getAllByRole, 1); - - expect(getAllByRole('textbox')).toHaveLength(1); - expect(getAllByRole('textbox')[0]).toHaveValue('A first reply'); - }); - }); - - it('renders a deleted-element hint when the thread is orphaned', () => { - const {getByText} = renderWithReviewState( - - ); - - expect(getByText('Refers to a deleted element')).toBeInTheDocument(); - }); - - it('does not render the hint for a normal thread', () => { - const {queryByText} = renderWithReviewState( - - ); - - expect(queryByText('Refers to a deleted element')).not.toBeInTheDocument(); - }); - - it('renders the hint above the first comment', () => { - const {getByText} = renderWithReviewState( - - ); - - const hint = getByText('Refers to a deleted element'); - const comment = getByText('On the pull quote'); - - expect(hint.compareDocumentPosition(comment) & Node.DOCUMENT_POSITION_FOLLOWING) - .toBeTruthy(); - }); - - describe('comment quotes', () => { - const seed = { - sections: [{id: 1, permaId: 1}], - contentElements: [ - { - id: 1, permaId: 10, sectionId: 1, typeName: 'textBlock', - configuration: {value: 'Current wording'} - } - ] - }; - - const quotingThread = { - ...thread, - subjectType: 'ContentElement', - subjectId: 10, - subjectRange: {anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 7}} - }; - - function threadWithQuotes(...quotes) { - return { - ...quotingThread, - comments: quotes.map((quote, index) => ({ - id: 10 + index, - body: `Comment ${index + 1}`, - creatorName: 'Bob', - creatorId: 2, - quote - })) - }; - } - - beforeEach(() => { - review.contentElementTypes.register('textBlock', { - extractQuote: configuration => configuration.value - }); - }); - - afterEach(() => { - review.contentElementTypes.types = {}; - }); - - it('renders a quote for a comment whose text has since changed', () => { - const {getByText} = renderWithReviewState( - , - {seed} - ); - - expect(getByText('Original wording')).toBeInTheDocument(); - }); - - it('renders no quote while the text still reads the same', () => { - const {container} = renderWithReviewState( - , - {seed} - ); - - expect(container.querySelector('blockquote')).toBeNull(); - }); - - it('renders the quote inside the comment, above its body', () => { - const {getByText} = renderWithReviewState( - , - {seed} - ); - - const quote = getByText('Original wording'); - const body = getByText('Comment 1'); - - expect(quote.parentNode).toBe(body.parentNode); - expect(quote.compareDocumentPosition(body) & Node.DOCUMENT_POSITION_FOLLOWING) - .toBeTruthy(); - }); - - it('renders a quote per version once the text changes mid thread', () => { - const {getByText} = renderWithReviewState( - , - {seed} - ); - - expect(getByText('First wording')).toBeInTheDocument(); - expect(getByText('Second wording')).toBeInTheDocument(); - }); - - it('renders the quote once for a run of replies about the same wording', () => { - const {queryAllByText} = renderWithReviewState( - , - {seed} - ); - - expect(queryAllByText('Same wording')).toHaveLength(1); - }); - - it('renders no quote for the last comment when it matches the current text', () => { - const {getByText, queryByText} = renderWithReviewState( - , - {seed} - ); - - expect(getByText('Original wording')).toBeInTheDocument(); - expect(queryByText('Current wording')).not.toBeInTheDocument(); - }); - - it('renders quotes for threads whose content element was deleted', () => { - const {getByText} = renderWithReviewState( - , - {seed} - ); - - expect(getByText('Original wording')).toBeInTheDocument(); - }); - - it('renders no quote for comments recorded without one', () => { - const {container} = renderWithReviewState( - , - {seed} - ); - - expect(container.querySelector('blockquote')).toBeNull(); - }); - }); -}); diff --git a/entry_types/scrolled/package/spec/review/Thread/features/deletedElementHint-spec.js b/entry_types/scrolled/package/spec/review/Thread/features/deletedElementHint-spec.js new file mode 100644 index 0000000000..5431778304 --- /dev/null +++ b/entry_types/scrolled/package/spec/review/Thread/features/deletedElementHint-spec.js @@ -0,0 +1,45 @@ +import React from 'react'; +import '@testing-library/jest-dom/extend-expect'; +import {useFakeTranslations} from 'pageflow/testHelpers'; + +import {Thread} from 'review/Thread'; +import {renderWithReviewState} from 'support/renderWithReviewState'; + +describe('Thread deleted element hint', () => { + useFakeTranslations({ + 'pageflow_scrolled.review.refers_to_deleted_element': 'Refers to a deleted element' + }); + + const thread = { + id: 1, + comments: [{id: 10, body: 'On the pull quote', creatorName: 'Bob', creatorId: 2}] + }; + + it('renders a deleted-element hint when the thread is orphaned', () => { + const {getByText} = renderWithReviewState( + + ); + + expect(getByText('Refers to a deleted element')).toBeInTheDocument(); + }); + + it('does not render the hint for a normal thread', () => { + const {queryByText} = renderWithReviewState( + + ); + + expect(queryByText('Refers to a deleted element')).not.toBeInTheDocument(); + }); + + it('renders the hint above the first comment', () => { + const {getByText} = renderWithReviewState( + + ); + + const hint = getByText('Refers to a deleted element'); + const comment = getByText('On the pull quote'); + + expect(hint.compareDocumentPosition(comment) & Node.DOCUMENT_POSITION_FOLLOWING) + .toBeTruthy(); + }); +}); diff --git a/entry_types/scrolled/package/spec/review/Thread/features/markingRead-spec.js b/entry_types/scrolled/package/spec/review/Thread/features/markingRead-spec.js new file mode 100644 index 0000000000..6f9b3bff2c --- /dev/null +++ b/entry_types/scrolled/package/spec/review/Thread/features/markingRead-spec.js @@ -0,0 +1,176 @@ +import React from 'react'; +import '@testing-library/jest-dom/extend-expect'; +import {act} from '@testing-library/react'; +import {useFakeTranslations} from 'pageflow/testHelpers'; + +import {Thread} from 'review/Thread'; +import {renderWithReviewState} from 'support/renderWithReviewState'; +import { + simulateScrollingIntoView, + simulateScrollingOutOfView +} from 'support/fakeIntersectionObserver'; + +describe('Thread marking read', () => { + useFakeTranslations({ + 'pageflow_scrolled.review.reply_placeholder': 'Reply...', + 'pageflow_scrolled.review.reply_count.one': '1 reply', + 'pageflow_scrolled.review.reply_count.other': '%{count} replies', + 'pageflow_scrolled.review.toggle_replies': 'Toggle replies' + }); + + const currentUser = {id: 42, name: 'Alice'}; + + const thread = { + id: 1, + permaId: 5, + subjectType: 'ContentElement', + subjectId: 10, + comments: [ + { + id: 100, + body: 'On the pull quote', + creatorId: 43, + creatorName: 'Bob', + createdAt: '2026-08-17T11:00:00.000Z' + } + ] + }; + + const threadWithReply = { + ...thread, + comments: [ + ...thread.comments, + { + id: 101, + body: 'Agreed', + creatorId: 44, + creatorName: 'Carol', + createdAt: '2026-08-17T12:00:00.000Z' + } + ] + }; + + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + jest.restoreAllMocks(); + }); + + function render(ui, options) { + const postMessage = jest.spyOn(window.top, 'postMessage').mockImplementation(() => {}); + + return { + ...renderWithReviewState(ui, {currentUser, commentThreads: [thread], ...options}), + postMessage + }; + } + + function passTime(ms) { + act(() => { + jest.advanceTimersByTime(ms); + }); + } + + function markReadMessages(postMessage) { + return postMessage.mock.calls.filter(([message]) => message.type === 'MARK_THREADS_READ'); + } + + it('marks thread read once it has been on screen long enough', () => { + const {container, postMessage} = render(); + + simulateScrollingIntoView(container); + passTime(1000); + + expect(postMessage).toHaveBeenCalledWith( + {type: 'MARK_THREADS_READ', payload: {permaIds: [5]}}, + window.location.origin + ); + }); + + it('does not mark thread read while it is off screen', () => { + const {postMessage} = render(); + + passTime(1000); + + expect(markReadMessages(postMessage)).toEqual([]); + }); + + it('does not mark thread read when it scrolls by', () => { + const {container, postMessage} = render(); + + simulateScrollingIntoView(container); + passTime(400); + simulateScrollingOutOfView(container); + passTime(1000); + + expect(markReadMessages(postMessage)).toEqual([]); + }); + + it('does not mark thread read while replies are collapsed', () => { + const {container, postMessage} = render( + , + {commentThreads: [threadWithReply]} + ); + + simulateScrollingIntoView(container); + passTime(1000); + + expect(markReadMessages(postMessage)).toEqual([]); + }); + + it('marks thread read once replies are expanded', () => { + const {container, postMessage} = render( + , + {commentThreads: [threadWithReply]} + ); + + simulateScrollingIntoView(container); + passTime(1000); + + expect(markReadMessages(postMessage)).toHaveLength(1); + }); + + it('does not mark thread read again once all comments have been read', () => { + const {container, postMessage} = render( + , + {commentThreadReads: {5: '2026-08-17T13:00:00.000Z'}} + ); + + simulateScrollingIntoView(container); + passTime(1000); + + expect(markReadMessages(postMessage)).toEqual([]); + }); + + it('does not mark thread read that only contains own comments', () => { + const ownThread = { + ...thread, + comments: [{...thread.comments[0], creatorId: currentUser.id, creatorName: 'Alice'}] + }; + + const {container, postMessage} = render( + , + {commentThreads: [ownThread]} + ); + + simulateScrollingIntoView(container); + passTime(1000); + + expect(markReadMessages(postMessage)).toEqual([]); + }); + + it('does not mark thread read before current user is known', () => { + const {container, postMessage} = render( + , + {currentUser: null} + ); + + simulateScrollingIntoView(container); + passTime(1000); + + expect(markReadMessages(postMessage)).toEqual([]); + }); +}); diff --git a/entry_types/scrolled/package/spec/review/Thread/features/newMarkers-spec.js b/entry_types/scrolled/package/spec/review/Thread/features/newMarkers-spec.js new file mode 100644 index 0000000000..ac944b9c97 --- /dev/null +++ b/entry_types/scrolled/package/spec/review/Thread/features/newMarkers-spec.js @@ -0,0 +1,144 @@ +import React from 'react'; +import '@testing-library/jest-dom/extend-expect'; +import {useFakeTranslations} from 'pageflow/testHelpers'; + +import {Thread} from 'review/Thread'; +import {renderWithReviewState} from 'support/renderWithReviewState'; +import styles from 'review/Thread.module.css'; + +describe('Thread new markers', () => { + useFakeTranslations({ + 'pageflow_scrolled.review.reply_placeholder': 'Reply...', + 'pageflow_scrolled.review.reply_count.one': '1 reply', + 'pageflow_scrolled.review.reply_count.other': '%{count} replies', + 'pageflow_scrolled.review.new_reply_count.one': '1 new', + 'pageflow_scrolled.review.new_reply_count.other': '%{count} new', + 'pageflow_scrolled.review.toggle_replies': 'Toggle replies', + 'pageflow_scrolled.review.unread_comment_count.one': '1 unread comment', + 'pageflow_scrolled.review.unread_comment_count.other': '%{count} unread comments' + }); + + const currentUser = {id: 42, name: 'Alice'}; + + function comment(attributes) { + return { + creatorId: 43, + creatorName: 'Bob', + createdAt: '2026-08-17T11:00:00.000Z', + ...attributes + }; + } + + const thread = { + id: 1, + permaId: 5, + subjectType: 'ContentElement', + subjectId: 10, + comments: [comment({id: 100, body: 'On the pull quote'})] + }; + + const threadWithReplies = { + ...thread, + comments: [ + comment({ + id: 100, body: 'On the pull quote', createdAt: '2026-08-17T09:00:00.000Z' + }), + comment({id: 101, body: 'Agreed', creatorId: 44, creatorName: 'Carol'}) + ] + }; + + function render(ui, options = {}) { + return renderWithReviewState(ui, {currentUser, commentThreads: [thread], ...options}); + } + + function newDot(container) { + return container.querySelector(`.${styles.newDot}`); + } + + describe('dot on the thread', () => { + it('marks a thread with unseen comments', () => { + const {container, getByLabelText} = render( + + ); + + expect(newDot(container)).not.toBeNull(); + expect(getByLabelText('1 unread comment')).toBeInTheDocument(); + }); + + it('marks a thread whose replies are unseen', () => { + const {container} = render( + , + { + commentThreads: [threadWithReplies], + commentThreadReads: {5: '2026-08-17T10:00:00.000Z'} + } + ); + + expect(newDot(container)).not.toBeNull(); + }); + + it('does not mark a thread without unseen comments', () => { + const {container} = render( + , + {commentThreadReads: {5: '2026-08-17T12:00:00.000Z'}} + ); + + expect(newDot(container)).toBeNull(); + }); + + // The badge that opened the list already carries the same information. + it('does not mark a thread shown on its own', () => { + const {container} = render(); + + expect(newDot(container)).toBeNull(); + }); + }); + + describe('new reply count', () => { + it('counts unseen replies hidden by collapsing', () => { + const {getByText} = render( + , + { + commentThreads: [threadWithReplies], + commentThreadReads: {5: '2026-08-17T10:00:00.000Z'} + } + ); + + expect(getByText('1 reply')).toBeInTheDocument(); + expect(getByText('1 new')).toBeInTheDocument(); + }); + + // An unread first comment must not be counted among the replies it + // is collapsed above, and own replies are never new. + it('counts neither the first comment nor own replies', () => { + const threadWithOwnReply = { + ...thread, + comments: [ + comment({id: 100, body: 'On the pull quote'}), + comment({id: 101, body: 'Will fix', creatorId: currentUser.id, creatorName: 'Alice'}) + ] + }; + + const {getByText, queryByText} = render( + , + {commentThreads: [threadWithOwnReply]} + ); + + expect(getByText('1 reply')).toBeInTheDocument(); + expect(queryByText('1 new')).toBeNull(); + }); + + it('does not show a count when all replies have been seen', () => { + const {queryByText} = render( + , + { + commentThreads: [threadWithReplies], + commentThreadReads: {5: '2026-08-17T13:00:00.000Z'} + } + ); + + expect(queryByText('1 reply')).toBeInTheDocument(); + expect(queryByText('1 new')).toBeNull(); + }); + }); +}); diff --git a/entry_types/scrolled/package/spec/review/Thread/features/newRepliesDivider-spec.js b/entry_types/scrolled/package/spec/review/Thread/features/newRepliesDivider-spec.js new file mode 100644 index 0000000000..c91f953f0c --- /dev/null +++ b/entry_types/scrolled/package/spec/review/Thread/features/newRepliesDivider-spec.js @@ -0,0 +1,108 @@ +import React from 'react'; +import '@testing-library/jest-dom/extend-expect'; +import {useFakeTranslations} from 'pageflow/testHelpers'; + +import {Thread} from 'review/Thread'; +import {renderWithReviewState} from 'support/renderWithReviewState'; + +describe('Thread new replies divider', () => { + useFakeTranslations({ + 'pageflow_scrolled.review.reply_placeholder': 'Reply...', + 'pageflow_scrolled.review.reply_count.one': '1 reply', + 'pageflow_scrolled.review.reply_count.other': '%{count} replies', + 'pageflow_scrolled.review.toggle_replies': 'Toggle replies', + 'pageflow_scrolled.review.new_replies': 'New replies' + }); + + const currentUser = {id: 42, name: 'Alice'}; + + function comment(attributes) { + return {creatorId: 43, creatorName: 'Bob', ...attributes}; + } + + const thread = { + id: 1, + permaId: 5, + subjectType: 'ContentElement', + subjectId: 10, + comments: [ + comment({id: 100, body: 'On the pull quote', createdAt: '2026-08-17T09:00:00.000Z'}), + comment({id: 101, body: 'Older reply', createdAt: '2026-08-17T09:30:00.000Z'}), + comment({ + id: 102, body: 'Newer reply', creatorId: 44, creatorName: 'Carol', + createdAt: '2026-08-17T11:00:00.000Z' + }) + ] + }; + + function render(options = {}) { + return renderWithReviewState( + , + {currentUser, commentThreads: [thread], ...options} + ); + } + + it('separates unseen replies from the ones already seen', () => { + const {getByText} = render({commentThreadReads: {5: '2026-08-17T10:00:00.000Z'}}); + + const divider = getByText('New replies'); + const seen = getByText('Older reply'); + const unseen = getByText('Newer reply'); + + expect(divider.compareDocumentPosition(seen) & Node.DOCUMENT_POSITION_PRECEDING) + .toBeTruthy(); + expect(divider.compareDocumentPosition(unseen) & Node.DOCUMENT_POSITION_FOLLOWING) + .toBeTruthy(); + }); + + it('renders no divider when every reply has been seen', () => { + const {queryByText} = render({commentThreadReads: {5: '2026-08-17T12:00:00.000Z'}}); + + expect(queryByText('New replies')).toBeNull(); + }); + + // The thread's own marker already says the whole thread is new. + it('renders no divider when the thread is new all through', () => { + const {queryByText} = render(); + + expect(queryByText('New replies')).toBeNull(); + }); + + it('renders no divider while replies are collapsed', () => { + const {queryByText} = renderWithReviewState( + , + { + currentUser, + commentThreads: [thread], + commentThreadReads: {5: '2026-08-17T10:00:00.000Z'} + } + ); + + expect(queryByText('New replies')).toBeNull(); + }); + + it('renders no divider before the current user own replies', () => { + const threadWithOwnReply = { + ...thread, + comments: [ + thread.comments[0], + thread.comments[1], + comment({ + id: 102, body: 'Will fix', creatorId: currentUser.id, creatorName: 'Alice', + createdAt: '2026-08-17T11:00:00.000Z' + }) + ] + }; + + const {queryByText} = renderWithReviewState( + , + { + currentUser, + commentThreads: [threadWithOwnReply], + commentThreadReads: {5: '2026-08-17T10:00:00.000Z'} + } + ); + + expect(queryByText('New replies')).toBeNull(); + }); +}); diff --git a/entry_types/scrolled/package/spec/review/Thread/features/quotes-spec.js b/entry_types/scrolled/package/spec/review/Thread/features/quotes-spec.js new file mode 100644 index 0000000000..585a3cd525 --- /dev/null +++ b/entry_types/scrolled/package/spec/review/Thread/features/quotes-spec.js @@ -0,0 +1,138 @@ +import React from 'react'; +import '@testing-library/jest-dom/extend-expect'; + +import {Thread} from 'review/Thread'; +import {review} from 'review/api'; +import {renderWithReviewState} from 'support/renderWithReviewState'; + +describe('Thread comment quotes', () => { + const thread = { + id: 1, + comments: [{id: 10, body: 'On the pull quote', creatorName: 'Bob', creatorId: 2}] + }; + + const seed = { + sections: [{id: 1, permaId: 1}], + contentElements: [ + { + id: 1, permaId: 10, sectionId: 1, typeName: 'textBlock', + configuration: {value: 'Current wording'} + } + ] + }; + + const quotingThread = { + ...thread, + subjectType: 'ContentElement', + subjectId: 10, + subjectRange: {anchor: {path: [0, 0], offset: 0}, focus: {path: [0, 0], offset: 7}} + }; + + function threadWithQuotes(...quotes) { + return { + ...quotingThread, + comments: quotes.map((quote, index) => ({ + id: 10 + index, + body: `Comment ${index + 1}`, + creatorName: 'Bob', + creatorId: 2, + quote + })) + }; + } + + beforeEach(() => { + review.contentElementTypes.register('textBlock', { + extractQuote: configuration => configuration.value + }); + }); + + afterEach(() => { + review.contentElementTypes.types = {}; + }); + + it('renders a quote for a comment whose text has since changed', () => { + const {getByText} = renderWithReviewState( + , + {seed} + ); + + expect(getByText('Original wording')).toBeInTheDocument(); + }); + + it('renders no quote while the text still reads the same', () => { + const {container} = renderWithReviewState( + , + {seed} + ); + + expect(container.querySelector('blockquote')).toBeNull(); + }); + + it('renders the quote inside the comment, above its body', () => { + const {getByText} = renderWithReviewState( + , + {seed} + ); + + const quote = getByText('Original wording'); + const body = getByText('Comment 1'); + + expect(quote.parentNode).toBe(body.parentNode); + expect(quote.compareDocumentPosition(body) & Node.DOCUMENT_POSITION_FOLLOWING) + .toBeTruthy(); + }); + + it('renders a quote per version once the text changes mid thread', () => { + const {getByText} = renderWithReviewState( + , + {seed} + ); + + expect(getByText('First wording')).toBeInTheDocument(); + expect(getByText('Second wording')).toBeInTheDocument(); + }); + + it('renders the quote once for a run of replies about the same wording', () => { + const {queryAllByText} = renderWithReviewState( + , + {seed} + ); + + expect(queryAllByText('Same wording')).toHaveLength(1); + }); + + it('renders no quote for the last comment when it matches the current text', () => { + const {getByText, queryByText} = renderWithReviewState( + , + {seed} + ); + + expect(getByText('Original wording')).toBeInTheDocument(); + expect(queryByText('Current wording')).not.toBeInTheDocument(); + }); + + it('renders quotes for threads whose content element was deleted', () => { + const {getByText} = renderWithReviewState( + , + {seed} + ); + + expect(getByText('Original wording')).toBeInTheDocument(); + }); + + it('renders no quote for comments recorded without one', () => { + const {container} = renderWithReviewState( + , + {seed} + ); + + expect(container.querySelector('blockquote')).toBeNull(); + }); +}); diff --git a/entry_types/scrolled/package/spec/review/Thread/features/replyForm-spec.js b/entry_types/scrolled/package/spec/review/Thread/features/replyForm-spec.js new file mode 100644 index 0000000000..abeccf5bf8 --- /dev/null +++ b/entry_types/scrolled/package/spec/review/Thread/features/replyForm-spec.js @@ -0,0 +1,110 @@ +import React from 'react'; +import '@testing-library/jest-dom/extend-expect'; +import userEvent from '@testing-library/user-event'; +import {useFakeTranslations} from 'pageflow/testHelpers'; + +import {Thread} from 'review/Thread'; +import {renderWithReviewState} from 'support/renderWithReviewState'; + +describe('Thread reply form', () => { + useFakeTranslations({ + 'pageflow_scrolled.review.reply_placeholder': 'Reply...', + 'pageflow_scrolled.review.reply_count.one': '1 reply', + 'pageflow_scrolled.review.reply_count.other': '%{count} replies', + 'pageflow_scrolled.review.toggle_replies': 'Toggle replies' + }); + + const thread = { + id: 1, + comments: [{id: 10, body: 'On the pull quote', creatorName: 'Bob', creatorId: 2}] + }; + + // The reply form is hidden while a thread with replies is collapsed, + // which would leave a drafted reply out of reach. + it('expands a collapsed thread that has a drafted reply', () => { + const threadWithReply = { + ...thread, + comments: [ + ...thread.comments, + {id: 11, body: 'A first reply', creatorName: 'Alice', creatorId: 1} + ] + }; + + const {getByPlaceholderText} = renderWithReviewState( + , + { + drafts: { + 'Thread:1': {threadId: 1, body: 'Half a reply', pending: false} + } + } + ); + + expect(getByPlaceholderText('Reply...')).toHaveValue('Half a reply'); + }); + + // Two textareas in one card leave it ambiguous which one typing lands in. + describe('while editing a comment', () => { + useFakeTranslations({ + 'pageflow_scrolled.review.comment_actions': 'Comment actions', + 'pageflow_scrolled.review.edit_comment': 'Edit', + 'pageflow_scrolled.review.cancel': 'Cancel' + }); + + const ownThread = { + id: 1, + comments: [ + {id: 10, body: 'On the pull quote', creatorName: 'Bob', creatorId: 2}, + {id: 11, body: 'A first reply', creatorName: 'Bob', creatorId: 2} + ] + }; + + async function startEditing(user, getAllByRole, index) { + await user.click(getAllByRole('button', {name: 'Comment actions'})[index]); + await user.click(getAllByRole('menuitem', {name: 'Edit'})[0]); + } + + it('hides the reply form', async () => { + const user = userEvent.setup(); + + const {getAllByRole, queryByPlaceholderText} = renderWithReviewState( + , + {currentUser: {id: 2, name: 'Bob'}} + ); + + expect(queryByPlaceholderText('Reply...')).toBeInTheDocument(); + + await startEditing(user, getAllByRole, 0); + + expect(queryByPlaceholderText('Reply...')).toBeNull(); + }); + + it('restores the reply form when editing ends', async () => { + const user = userEvent.setup(); + + const {getAllByRole, getByRole, queryByPlaceholderText} = renderWithReviewState( + , + {currentUser: {id: 2, name: 'Bob'}} + ); + + await startEditing(user, getAllByRole, 0); + await user.click(getByRole('button', {name: 'Cancel'})); + + expect(queryByPlaceholderText('Reply...')).toBeInTheDocument(); + }); + + it('keeps only one comment in edit mode', async () => { + const user = userEvent.setup(); + + const {getAllByRole} = renderWithReviewState( + , + {currentUser: {id: 2, name: 'Bob'}} + ); + + await startEditing(user, getAllByRole, 0); + await startEditing(user, getAllByRole, 1); + + expect(getAllByRole('textbox')).toHaveLength(1); + expect(getAllByRole('textbox')[0]).toHaveValue('A first reply'); + }); + }); +}); diff --git a/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js b/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js index f992922aa9..1a9575fa2d 100644 --- a/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js +++ b/entry_types/scrolled/package/spec/review/ThreadsBadge-spec.js @@ -1,5 +1,6 @@ import React from 'react'; import '@testing-library/jest-dom/extend-expect'; +import {useFakeTranslations} from 'pageflow/testHelpers'; import {ThreadsBadge} from 'review/ThreadsBadge'; import {renderWithReviewState} from 'support/renderWithReviewState'; @@ -12,11 +13,92 @@ const seed = { contentElements: [{id: 1, permaId: 10, sectionId: 1, typeName: 'textBlock'}] }; -function renderThreadsBadge(ui, {commentThreads = []} = {}) { - return renderWithReviewState(ui, {seed, commentThreads}); +function renderThreadsBadge(ui, {commentThreads = [], ...options} = {}) { + return renderWithReviewState(ui, {seed, commentThreads, ...options}); } describe('ThreadsBadge', () => { + describe('unread comments', () => { + useFakeTranslations({ + 'pageflow_scrolled.review.unread_comment_count.one': '1 unread comment', + 'pageflow_scrolled.review.unread_comment_count.other': '%{count} unread comments' + }); + + const currentUser = {id: 42, name: 'Alice'}; + + function threadWithComment(attributes) { + return { + id: 1, + permaId: 5, + subjectType: 'ContentElement', + subjectId: 10, + comments: [{ + id: 100, + creatorId: 43, + createdAt: '2026-08-17T11:00:00.000Z', + ...attributes + }] + }; + } + + it('marks badge as unread and names the count', () => { + const {getByRole} = renderThreadsBadge( + , + {currentUser, commentThreads: [threadWithComment()]} + ); + + expect(getByRole('status')).toHaveClass(badgeStyles.unread); + expect(getByRole('status')).toHaveAttribute('aria-label', '1 unread comment'); + }); + + it('counts unread comments across threads of the subject', () => { + const {getByRole} = renderThreadsBadge( + , + { + currentUser, + commentThreads: [ + threadWithComment(), + {...threadWithComment({id: 101}), id: 2, permaId: 6} + ] + } + ); + + expect(getByRole('status')).toHaveAttribute('aria-label', '2 unread comments'); + }); + + it('does not mark badge as unread once comments have been read', () => { + const {getByRole} = renderThreadsBadge( + , + { + currentUser, + commentThreads: [threadWithComment()], + commentThreadReads: {5: '2026-08-17T12:00:00.000Z'} + } + ); + + expect(getByRole('status')).not.toHaveClass(badgeStyles.unread); + expect(getByRole('status')).not.toHaveAttribute('aria-label'); + }); + + it('does not mark badge as unread for own comments', () => { + const {getByRole} = renderThreadsBadge( + , + {currentUser, commentThreads: [threadWithComment({creatorId: currentUser.id})]} + ); + + expect(getByRole('status')).not.toHaveClass(badgeStyles.unread); + }); + + it('does not mark badge as unread before current user is known', () => { + const {getByRole} = renderThreadsBadge( + , + {commentThreads: [threadWithComment()]} + ); + + expect(getByRole('status')).not.toHaveClass(badgeStyles.unread); + }); + }); + it('does not display count for single thread', () => { const {getByRole} = renderThreadsBadge( , @@ -185,6 +267,28 @@ describe('ThreadsBadge', () => { expect(getByRole('status')).toBeInTheDocument(); expect(getByRole('status')).not.toHaveTextContent(/\d/); }); + + // Two dots on one badge say less than one, so unseen comments keep + // the badge from collapsing. + it('renders full badge when comments are unread', () => { + const {container, getByRole} = renderThreadsBadge( + , + { + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [ + { + id: 1, permaId: 5, subjectType: 'ContentElement', subjectId: 10, + comments: [{ + id: 100, creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z' + }] + } + ] + } + ); + + expect(getByRole('status')).not.toHaveClass(badgeStyles.dot); + expect(container.querySelector(`.${badgeStyles.icon}`)).not.toBeNull(); + }); }); describe('mode active', () => { diff --git a/entry_types/scrolled/package/spec/review/commentThreadReadsSnapshot-spec.js b/entry_types/scrolled/package/spec/review/commentThreadReadsSnapshot-spec.js new file mode 100644 index 0000000000..96773da29b --- /dev/null +++ b/entry_types/scrolled/package/spec/review/commentThreadReadsSnapshot-spec.js @@ -0,0 +1,115 @@ +import React from 'react'; +import '@testing-library/jest-dom/extend-expect'; +import {act} from '@testing-library/react'; + +import { + CommentThreadReadsSnapshot, useDisplayedCommentThreadReads +} from 'review/commentThreadReadsSnapshot'; +import { + postReviewStateResetMessage, postReviewStateReadsChangeMessage +} from 'review/postMessage'; +import {renderWithReviewState} from 'support/renderWithReviewState'; + +describe('CommentThreadReadsSnapshot', () => { + const currentUser = {id: 42, name: 'Alice'}; + const commentThreads = [{id: 1, permaId: 5, subjectType: 'CE', subjectId: 10, comments: []}]; + + function DisplayedReads() { + return {JSON.stringify(useDisplayedCommentThreadReads())}; + } + + // Messages are delivered in a later task, so posting has to be + // flushed before the rendered output reflects them. + async function post(postMessage) { + await act(async () => { + postMessage(); + await new Promise(resolve => setTimeout(resolve, 0)); + }); + } + + function postReadsChange(reads) { + return post(() => postReviewStateReadsChangeMessage(window, reads)); + } + + function reads(getByTestId) { + return JSON.parse(getByTestId('reads').textContent); + } + + it('follows live read state without a snapshot', async () => { + const {getByTestId} = renderWithReviewState( + , + {currentUser, commentThreads, commentThreadReads: {}} + ); + + await postReadsChange({5: '2026-08-17T12:00:00.000Z'}); + + expect(reads(getByTestId)).toEqual({5: '2026-08-17T12:00:00.000Z'}); + }); + + it('keeps the read state it was mounted with', async () => { + const {getByTestId} = renderWithReviewState( + + + , + {currentUser, commentThreads, commentThreadReads: {5: '2026-08-17T10:00:00.000Z'}} + ); + + await postReadsChange({5: '2026-08-17T12:00:00.000Z'}); + + expect(reads(getByTestId)).toEqual({5: '2026-08-17T10:00:00.000Z'}); + }); + + it('follows live read state while disabled', async () => { + const {getByTestId} = renderWithReviewState( + + + , + {currentUser, commentThreads, commentThreadReads: {}} + ); + + await postReadsChange({5: '2026-08-17T12:00:00.000Z'}); + + expect(reads(getByTestId)).toEqual({5: '2026-08-17T12:00:00.000Z'}); + }); + + it('waits for the current user before freezing', async () => { + const {getByTestId} = renderWithReviewState( + + + , + {commentThreads, commentThreadReads: {}} + ); + + await post(() => postReviewStateResetMessage(window, { + currentUser, + commentThreads, + commentThreadReads: {5: '2026-08-17T10:00:00.000Z'} + })); + await postReadsChange({5: '2026-08-17T12:00:00.000Z'}); + + expect(reads(getByTestId)).toEqual({5: '2026-08-17T10:00:00.000Z'}); + }); + + it('reuses the outer snapshot when nested', async () => { + const {getByTestId} = renderWithReviewState( + + + , + {currentUser, commentThreads, commentThreadReads: {5: '2026-08-17T10:00:00.000Z'}} + ); + + await postReadsChange({5: '2026-08-17T12:00:00.000Z'}); + + expect(reads(getByTestId)).toEqual({5: '2026-08-17T10:00:00.000Z'}); + }); + + // Mounted after the outer snapshot froze, as a thread list opening + // inside an already frozen scope does. + function NestedSnapshot() { + return ( + + + + ); + } +}); diff --git a/entry_types/scrolled/package/spec/review/unreadComments-spec.js b/entry_types/scrolled/package/spec/review/unreadComments-spec.js new file mode 100644 index 0000000000..b02437e3b6 --- /dev/null +++ b/entry_types/scrolled/package/spec/review/unreadComments-spec.js @@ -0,0 +1,149 @@ +import {unreadComments, useUnreadComments} from 'review/unreadComments'; +import {renderHookWithReviewState} from 'support/renderWithReviewState'; + +describe('unreadComments', () => { + const currentUser = {id: 42, name: 'Alice'}; + + function thread(comments) { + return {id: 1, permaId: 5, comments}; + } + + it('returns comments created after read timestamp', () => { + const result = unreadComments( + thread([ + {id: 100, creatorId: 43, createdAt: '2026-08-17T09:00:00.000Z'}, + {id: 101, creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z'} + ]), + {currentUser, readAt: '2026-08-17T10:00:00.000Z'} + ); + + expect(result.map(comment => comment.id)).toEqual([101]); + }); + + it('returns all comments of never read thread', () => { + const result = unreadComments( + thread([ + {id: 100, creatorId: 43, createdAt: '2026-08-17T09:00:00.000Z'}, + {id: 101, creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z'} + ]), + {currentUser, readAt: undefined} + ); + + expect(result.map(comment => comment.id)).toEqual([100, 101]); + }); + + it('excludes comments of current user', () => { + const result = unreadComments( + thread([ + {id: 100, creatorId: 42, createdAt: '2026-08-17T11:00:00.000Z'}, + {id: 101, creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z'} + ]), + {currentUser, readAt: undefined} + ); + + expect(result.map(comment => comment.id)).toEqual([101]); + }); + + it('returns nothing while current user is unknown', () => { + const result = unreadComments( + thread([{id: 100, creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z'}]), + {currentUser: null, readAt: undefined} + ); + + expect(result).toEqual([]); + }); + + describe('with a baseline on the current user', () => { + const joinedUser = {...currentUser, unreadCommentsSinceAt: '2026-08-17T10:00:00.000Z'}; + + it('ignores comments from before the baseline', () => { + const result = unreadComments( + thread([{id: 100, creatorId: 43, createdAt: '2026-08-17T09:00:00.000Z'}]), + {currentUser: joinedUser, readAt: undefined} + ); + + expect(result).toEqual([]); + }); + + it('returns comments from after the baseline', () => { + const result = unreadComments( + thread([{id: 100, creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z'}]), + {currentUser: joinedUser, readAt: undefined} + ); + + expect(result.map(comment => comment.id)).toEqual([100]); + }); + + // A thread read after the baseline has moved past it. + it('prefers a later read timestamp over the baseline', () => { + const result = unreadComments( + thread([{id: 100, creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z'}]), + {currentUser: joinedUser, readAt: '2026-08-17T12:00:00.000Z'} + ); + + expect(result).toEqual([]); + }); + + // A thread last read before the baseline says nothing newer than it. + it('prefers the baseline over an earlier read timestamp', () => { + const result = unreadComments( + thread([{id: 100, creatorId: 43, createdAt: '2026-08-17T09:30:00.000Z'}]), + {currentUser: joinedUser, readAt: '2026-08-17T09:00:00.000Z'} + ); + + expect(result).toEqual([]); + }); + }); + + it('compares timestamps of different time zone offsets', () => { + const result = unreadComments( + thread([{id: 100, creatorId: 43, createdAt: '2026-08-17T12:00:00.000+02:00'}]), + {currentUser, readAt: '2026-08-17T11:00:00.000Z'} + ); + + expect(result).toEqual([]); + }); + + describe('useUnreadComments', () => { + it('reads current user and read timestamp from review state', () => { + const commentThread = { + id: 1, + permaId: 5, + subjectType: 'ContentElement', + subjectId: 10, + comments: [ + {id: 100, creatorId: 43, createdAt: '2026-08-17T09:00:00.000Z'}, + {id: 101, creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z'} + ] + }; + + const {result} = renderHookWithReviewState( + () => useUnreadComments(commentThread), + { + currentUser, + commentThreads: [commentThread], + commentThreadReads: {5: '2026-08-17T10:00:00.000Z'} + } + ); + + expect(result.current.map(comment => comment.id)).toEqual([101]); + }); + + it('returns nothing before review state has been fetched', () => { + const commentThread = { + id: 1, + permaId: 5, + subjectType: 'ContentElement', + subjectId: 10, + comments: [{id: 100, creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z'}] + }; + + const {result} = renderHookWithReviewState( + () => useUnreadComments(commentThread), + {commentThreads: [commentThread]} + ); + + expect(result.current).toEqual([]); + }); + }); +}); diff --git a/entry_types/scrolled/package/spec/review/watchUnreadComments-spec.js b/entry_types/scrolled/package/spec/review/watchUnreadComments-spec.js new file mode 100644 index 0000000000..d77df4eb11 --- /dev/null +++ b/entry_types/scrolled/package/spec/review/watchUnreadComments-spec.js @@ -0,0 +1,120 @@ +import BackboneEvents from 'backbone-events-standalone'; +import {Model} from 'backbone'; + +import {watchUnreadComments} from 'review/watchUnreadComments'; + +describe('watchUnreadComments', () => { + const currentUser = {id: 42, name: 'Alice'}; + + function fakeSession(state = null) { + const session = {state}; + Object.assign(session, BackboneEvents); + return session; + } + + function thread(attributes = {}) { + return { + id: 1, + permaId: 5, + comments: [{ + id: 100, creatorId: 43, createdAt: '2026-08-17T11:00:00.000Z' + }], + ...attributes + }; + } + + function watch(state) { + const entry = new Model(); + const session = fakeSession(state); + + watchUnreadComments({entry, session}); + + return {entry, session}; + } + + it('is false before the session has state', () => { + const {entry} = watch(null); + + expect(entry.get('hasUnreadComments')).toBe(false); + }); + + it('is true while a thread holds unseen comments', () => { + const {entry} = watch({ + currentUser, + commentThreads: [thread()], + commentThreadReads: {} + }); + + expect(entry.get('hasUnreadComments')).toBe(true); + }); + + it('is false once every comment has been read', () => { + const {entry} = watch({ + currentUser, + commentThreads: [thread()], + commentThreadReads: {5: '2026-08-17T12:00:00.000Z'} + }); + + expect(entry.get('hasUnreadComments')).toBe(false); + }); + + it('is false for own comments', () => { + const {entry} = watch({ + currentUser, + commentThreads: [thread({ + comments: [{id: 100, creatorId: currentUser.id, createdAt: '2026-08-17T11:00:00.000Z'}] + })], + commentThreadReads: {} + }); + + expect(entry.get('hasUnreadComments')).toBe(false); + }); + + it('follows threads arriving with a fetch', () => { + const {entry, session} = watch(null); + + session.state = { + currentUser, + commentThreads: [thread()], + commentThreadReads: {} + }; + session.trigger('reset', session.state); + + expect(entry.get('hasUnreadComments')).toBe(true); + }); + + it('follows threads being read', () => { + const {entry, session} = watch({ + currentUser, + commentThreads: [thread()], + commentThreadReads: {} + }); + + session.state = { + ...session.state, + commentThreadReads: {5: '2026-08-17T12:00:00.000Z'} + }; + session.trigger('change:reads', session.state.commentThreadReads); + + expect(entry.get('hasUnreadComments')).toBe(false); + }); + + it('follows replies being added', () => { + const {entry, session} = watch({ + currentUser, + commentThreads: [thread()], + commentThreadReads: {5: '2026-08-17T12:00:00.000Z'} + }); + + const updatedThread = thread({ + comments: [ + ...thread().comments, + {id: 101, creatorId: 44, createdAt: '2026-08-17T13:00:00.000Z'} + ] + }); + session.state = {...session.state, commentThreads: [updatedThread]}; + session.trigger('change:thread', updatedThread); + + expect(entry.get('hasUnreadComments')).toBe(true); + }); +}); diff --git a/entry_types/scrolled/package/spec/support/pageObjects/commenting.js b/entry_types/scrolled/package/spec/support/pageObjects/commenting.js index 6efefe02c9..bac9dd65f0 100644 --- a/entry_types/scrolled/package/spec/support/pageObjects/commenting.js +++ b/entry_types/scrolled/package/spec/support/pageObjects/commenting.js @@ -4,6 +4,7 @@ import {useFakeTranslations} from 'pageflow/testHelpers'; import {loadCommentingExtensions} from 'frontend/commenting'; import {clearExtensions} from 'frontend/extensionRegistry'; import contentElementDecoratorStyles from 'frontend/commenting/ContentElementDecorator.module.css'; +import badgeStyles from 'review/Badge.module.css'; import { renderEntry as baseRenderEntry, @@ -26,14 +27,18 @@ export function renderEntry({ getCommentToolbar: () => result.getByRole('group', {name: 'Comments'}), queryCommentToolbar: () => result.queryByRole('group', {name: 'Comments'}), getHideCommentsButton: () => result.getByRole('button', {name: 'Hide comments'}), - getShowCommentsButton: () => result.getByRole('button', {name: 'Show comments'}), - queryShowCommentsButton: () => result.queryByRole('button', {name: 'Show comments'}), + // Matched by prefix since the button also names unread comments. + getShowCommentsButton: () => result.getByRole('button', {name: /^Show comments/}), + queryShowCommentsButton: () => result.queryByRole('button', {name: /^Show comments/}), getAddCommentButton: () => result.getByRole('button', {name: 'Add comment'}), getCancelAddCommentButton: () => result.getByRole('button', {name: 'Cancel add comment'}), getNewThreadInput: () => result.getByPlaceholderText('Add a comment...'), queryNewThreadInput: () => result.queryByPlaceholderText('Add a comment...'), getAllCommentBadges: () => result.getAllByRole('status'), queryAllCommentBadges: () => result.queryAllByRole('status'), + queryAllUnreadCommentBadges: () => result.queryAllByRole('status').filter( + badge => badge.classList.contains(badgeStyles.unread) + ), getCommentFilterButton: resolution => result.getByRole('button', {name: resolution === 'all' ? 'All' : 'Unresolved'}), getPreviousCommentButton: () => result.getByRole('button', {name: 'Previous comment'}), diff --git a/entry_types/scrolled/package/spec/support/renderWithReviewState.js b/entry_types/scrolled/package/spec/support/renderWithReviewState.js index ad31f6435c..1035e58ace 100644 --- a/entry_types/scrolled/package/spec/support/renderWithReviewState.js +++ b/entry_types/scrolled/package/spec/support/renderWithReviewState.js @@ -9,25 +9,25 @@ import {LocatedCommentThreadsProvider} from 'review/useLocatedCommentThreads'; // lives in), so it needs the entry scope just like any other frontend // component — review state is layered on top via the entry helper's // `wrapper` option. -export function renderWithReviewState(ui, {commentThreads = [], currentUser = null, drafts, setDraft, seed = {}} = {}) { +export function renderWithReviewState(ui, {commentThreads = [], commentThreadReads = {}, currentUser = null, drafts, setDraft, seed = {}} = {}) { return renderInEntry(ui, { seed, - wrapper: reviewStateWrapper({commentThreads, currentUser, drafts, setDraft}) + wrapper: reviewStateWrapper({commentThreads, commentThreadReads, currentUser, drafts, setDraft}) }); } // Counterpart for selector hooks that read entry state and review state. -export function renderHookWithReviewState(hook, {commentThreads = [], currentUser = null, drafts, setDraft, seed = {}} = {}) { +export function renderHookWithReviewState(hook, {commentThreads = [], commentThreadReads = {}, currentUser = null, drafts, setDraft, seed = {}} = {}) { return renderHookInEntry(hook, { seed, - wrapper: reviewStateWrapper({commentThreads, currentUser, drafts, setDraft}) + wrapper: reviewStateWrapper({commentThreads, commentThreadReads, currentUser, drafts, setDraft}) }); } -function reviewStateWrapper({commentThreads = [], currentUser = null, drafts, setDraft} = {}) { +function reviewStateWrapper({commentThreads = [], commentThreadReads = {}, currentUser = null, drafts, setDraft} = {}) { return function ReviewStateWrapper({children}) { return ( - diff --git a/entry_types/scrolled/package/src/editor/config.js b/entry_types/scrolled/package/src/editor/config.js index 4a258fd210..056eb53672 100644 --- a/entry_types/scrolled/package/src/editor/config.js +++ b/entry_types/scrolled/package/src/editor/config.js @@ -63,7 +63,8 @@ editor.addInitializer(() => { editor.registerMainMenuItem({ translationKey: 'pageflow_scrolled.editor.main_menu.comments', path: '/scrolled/comments', - id: 'comments' + id: 'comments', + indicatorAttribute: 'hasUnreadComments' }); } }); diff --git a/entry_types/scrolled/package/src/editor/models/ScrolledEntry/index.js b/entry_types/scrolled/package/src/editor/models/ScrolledEntry/index.js index 63a49cff12..41f8b9d00e 100644 --- a/entry_types/scrolled/package/src/editor/models/ScrolledEntry/index.js +++ b/entry_types/scrolled/package/src/editor/models/ScrolledEntry/index.js @@ -2,6 +2,7 @@ import {Entry, editor} from 'pageflow/editor'; import I18n from 'i18n-js'; import {features} from 'pageflow/frontend'; import {createReviewSession} from 'pageflow/review'; +import {watchUnreadComments} from 'pageflow-scrolled/review'; import {ConsentVendors} from '../ConsentVendors'; @@ -81,6 +82,7 @@ export const ScrolledEntry = Entry.extend({ if (features.isEnabled('commenting')) { this.reviewSession = createReviewSession({entryId: this.id}); + watchUnreadComments({entry: this, session: this.reviewSession}); } }, diff --git a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js index 41c401ad77..e4483f85ab 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js +++ b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.js @@ -1,7 +1,7 @@ import React, {useEffect} from 'react'; import classNames from 'classnames'; -import {useLocatedCommentThreads} from 'pageflow-scrolled/review'; +import {useLocatedCommentThreads, useUnreadCommentCount} from 'pageflow-scrolled/review'; import {useI18n} from '../i18n'; import {useAddCommentMode} from './AddCommentModeProvider'; import {useCommentDisplayFilter} from './CommentDisplayFilterProvider'; @@ -62,7 +62,17 @@ function HideCommentsButton() { function ShowCommentsButton() { const {t} = useI18n({locale: 'ui'}); const {toggle} = useCommentingVisibility(); - const label = t('pageflow_scrolled.review.show_comments'); + const {threads} = useLocatedCommentThreads(); + + const unreadCommentCount = useUnreadCommentCount(threads); + const unread = unreadCommentCount > 0; + + // Named rather than only marked, since the dot is the sole cue that + // comments are waiting behind the collapsed toolbar. + const label = unread ? + t('pageflow_scrolled.review.show_comments_with_unread', + {count: unreadCommentCount}) : + t('pageflow_scrolled.review.show_comments'); return ( ); } diff --git a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.module.css b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.module.css index 9add2f8ee8..814224bded 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.module.css +++ b/entry_types/scrolled/package/src/frontend/commenting/FloatingToolbar.module.css @@ -4,10 +4,10 @@ puck together (the content deforms and cross-fades along with the rect, having no counterpart in the puck). The toggle icons (eye and bubble) share `comment-toolbar-icon` and morph on their own — naming them lifts them out - of the surface snapshot so the icon swap stays crisp. The - ::view-transition old/new overrides below stretch the surface snapshots to - fill the animating box, so it reshapes into the puck rather than scaling - down with a fixed aspect ratio. */ + of the surface snapshot so the icon swap stays crisp, and the unread dot is + named for the same reason. The ::view-transition old/new overrides below + stretch the surface snapshots to fill the animating box, so it reshapes into + the puck rather than scaling down with a fixed aspect ratio. */ .toolbar { position: fixed; bottom: space(3); @@ -74,6 +74,21 @@ animation: none; } +.unreadDot { + position: absolute; + top: space(-1); + right: space(-1); + width: space(2); + height: space(2); + border-radius: 50%; + background: var(--ui-warning-color); + box-shadow: 0 0 0 1px var(--ui-on-surface-color-lighter); + + /* Naming the dot lifts it out of the puck's surface snapshot, so it + does not deform along with the rect while the toolbar expands. */ + view-transition-name: comment-toolbar-unread-dot; +} + @keyframes puckIdle { from { opacity: 1; diff --git a/entry_types/scrolled/package/src/frontend/commenting/Popover.js b/entry_types/scrolled/package/src/frontend/commenting/Popover.js index 673bee4db9..4857d111a2 100644 --- a/entry_types/scrolled/package/src/frontend/commenting/Popover.js +++ b/entry_types/scrolled/package/src/frontend/commenting/Popover.js @@ -4,7 +4,7 @@ import { offset, flip, shift, autoUpdate } from '@floating-ui/react'; -import {ThreadsBadge, ThreadList} from 'pageflow-scrolled/review'; +import {ThreadsBadge, ThreadList, CommentThreadReadsSnapshot} from 'pageflow-scrolled/review'; import {useFloatingPortalRoot} from '../FloatingPortalRootProvider'; import {useCommentDisplayFilter} from './CommentDisplayFilterProvider'; import {useSelectedSubject} from './SelectedSubjectProvider'; @@ -43,24 +43,26 @@ export function Popover({ return ( - - {isSelected && - } + + + {isSelected && + } + ); } diff --git a/entry_types/scrolled/package/src/review/Badge.js b/entry_types/scrolled/package/src/review/Badge.js index 0cc0f23295..ac3d2bc04d 100644 --- a/entry_types/scrolled/package/src/review/Badge.js +++ b/entry_types/scrolled/package/src/review/Badge.js @@ -4,8 +4,10 @@ import classNames from 'classnames'; import CommentIcon from './images/comment.svg'; import styles from './Badge.module.css'; -export const Badge = forwardRef(function Badge({counter, mode, resolved, onClick}, ref) { - const variant = resolveVariant(mode, counter > 0); +export const Badge = forwardRef(function Badge({ + counter, mode, resolved, unread, label, onClick +}, ref) { + const variant = resolveVariant(mode, counter > 0, unread); if (!variant) { return null; @@ -14,8 +16,10 @@ export const Badge = forwardRef(function Badge({counter, mode, resolved, onClick return ( } {!collapsed && replies.map(comment => ( - + + {comment.id === firstNewReplyId && +
+ {t('pageflow_scrolled.review.new_replies')} +
} + + ))} {interactive && !thread.resolvedAt && !repliesCollapsed && !editing && diff --git a/entry_types/scrolled/package/src/review/Thread.module.css b/entry_types/scrolled/package/src/review/Thread.module.css index 4334579c34..b4d83849ae 100644 --- a/entry_types/scrolled/package/src/review/Thread.module.css +++ b/entry_types/scrolled/package/src/review/Thread.module.css @@ -29,6 +29,54 @@ background: var(--review-resolved-thread-background, var(--ui-surface-color)); } +/* Locates the new thread among several. Matches the badge dot, which + carries the same meaning one level up. Straddles the corner so it + stays clear of the chevron button inside it. */ +.newDot { + position: absolute; + top: space(-1); + right: space(-1); + width: space(2); + height: space(2); + border-radius: 50%; + background: var(--ui-warning-color); + box-shadow: 0 0 0 1px var(--ui-on-surface-color-lighter); +} + +.replyCount { + display: flex; + align-items: center; + gap: space(2); +} + +.newReplyCount::before { + content: '·'; + margin-right: space(2); + color: var(--ui-on-surface-color-light); +} + +.newReplyCount { + color: var(--ui-warning-color); + font-weight: 600; +} + +.newRepliesDivider { + display: flex; + align-items: center; + gap: space(2); + font-weight: 500; + color: var(--ui-warning-color); +} + +.newRepliesDivider::before, +.newRepliesDivider::after { + content: ''; + flex: 1; + height: 1px; + background: var(--ui-warning-color); + opacity: 0.4; +} + .deletedHint { margin: 0; font-size: space(3); @@ -69,14 +117,14 @@ padding: 0 0 0 space(8); font: inherit; font-weight: 500; - color: var(--ui-primary-color); + color: var(--ui-on-surface-color-light); background: none; border: none; cursor: pointer; } .expandButton:hover { - text-decoration: underline; + color: var(--ui-primary-color); } .resolveRow { diff --git a/entry_types/scrolled/package/src/review/ThreadList.js b/entry_types/scrolled/package/src/review/ThreadList.js index e29b1e608a..e9b8dfff5c 100644 --- a/entry_types/scrolled/package/src/review/ThreadList.js +++ b/entry_types/scrolled/package/src/review/ThreadList.js @@ -4,6 +4,7 @@ import classNames from 'classnames'; import {useI18n} from 'pageflow-scrolled/frontend'; import {useCommentDraft} from './ReviewStateProvider'; import {useLocatedCommentThreadsForSubject} from './useLocatedCommentThreadsForSubject'; +import {CommentThreadReadsSnapshot} from './commentThreadReadsSnapshot'; import {Thread} from './Thread'; import {NewThreadForm} from './NewThreadForm'; import {postUpdateThreadMessage} from './postMessage'; @@ -59,58 +60,62 @@ export function ThreadList({subjectType, subjectId, subjectRange, filter, highli } return ( -
- {!showNewForm && !hideNewTopicButton && - } - - {showNewForm && - setFormToggled(false)} />} - - {noThreads && !showNewForm && -

- {t('pageflow_scrolled.review.no_threads_yet')} -

} - - {activeThreads.map(thread => ( - 1 && expandedThreadId !== thread.id} - onToggle={() => toggleThread(thread.id)} - onResolve={() => postUpdateThreadMessage({threadId: thread.id, resolved: true})} - onClick={onThreadClick && (() => onThreadClick(thread))} - highlighted={isHighlighted(thread)} - interactive={!restrictInteractionsToHighlighted || isHighlighted(thread)} /> - ))} - - {resolvedThreads.length > 0 && -
- - - {showResolved && resolvedThreads.map(thread => ( - 1 && expandedThreadId !== thread.id} - onToggle={() => toggleThread(thread.id)} - onResolve={() => postUpdateThreadMessage({threadId: thread.id, resolved: false})} - onClick={onThreadClick && (() => onThreadClick(thread))} - highlighted={isHighlighted(thread)} - interactive={!restrictInteractionsToHighlighted || isHighlighted(thread)} /> - ))} -
} -
+ +
+ {!showNewForm && !hideNewTopicButton && + } + + {showNewForm && + setFormToggled(false)} />} + + {noThreads && !showNewForm && +

+ {t('pageflow_scrolled.review.no_threads_yet')} +

} + + {activeThreads.map(thread => ( + 1 && expandedThreadId !== thread.id} + showNewMarker={activeThreads.length > 1} + onToggle={() => toggleThread(thread.id)} + onResolve={() => postUpdateThreadMessage({threadId: thread.id, resolved: true})} + onClick={onThreadClick && (() => onThreadClick(thread))} + highlighted={isHighlighted(thread)} + interactive={!restrictInteractionsToHighlighted || isHighlighted(thread)} /> + ))} + + {resolvedThreads.length > 0 && +
+ + + {showResolved && resolvedThreads.map(thread => ( + 1 && expandedThreadId !== thread.id} + showNewMarker={resolvedThreads.length > 1} + onToggle={() => toggleThread(thread.id)} + onResolve={() => postUpdateThreadMessage({threadId: thread.id, resolved: false})} + onClick={onThreadClick && (() => onThreadClick(thread))} + highlighted={isHighlighted(thread)} + interactive={!restrictInteractionsToHighlighted || isHighlighted(thread)} /> + ))} +
} +
+ ); } diff --git a/entry_types/scrolled/package/src/review/ThreadsBadge.js b/entry_types/scrolled/package/src/review/ThreadsBadge.js index f7ad19ef30..c02fa60e87 100644 --- a/entry_types/scrolled/package/src/review/ThreadsBadge.js +++ b/entry_types/scrolled/package/src/review/ThreadsBadge.js @@ -1,19 +1,33 @@ import React, {useCallback} from 'react'; +import {useI18n} from 'pageflow-scrolled/frontend'; import {useLocatedCommentThreadsForSubject} from './useLocatedCommentThreadsForSubject'; +import {useUnreadCommentCount} from './unreadComments'; import {Badge} from './Badge'; export function ThreadsBadge({subjectType, subjectId, subjectRange, onClick, mode, resolution = 'unresolved'}) { + const {t} = useI18n({locale: 'ui'}); + const threads = useLocatedCommentThreadsForSubject({subjectType, subjectId, subjectRange, resolution}); const unresolvedThreads = useLocatedCommentThreadsForSubject({subjectType, subjectId, subjectRange, resolution: 'unresolved'}); + const unreadCommentCount = useUnreadCommentCount(threads); + const handleClick = useCallback(() => { if (onClick) onClick(threads); }, [onClick, threads]); const resolved = threads.length > 0 && unresolvedThreads.length === 0; - return ; + return 0} + label={unreadCommentCount > 0 ? + t('pageflow_scrolled.review.unread_comment_count', + {count: unreadCommentCount}) : + undefined} + onClick={handleClick} />; } diff --git a/entry_types/scrolled/package/src/review/commentThreadReadsSnapshot.js b/entry_types/scrolled/package/src/review/commentThreadReadsSnapshot.js new file mode 100644 index 0000000000..87f1825c96 --- /dev/null +++ b/entry_types/scrolled/package/src/review/commentThreadReadsSnapshot.js @@ -0,0 +1,45 @@ +import React, {createContext, useContext, useRef} from 'react'; + +import {useCommentThreadReads, useCurrentUser} from './ReviewStateProvider'; + +const CommentThreadReadsSnapshotContext = createContext(null); + +// Threads mark themselves read while they are on screen, so markers +// derived from live read state would disappear from under the reviewer +// mid-read. Displaying threads therefore freezes read state for as long +// as they are shown: markers hold still until the list goes away, and +// the next visit reflects what was read. +// +// Nesting reuses the outermost snapshot, so a list rendered inside an +// already frozen scope keeps that scope's idea of what is new. +export function CommentThreadReadsSnapshot({enabled = true, children}) { + const outerSnapshot = useContext(CommentThreadReadsSnapshotContext); + const liveReads = useCommentThreadReads(); + + // Read state only means anything once the current user is known. + // Freezing before that would keep every thread marked new. + const currentUser = useCurrentUser(); + const snapshot = useRef(null); + + if (!enabled) { + snapshot.current = null; + } + else if (!snapshot.current && currentUser) { + snapshot.current = liveReads; + } + + return ( + + {children} + + ); +} + +// Read state to derive markers from. Falls back to live state where +// nothing has been frozen. +export function useDisplayedCommentThreadReads() { + const snapshot = useContext(CommentThreadReadsSnapshotContext); + const liveReads = useCommentThreadReads(); + + return snapshot || liveReads; +} diff --git a/entry_types/scrolled/package/src/review/index.js b/entry_types/scrolled/package/src/review/index.js index bf53c4da55..6a67404272 100644 --- a/entry_types/scrolled/package/src/review/index.js +++ b/entry_types/scrolled/package/src/review/index.js @@ -3,9 +3,12 @@ export {ReviewStateProvider, useCommentThreads, useCommentThread} from './Review export {LocatedCommentThreadsProvider, useLocatedCommentThreads} from './useLocatedCommentThreads'; export {useLocatedCommentThreadsForSubject} from './useLocatedCommentThreadsForSubject'; export {ReviewMessageHandler} from './ReviewMessageHandler'; +export {watchUnreadComments} from './watchUnreadComments'; +export {useUnreadCommentCount} from './unreadComments'; export {ThreadsBadge} from './ThreadsBadge'; export {Badge} from './Badge'; export {ThreadList} from './ThreadList'; +export {CommentThreadReadsSnapshot} from './commentThreadReadsSnapshot'; export {Thread} from './Thread'; export {ScrollHighlightedThreadIntoViewProvider} from './scrollHighlightedThreadIntoView'; export {NewThreadForm} from './NewThreadForm'; diff --git a/entry_types/scrolled/package/src/review/markThreadReadWhenSeen.js b/entry_types/scrolled/package/src/review/markThreadReadWhenSeen.js new file mode 100644 index 0000000000..93a7b0860b --- /dev/null +++ b/entry_types/scrolled/package/src/review/markThreadReadWhenSeen.js @@ -0,0 +1,48 @@ +import {useEffect} from 'react'; + +import {useMarkThreadRead} from './ReviewStateProvider'; +import {useLiveUnreadComments} from './unreadComments'; + +const DWELL_TIME = 800; + +// Threads that only peek in at the very edge of the viewport have not +// been read, but anything substantially on screen has. Expressed as a +// margin rather than a visibility ratio, which a thread taller than the +// viewport could never reach. +const ROOT_MARGIN = '-10% 0px -10% 0px'; + +// A thread counts as read once it has been on screen long enough to +// actually read it. Scrolling past it therefore leaves it unread. +// +// Callers pass `enabled: false` while the thread hides part of itself, +// so collapsed replies do not get marked as read unseen. +export function useMarkThreadReadWhenSeen({thread, ref, enabled}) { + const unreadComments = useLiveUnreadComments(thread); + const markThreadRead = useMarkThreadRead(); + + const {permaId} = thread; + const hasUnreadComments = unreadComments.length > 0; + + useEffect(() => { + const element = ref.current; + + if (!enabled || !hasUnreadComments || !markThreadRead || !element) return; + + let timeout; + + const observer = new IntersectionObserver(entries => { + clearTimeout(timeout); + + if (entries[entries.length - 1].isIntersecting) { + timeout = setTimeout(() => markThreadRead(permaId), DWELL_TIME); + } + }, {rootMargin: ROOT_MARGIN}); + + observer.observe(element); + + return () => { + clearTimeout(timeout); + observer.disconnect(); + }; + }, [enabled, hasUnreadComments, markThreadRead, permaId, ref]); +} diff --git a/entry_types/scrolled/package/src/review/postMessage.js b/entry_types/scrolled/package/src/review/postMessage.js index 31c2f51477..df54fde3be 100644 --- a/entry_types/scrolled/package/src/review/postMessage.js +++ b/entry_types/scrolled/package/src/review/postMessage.js @@ -31,6 +31,13 @@ export function postSetCommentDraftMessage(draft) { ); } +export function postMarkThreadsReadMessage(permaIds) { + window.top.postMessage( + {type: 'MARK_THREADS_READ', payload: {permaIds}}, + window.location.origin + ); +} + export function postUpdateThreadMessage({threadId, resolved}) { window.top.postMessage( {type: 'UPDATE_THREAD', payload: {threadId, resolved}}, @@ -58,3 +65,10 @@ export function postReviewStateDraftsChangeMessage(targetWindow, drafts) { window.location.origin ); } + +export function postReviewStateReadsChangeMessage(targetWindow, reads) { + targetWindow.postMessage( + {type: 'REVIEW_STATE_READS_CHANGE', payload: reads}, + window.location.origin + ); +} diff --git a/entry_types/scrolled/package/src/review/unreadComments.js b/entry_types/scrolled/package/src/review/unreadComments.js new file mode 100644 index 0000000000..38c0f89c11 --- /dev/null +++ b/entry_types/scrolled/package/src/review/unreadComments.js @@ -0,0 +1,78 @@ +import {useMemo} from 'react'; + +import {useCommentThreadReads, useCurrentUser} from './ReviewStateProvider'; +import {useDisplayedCommentThreadReads} from './commentThreadReadsSnapshot'; + +export function useUnreadCommentCount(threads) { + const currentUser = useCurrentUser(); + const commentThreadReads = useDisplayedCommentThreadReads(); + + return useMemo( + () => threads.reduce( + (count, thread) => count + unreadComments(thread, { + currentUser, + readAt: commentThreadReads[thread.permaId] + }).length, + 0 + ), + [threads, currentUser, commentThreadReads] + ); +} + +export function useUnreadComments(thread) { + const currentUser = useCurrentUser(); + const commentThreadReads = useDisplayedCommentThreadReads(); + + return useMemo( + () => unreadComments(thread, { + currentUser, + readAt: commentThreadReads[thread.permaId] + }), + [thread, currentUser, commentThreadReads] + ); +} + +// Counterpart of useUnreadComments for deciding whether a thread still +// needs to be marked read. Reading frozen state here would keep the +// thread unread no matter how often it was marked, leaving the read +// signal firing forever. +export function useLiveUnreadComments(thread) { + const currentUser = useCurrentUser(); + const commentThreadReads = useCommentThreadReads(); + + return useMemo( + () => unreadComments(thread, { + currentUser, + readAt: commentThreadReads[thread.permaId] + }), + [thread, currentUser, commentThreadReads] + ); +} + +// Comments the reviewer has not seen yet. Own comments never count: the +// reviewer has read what they just wrote, and a thread would otherwise +// turn unread by replying to it. +// +// Comments from before the reviewer's baseline do not count either. It +// keeps the comments that were already there when read tracking started +// - or when the reviewer joined - from all turning up as unread at once. +// +// Read state is only known once the current user has been fetched. Until +// then nothing counts as unread, so lists do not briefly show every +// thread as new. +export function unreadComments(thread, {currentUser, readAt}) { + if (!currentUser) return []; + + const seenUpTo = latestTime([readAt, currentUser.unreadCommentsSinceAt]); + + return thread.comments.filter( + comment => comment.creatorId !== currentUser.id && + (seenUpTo === null || new Date(comment.createdAt).getTime() > seenUpTo) + ); +} + +function latestTime(timestamps) { + const times = timestamps.filter(Boolean).map(timestamp => new Date(timestamp).getTime()); + + return times.length ? Math.max(...times) : null; +} diff --git a/entry_types/scrolled/package/src/review/watchUnreadComments.js b/entry_types/scrolled/package/src/review/watchUnreadComments.js new file mode 100644 index 0000000000..fdc3a3f9ea --- /dev/null +++ b/entry_types/scrolled/package/src/review/watchUnreadComments.js @@ -0,0 +1,31 @@ +import {unreadComments} from './unreadComments'; + +// Keeps an entry attribute in sync with whether the review session holds +// comments the user has not seen, so that the comments main menu item can +// point at them from the root of the sidebar. +export function watchUnreadComments({entry, session}) { + function update() { + entry.set('hasUnreadComments', hasUnreadComments(session.state)); + } + + session.on('reset', update); + session.on('change:thread', update); + session.on('change:reads', update); + + update(); +} + +function hasUnreadComments(state) { + if (!state) { + return false; + } + + const {currentUser, commentThreads, commentThreadReads = {}} = state; + + return commentThreads.some( + thread => unreadComments(thread, { + currentUser, + readAt: commentThreadReads[thread.permaId] + }).length > 0 + ); +} diff --git a/lib/pageflow/user_mixin.rb b/lib/pageflow/user_mixin.rb index 5ea782ad46..ab6702a321 100644 --- a/lib/pageflow/user_mixin.rb +++ b/lib/pageflow/user_mixin.rb @@ -23,10 +23,22 @@ module UserMixin has_many :revisions, class_name: 'Pageflow::Revision', foreign_key: :creator_id + has_many :comment_thread_reads, + dependent: :destroy, + class_name: 'Pageflow::CommentThreadRead' + validates :first_name, :last_name, presence: true validates_inclusion_of :locale, in: Pageflow.config.available_locales.map(&:to_s) scope :admins, -> { where(admin: true) } + + before_create :ensure_unread_comments_since_at + end + + # Comments predating a user are not new to them. Without a baseline, + # joining would mean facing every comment ever written as unread. + def ensure_unread_comments_since_at + self.unread_comments_since_at ||= Time.current end def admin? diff --git a/package/spec/editor/views/EditEntryView-spec.js b/package/spec/editor/views/EditEntryView-spec.js index 8cfe2294e7..c11c9ea330 100644 --- a/package/spec/editor/views/EditEntryView-spec.js +++ b/package/spec/editor/views/EditEntryView-spec.js @@ -36,4 +36,44 @@ describe('EditEntryView', () => { expect(item).toHaveText('some translation'); }); + + describe('menu item indicator', () => { + function renderMenuItem(entry) { + editor.registerMainMenuItem({ + translationKey: 'some.key', + id: 'some-id', + indicatorAttribute: 'somethingUnseen' + }); + const view = new EditEntryView({model: entry}); + + view.render(); + + return view.$el.find('[data-main-menu-item="some-id"]'); + } + + it('is absent while the entry attribute is falsy', () => { + const item = renderMenuItem(factories.entry()); + + expect(item).not.toHaveClass('indicator'); + }); + + it('is present while the entry attribute is truthy', () => { + const item = renderMenuItem(factories.entry({somethingUnseen: true})); + + expect(item).toHaveClass('indicator'); + }); + + it('follows changes of the entry attribute', () => { + const entry = factories.entry(); + const item = renderMenuItem(entry); + + entry.set('somethingUnseen', true); + + expect(item).toHaveClass('indicator'); + + entry.set('somethingUnseen', false); + + expect(item).not.toHaveClass('indicator'); + }); + }); }); diff --git a/package/spec/review/ReviewSession-spec.js b/package/spec/review/ReviewSession-spec.js index d8fc8ebe59..40eaf166f7 100644 --- a/package/spec/review/ReviewSession-spec.js +++ b/package/spec/review/ReviewSession-spec.js @@ -29,7 +29,8 @@ describe('ReviewSession', () => { id: 1, comments: [expect.objectContaining({body: 'Hello'})] }) - ] + ], + commentThreadReads: {} }); }); @@ -66,7 +67,8 @@ describe('ReviewSession', () => { currentUser: {id: 42, name: 'Alice'}, commentThreads: [ expect.objectContaining({id: 1}) - ] + ], + commentThreadReads: {} }); }); @@ -1081,4 +1083,140 @@ describe('ReviewSession', () => { expect(session.state.commentThreads).toEqual([]); }); }); + + describe('#markThreadsRead', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + async function createFetchedSession({commentThreadReads = {}} = {}) { + const request = jest.fn().mockResolvedValue({ + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [{id: 1, permaId: 5, comments: []}], + commentThreadReads + }); + + const session = new ReviewSession({entryId: 5, request}); + await session.fetch(); + request.mockClear(); + request.mockResolvedValue(null); + + return {session, request}; + } + + it('exposes read timestamps from fetch in state', async () => { + const {session} = await createFetchedSession({ + commentThreadReads: {5: '2026-08-17T10:00:00.000Z'} + }); + + expect(session.state.commentThreadReads).toEqual({5: '2026-08-17T10:00:00.000Z'}); + }); + + it('emits change:reads with read timestamp for marked threads', async () => { + const {session} = await createFetchedSession(); + const listener = jest.fn(); + session.on('change:reads', listener); + + session.markThreadsRead([5]); + + expect(listener).toHaveBeenCalledWith({5: expect.any(String)}); + expect(session.state.commentThreadReads[5]).toEqual(expect.any(String)); + }); + + it('keeps read timestamps of other threads', async () => { + const {session} = await createFetchedSession({ + commentThreadReads: {7: '2026-08-17T10:00:00.000Z'} + }); + + session.markThreadsRead([5]); + + expect(session.state.commentThreadReads[7]).toEqual('2026-08-17T10:00:00.000Z'); + }); + + it('sends marked perma ids in single request after delay', async () => { + const {session, request} = await createFetchedSession(); + + session.markThreadsRead([5]); + session.markThreadsRead([6]); + + expect(request).not.toHaveBeenCalled(); + + jest.runAllTimers(); + + expect(request).toHaveBeenCalledTimes(1); + expect(request).toHaveBeenCalledWith({ + url: '/review/entries/5/comment_thread_reads', + method: 'POST', + payload: {comment_thread_perma_ids: [5, 6]} + }); + }); + + it('does not send perma id twice', async () => { + const {session, request} = await createFetchedSession(); + + session.markThreadsRead([5]); + session.markThreadsRead([5]); + jest.runAllTimers(); + + expect(request).toHaveBeenCalledWith( + expect.objectContaining({payload: {comment_thread_perma_ids: [5]}}) + ); + }); + + it('does not send request for empty list of perma ids', async () => { + const {session, request} = await createFetchedSession(); + + session.markThreadsRead([]); + jest.runAllTimers(); + + expect(request).not.toHaveBeenCalled(); + }); + + it('ignores marks before state has been fetched', () => { + const request = jest.fn(); + const session = new ReviewSession({entryId: 5, request}); + + session.markThreadsRead([5]); + jest.runAllTimers(); + + expect(request).not.toHaveBeenCalled(); + }); + + it('sends pending perma ids on flushReads', async () => { + const {session, request} = await createFetchedSession(); + + session.markThreadsRead([5]); + await session.flushReads(); + + expect(request).toHaveBeenCalledTimes(1); + }); + + it('does not send request again when nothing is pending', async () => { + const {session, request} = await createFetchedSession(); + + session.markThreadsRead([5]); + await session.flushReads(); + await session.flushReads(); + + expect(request).toHaveBeenCalledTimes(1); + }); + + it('retries perma ids of failed request on next flush', async () => { + const {session, request} = await createFetchedSession(); + request.mockRejectedValueOnce(new Error('Network down')); + + session.markThreadsRead([5]); + await session.flushReads(); + await session.flushReads(); + + expect(request).toHaveBeenCalledTimes(2); + expect(request).toHaveBeenLastCalledWith( + expect.objectContaining({payload: {comment_thread_perma_ids: [5]}}) + ); + }); + }); }); diff --git a/package/spec/review/index-spec.js b/package/spec/review/index-spec.js new file mode 100644 index 0000000000..faa135f3b0 --- /dev/null +++ b/package/spec/review/index-spec.js @@ -0,0 +1,32 @@ +import {createReviewSession} from 'review'; + +describe('createReviewSession', () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('sends pending read marks when page is hidden', async () => { + window.fetch = jest.fn().mockResolvedValue({ok: true, status: 204}); + + const session = createReviewSession({ + entryId: 5, + initialState: { + currentUser: {id: 42, name: 'Alice'}, + commentThreads: [{id: 1, permaId: 7, comments: []}], + commentThreadReads: {} + } + }); + + session.markThreadsRead([7]); + window.dispatchEvent(new window.Event('pagehide')); + await Promise.resolve(); + + expect(window.fetch).toHaveBeenCalledWith( + '/review/entries/5/comment_thread_reads', + expect.objectContaining({ + method: 'POST', + body: JSON.stringify({comment_thread_perma_ids: [7]}) + }) + ); + }); +}); diff --git a/package/spec/review/request-spec.js b/package/spec/review/request-spec.js index 8cbda1277d..de0e61c378 100644 --- a/package/spec/review/request-spec.js +++ b/package/spec/review/request-spec.js @@ -23,6 +23,18 @@ describe('request', () => { expect(result).toEqual({some: 'data'}); }); + it('returns null for responses without content', async () => { + window.fetch = jest.fn().mockResolvedValue({ + ok: true, + status: 204, + json: () => Promise.reject(new Error('Unexpected end of JSON input')) + }); + + const result = await request({url: '/some/path', method: 'POST', payload: {}}); + + expect(result).toBeNull(); + }); + it('sends JSON body with CSRF token for POST requests', async () => { window.fetch = jest.fn().mockResolvedValue({ ok: true, diff --git a/package/src/editor/api/index.js b/package/src/editor/api/index.js index 07f6304977..b59060f09b 100644 --- a/package/src/editor/api/index.js +++ b/package/src/editor/api/index.js @@ -190,6 +190,8 @@ export const EditorApi = Object.extend( * - translationKey: for the label * - path: route to link to * - click: click handler + * - indicatorAttribute: name of an entry attribute. While it is + * truthy, the item displays an indicator dot. */ registerMainMenuItem: function(options) { this.mainMenuItems.push(options); diff --git a/package/src/editor/views/EditEntryView.js b/package/src/editor/views/EditEntryView.js index b97b1e90fb..21b1ade964 100644 --- a/package/src/editor/views/EditEntryView.js +++ b/package/src/editor/views/EditEntryView.js @@ -98,7 +98,22 @@ export const EditEntryView = Marionette.Layout.extend({ $(link).click(options.click); } + if (options.indicatorAttribute) { + view._bindMenuItemIndicator(link, options.indicatorAttribute); + } + view.ui.menu.append(item); }); + }, + + _bindMenuItemIndicator: function(link, attribute) { + var view = this; + + var update = function() { + link.toggleClass('indicator', !!view.model.get(attribute)); + }; + + this.listenTo(this.model, 'change:' + attribute, update); + update(); } }); diff --git a/package/src/review/ReviewSession.js b/package/src/review/ReviewSession.js index 3fba870ee8..4172393b02 100644 --- a/package/src/review/ReviewSession.js +++ b/package/src/review/ReviewSession.js @@ -1,11 +1,15 @@ import BackboneEvents from 'backbone-events-standalone'; +const FLUSH_READS_DELAY = 1000; + export class ReviewSession { constructor({entryId, request, initialState = null}) { this._entryId = entryId; this._request = request; this._state = initialState; this._drafts = {}; + this._pendingReads = new Set(); + this._flushReadsTimeout = null; } get state() { @@ -181,12 +185,61 @@ export class ReviewSession { this._state = { currentUser: data.currentUser, - commentThreads: data.commentThreads + commentThreads: data.commentThreads, + commentThreadReads: data.commentThreadReads || {} }; this.trigger('reset', this._state); } + // Read marks are frequent and individually unimportant, so they are + // collected and sent as one request instead of one request per thread. + markThreadsRead(permaIds) { + if (!this._state || !permaIds.length) return; + + const readAt = new Date().toISOString(); + + this._state = { + ...this._state, + commentThreadReads: { + ...this._state.commentThreadReads, + ...Object.fromEntries(permaIds.map(permaId => [permaId, readAt])) + } + }; + + permaIds.forEach(permaId => this._pendingReads.add(permaId)); + + this.trigger('change:reads', this._state.commentThreadReads); + this._scheduleFlushReads(); + } + + async flushReads() { + clearTimeout(this._flushReadsTimeout); + this._flushReadsTimeout = null; + + if (!this._pendingReads.size) return; + + const permaIds = [...this._pendingReads]; + this._pendingReads.clear(); + + await this._request({ + url: `/review/entries/${this._entryId}/comment_thread_reads`, + method: 'POST', + payload: {comment_thread_perma_ids: permaIds} + }).catch(() => { + // Local state already counts the threads as read. Keeping the + // perma ids pending lets the next flush try again instead of + // leaving them unread until the next page load. + permaIds.forEach(permaId => this._pendingReads.add(permaId)); + }); + } + + _scheduleFlushReads() { + if (this._flushReadsTimeout) return; + + this._flushReadsTimeout = setTimeout(() => this.flushReads(), FLUSH_READS_DELAY); + } + _writeDraft({body, pending = false, ...of}) { this._drafts = { ...this._drafts, diff --git a/package/src/review/index.js b/package/src/review/index.js index 33563650b3..d969fcf986 100644 --- a/package/src/review/index.js +++ b/package/src/review/index.js @@ -4,5 +4,11 @@ import {request} from './request'; export {ReviewSession}; export function createReviewSession({entryId, initialState}) { - return new ReviewSession({entryId, request, initialState}); + const session = new ReviewSession({entryId, request, initialState}); + + // Read marks are sent with a delay, which a tab closing right after + // reading would otherwise cut short. + window.addEventListener('pagehide', () => session.flushReads()); + + return session; } diff --git a/package/src/review/request.js b/package/src/review/request.js index 502c229b26..8b4ae09cf2 100644 --- a/package/src/review/request.js +++ b/package/src/review/request.js @@ -21,6 +21,10 @@ export async function request({url, method, payload}) { throw new Error(`${response.status} ${response.statusText}`); } + if (response.status === 204) { + return null; + } + return response.json(); } diff --git a/spec/controllers/pageflow/review/comment_thread_reads_controller_spec.rb b/spec/controllers/pageflow/review/comment_thread_reads_controller_spec.rb new file mode 100644 index 0000000000..62e046f0a4 --- /dev/null +++ b/spec/controllers/pageflow/review/comment_thread_reads_controller_spec.rb @@ -0,0 +1,94 @@ +require 'spec_helper' + +module Pageflow + describe Review::CommentThreadReadsController do + routes { Engine.routes } + + describe '#create' do + it 'marks comment threads as read for current user' do + user = create(:user) + entry = create(:entry, with_previewer: user) + thread = create(:comment_thread, revision: entry.draft) + + sign_in(user, scope: :user) + post(:create, params: { + entry_id: entry.id, + comment_thread_perma_ids: [thread.perma_id] + }, format: 'json') + + expect(response.status).to eq(204) + expect(CommentThreadRead.where(entry:, user:).pluck(:comment_thread_perma_id)) + .to eq([thread.perma_id]) + end + + it 'moves read at timestamp of already read thread forward' do + user = create(:user) + entry = create(:entry, with_previewer: user) + thread = create(:comment_thread, revision: entry.draft) + read = create(:comment_thread_read, + entry:, + user:, + comment_thread_perma_id: thread.perma_id, + read_at: 2.hours.ago) + + sign_in(user, scope: :user) + + Timecop.freeze(1.hour.from_now) do + post(:create, params: { + entry_id: entry.id, + comment_thread_perma_ids: [thread.perma_id] + }, format: 'json') + + expect(read.reload.read_at).to eq(Time.current) + end + + expect(CommentThreadRead.count).to eq(1) + end + + it 'ignores perma ids of threads not belonging to entry' do + user = create(:user) + entry = create(:entry, with_previewer: user) + other_entry = create(:entry, with_previewer: user) + other_thread = create(:comment_thread, revision: other_entry.draft) + + sign_in(user, scope: :user) + post(:create, params: { + entry_id: entry.id, + comment_thread_perma_ids: [other_thread.perma_id] + }, format: 'json') + + expect(response.status).to eq(204) + expect(CommentThreadRead.count).to eq(0) + end + + it 'ignores blank list of perma ids' do + user = create(:user) + entry = create(:entry, with_previewer: user) + + sign_in(user, scope: :user) + post(:create, params: {entry_id: entry.id}, format: 'json') + + expect(response.status).to eq(204) + expect(CommentThreadRead.count).to eq(0) + end + + it 'requires user to be signed in' do + entry = create(:entry) + + post(:create, params: {entry_id: entry.id}, format: 'json') + + expect(response.status).to eq(401) + end + + it 'requires read permission on entry' do + user = create(:user) + entry = create(:entry) + + sign_in(user, scope: :user) + post(:create, params: {entry_id: entry.id}, format: 'json') + + expect(response.status).to eq(403) + end + end + end +end diff --git a/spec/controllers/pageflow/review/comment_threads_controller_spec.rb b/spec/controllers/pageflow/review/comment_threads_controller_spec.rb index 416a4ea2cf..a6879ea04b 100644 --- a/spec/controllers/pageflow/review/comment_threads_controller_spec.rb +++ b/spec/controllers/pageflow/review/comment_threads_controller_spec.rb @@ -113,6 +113,53 @@ module Pageflow end end + it 'includes the unread baseline of the current user' do + unread_comments_since_at = 2.hours.ago + user = create(:user, unread_comments_since_at:) + entry = create(:entry, with_previewer: user) + + sign_in(user, scope: :user) + get(:index, params: {entry_id: entry.id}, format: 'json') + + baseline = JSON.parse(response.body)['currentUser']['unreadCommentsSinceAt'] + + expect(Time.zone.parse(baseline)).to eq(unread_comments_since_at) + end + + it 'includes read timestamps of current user by thread perma id' do + user = create(:user) + entry = create(:entry, with_previewer: user) + thread = create(:comment_thread, revision: entry.draft, creator: user) + read_at = 2.hours.ago + create(:comment_thread_read, + entry:, + user:, + comment_thread_perma_id: thread.perma_id, + read_at:) + + sign_in(user, scope: :user) + get(:index, params: {entry_id: entry.id}, format: 'json') + + reads = JSON.parse(response.body)['commentThreadReads'] + + expect(Time.zone.parse(reads[thread.perma_id.to_s])).to eq(read_at) + end + + it 'does not include read timestamps of other users' do + user = create(:user) + entry = create(:entry, with_previewer: user) + thread = create(:comment_thread, revision: entry.draft, creator: user) + create(:comment_thread_read, + entry:, + user: create(:user), + comment_thread_perma_id: thread.perma_id) + + sign_in(user, scope: :user) + get(:index, params: {entry_id: entry.id}, format: 'json') + + expect(JSON.parse(response.body)['commentThreadReads']).to eq({}) + end + it 'requires user to be signed in' do entry = create(:entry) diff --git a/spec/factories/comment_thread_reads.rb b/spec/factories/comment_thread_reads.rb new file mode 100644 index 0000000000..5993a29c44 --- /dev/null +++ b/spec/factories/comment_thread_reads.rb @@ -0,0 +1,10 @@ +module Pageflow + FactoryBot.define do + factory :comment_thread_read, class: CommentThreadRead do + entry + user + comment_thread_perma_id { 1 } + read_at { Time.current } + end + end +end diff --git a/spec/features/account_previewer/seeing_comment_activity_of_entries_spec.rb b/spec/features/account_previewer/seeing_comment_activity_of_entries_spec.rb new file mode 100644 index 0000000000..ba62f63f73 --- /dev/null +++ b/spec/features/account_previewer/seeing_comment_activity_of_entries_spec.rb @@ -0,0 +1,50 @@ +require 'spec_helper' + +feature 'as account previewer, seeing comment activity in the entries table' do + scenario 'entry without comments shows no indicator' do + entry = create(:entry, title: 'Quiet Entry') + Dom::Admin::Page.sign_in_as(:previewer, on: entry.account) + + visit(admin_entries_path) + + expect(Dom::Admin::EntryInIndexTable.find_by_title('Quiet Entry').comments_indicator) + .to be_nil + end + + scenario 'entry with unseen comments shows marked indicator naming them' do + entry = create(:entry, title: 'Discussed Entry') + user = Dom::Admin::Page.sign_in_as(:previewer, on: entry.account) + user.update!(unread_comments_since_at: 1.day.ago) + + thread = create(:comment_thread, revision: entry.draft) + create(:comment, comment_thread: thread, creator: create(:user)) + create(:comment, comment_thread: thread, creator: create(:user)) + + visit(admin_entries_path) + indicator = Dom::Admin::EntryInIndexTable.find_by_title('Discussed Entry') + .comments_indicator + + expect(indicator).to have_text('1') + expect(indicator).to have_selector('.unread_dot') + expect(indicator['data-tooltip']) + .to eq('Comments: 1 unresolved topic, 1 new topic, 1 new reply') + end + + scenario 'entry with comments predating the user shows unmarked indicator' do + entry = create(:entry, title: 'Settled Entry') + Dom::Admin::Page.sign_in_as(:previewer, on: entry.account) + + Timecop.travel(2.days.ago) do + thread = create(:comment_thread, revision: entry.draft) + create(:comment, comment_thread: thread, creator: create(:user)) + end + + visit(admin_entries_path) + indicator = Dom::Admin::EntryInIndexTable.find_by_title('Settled Entry') + .comments_indicator + + expect(indicator).to have_text('1') + expect(indicator).to have_no_selector('.unread_dot') + expect(indicator['data-tooltip']).to eq('Comments: 1 unresolved topic') + end +end diff --git a/spec/helpers/pageflow/admin/entries_helper_spec.rb b/spec/helpers/pageflow/admin/entries_helper_spec.rb index 4e09231963..82aa17ab0e 100644 --- a/spec/helpers/pageflow/admin/entries_helper_spec.rb +++ b/spec/helpers/pageflow/admin/entries_helper_spec.rb @@ -4,6 +4,90 @@ module Pageflow module Admin describe EntriesHelper do + describe '#entry_comments_indicator' do + def render_indicator(entry, user) + allow(helper).to receive(:collection).and_return([entry]) + allow(helper).to receive(:current_user).and_return(user) + + helper.entry_comments_indicator(entry) + end + + it 'renders nothing without unresolved threads' do + user = create(:user) + entry = create(:entry) + + expect(render_indicator(entry, user)).to be_nil + end + + it 'renders the number of unresolved topics' do + user = create(:user) + entry = create(:entry) + create(:comment_thread, revision: entry.draft) + create(:comment_thread, revision: entry.draft) + + result = render_indicator(entry, user) + + expect(result).to have_selector('span.entry_comments_indicator', text: '2') + end + + it 'marks the indicator when comments are unseen' do + user = create(:user, unread_comments_since_at: 3.hours.ago) + entry = create(:entry) + thread = create(:comment_thread, revision: entry.draft) + create(:comment, comment_thread: thread, creator: create(:user)) + + result = render_indicator(entry, user) + + expect(result).to have_selector('span.entry_comments_indicator .unread_dot') + end + + it 'does not mark the indicator when everything has been seen' do + user = create(:user, unread_comments_since_at: Time.current) + entry = create(:entry) + create(:comment_thread, revision: entry.draft) + + result = render_indicator(entry, user) + + expect(result).to have_selector('span.entry_comments_indicator') + expect(result).not_to have_selector('span.entry_comments_indicator .unread_dot') + end + + it 'names the topic count in the tooltip' do + user = create(:user, unread_comments_since_at: Time.current) + entry = create(:entry) + create(:comment_thread, revision: entry.draft) + + result = render_indicator(entry, user) + + expect(result).to have_selector("[data-tooltip='Comments: 1 unresolved topic']") + end + + it 'renders summaries passed in instead of querying the collection' do + user = create(:user) + entry = create(:entry) + create(:comment_thread, revision: entry.draft) + summaries = EntryCommentSummary.for_entries([entry], user:) + + result = helper.entry_comments_indicator(entry, summaries:) + + expect(result).to have_selector('span.entry_comments_indicator', text: '1') + end + + it 'names new topics and replies in the tooltip' do + user = create(:user, unread_comments_since_at: 3.hours.ago) + entry = create(:entry) + thread = create(:comment_thread, revision: entry.draft) + create(:comment, comment_thread: thread, creator: create(:user)) + create(:comment, comment_thread: thread, creator: create(:user)) + + result = render_indicator(entry, user) + + expect(result).to have_selector( + "[data-tooltip='Comments: 1 unresolved topic, 1 new topic, 1 new reply']" + ) + end + end + describe '#entry_type_collection' do include_context 'fake translations' diff --git a/spec/models/pageflow/comment_thread_read_spec.rb b/spec/models/pageflow/comment_thread_read_spec.rb new file mode 100644 index 0000000000..7ebf746cc5 --- /dev/null +++ b/spec/models/pageflow/comment_thread_read_spec.rb @@ -0,0 +1,119 @@ +require 'spec_helper' + +module Pageflow + describe CommentThreadRead do + describe '.mark' do + it 'creates records for the given comment thread perma ids' do + entry = create(:entry) + user = create(:user) + + CommentThreadRead.mark(entry:, user:, comment_thread_perma_ids: [5, 6]) + + expect(CommentThreadRead.where(entry:, user:).pluck(:comment_thread_perma_id)) + .to contain_exactly(5, 6) + end + + it 'records the given read at timestamp' do + entry = create(:entry) + user = create(:user) + read_at = 2.hours.ago + + CommentThreadRead.mark(entry:, user:, comment_thread_perma_ids: [5], read_at:) + + expect(CommentThreadRead.last.read_at).to eq(read_at) + end + + it 'moves read at timestamp of existing record forward' do + read = create(:comment_thread_read, + comment_thread_perma_id: 5, + read_at: 2.hours.ago) + + Timecop.freeze(1.hour.from_now) do + CommentThreadRead.mark(entry: read.entry, + user: read.user, + comment_thread_perma_ids: [5]) + + expect(read.reload.read_at).to eq(Time.current) + end + + expect(CommentThreadRead.count).to eq(1) + end + + it 'keeps records of other users separate' do + entry = create(:entry) + other_read = create(:comment_thread_read, + entry:, + comment_thread_perma_id: 5, + read_at: 2.hours.ago) + + CommentThreadRead.mark(entry:, + user: create(:user), + comment_thread_perma_ids: [5]) + + expect(other_read.reload.read_at).to eq(2.hours.ago) + expect(CommentThreadRead.count).to eq(2) + end + + it 'keeps records of other entries separate' do + user = create(:user) + other_read = create(:comment_thread_read, + user:, + comment_thread_perma_id: 5, + read_at: 2.hours.ago) + + CommentThreadRead.mark(entry: create(:entry), + user:, + comment_thread_perma_ids: [5]) + + expect(other_read.reload.read_at).to eq(2.hours.ago) + expect(CommentThreadRead.count).to eq(2) + end + + it 'does nothing for blank list of perma ids' do + entry = create(:entry) + user = create(:user) + + CommentThreadRead.mark(entry:, user:, comment_thread_perma_ids: []) + + expect(CommentThreadRead.count).to eq(0) + end + end + + describe '.read_at_by_perma_id' do + it 'returns read timestamps of user in entry keyed by thread perma id' do + read = create(:comment_thread_read, comment_thread_perma_id: 5) + + result = CommentThreadRead.read_at_by_perma_id(entry: read.entry, user: read.user) + + expect(result.keys).to eq([5]) + expect(result[5]).to eq(read.read_at) + end + + it 'ignores records of other users and entries' do + read = create(:comment_thread_read, comment_thread_perma_id: 5) + create(:comment_thread_read, entry: read.entry, comment_thread_perma_id: 6) + create(:comment_thread_read, user: read.user, comment_thread_perma_id: 7) + + result = CommentThreadRead.read_at_by_perma_id(entry: read.entry, user: read.user) + + expect(result.keys).to eq([5]) + end + end + + it 'is destroyed together with entry' do + read = create(:comment_thread_read) + + read.entry.destroy + + expect(CommentThreadRead.count).to eq(0) + end + + it 'is destroyed together with user' do + read = create(:comment_thread_read) + + read.user.destroy + + expect(CommentThreadRead.count).to eq(0) + end + end +end diff --git a/spec/models/pageflow/entry_comment_summary_spec.rb b/spec/models/pageflow/entry_comment_summary_spec.rb new file mode 100644 index 0000000000..6f03d1fddb --- /dev/null +++ b/spec/models/pageflow/entry_comment_summary_spec.rb @@ -0,0 +1,155 @@ +require 'spec_helper' + +module Pageflow + describe EntryCommentSummary do + describe '.for_entries' do + def summary_for(entry, user) + described_class.for_entries([entry], user:)[entry.id] + end + + it 'is empty without entries' do + expect(described_class.for_entries([], user: create(:user))).to eq({}) + end + + it 'counts unresolved threads of the draft revision as topics' do + user = create(:user) + entry = create(:entry) + create(:comment_thread, revision: entry.draft) + create(:comment_thread, revision: entry.draft) + + expect(summary_for(entry, user).topic_count).to eq(2) + end + + it 'ignores resolved threads' do + user = create(:user) + entry = create(:entry) + create(:comment_thread, revision: entry.draft, resolved_at: Time.current) + + expect(summary_for(entry, user).topic_count).to eq(0) + end + + it 'ignores threads of other entries' do + user = create(:user) + entry = create(:entry) + create(:comment_thread, revision: create(:entry).draft) + + expect(summary_for(entry, user).topic_count).to eq(0) + end + + it 'counts threads whose first comment is unseen as new topics' do + user = create(:user, unread_comments_since_at: 3.hours.ago) + entry = create(:entry) + thread = create(:comment_thread, revision: entry.draft) + create(:comment, comment_thread: thread, creator: create(:user)) + + summary = summary_for(entry, user) + + expect(summary.new_topic_count).to eq(1) + expect(summary.new_reply_count).to eq(0) + end + + it 'counts unseen comments after the first as new replies' do + user = create(:user, unread_comments_since_at: 3.hours.ago) + entry = create(:entry) + thread = create(:comment_thread, revision: entry.draft) + create(:comment, comment_thread: thread, creator: create(:user)) + create(:comment, comment_thread: thread, creator: create(:user)) + + summary = summary_for(entry, user) + + expect(summary.new_topic_count).to eq(1) + expect(summary.new_reply_count).to eq(1) + end + + it 'ignores comments the user wrote' do + user = create(:user, unread_comments_since_at: 3.hours.ago) + entry = create(:entry) + thread = create(:comment_thread, revision: entry.draft) + create(:comment, comment_thread: thread, creator: user) + + expect(summary_for(entry, user).new_topic_count).to eq(0) + end + + it 'ignores comments from before the users baseline' do + user = create(:user, unread_comments_since_at: Time.current) + entry = create(:entry) + thread = create(:comment_thread, revision: entry.draft) + + Timecop.freeze(2.hours.ago) do + create(:comment, comment_thread: thread, creator: create(:user)) + end + + expect(summary_for(entry, user).new_topic_count).to eq(0) + end + + it 'ignores comments from before the thread was read' do + user = create(:user, unread_comments_since_at: 3.hours.ago) + entry = create(:entry) + thread = create(:comment_thread, revision: entry.draft) + + Timecop.freeze(2.hours.ago) do + create(:comment, comment_thread: thread, creator: create(:user)) + end + + create(:comment_thread_read, + entry:, + user:, + comment_thread_perma_id: thread.perma_id, + read_at: 1.hour.ago) + + expect(summary_for(entry, user).new_topic_count).to eq(0) + end + + it 'keeps read state of other users apart' do + user = create(:user, unread_comments_since_at: 3.hours.ago) + entry = create(:entry) + thread = create(:comment_thread, revision: entry.draft) + create(:comment, comment_thread: thread, creator: create(:user)) + create(:comment_thread_read, + entry:, + user: create(:user), + comment_thread_perma_id: thread.perma_id) + + expect(summary_for(entry, user).new_topic_count).to eq(1) + end + + it 'returns a summary per entry' do + user = create(:user, unread_comments_since_at: 3.hours.ago) + entry = create(:entry) + other_entry = create(:entry) + create(:comment_thread, revision: entry.draft) + + result = described_class.for_entries([entry, other_entry], user:) + + expect(result[entry.id].topic_count).to eq(1) + expect(result[other_entry.id].topic_count).to eq(0) + end + + it 'does not have N+1 queries' do + user = create(:user) + entries = Array.new(3) { create(:entry) } + entries.each do |entry| + thread = create(:comment_thread, revision: entry.draft) + create(:comment, comment_thread: thread, creator: create(:user)) + create(:comment, comment_thread: thread, creator: create(:user)) + end + + detect_n_plus_one_queries do + described_class.for_entries(entries, user:) + end + end + end + + describe '#new?' do + it 'is true with new topics or new replies' do + expect(build_summary(new_topic_count: 1)).to be_new + expect(build_summary(new_reply_count: 1)).to be_new + expect(build_summary).not_to be_new + end + + def build_summary(new_topic_count: 0, new_reply_count: 0) + described_class.new(topic_count: 1, new_topic_count:, new_reply_count:) + end + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb new file mode 100644 index 0000000000..d09b371326 --- /dev/null +++ b/spec/models/user_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe User do + describe 'unread_comments_since_at' do + it 'is set to creation time' do + user = create(:user) + + expect(user.unread_comments_since_at).to eq(Time.current) + end + + it 'is not overwritten when given' do + unread_comments_since_at = 2.hours.ago + + user = create(:user, unread_comments_since_at:) + + expect(user.unread_comments_since_at).to eq(unread_comments_since_at) + end + + it 'stays put when the user is updated' do + user = create(:user) + + Timecop.freeze(1.hour.from_now) do + user.update!(first_name: 'Renamed') + end + + expect(user.reload.unread_comments_since_at).to eq(user.created_at) + end + end +end diff --git a/spec/support/dominos/admin/entry_in_index_table.rb b/spec/support/dominos/admin/entry_in_index_table.rb index 2669a7c48d..6b9cb02c2e 100644 --- a/spec/support/dominos/admin/entry_in_index_table.rb +++ b/spec/support/dominos/admin/entry_in_index_table.rb @@ -3,8 +3,14 @@ module Admin class EntryInIndexTable < Domino selector '.admin_entries.index .index_table tbody tr' - attribute :title, 'td.col-title' + # Scoped to the link since the title cell also carries the comment + # indicator. + attribute :title, 'td.col-title a' attribute :account_name, 'td.col-account' + + def comments_indicator + node.first('.entry_comments_indicator', minimum: 0) + end end end end