fix: template selection blocked when product uses default (allow all) mode#754
Conversation
… mode get_available_site_templates() returned [] for MODE_DEFAULT, causing the validation rule to block all templates with 'not available for this product'. Return false for MODE_DEFAULT to signal no restriction, matching the is_array() gate in the Site_Template validation rule. Guard is_template_available() against the false return to avoid a PHP warning.
📝 WalkthroughWalkthroughTwo files modified to refine site template availability logic. The Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
inc/limitations/class-limit-site-templates.php (1)
204-214:⚠️ Potential issue | 🔴 CriticalBreaking change: unguarded caller in
class-template-switching-element.phpwill crash.The return type changed from
arraytofalse|array. The call atinc/ui/class-template-switching-element.php:277passes the result directly toarray_map()without checking forfalse:$available_templates = array_map('intval', $this->site->get_limitations()->site_templates->get_available_site_templates());When
get_available_site_templates()returnsfalse(MODE_DEFAULT),array_map('intval', false)throws aTypeErrorin PHP 8.0+ or returns unexpected results in PHP 7.4, causing the subsequentin_array()call to fail.Additionally, the PHPDoc at line 204 states
@return arraybut should reflect@return array|false.Suggested PHPDoc fix
/** * Get available themes. * * `@since` 2.0.0 - * `@return` array + * `@return` array|false Array of available template IDs, or false when MODE_DEFAULT (unrestricted). */ public function get_available_site_templates() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@inc/limitations/class-limit-site-templates.php` around lines 204 - 214, get_available_site_templates() in class-limit-site-templates.php now returns false|array but its PHPDoc still says `@return` array and callers like class-template-switching-element.php::line ~277 call array_map(...) directly which will TypeError on false; update the PHPDoc for get_available_site_templates() to `@return` array|false and modify callers (e.g., the call in class-template-switching-element.php where $available_templates = array_map('intval', $this->site->get_limitations()->site_templates->get_available_site_templates());) to guard for false by treating false as an empty array before mapping (e.g., fetch the value into a variable, coalesce or conditional to [] when false, then call array_map), ensuring subsequent in_array() checks operate on an array.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@inc/limitations/class-limit-site-templates.php`:
- Around line 204-214: get_available_site_templates() in
class-limit-site-templates.php now returns false|array but its PHPDoc still says
`@return` array and callers like class-template-switching-element.php::line ~277
call array_map(...) directly which will TypeError on false; update the PHPDoc
for get_available_site_templates() to `@return` array|false and modify callers
(e.g., the call in class-template-switching-element.php where
$available_templates = array_map('intval',
$this->site->get_limitations()->site_templates->get_available_site_templates());)
to guard for false by treating false as an empty array before mapping (e.g.,
fetch the value into a variable, coalesce or conditional to [] when false, then
call array_map), ensuring subsequent in_array() checks operate on an array.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 59281a7a-4c89-4512-b76e-ff32dfb40e1a
📒 Files selected for processing (2)
inc/limitations/class-limit-site-templates.phpinc/limits/class-site-template-limits.php
|
Performance Test Results Performance test results for cd4fdb1 are in 🛎️! Note: the numbers in parentheses show the difference to the previous (baseline) test run. Differences below 2% or 0.5 in absolute values are not shown. URL:
|
Summary
Changes
Reproduction
Merged via PR #754 to main.
|
1 participant
Summary
get_available_site_templates()returned[](empty array) when a product's site template mode isMODE_DEFAULT("Default - Allow All Site Templates")class-site-template.phpchecksis_array($allowed_templates) && !in_array(...)— an empty array passesis_array(), so every template was blocked with "The selected template is not available for this product"falseforMODE_DEFAULTto signal "no restriction", matching the existingis_array()gate used by the validatorChanges
inc/limitations/class-limit-site-templates.phpget_available_site_templates()returnsfalseimmediately whenmode === MODE_DEFAULTMODE_DEFAULTbranch inside the loop (unreachable after the early return)inc/limits/class-site-template-limits.phpis_template_available()guards againstfalsefromget_available_site_templates()before callingin_array(), preventing a PHP warning and correctly returningtruefor the default modeReproduction
Summary by CodeRabbit