Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
15 changes: 15 additions & 0 deletions app/controllers/pageflow/review/comment_threads_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions app/models/pageflow/comment_thread.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions entry_types/scrolled/config/locales/de.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
5 changes: 5 additions & 0 deletions entry_types/scrolled/config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
28 changes: 28 additions & 0 deletions entry_types/scrolled/package/spec/review/CommentBadge-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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 = {};
Expand Down
127 changes: 126 additions & 1 deletion entry_types/scrolled/package/spec/review/ThreadList-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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} />,
Expand Down
5 changes: 3 additions & 2 deletions entry_types/scrolled/package/src/review/CommentBadge.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ export const ReviewMessageHandler = {
else if (type === 'CREATE_COMMENT') {
session.createComment(payload);
}
else if (type === 'UPDATE_THREAD') {
session.updateThread(payload);
}
}

function handleReset(state) {
Expand Down
20 changes: 17 additions & 3 deletions entry_types/scrolled/package/src/review/Thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@ import {Comment} from './Comment';
import {ReplyForm} from './ReplyForm';

import ChevronIcon from './images/chevron.svg';
import ResolveIcon from './images/resolve.svg';
import UnresolveIcon from './images/unresolve.svg';
import styles from './Thread.module.css';

export function Thread({thread, collapsed, onToggle}) {
export function Thread({thread, collapsed, onToggle, onResolve}) {
const {t} = useI18n({locale: 'ui'});
const firstComment = thread.comments[0];
const replies = thread.comments.slice(1);
const repliesCollapsed = collapsed && replies.length > 0;

return (
<div className={styles.thread}>
Expand All @@ -24,7 +27,7 @@ export function Thread({thread, collapsed, onToggle}) {

{firstComment && <Comment comment={firstComment} />}

{collapsed && replies.length > 0 &&
{repliesCollapsed &&
<button className={styles.expandButton} onClick={onToggle}>
{t('pageflow_scrolled.review.reply_count', {count: replies.length})}
<AvatarStack names={replies.map(c => c.creatorName)} />
Expand All @@ -34,7 +37,18 @@ export function Thread({thread, collapsed, onToggle}) {
<Comment key={comment.id} comment={comment} />
))}

{(!collapsed || replies.length === 0) && <ReplyForm threadId={thread.id} />}
{!thread.resolvedAt && !repliesCollapsed &&
<ReplyForm threadId={thread.id} />}

{onResolve && !repliesCollapsed &&
<div className={styles.resolveRow}>
<button className={styles.resolveButton} onClick={onResolve}>
{thread.resolvedAt ? <UnresolveIcon /> : <ResolveIcon />}
{t(thread.resolvedAt
? 'pageflow_scrolled.review.unresolve'
: 'pageflow_scrolled.review.resolve')}
</button>
</div>}
</div>
);
}
Loading
Loading