Skip to content

fix: address PR #383 post-merge review feedback#399

Merged
imantsk merged 11 commits into
core-betafrom
fix/pr-383-issues
May 26, 2026
Merged

fix: address PR #383 post-merge review feedback#399
imantsk merged 11 commits into
core-betafrom
fix/pr-383-issues

Conversation

@imantsk
Copy link
Copy Markdown
Member

@imantsk imantsk commented May 25, 2026

Summary

Addresses unresolved reviewer feedback from PR #383. PHP, test, and standards fixes only — JS/TS refactoring handled separately by maintainer in core-beta.

PHP

  • Replace raw SQL DELETE FROM wp_options WHERE option_name LIKE with version-counter cache invalidation (cs_featured_cache_version option bumped on flush, old keys expire naturally via WordPress transient expiry)
  • Filter empty values with array_filter before hashing cache key to prevent cache fragmentation
  • Delete unused get_cloud_types(), get_cloud_categories() methods and their /types, /categories REST routes (filters delivered inline via response body, no JS consumer exists)
  • Fix get_filter_args docstrings: descriptions now say "ID" instead of "name (comma-separated)" matching actual accepted input
  • Fix Cloud_Snippets docblock: $snippets typed as Cloud_Snippet[], remove dead $success property
  • Clean up agent-context references in docblocks
  • Expose FEATURED_VERSION_OPTION as public constant, reference from Uninstaller instead of duplicated string literal
  • Add delete_option() for cs_featured_cache_version in uninstall cleanup path

JS (minor, non-architectural)

  • Add missing code-snippets text domain to error banner i18n string in CloudSearch.tsx
  • Add missing noopener to target="_blank" rel attribute in SearchResult.tsx
  • Remove dead connectCloud property from Window.ts type (never localized from PHP)

Tests

  • Fix Playwright selector: .cloud-featured-heading to .cloud-snippets-heading with hasText filter
  • Add PHPUnit test: clear_caches() invalidates featured cache (next call re-fetches)
  • Add PHPUnit test: empty filter values produce same cache key as omitted filters
  • Update test helper for versioned transient key format

Backwards compatibility

Cloud_API::get_cloud_types(), Cloud_API::get_cloud_categories(), and their REST routes were introduced in PR #383 on core-beta only, never shipped to production. No external consumers — confirmed via grep. Filter options are delivered inline in every search and featured API response.

New cs_featured_cache_version option added to wp_options. Cleaned up in Uninstaller::uninstall_current_site().

Test results

  • npm run lint — 0 errors, 1 pre-existing PHPCS warning (Cloud_API direct DB query — removed by this PR)
  • npm run test:php — 86 tests, 201 assertions, 0 failures
  • npm run test:playwright — 63 passed, 9 pre-existing failures (download bulk actions + quicknav timeouts). All 6 community-featured tests pass

@imantsk imantsk added the run-tests Trigger automated tests label May 25, 2026
@imantsk imantsk force-pushed the fix/pr-383-issues branch from 93ad6c5 to 3e33cbc Compare May 26, 2026 15:03
@github-actions
Copy link
Copy Markdown

PHPUnit Test Failure

One or more PHP version targets failed in this workflow run.

See all PHPUnit errors (click to expand)

Affected PHP version: 7.4, 8.0, 8.1, 8.3, 8.4

PHPUnit\Framework\ExpectationFailedException
Code_Snippets\Tests\Cloud_API_Featured_Test::test_empty_filters_produce_same_cache_key
Empty filter values should hash identically to no filters.
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'cs_featured_snippets_v1779809218531.1_p1_pp10_d751713988987e9331980363e24189ce'
+'cs_featured_snippets_v1779809218531_p1_pp10_d751713988987e9331980363e24189ce'

/home/runner/work/code-snippets/code-snippets/tests/phpunit/test-cloud-api-featured.php:383
phpvfscomposer:///home/runner/work/code-snippets/code-snippets/src/vendor/phpunit/phpunit/phpunit:106

Please review the failing jobs and fix the issues before merging.

@imantsk imantsk merged commit b633e06 into core-beta May 26, 2026
5 of 13 checks passed
@github-actions
Copy link
Copy Markdown

Playwright Test Failure

One or more Playwright targets failed in this workflow run.

See all Playwright errors (click to expand)

Affected Playwright test: all

FAILURE: code-snippets-list.spec.ts:170:6 Can download a single snippet from bulk actions
[chromium-db-snippets] › code-snippets-list.spec.ts:170:6 › Code Snippets List Page Actions › Can download a single snippet from bulk actions 

    Test timeout of 60000ms exceeded.

    Error: page.waitForEvent: Test timeout of 60000ms exceeded.
    =========================== logs ===========================
    waiting for event "download"
    ============================================================

      178 |
      179 | 		const download = await Promise.all([
    > 180 | 			page.waitForEvent('download'),
          | 			     ^
      181 | 			page.locator('#doaction').click()
      182 | 		]).then(([downloadEvent]) => downloadEvent)
      183 |
        at /home/runner/work/code-snippets/code-snippets/tests/e2e/code-snippets-list.spec.ts:180:9

    attachment #1: screenshot (image/png) ──────────────────────────────────────────────────────────
    ../../test-results/code-snippets-list-Code-Sn-04ad4-e-snippet-from-bulk-actions-chromium-db-snippets/test-failed-1.png
    ────────────────────────────────────────────────────────────────────────────────────────────────

    attachment #2: video (video/webm) ──────────────────────────────────────────────────────────────
    ../../test-results/code-snippets-list-Code-Sn-04ad4-e-snippet-from-bulk-actions-chromium-db-snippets/video.webm
    ────────────────────────────────────────────────────────────────────────────────────────────────

    Error Context: ../../test-results/code-snippets-list-Code-Sn-04ad4-e-snippet-from-bulk-actions-chromium-db-snippets/error-context.md

    attachment #4: trace (application/zip) ─────────────────────────────────────────────────────────
    ../../test-results/code-snippets-list-Code-Sn-04ad4-e-snippet-from-bulk-actions-chromium-db-snippets/trace.zip
    Usage:

        npx playwright show-trace ../../test-results/code-snippets-list-Code-Sn-04ad4-e-snippet-from-bulk-actions-chromium-db-snippets/trace.zip

    ────────────────────────────────────────────────────────────────────────────────────────────────

Affected Playwright test: all

FAILURE: code-snippets-list.spec.ts:187:6 Can download multiple snippets from bulk actions as a zip archive
[chromium-db-snippets] › code-snippets-list.spec.ts:187:6 › Code Snippets List Page Actions › Can download multiple snippets from bulk actions as a zip archive 

    Test timeout of 60000ms exceeded.

    Error: page.waitForEvent: Test timeout of 60000ms exceeded.
    =========================== logs ===========================
    waiting for event "download"
    ============================================================

      208 |
      209 | 		const download = await Promise.all([
    > 210 | 			page.waitForEvent('download'),
          | 			     ^
      211 | 			page.locator('#doaction').click()
      212 | 		]).then(([downloadEvent]) => downloadEvent)
      213 |
        at /home/runner/work/code-snippets/code-snippets/tests/e2e/code-snippets-list.spec.ts:210:9

    attachment #1: screenshot (image/png) ──────────────────────────────────────────────────────────
    ../../test-results/code-snippets-list-Code-Sn-ac58b-lk-actions-as-a-zip-archive-chromium-db-snippets/test-failed-1.png
    ────────────────────────────────────────────────────────────────────────────────────────────────

    attachment #2: video (video/webm) ──────────────────────────────────────────────────────────────
    ../../test-results/code-snippets-list-Code-Sn-ac58b-lk-actions-as-a-zip-archive-chromium-db-snippets/video.webm
    ────────────────────────────────────────────────────────────────────────────────────────────────

    Error Context: ../../test-results/code-snippets-list-Code-Sn-ac58b-lk-actions-as-a-zip-archive-chromium-db-snippets/error-context.md

    attachment #4: trace (application/zip) ─────────────────────────────────────────────────────────
    ../../test-results/code-snippets-list-Code-Sn-ac58b-lk-actions-as-a-zip-archive-chromium-db-snippets/trace.zip
    Usage:

        npx playwright show-trace ../../test-results/code-snippets-list-Code-Sn-ac58b-lk-actions-as-a-zip-archive-chromium-db-snippets/trace.zip

    ────────────────────────────────────────────────────────────────────────────────────────────────

Affected Playwright test: all

FAILURE: code-snippets-list.spec.ts:219:6 Bulk download stays scoped to the current page selection
[chromium-db-snippets] › code-snippets-list.spec.ts:219:6 › Code Snippets List Page Actions › Bulk download stays scoped to the current page selection 

    Test timeout of 60000ms exceeded.

    Error: page.waitForEvent: Test timeout of 60000ms exceeded.
    =========================== logs ===========================
    waiting for event "download"
    ============================================================

      250 |
      251 | 			const download = await Promise.all([
    > 252 | 				page.waitForEvent('download'),
          | 				     ^
      253 | 				page.locator('#doaction').click()
      254 | 			]).then(([downloadEvent]) => downloadEvent)
      255 |
        at /home/runner/work/code-snippets/code-snippets/tests/e2e/code-snippets-list.spec.ts:252:10

    attachment #1: screenshot (image/png) ──────────────────────────────────────────────────────────
    ../../test-results/code-snippets-list-Code-Sn-762db--the-current-page-selection-chromium-db-snippets/test-failed-1.png
    ────────────────────────────────────────────────────────────────────────────────────────────────

    attachment #2: video (video/webm) ──────────────────────────────────────────────────────────────
    ../../test-results/code-snippets-list-Code-Sn-762db--the-current-page-selection-chromium-db-snippets/video.webm
    ────────────────────────────────────────────────────────────────────────────────────────────────

    Error Context: ../../test-results/code-snippets-list-Code-Sn-762db--the-current-page-selection-chromium-db-snippets/error-context.md

    attachment #4: trace (application/zip) ─────────────────────────────────────────────────────────
    ../../test-results/code-snippets-list-Code-Sn-762db--the-current-page-selection-chromium-db-snippets/trace.zip
    Usage:

        npx playwright show-trace ../../test-results/code-snippets-list-Code-Sn-762db--the-current-page-selection-chromium-db-snippets/trace.zip

    ────────────────────────────────────────────────────────────────────────────────────────────────

Please review the failing jobs and fix the issues before merging.

@imantsk imantsk deleted the fix/pr-383-issues branch May 26, 2026 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-tests Trigger automated tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant