Merge pull request #2399 from tf/resolve-comments

Resolving comments

Author: tf

.rubocop.yml

@@ -173,6 +173,10 @@ Lint/UselessConstantScoping:
 Gemspec/DevelopmentDependencies:
   Enabled: false
 
+# Allow I18n-style %{var} tokens
+Style/FormatStringToken:
+  Mode: conservative
+
 # Ignore empty specs and factory traits
 Lint/EmptyBlock:
   Exclude:

app/controllers/pageflow/review/comment_threads_controller.rb

@@ -28,6 +28,21 @@ def create
         render :create, status: :created
       end
 
+      def update
+        entry = DraftEntry.find(params[:entry_id])
+        authorize!(:read, entry.to_model)
+
+        @comment_thread = entry.comment_threads.find(params[:id])
+
+        if ActiveModel::Type::Boolean.new.cast(params[:comment_thread][:resolved])
+          @comment_thread.resolve(current_user)
+        else
+          @comment_thread.unresolve
+        end
+
+        render :create
+      end
+
       private
 
       def thread_params

app/models/pageflow/comment_thread.rb

@@ -4,10 +4,19 @@ class CommentThread < ApplicationRecord
     include RevisionComponent
 
     belongs_to :creator, class_name: 'User'
+    belongs_to :resolver, class_name: 'User', foreign_key: :resolved_by_id, optional: true
     has_many :comments, dependent: :destroy
 
     nested_revision_components :comments
 
     validates :subject_type, :subject_id, presence: true
+
+    def resolve(user)
+      update!(resolved_at: Time.current, resolver: user) unless resolved_at
+    end
+
+    def unresolve
+      update!(resolved_at: nil, resolver: nil)
+    end
   end
 end

config/routes.rb

@@ -79,7 +79,7 @@
 
     namespace :review do
       resources :entries, only: [] do
-        resources :comment_threads, only: [:index, :create] do
+        resources :comment_threads, only: [:index, :create, :update] do
           resources :comments, only: [:create]
         end
       end

entry_types/scrolled/config/locales/de.yml

@@ -1922,6 +1922,11 @@ de:
       send: Senden
       enter_for_new_line: Enter für neue Zeile
       toggle_replies: Antworten umschalten
+      resolve: Als gelöst markieren
+      unresolve: Als ungelöst markieren
+      resolved_count:
+        one: 1 erledigt
+        other: '%{count} erledigt'
       reply_count:
         one: 1 Antwort
         other: '%{count} Antworten'

entry_types/scrolled/config/locales/en.yml

@@ -1751,6 +1751,11 @@ en:
       send: Send
       enter_for_new_line: Enter for new line
       toggle_replies: Toggle replies
+      resolve: Mark as resolved
+      unresolve: Mark as unresolved
+      resolved_count:
+        one: 1 resolved
+        other: '%{count} resolved'
       reply_count:
         one: 1 reply
         other: '%{count} replies'

entry_types/scrolled/package/spec/review/CommentBadge-spec.js

@@ -33,6 +33,34 @@ describe('CommentBadge', () => {
     expect(getByRole('status')).toHaveTextContent('2');
   });
 
+  it('only counts unresolved threads', () => {
+    const {getByRole} = renderWithReviewState(
+      <CommentBadge subjectType="ContentElement" subjectId={10} />,
+      {
+        commentThreads: [
+          {id: 1, subjectType: 'ContentElement', subjectId: 10, resolvedAt: null, comments: []},
+          {id: 2, subjectType: 'ContentElement', subjectId: 10, resolvedAt: null, comments: []},
+          {id: 3, subjectType: 'ContentElement', subjectId: 10, resolvedAt: '2026-04-09T10:00:00Z', comments: []}
+        ]
+      }
+    );
+
+    expect(getByRole('status')).toHaveTextContent('2');
+  });
+
+  it('renders nothing when all threads are resolved', () => {
+    const {container} = renderWithReviewState(
+      <CommentBadge subjectType="ContentElement" subjectId={10} />,
+      {
+        commentThreads: [
+          {id: 1, subjectType: 'ContentElement', subjectId: 10, resolvedAt: '2026-04-09T10:00:00Z', comments: []}
+        ]
+      }
+    );
+
+    expect(container).toBeEmptyDOMElement();
+  });
+
   it('renders nothing when no threads exist for subject', () => {
     const {container} = renderWithReviewState(
       <CommentBadge subjectType="ContentElement" subjectId={10} />

entry_types/scrolled/package/spec/review/ReviewMessageHandler-spec.js

@@ -5,7 +5,8 @@ import {ReviewMessageHandler} from 'review/ReviewMessageHandler';
 function fakeReviewSession() {
   const session = {
     createThread: jest.fn().mockResolvedValue(),
-    createComment: jest.fn().mockResolvedValue()
+    createComment: jest.fn().mockResolvedValue(),
+    updateThread: jest.fn().mockResolvedValue()
   };
 
   Object.assign(session, BackboneEvents);
@@ -91,6 +92,27 @@ describe('ReviewMessageHandler', () => {
     window.postMessage.mockRestore();
   });
 
+  it('calls session.updateThread on UPDATE_THREAD message from targetWindow', async () => {
+    const session = fakeReviewSession();
+
+    ReviewMessageHandler.create({session, targetWindow: window});
+
+    window.dispatchEvent(new MessageEvent('message', {
+      data: {
+        type: 'UPDATE_THREAD',
+        payload: {threadId: 1, resolved: true}
+      },
+      origin: window.location.origin,
+      source: window
+    }));
+
+    await new Promise(resolve => setTimeout(resolve, 0));
+
+    expect(session.updateThread).toHaveBeenCalledWith({
+      threadId: 1, resolved: true
+    });
+  });
+
   it('ignores messages not from targetWindow', async () => {
     const session = fakeReviewSession();
     const iframeWindow = {};

entry_types/scrolled/package/spec/review/ThreadList-spec.js

@@ -16,7 +16,11 @@ describe('ThreadList', () => {
     'pageflow_scrolled.review.reply_placeholder': 'Reply...',
     'pageflow_scrolled.review.send': 'Send',
     'pageflow_scrolled.review.enter_for_new_line': 'Enter for new line',
-    'pageflow_scrolled.review.toggle_replies': 'Toggle replies'
+    'pageflow_scrolled.review.toggle_replies': 'Toggle replies',
+    'pageflow_scrolled.review.resolve': 'Mark as resolved',
+    'pageflow_scrolled.review.unresolve': 'Mark as unresolved',
+    'pageflow_scrolled.review.resolved_count.one': '1 resolved',
+    'pageflow_scrolled.review.resolved_count.other': '%{count} resolved'
   });
   it('displays comments of threads for subject', () => {
     const {getByText} = renderWithReviewState(
@@ -389,6 +393,127 @@ describe('ThreadList', () => {
     expect(getByRole('button', {name: 'Send'})).toBeInTheDocument();
   });
 
+  it('hides resolved threads and shows resolved count pill', () => {
+    const {queryByText, getByText} = renderWithReviewState(
+      <ThreadList subjectType="ContentElement" subjectId={10} />,
+      {
+        commentThreads: [
+          {id: 1, subjectType: 'ContentElement', subjectId: 10,
+           resolvedAt: '2026-04-09T10:00:00Z',
+           comments: [{id: 10, body: 'Resolved thread', creatorName: 'Bob', creatorId: 2}]},
+          {id: 2, subjectType: 'ContentElement', subjectId: 10,
+           resolvedAt: null,
+           comments: [{id: 20, body: 'Active thread', creatorName: 'Alice', creatorId: 1}]}
+        ]
+      }
+    );
+
+    expect(getByText('Active thread')).toBeInTheDocument();
+    expect(queryByText('Resolved thread')).not.toBeInTheDocument();
+    expect(getByText('1 resolved')).toBeInTheDocument();
+  });
+
+  it('toggles resolved threads when pill is clicked', async () => {
+    const user = userEvent.setup();
+
+    const {getByText, queryByText} = renderWithReviewState(
+      <ThreadList subjectType="ContentElement" subjectId={10} />,
+      {
+        commentThreads: [
+          {id: 1, subjectType: 'ContentElement', subjectId: 10,
+           resolvedAt: '2026-04-09T10:00:00Z',
+           comments: [{id: 10, body: 'Resolved thread', creatorName: 'Bob', creatorId: 2}]},
+          {id: 2, subjectType: 'ContentElement', subjectId: 10,
+           resolvedAt: null,
+           comments: [{id: 20, body: 'Active thread', creatorName: 'Alice', creatorId: 1}]}
+        ]
+      }
+    );
+
+    await user.click(getByText('1 resolved'));
+    expect(getByText('Resolved thread')).toBeInTheDocument();
+    expect(getByText('1 resolved')).toBeInTheDocument();
+
+    await user.click(getByText('1 resolved'));
+    expect(queryByText('Resolved thread')).not.toBeInTheDocument();
+  });
+
+  it('posts resolve message when resolve button is clicked', async () => {
+    const user = userEvent.setup();
+    const postMessage = jest.spyOn(window.top, 'postMessage').mockImplementation(() => {});
+
+    const {getByText} = renderWithReviewState(
+      <ThreadList subjectType="ContentElement" subjectId={10} />,
+      {
+        commentThreads: [
+          {id: 1, subjectType: 'ContentElement', subjectId: 10,
+           resolvedAt: null,
+           comments: [{id: 10, body: 'Open thread', creatorName: 'Bob', creatorId: 2}]}
+        ]
+      }
+    );
+
+    await user.click(getByText('Mark as resolved'));
+
+    expect(postMessage).toHaveBeenCalledWith(
+      {type: 'UPDATE_THREAD', payload: {threadId: 1, resolved: true}},
+      window.location.origin
+    );
+
+    postMessage.mockRestore();
+  });
+
+  it('does not show resolve button on collapsed threads with replies', () => {
+    const {queryByText} = renderWithReviewState(
+      <ThreadList subjectType="ContentElement" subjectId={10} />,
+      {
+        commentThreads: [
+          {id: 1, subjectType: 'ContentElement', subjectId: 10,
+           resolvedAt: null,
+           comments: [
+             {id: 10, body: 'First thread', creatorName: 'Bob', creatorId: 2},
+             {id: 11, body: 'Reply', creatorName: 'Alice', creatorId: 1}
+           ]},
+          {id: 2, subjectType: 'ContentElement', subjectId: 10,
+           resolvedAt: null,
+           comments: [
+             {id: 20, body: 'Second thread', creatorName: 'Alice', creatorId: 1},
+             {id: 21, body: 'Reply', creatorName: 'Bob', creatorId: 2}
+           ]}
+        ]
+      }
+    );
+
+    expect(queryByText('Mark as resolved')).not.toBeInTheDocument();
+  });
+
+  it('does not show reply form on resolved threads', async () => {
+    const user = userEvent.setup();
+
+    const {queryByPlaceholderText, getByText} = renderWithReviewState(
+      <ThreadList subjectType="ContentElement" subjectId={10} />,
+      {
+        commentThreads: [
+          {id: 1, subjectType: 'ContentElement', subjectId: 10,
+           resolvedAt: null,
+           comments: [{id: 10, body: 'Active thread', creatorName: 'Bob', creatorId: 2}]},
+          {id: 2, subjectType: 'ContentElement', subjectId: 10,
+           resolvedAt: '2026-04-09T10:00:00Z',
+           comments: [{id: 20, body: 'Resolved thread', creatorName: 'Alice', creatorId: 1}]}
+        ]
+      }
+    );
+
+    await user.click(getByText('1 resolved'));
+
+    const replyFields = queryByPlaceholderText('Reply...');
+    expect(replyFields).toBeInTheDocument();
+
+    expect(getByText('Resolved thread')).toBeInTheDocument();
+    const resolvedThread = getByText('Resolved thread').closest('[class*="thread"]');
+    expect(resolvedThread.querySelector('textarea[placeholder="Reply..."]')).toBeNull();
+  });
+
   it('shows reply form in collapsed thread without replies', () => {
     const {getAllByPlaceholderText} = renderWithReviewState(
       <ThreadList subjectType="ContentElement" subjectId={10} />,

entry_types/scrolled/package/src/review/CommentBadge.js

@@ -8,7 +8,8 @@ import styles from './CommentBadge.module.css';
 
 export function CommentBadge({subjectType, subjectId, onClick, mode}) {
   const threads = useCommentThreads(subjectType, subjectId);
-  const hasThreads = threads.length > 0;
+  const unresolvedCount = threads.filter(t => !t.resolvedAt).length;
+  const hasThreads = unresolvedCount > 0;
 
   const variant = resolveVariant(mode, hasThreads);
 
@@ -21,7 +22,7 @@ export function CommentBadge({subjectType, subjectId, onClick, mode}) {
             className={classNames(styles.badge, styles[variant])}
             onClick={onClick}>
       {variant !== 'dot' && <CommentIcon className={styles.icon} />}
-      {(variant === 'active' || variant === 'expanded') && threads.length > 1 ? threads.length : null}
+      {(variant === 'active' || variant === 'expanded') && unresolvedCount > 1 ? unresolvedCount : null}
     </button>
   );
 }