🐛 Fixed comment deep-link review regressions

ref #27043
Preserves thread-scoped browsing and exits single-comment mode when row filters are applied

Author: jonatansberg

apps/posts/src/hooks/filter-sources/create-remote-value-source.ts

@@ -53,6 +53,26 @@ export type RemoteValueSource<T = string, Item = unknown> = ValueSource<T> & {
 
 export type RemoteValueSourceHook<T = string, Item = unknown> = (options?: ValueSourceHookOptions) => RemoteValueSource<T, Item>;
 
+function buildFallbackOptions<T>(
+    selectedValues: T[],
+    mergedOptions: FilterOption<T>[],
+    getMissingSelectedOption?: (selectedValue: T) => FilterOption<T>
+): FilterOption<T>[] {
+    if (!getMissingSelectedOption) {
+        return [];
+    }
+
+    return selectedValues.flatMap((selectedValue) => {
+        const hasMatch = mergedOptions.some(option => option.value === selectedValue);
+
+        if (hasMatch) {
+            return [];
+        }
+
+        return [getMissingSelectedOption(selectedValue)];
+    });
+}
+
 export function createRemoteValueSource<Item, T = string>(
     config: RemoteValueSourceConfig<Item, T>
 ): RemoteValueSourceHook<T, Item> {
@@ -90,21 +110,11 @@ export function createRemoteValueSource<Item, T = string>(
                 return mergeFilterOptions(hydratedOptions, visibleOptions);
             }, [hydratedOptions, visibleOptions]);
             const fallbackOptions = useMemo(() => {
-                const getMissingSelectedOption = config.getMissingSelectedOption;
-
-                if (!getMissingSelectedOption) {
-                    return [];
-                }
-
-                return selectedValues.flatMap((selectedValue) => {
-                    const hasMatch = mergedOptions.some(option => option.value === selectedValue);
-
-                    if (hasMatch) {
-                        return [];
-                    }
-
-                    return [getMissingSelectedOption(selectedValue)];
-                });
+                return buildFallbackOptions(
+                    selectedValues,
+                    mergedOptions,
+                    config.getMissingSelectedOption
+                );
             }, [mergedOptions, selectedValues]);
 
             if (!enabled) {

apps/posts/src/views/comments/comments.tsx

@@ -7,6 +7,7 @@ import React, {useCallback, useMemo} from 'react';
 import {Button, EmptyIndicator, LoadingIndicator, LucideIcon, createFilter} from '@tryghost/shade';
 import {escapeNqlString} from '../filters/filter-normalization';
 import {getSiteTimezone} from '@src/utils/get-site-timezone';
+import {serializeCommentFilters} from './comment-filter-query';
 import {shouldDelayCommentDateFilterHydration, useFilterState} from './hooks/use-filter-state';
 import {useBrowseComments} from '@tryghost/admin-x-framework/api/comments';
 import {useBrowseSettings} from '@tryghost/admin-x-framework/api/settings';
@@ -23,14 +24,32 @@ const CommentsPage: React.FC<{timezone: string; singleCommentId?: string}> = ({
     timezone,
     singleCommentId
 }) => {
-    const [, setSearchParams] = useSearchParams();
+    const [searchParams, setSearchParams] = useSearchParams();
     const {filters, nql, setFilters} = useFilterState(timezone);
+    const threadParam = searchParams.get('thread') ?? undefined;
     const handleAddFilter = useCallback((field: string, value: string, operator: string = 'is') => {
-        setFilters((prevFilters) => {
-            const filtered = prevFilters.filter(f => f.field !== field);
-            return [...filtered, createFilter(field, operator, [value])];
-        }, {replace: false});
-    }, [setFilters]);
+        const nextFilters = [
+            ...filters.filter(filter => filter.field !== field),
+            createFilter(field, operator, [value])
+        ];
+
+        if (!singleCommentId) {
+            setFilters(nextFilters, {replace: false});
+            return;
+        }
+
+        const nextSearchParams = new URLSearchParams(searchParams);
+        const nextNql = serializeCommentFilters(nextFilters, timezone);
+
+        nextSearchParams.delete('id');
+        nextSearchParams.delete('filter');
+
+        if (nextNql) {
+            nextSearchParams.set('filter', nextNql);
+        }
+
+        setSearchParams(nextSearchParams, {replace: false});
+    }, [filters, searchParams, setFilters, setSearchParams, singleCommentId, timezone]);
     const effectiveFilter = useMemo(() => {
         if (singleCommentId) {
             return `id:${escapeNqlString(singleCommentId)}`;
@@ -52,7 +71,10 @@ const CommentsPage: React.FC<{timezone: string; singleCommentId?: string}> = ({
         fetchNextPage,
         hasNextPage
     } = useBrowseComments({
-        searchParams: effectiveFilter ? {filter: effectiveFilter} : {},
+        searchParams: {
+            ...(threadParam && !singleCommentId ? {thread: threadParam} : {}),
+            ...(effectiveFilter ? {filter: effectiveFilter} : {})
+        },
         keepPreviousData: true
     });
     const shouldShowLoading = isFetching && !isFetchingNextPage && !isRefetching;

apps/posts/test/unit/views/comments/comments.test.tsx

@@ -38,7 +38,18 @@ vi.mock('../../../../src/views/comments/components/comments-content', () => ({
 }));
 
 vi.mock('../../../../src/views/comments/components/comments-list', () => ({
-    default: () => <div data-testid="comments-list" />
+    default: ({onAddFilter}: {onAddFilter: (field: string, value: string, operator?: string) => void}) => (
+        <div>
+            <div data-testid="comments-list" />
+            <button
+                data-testid="add-author-filter"
+                type="button"
+                onClick={() => onAddFilter('author', 'member_456')}
+            >
+                Add author filter
+            </button>
+        </div>
+    )
 }));
 
 vi.mock('../../../../src/views/comments/components/comments-filters', () => ({
@@ -123,11 +134,36 @@ describe('Comments', () => {
     it('keeps thread state separate from canonical filter state updates', async () => {
         await renderComments('/?thread=is:comment_456');
 
+        expect(useBrowseCommentsMock).toHaveBeenCalledWith(expect.objectContaining({
+            searchParams: {
+                thread: 'is:comment_456'
+            }
+        }));
+
         fireEvent.click(screen.getByTestId('apply-filter'));
 
+        expect(useBrowseCommentsMock).toHaveBeenLastCalledWith(expect.objectContaining({
+            searchParams: {
+                filter: 'status:published',
+                thread: 'is:comment_456'
+            }
+        }));
         expect(screen.getByTestId('location-search')).toHaveTextContent('?thread=is%3Acomment_456&filter=status%3Apublished');
     });
 
+    it('leaves single-comment mode when applying a row-level filter', async () => {
+        await renderComments('/?id=is:comment_123');
+
+        fireEvent.click(screen.getByTestId('add-author-filter'));
+
+        expect(useBrowseCommentsMock).toHaveBeenLastCalledWith(expect.objectContaining({
+            searchParams: {
+                filter: 'member_id:member_456'
+            }
+        }));
+        expect(screen.getByTestId('location-search')).toHaveTextContent('?filter=member_id%3Amember_456');
+    });
+
     it('delays date-filter hydration until the site timezone is resolved', async () => {
         const encodedFilter = encodeURIComponent(
             'created_at:>=\'2024-03-10T05:00:00.000Z\'+created_at:<=\'2024-03-11T03:59:59.999Z\''