diff --git a/CHANGELOG.md b/CHANGELOG.md index e94b17f42..fb6fdb5e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,9 @@ * Fixed featured Community Cloud snippets failing to load with some cloud API responses. (PRO) * Fixed bulk actions in Community Cloud running against an empty selection, so selected snippets were never downloaded. (PRO) +* Fixed the snippets list showing only the first 100 snippets on sites with larger libraries, with the type counts + agreeing with that shortened list rather than the real total. The list no longer carries snippet code, making the + request far smaller, and the screen now reports a problem instead of presenting a partial list as complete. ## [3.10.2] (2026-09-01) diff --git a/src/js/components/ManageMenu/SnippetsTable/ManageSnippetCard.tsx b/src/js/components/ManageMenu/SnippetsTable/ManageSnippetCard.tsx index 3648a397e..db0aa03c0 100644 --- a/src/js/components/ManageMenu/SnippetsTable/ManageSnippetCard.tsx +++ b/src/js/components/ManageMenu/SnippetsTable/ManageSnippetCard.tsx @@ -51,7 +51,8 @@ const CloneExportMenuItems: React.FC = ({ snippet }) => <> { - api.create(cloneSnippetObject(snippet)) + api.ensureCode(snippet) + .then(full => api.create(cloneSnippetObject(full))) .then(refreshSnippetsList) .catch(handleUnknownError) }} diff --git a/src/js/components/ManageMenu/SnippetsTable/RowActions.tsx b/src/js/components/ManageMenu/SnippetsTable/RowActions.tsx index ae320c95f..804f706fe 100644 --- a/src/js/components/ManageMenu/SnippetsTable/RowActions.tsx +++ b/src/js/components/ManageMenu/SnippetsTable/RowActions.tsx @@ -135,7 +135,9 @@ const ActionLinks: React.FC = ({ snippet }) => { ? api.create(cloneSnippetObject(snippet)).then(refreshSnippetsList)} + action={() => api.ensureCode(snippet) + .then(full => api.create(cloneSnippetObject(full))) + .then(refreshSnippetsList)} /> : null diff --git a/src/js/components/ManageMenu/SnippetsTable/SnippetsTable.tsx b/src/js/components/ManageMenu/SnippetsTable/SnippetsTable.tsx index 7d6d1a81f..4777c8edb 100644 --- a/src/js/components/ManageMenu/SnippetsTable/SnippetsTable.tsx +++ b/src/js/components/ManageMenu/SnippetsTable/SnippetsTable.tsx @@ -84,9 +84,13 @@ const SafeModeNotice = () => : null -// Counts render immediately from localized values, then switch to live values. +// Counts render immediately from the values localized with the page, then switch +// to live values derived from the snippets list once it has loaded. The switch is +// driven by isListLoaded rather than by the list being present, because the list +// starts out holding the capped set embedded in the page: counting that would +// quietly report a truncated library as the whole of it. const useSnippetTypeCounts = () => { - const { snippetsList } = useSnippetsList() + const { snippetsList, isListLoaded } = useSnippetsList() const countedSnippets = useMemo( () => snippetsList?.filter(snippet => !snippet.trashed), @@ -104,15 +108,23 @@ const useSnippetTypeCounts = () => { const localized = window.CODE_SNIPPETS_MANAGE?.typeCounts const getCount = useCallback( - (type?: SnippetType) => countedSnippets - ? type ? typeCounts?.get(type) ?? 0 : countedSnippets.length + (type?: SnippetType) => isListLoaded + ? type ? typeCounts?.get(type) ?? 0 : countedSnippets?.length ?? 0 : localized?.[type ?? 'all'], - [countedSnippets, typeCounts, localized] + [isListLoaded, countedSnippets, typeCounts, localized] ) return { getCount } } +const ListErrorNotice: React.FC = () => { + const { listError } = useSnippetsList() + + return listError + ?

{listError}

+ : null +} + const SnippetsTableInner = () => { const { snippetView, setSnippetView } = useSnippetView() const { currentType } = useSnippetsFilters() @@ -156,6 +168,8 @@ const SnippetsTableInner = () => { + + {currentType && !isLicensed() && isProType(currentType) ? : diff --git a/src/js/components/ManageMenu/SnippetsTable/WithFilteredSnippetsContext.tsx b/src/js/components/ManageMenu/SnippetsTable/WithFilteredSnippetsContext.tsx index f0f88a0c0..f773e6bf3 100644 --- a/src/js/components/ManageMenu/SnippetsTable/WithFilteredSnippetsContext.tsx +++ b/src/js/components/ManageMenu/SnippetsTable/WithFilteredSnippetsContext.tsx @@ -1,4 +1,4 @@ -import React, { useMemo } from 'react' +import React, { useEffect, useMemo } from 'react' import { createContextHook } from '../../../utils/bootstrap' import { parseSnippetObject } from '../../../utils/snippets/objects' import { getSnippetType, isSnippetActive } from '../../../utils/snippets/snippets' @@ -48,9 +48,17 @@ export interface FilteredSnippetsContext { const [Context, useFilteredSnippets] = createContextHook('useFilteredSnippets') export const WithFilteredSnippetsContext: React.FC = ({ children }) => { - const { snippetsList } = useSnippetsList() + const { snippetsList, ensureSnippetCode } = useSnippetsList() const { currentType, currentTag, searchLineNumber, searchQueryText } = useSnippetsFilters() + // The list is fetched without snippet code, so ask for it the moment a search + // could match against it. Until it arrives, code simply matches nothing. + useEffect(() => { + if (searchQueryText?.trim()) { + ensureSnippetCode() + } + }, [searchQueryText, ensureSnippetCode]) + const snippets = useMemo( () => snippetsList ?? window.CODE_SNIPPETS_MANAGE?.snippetsList?.map(parseSnippetObject) ?? [], [snippetsList]) diff --git a/src/js/components/common/snippets/SnippetPreviewModal.tsx b/src/js/components/common/snippets/SnippetPreviewModal.tsx index 160535a61..5a8692f70 100644 --- a/src/js/components/common/snippets/SnippetPreviewModal.tsx +++ b/src/js/components/common/snippets/SnippetPreviewModal.tsx @@ -116,6 +116,8 @@ export interface PreviewModalProps { export const PreviewModal: React.FC = ({ onRequestClose, title, type, code, children }) => { const textareaRef = useRef(null) + const editorRef = useRef() + const contents = `${'php' === type ? ' { if (!textareaRef.current || !window.wp.codeEditor) { @@ -127,16 +129,30 @@ export const PreviewModal: React.FC = ({ onRequestClose, titl { codemirror: getPreviewEditorSettings(type) } ) + editorRef.current = instance.codemirror as EditorFromTextArea + // CodeMirror hides the labeled source textarea and creates an unlabelled // internal input. The screenReaderLabel option only exists from CodeMirror // 5.59, while WordPress 5.5 ships 5.29, so label the input directly. instance.codemirror.getInputField().setAttribute('aria-label', __('Snippet code preview', 'code-snippets')) return () => { + editorRef.current = undefined; (instance.codemirror as EditorFromTextArea).toTextArea() } }, [type]) + // The editor is created as soon as the modal opens, but a snippet opened from + // the list arrives without its body and is fetched, so the code can turn up + // afterwards. Keep the editor showing whatever the current code is. + useEffect(() => { + const editor = editorRef.current + + if (editor && editor.getValue() !== contents) { + editor.setValue(contents) + } + }, [contents]) + return ( = ({ onRequestClose, titl ref={textareaRef} readOnly aria-label={__('Snippet code preview', 'code-snippets')} - defaultValue={`${'php' === type ? ' {children} @@ -206,7 +222,8 @@ const CloneButton: React.FC = ({ snippet, isWorking, setIsWork const handleClone = () => { setIsWorking(true) - api.create(cloneSnippetObject(snippet)) + api.ensureCode(snippet) + .then(full => api.create(cloneSnippetObject(full))) .then(refreshSnippetsList) .then(() => setIsOpen(false)) .catch(handleUnknownError) @@ -249,7 +266,37 @@ export interface SnippetPreviewModalProps { extraActions?: (working: boolean) => ReactNode } -export const SnippetPreviewModal: React.FC = ({ snippet, setIsOpen, extraActions }) => { +/** + * The snippets list is fetched without code, so a snippet opened from it arrives + * with an empty body. Fetch that one snippet so the preview has something to show. + */ +const useSnippetWithCode = (snippet: Snippet): Snippet => { + const api = useSnippetsAPI() + const [resolved, setResolved] = useState(snippet) + + useEffect(() => { + let cancelled = false + + api.ensureCode(snippet) + .then(full => { + if (!cancelled) { + setResolved(full) + } + }) + .catch(handleUnknownError) + + return () => { + cancelled = true + } + // Refetching whenever the api object changes identity would loop; the snippet is what matters. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [snippet]) + + return resolved +} + +export const SnippetPreviewModal: React.FC = ({ snippet: listSnippet, setIsOpen, extraActions }) => { + const snippet = useSnippetWithCode(listSnippet) const { refreshSnippetsList } = useSnippetsList() const { isWorking, setIsWorking } = useWorkingState() diff --git a/src/js/hooks/useSnippetsAPI.tsx b/src/js/hooks/useSnippetsAPI.tsx index 3f9160173..535d88606 100644 --- a/src/js/hooks/useSnippetsAPI.tsx +++ b/src/js/hooks/useSnippetsAPI.tsx @@ -10,9 +10,23 @@ import type { SnippetIdentifierSchema, SnippetSchema, WritableSnippetSchema } fr import type { RestAPI } from './useRestAPI' import type { PropsWithChildren } from 'react' +export interface FetchAllOptions { + /** + * Whether to include each snippet's code. The list screen never renders it, + * so it is left out by default; only searching code contents needs it. + */ + withCode?: boolean +} + export interface SnippetsAPI { - fetchAll: (network?: boolean | null) => Promise + fetchAll: (network?: boolean | null, options?: FetchAllOptions) => Promise fetch: (snippetId: number, network?: boolean | null) => Promise + /** + * Resolve a snippet that may have come from the list, which omits code. + * Anything needing the body — cloning, previewing, copying — goes through + * this so it works whichever source the snippet came from. + */ + ensureCode: (snippet: Snippet) => Promise create: (snippet: Partial) => Promise update: (snippet: Pick & Partial) => Promise delete: (snippet: Pick) => Promise @@ -25,6 +39,18 @@ export interface SnippetsAPI { detach: (snippet: Pick) => Promise } +/** + * Fields the snippets list needs, which is every schema field except `code`. + * A list carrying full snippet bodies is by far the largest thing the manage + * screen transfers and grows without bound as a library does, while nothing in + * the list renders it. Keep in step with the item schema. + */ +const LIST_FIELDS = [ + 'id', 'name', 'desc', 'tags', 'scope', 'condition_id', 'active', 'trashed', 'locked', + 'priority', 'network', 'shared_network', 'modified', 'last_active', 'created_by', 'updated_by', + 'code_error', 'code_error_trace' +].join(',') + const buildSnippetUrl = ({ id, network }: Pick, action?: string) => buildUrl([REST_BASES.snippets, id, action].filter(Boolean).join('/'), { network }) @@ -57,14 +83,23 @@ const mapToSchema = ({ }) const buildSnippetsAPI = ({ get, post, del, put }: RestAPI): SnippetsAPI => ({ - fetchAll: network => - get(buildUrl(REST_BASES.snippets, { network })) + fetchAll: (network, options) => + get(buildUrl(REST_BASES.snippets, { + network, + ...options?.withCode ? {} : { _fields: LIST_FIELDS } + })) .then(response => response.map(createSnippetObject)), fetch: (snippetId, network) => get(buildUrl(`${REST_BASES.snippets}/${snippetId}`, { network })) .then(createSnippetObject), + ensureCode: snippet => + snippet.code + ? Promise.resolve(snippet) + : get(buildUrl(`${REST_BASES.snippets}/${snippet.id}`, { network: snippet.network })) + .then(createSnippetObject), + create: snippet => post(REST_BASES.snippets, mapToSchema(snippet)) .then(createSnippetObject), diff --git a/src/js/hooks/useSnippetsList.tsx b/src/js/hooks/useSnippetsList.tsx index 999004730..23a23deda 100644 --- a/src/js/hooks/useSnippetsList.tsx +++ b/src/js/hooks/useSnippetsList.tsx @@ -1,4 +1,5 @@ import React, { useCallback, useEffect, useState } from 'react' +import { __ } from '@wordpress/i18n' import { createContextHook } from '../utils/bootstrap' import { isNetworkAdmin } from '../utils/screen' import { parseSnippetObject } from '../utils/snippets/objects' @@ -8,7 +9,17 @@ import type { Snippet } from '../types/Snippet' export interface SnippetsListContext { snippetsList: readonly Snippet[] | undefined + /** + * Whether the complete list has arrived from the API. Until it has, the list + * is whatever was embedded in the page, which is capped and may be partial, + * so nothing should present it as the whole library. + */ + isListLoaded: boolean + /** Set when the list could not be fetched, so the screen can say so rather than show a partial list as if it were complete. */ + listError: string | undefined refreshSnippetsList: () => Promise + /** Ask for snippet code, which the list omits. Only searching code contents needs it. */ + ensureSnippetCode: () => void } const [Context, useSnippetsList] = createContextHook('useSnippetsList') @@ -18,16 +29,25 @@ export const WithSnippetsListContext: React.FC = ({ children const [snippetsList, setSnippetsList] = useState( () => window.CODE_SNIPPETS_MANAGE?.snippetsList?.map(parseSnippetObject) ) + const [isListLoaded, setIsListLoaded] = useState(false) + const [listError, setListError] = useState() + const [withCode, setWithCode] = useState(false) + + // Flipping this re-runs the fetch below, so code is loaded through the same + // path as everything else rather than racing a second request against it. + const ensureSnippetCode = useCallback(() => setWithCode(true), []) const refreshSnippetsList = useCallback(async (): Promise => { try { - console.info('Fetching snippets list') - const response = await fetchAll(isNetworkAdmin()) + const response = await fetchAll(isNetworkAdmin(), { withCode }) setSnippetsList(response) + setIsListLoaded(true) + setListError(undefined) } catch (error: unknown) { console.error('Error fetching snippets list', error) + setListError(__('Could not load the complete list of snippets, so some may be missing below.', 'code-snippets')) } - }, [fetchAll]) + }, [fetchAll, withCode]) useEffect(() => { refreshSnippetsList() @@ -36,7 +56,10 @@ export const WithSnippetsListContext: React.FC = ({ children const value: SnippetsListContext = { snippetsList, - refreshSnippetsList + isListLoaded, + listError, + refreshSnippetsList, + ensureSnippetCode } return {children} diff --git a/tests/e2e/snippets-list-size.spec.ts b/tests/e2e/snippets-list-size.spec.ts new file mode 100644 index 000000000..4383950c6 --- /dev/null +++ b/tests/e2e/snippets-list-size.spec.ts @@ -0,0 +1,122 @@ +import { expect, test } from '@playwright/test' +import { SnippetsTestHelper } from './helpers/SnippetsTestHelper' +import { TIMEOUTS } from './helpers/constants' +import { wpCli } from './helpers/wpCli' + +/** + * The manage screen embeds a capped list of snippets in the page so the table + * paints immediately, then replaces it with the complete list from the REST API. + * A library larger than that cap is therefore only shown correctly once the + * request lands, and must never be presented as complete before it does. + */ + +const PREFIX = 'E2E ListSize' + +// Comfortably past the 100-snippet cap the page embeds. +const CREATE_COUNT = 120 + +const ALL_COUNT = '.all-type-link .subnav-count' + +/** Matches the snippets collection request, on pretty and plain permalinks alike, but not a single snippet. */ +const isListRequest = (url: string): boolean => + /rest_route=%2Fcode-snippets%2Fv1%2Fsnippets(?:&|$)/.test(url) || + /\/wp-json\/code-snippets\/v1\/snippets(?:\?|$)/.test(url) + +const createManySnippets = async (count: number): Promise => { + const php = ` + $body = str_repeat( "// padding so each snippet carries a realistic code body\\n", 20 ); + for ( $i = 1; $i <= ${count}; $i++ ) { + $snippet = new \\Code_Snippets\\Model\\Snippet(); + $snippet->name = sprintf( ${JSON.stringify(`${PREFIX} %03d`)}, $i ); + $snippet->code = $body . sprintf( 'add_action( "init", function () { /* %03d */ } );', $i ); + $snippet->scope = 'global'; + $snippet->active = false; + \\Code_Snippets\\save_snippet( $snippet ); + } + ` + await wpCli(['eval', php]) +} + +/** How many snippets the screen should report, which excludes trashed ones. */ +const untrashedCount = async (): Promise => + Number(await wpCli([ + 'eval', + 'global $wpdb; echo (int) $wpdb->get_var( "SELECT COUNT(*) FROM {$wpdb->prefix}snippets WHERE active <> -1" );' + ])) + +test.describe('A library larger than the embedded list', () => { + let expectedTotal = 0 + + test.beforeAll(async () => { + await createManySnippets(CREATE_COUNT) + expectedTotal = await untrashedCount() + }) + + test.afterAll(async () => { + await SnippetsTestHelper.cleanupSnippetsByPrefix(PREFIX) + }) + + test('shows every snippet, not only the set embedded in the page', async ({ page }) => { + await page.goto('/wp-admin/admin.php?page=snippets') + + const embedded = await page.evaluate(() => + (<{ CODE_SNIPPETS_MANAGE?: { snippetsList?: unknown[] } }>window).CODE_SNIPPETS_MANAGE?.snippetsList?.length ?? 0) + + expect(embedded, 'the page should embed only a partial list, or this test proves nothing') + .toBeLessThan(expectedTotal) + + await expect(page.locator(ALL_COUNT)) + .toHaveText(String(expectedTotal), { timeout: TIMEOUTS.DEFAULT }) + }) + + test('never asks for snippet code when it only needs to list them', async ({ page }) => { + const listUrls: string[] = [] + page.on('request', request => { + if (isListRequest(request.url())) { + listUrls.push(request.url()) + } + }) + + await page.goto('/wp-admin/admin.php?page=snippets') + await expect(page.locator(ALL_COUNT)).toHaveText(String(expectedTotal), { timeout: TIMEOUTS.DEFAULT }) + + expect(listUrls.length, 'the list should have been requested').toBeGreaterThan(0) + for (const url of listUrls) { + expect(decodeURIComponent(url), 'the list request should name the fields it wants').toContain('_fields=') + expect(decodeURIComponent(url), 'the list request should not ask for snippet code').not.toMatch(/(?:^|,)code(?:,|&|$)/) + } + }) + + test('still shows a snippet\'s code when it is previewed from the list', async ({ page }) => { + await page.goto('/wp-admin/admin.php?page=snippets') + await expect(page.locator(ALL_COUNT)).toHaveText(String(expectedTotal), { timeout: TIMEOUTS.DEFAULT }) + + // The list omits code, so the modal has to fetch the body it displays. + const row = page.locator('tbody tr').filter({ hasText: PREFIX }).first() + await row.getByRole('button', { name: 'Preview', exact: true }).click() + + const editor = page.locator('.code-snippets-preview-modal .CodeMirror') + await expect(editor).toBeVisible({ timeout: TIMEOUTS.DEFAULT }) + + await expect + .poll( + () => editor.evaluate(el => + (<{ CodeMirror?: { getValue: () => string } }>el).CodeMirror?.getValue() ?? ''), + { timeout: TIMEOUTS.DEFAULT } + ) + .toContain('add_action') + }) + + test('keeps the count honest and says so when the list cannot be loaded', async ({ page }) => { + await page.route(url => isListRequest(url.toString()), route => route.abort('failed')) + + await page.goto('/wp-admin/admin.php?page=snippets') + + // The count comes from the server, so it stays true even though the table below it is short. + await expect(page.locator(ALL_COUNT)) + .toHaveText(String(expectedTotal), { timeout: TIMEOUTS.DEFAULT }) + + await expect(page.locator('.notice-error')) + .toContainText('Could not load the complete list of snippets', { timeout: TIMEOUTS.DEFAULT }) + }) +}) diff --git a/tests/unit/REST_API/REST_API_Snippets_List_Fields_Test.php b/tests/unit/REST_API/REST_API_Snippets_List_Fields_Test.php new file mode 100644 index 000000000..313f2c52a --- /dev/null +++ b/tests/unit/REST_API/REST_API_Snippets_List_Fields_Test.php @@ -0,0 +1,128 @@ +dispatch( + new WP_REST_Request( 'GET', '/code-snippets/v1/snippets/schema' ) + ); + + $this->assertSame( 200, $response->get_status(), 'the snippets schema route should be readable' ); + + $schema = $response->get_data(); + $this->assertArrayHasKey( 'properties', $schema, 'the schema should describe its properties' ); + + return array_keys( $schema['properties'] ); + } + + /** + * Field names the list request asks for. + * + * @return string[] + */ + private function requested_fields(): array { + $path = dirname( __DIR__, 3 ) . '/' . self::SOURCE_FILE; + $this->assertFileExists( $path, 'the list request should still be defined here' ); + + $source = (string) file_get_contents( $path ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents -- reading a source file in a test. + $matched = preg_match( '/const LIST_FIELDS\s*=\s*\[(.*?)\]/s', $source, $matches ); + + $this->assertSame( 1, $matched, 'LIST_FIELDS should still be a literal array in ' . self::SOURCE_FILE ); + + preg_match_all( "/'([a-z_]+)'/", $matches[1], $fields ); + + return $fields[1]; + } + + /** + * Every schema property except the code body is requested, and nothing else is. + * + * @return void + */ + public function test_list_request_asks_for_every_field_except_code(): void { + $expected = array_values( array_diff( $this->schema_properties(), [ self::OMITTED_FIELD ] ) ); + $requested = $this->requested_fields(); + + sort( $expected ); + sort( $requested ); + + $this->assertSame( + $expected, + $requested, + 'LIST_FIELDS in ' . self::SOURCE_FILE . ' is out of step with the snippets schema. ' + . 'A property added to the schema has to be added there too, or it will not reach the manage screen.' + ); + } + + /** + * The code body is left out on purpose, so guard against it creeping back in. + * + * @return void + */ + public function test_list_request_does_not_ask_for_code(): void { + $this->assertContains( + self::OMITTED_FIELD, + $this->schema_properties(), + 'the schema should still offer a code property for the editor to request' + ); + + $this->assertNotContains( + self::OMITTED_FIELD, + $this->requested_fields(), + 'the snippets list should not request snippet code; it is fetched separately when a search needs it' + ); + } +}