Repository navigation
Supported media rename and video blocks on older WP versions - #1208
Conversation
There was a problem hiding this comment.
🟡 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 -…printedOK (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>&…printedOK (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 %sthat 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
There was a problem hiding this comment.
🟡 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—correctnessThe PR keeps WordPress 5.5 as the minimum, but the settings dashboard still fails there.api.js:12importsstorefrom@wordpress/notices, and line 55 runsconst { createNotice } = dispatch( noticesStore );at module load. WordPress 5.5 core does not exportwp.notices.store, and itsdispatch( undefined )returnsnull. The destructuring throws aTypeError, so the whole dashboard bundle stops on the Optimole settings page.Toasts.js:7-12uses the same store. Pass the store name'core/notices'todispatch,selectanduseDispatchin both files. Add a case toassets/src/dashboard/utils/__tests__/api.test.jsthat mocks@wordpress/noticeswithoutstoreand 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::' …printedOK (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>&…printedOK (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…printedC1-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
There was a problem hiding this comment.
🟡 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—correctnessThe PR keeps WordPress 5.5 as the minimum, but the Compression settings still crash there. Lines 20-21 import__experimentalToggleGroupControland__experimentalToggleGroupControlOptionfrom@wordpress/components. WordPress 5.5 core does not export them, so both areundefined. 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 asRadioControl, whenToggleGroupControlis undefined. Addassets/src/dashboard/parts/connected/settings/__tests__/Compression.test.jsthat renders the component with both exports missing and expects the three modes.
Validation details
- Files reviewed: 8/8 changed files.
- The earlier
api.js:55finding is fixed. Both files now use thecore/noticesstore 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/…printedCannot destructure property 'createNotice'(run R4) - The side-effect import keeps
wp-noticesin 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_EXTERNALSwas not checked. - WordPress 5.5 exports for
url,hooks,i18nandapi-fetchwere 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
There was a problem hiding this comment.
🟢 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…printedElement 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
All Submissions:
Changes proposed in this Pull Request:
Maintained WordPress 5.5 compatibility by replacing the renamer’s
%iplaceholders with explicit backtick quoting for table and column identifiers, adding a class-only wrapper fallback whenget_block_wrapper_attributes()is unavailable, and falling back torender()when React’screateRootis 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 proposedhas_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: