Skip to content

Supported media rename and video blocks on older WP versions - #1208

Merged
pirate-bot merged 5 commits into
developmentfrom
bugfix/1201
Nov 5, 2026
Merged

pirate-bot merged 5 commits into
developmentfrom
bugfix/1201

Conversation

@girishpanchal30

Copy link
Copy Markdown
Contributor

All Submissions:

Changes proposed in this Pull Request:

Maintained WordPress 5.5 compatibility by replacing the renamer’s %i placeholders with explicit backtick quoting for table and column identifiers, adding a class-only wrapper fallback when get_block_wrapper_attributes() is unavailable, and falling back to render() when React’s createRoot is unavailable. The minimum supported WordPress version remains 5.5, preserving the intentional compatibility change introduced in v3.8.0 rather than raising the requirement to 6.2. The proposed has_cap( 'identifier_placeholders' ) branch was not used because a single quoting approach works across supported versions and can be tested in CI.

Closes #1201

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@pirate-bot

pirate-bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

Plugin build for 4487913 is ready 🛎️!

@pirate-bot pirate-bot left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes requested

Rename now runs on WordPress below 6.2, but its updates on tables without an ID column fail and still count as replacements.

Validation details
  • Files reviewed: 5/5 changed files.
  • cd /repo; grep -n '^class\|^final class' tests/media_rename/test-db-renamer.php tests/test-video-player-block.php; set -… printed OK (16 tests, 59 assertions) (run R2) The renamer tests pass at HEAD.
  • cd /repo; set -o pipefail; timeout 280 vendor/bin/phpunit --do-not-cache-result --filter 'Test_Video_Player_Block::' 2>&… printed OK (8 tests, 60 assertions) (run R3) The video block tests pass at HEAD.
  • The new renamer test fails on the base code with Failed asserting that 0 is greater than 0. So it detects the fix.
  • The security check found no injection in the quoted identifiers or the legacy wrapper classes.
  • The serialized-data query keeps a WHERE %s that never matches. It was the same before this change.
Untested areas
  • The widget render() fallback did not run on WordPress below 6.2. The widget needs a connected account.
  • The video block fallback did not run on WordPress 5.5. Only the helper ran, through reflection.
  • The new video test cannot fail on the base code. Its method is new, so that check is inconclusive.

🤖 Automated review · run code-review-agent_6ae1f39edafb58.36565626.


🤖 Review agent — review posted ✅ on fe74b66b · changes requested · 1 finding · 23 min

Run code-review-agent_6ae1f39edafb58.36565626 · trail

# Conflicts:
#	inc/media_rename/attachment_db_renamer.php
#	tests/media_rename/test-db-renamer.php

@pirate-bot pirate-bot left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes requested

The rename fix now holds, but the settings dashboard still crashes on WordPress 5.5, the minimum this PR keeps.

  • assets/src/dashboard/utils/api.js:55 — correctness The PR keeps WordPress 5.5 as the minimum, but the settings dashboard still fails there. api.js:12 imports store from @wordpress/notices, and line 55 runs const { createNotice } = dispatch( noticesStore ); at module load. WordPress 5.5 core does not export wp.notices.store, and its dispatch( undefined ) returns null. The destructuring throws a TypeError, so the whole dashboard bundle stops on the Optimole settings page. Toasts.js:7-12 uses the same store. Pass the store name 'core/notices' to dispatch, select and useDispatch in both files. Add a case to assets/src/dashboard/utils/__tests__/api.test.js that mocks @wordpress/notices without store and imports the module without an error.
Validation details
  • Files reviewed: 5/5 changed files.
  • The renamer tests pass at this head: cd /repo; set -o pipefail; timeout 300 vendor/bin/phpunit --do-not-cache-result --filter 'Test_Attachment_Db_Renamer::' … printed OK (28 tests, 73 assertions) (run R1)
  • The video block tests pass: cd /repo; set -o pipefail; timeout 300 vendor/bin/phpunit --do-not-cache-result --filter 'Test_Video_Player_Block::' 2>&… printed OK (8 tests, 60 assertions) (run R2)
  • A comment rename under the pre-6.2 prepare() shim updated the comment: cd /workspace/checks/C1 && python3 - <<'EOF' p='Probe_C1_Comment_Without_Placeholders.php' s=open(p).read() old=""" $or… printed C1-PROBE count=1 (run R5)
  • The new renamer test fails on the base code with Failed asserting that 0 is greater than 0. The helper run has no record ID.
  • The security check found no injection in the quoted identifiers or the legacy wrapper classes.
Untested areas
  • The compression tab imports __experimentalToggleGroupControl, which WordPress 5.5 lacks. Scope against this PR is unsettled.
  • The widget render() fallback did not run on WordPress below 6.2.
  • The video block fallback did not run on WordPress 5.5.
  • No dashboard run on WordPress 5.5. The crash is traced from source.

🤖 Automated review · run code-review-agent_6aec0a6bdfa9f3.52186680.


🤖 Review agent — review posted ✅ on 0cd4a023 · changes requested · 1 finding · 27 min

Run code-review-agent_6aec0a6bdfa9f3.52186680 · trail

@pirate-bot pirate-bot left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes requested

The notices crash is fixed, but the Compression settings still crash on WordPress 5.5, the minimum this PR keeps.

  • assets/src/dashboard/parts/connected/settings/Compression.js:201 — correctness The PR keeps WordPress 5.5 as the minimum, but the Compression settings still crash there. Lines 20-21 import __experimentalToggleGroupControl and __experimentalToggleGroupControlOption from @wordpress/components. WordPress 5.5 core does not export them, so both are undefined. Rendering them at lines 179-201 throws "Element type is invalid" when a user opens Settings → Compression on a connected WordPress 5.5 site with image replacement enabled. The dashboard tree then unmounts. This code was already in the code the earlier review saw. Render a control that WordPress 5.5 ships, such as RadioControl, when ToggleGroupControl is undefined. Add assets/src/dashboard/parts/connected/settings/__tests__/Compression.test.js that renders the component with both exports missing and expects the three modes.
Validation details
  • Files reviewed: 8/8 changed files.
  • The earlier api.js:55 finding is fixed. Both files now use the core/notices store name.
  • The changed test passes at this head: "Tests: 3 passed, 3 total" (not in the output of run R3)
  • The test fails with the base api.js: cd /repo && ls -a | grep -i babel; diff <(git show 0cd4a02:assets/src/dashboard/utils/api.js) <(git show 7e99825:assets/… printed Cannot destructure property 'createNotice' (run R4)
  • The side-effect import keeps wp-notices in the script dependencies, from source.
  • The renamer and video results of run code-review-agent_6aec0a6bdfa9f3.52186680 are reused, not rerun. Those files did not change since.
Untested areas
  • No dashboard run on WordPress 5.5. The Compression crash is traced from source.
  • A release build with WP_NO_EXTERNALS was not checked.
  • WordPress 5.5 exports for url, hooks, i18n and api-fetch were not audited.
  • The widget and video block fallbacks did not run on old WordPress.

🤖 Automated review · run code-review-agent_6aec13f01d4942.58342586.


🤖 Review agent — review posted ✅ on cf1f2bd4 · changes requested · 1 finding · 13 min

Run code-review-agent_6aec13f01d4942.58342586 · trail

@pirate-bot pirate-bot left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approved

The PR passes: the Compression settings now fall back to RadioControl on WordPress 5.5 to 5.8.

Question for the author: does dispatch( 'core/notices' ) in api.js:57 reach the same store as the removed noticesStore import on every supported version?

Validation details
  • Files reviewed: 3/3 files changed since the last review. The issue #1199 rename history row was not rechecked.
  • The new test passes: "Tests: 1 passed, 1 total" (not in the output of run R2)
  • It fails with the old Compression.js: cd /workspace/checks/C1/pre && set -o pipefail; timeout 300 npx jest assets/src/dashboard/parts/connected/settings/__tes… printed Element type is invalid (run R3)
  • The full Jest suite passes: "Tests: 237 passed, 237 total" (not in the output of run R5)
  • WordPress 5.5 exports RadioControl. Both toggle group exports first ship in 5.9, from source.
Untested areas
  • No dashboard run on a real WordPress 5.5 site.
  • The issue #1199 rename history row was not rechecked. The renamer file is unchanged.
  • The security result for the renamer query is reused, not rerun.
  • No exhaustive PHP version audit was done.

🤖 Automated review · run code-review-agent_6aec190128bf97.21130957.


🤖 Review agent — review posted ✅ on 44879136 · approved · 0 findings · 7 min

Run code-review-agent_6aec190128bf97.21130957 · trail

@pirate-bot
pirate-bot merged commit eb31d9b into development Nov 5, 2026
13 checks passed
@pirate-bot
pirate-bot deleted the bugfix/1201 branch November 5, 2026 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants