Skip to content

t531: fix set_demo_behavior fatal TypeError on PHP 8 when null passed via deprecated compat layer#914

Merged
superdav42 merged 1 commit intomainfrom
bugfix/t531-set-demo-behavior-null-php8
Apr 22, 2026
Merged

t531: fix set_demo_behavior fatal TypeError on PHP 8 when null passed via deprecated compat layer#914
superdav42 merged 1 commit intomainfrom
bugfix/t531-set-demo-behavior-null-php8

Conversation

@superdav42
Copy link
Copy Markdown
Collaborator

@superdav42 superdav42 commented Apr 22, 2026

What

Remove the non-nullable string type hint from Product::set_demo_behavior() and add a null guard, preventing a TypeError fatal on PHP 8.0+ when null is passed through the generic Base_Model::attributes() setter dispatch.

Why

Customers on PHP 8.x hit a fatal error loading any admin page that calls wu_get_plan() through the deprecated WU_Plan compat layer. The call chain:

wu_get_plan() → WU_Plan::__construct() → Base_Model::attributes()
  → call_user_func(['set_demo_behavior'], null)
  → TypeError: Argument #1 must be of type string, null given

Three compounding causes:

  1. set_demo_behavior(string $behavior) — non-nullable type hint on a public model setter, violating our coding standard (AGENTS.md: "NEVER add PHP return type declarations to public methods on model classes")
  2. PHP 8.0 removed implicit null → "" coercion for typed parameters that PHP 7.4 allowed silently
  3. Base_Model::attributes() passes raw DB/compat-layer values to setters without null filtering

How

inc/models/class-product.php:1367

  • Drop string parameter type and : void return type (per coding standard — use @param/@return PHPDoc instead)
  • Add if (null === $behavior) { return; } guard — null from legacy data is silently ignored; get_demo_behavior() already falls back to 'delete_after_time' when the property is unset

Verification

The fatal is reproducible by constructing a Product via attributes(['demo_behavior' => null]). After this fix, null is a no-op and get_demo_behavior() returns the default 'delete_after_time'.

Reported environment: PHP 8.5.5, WordPress 6.9.4, Ultimate Multisite 2.6.3.


aidevops.sh v3.8.94 plugin for OpenCode v1.3.17 with claude-sonnet-4-6 spent 4m and 6,734 tokens on this with the user in an interactive session.

Summary by CodeRabbit

  • Refactor
    • Improved product demo behavior handling to support flexible input values, allowing the system to gracefully manage edge cases and enhance overall stability.

@superdav42 superdav42 added the origin:interactive Created by interactive user session label Apr 22, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 22, 2026

📝 Walkthrough

Walkthrough

The set_demo_behavior method in the Product class was updated to accept nullable input, allowing the method to exit early without persisting changes when a null value is passed, while maintaining existing behavior for non-null values.

Changes

Cohort / File(s) Summary
Method Signature Relaxation
inc/models/class-product.php
Updated set_demo_behavior() to accept nullable input; added early exit when $behavior is null to skip persisting to meta and cache, while preserving existing persistence logic for non-null values.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A null check hops into view,
Early exits, fresh and new,
When nothing's there, we simply flee—
No meta stored, just liberty!
The demo dances on, so free. 🎭✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main fix: resolving a TypeError in set_demo_behavior when null is passed on PHP 8, addressing a specific bug in the deprecated compat layer.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bugfix/t531-set-demo-behavior-null-php8

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
inc/models/class-product.php (1)

1367-1376: Consider keeping : void return type for consistency.

The fix correctly resolves the PHP 8 TypeError. However, dropping the : void return declaration isn't necessary — an early return; is fully compatible with void. All sibling setters in this file (set_featured_image_id, set_slug, set_tax_category, etc.) retain : void, so preserving it here keeps the setter API consistent and still allows the nullable $behavior parameter.

♻️ Optional refactor
-	public function set_demo_behavior($behavior) {
+	public function set_demo_behavior($behavior): void {

 		if (null === $behavior) {
 			return;
 		}

Note: the broader root cause (Base_Model::attributes() forwarding raw nulls into setters) means other strictly-typed setters in the codebase could be susceptible to the same fatal. Worth a follow-up audit of set_* methods called through attributes() / the WU_Plan compat layer.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@inc/models/class-product.php` around lines 1367 - 1376, Restore the explicit
void return type on the setter to match sibling setters and keep the API
consistent: change the method signature of set_demo_behavior to include ": void"
while keeping the early "if (null === $behavior) { return; }" behavior and the
existing assignments to $this->meta[self::META_DEMO_BEHAVIOR] and
$this->demo_behavior; no other logic changes required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@inc/models/class-product.php`:
- Around line 1367-1376: Restore the explicit void return type on the setter to
match sibling setters and keep the API consistent: change the method signature
of set_demo_behavior to include ": void" while keeping the early "if (null ===
$behavior) { return; }" behavior and the existing assignments to
$this->meta[self::META_DEMO_BEHAVIOR] and $this->demo_behavior; no other logic
changes required.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 07a25db1-6f25-4ba6-a596-0e644563fcf7

📥 Commits

Reviewing files that changed from the base of the PR and between 69feaf4 and 70b08fb.

📒 Files selected for processing (1)
  • inc/models/class-product.php

@github-actions
Copy link
Copy Markdown

🔨 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!
Playground support for multisite is very limitied, hopefully it will get better in the future.

🚀 Launch in Playground

Login credentials: admin / password

@github-actions
Copy link
Copy Markdown

Performance Test Results

Performance test results for 3d4e5d6 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: /

Run DB Queries Memory Before Template Template WP Total LCP TTFB LCP - TTFB
0 41 37.80 MB 854.00 ms (+47.00 ms / +6% ) 152.00 ms 1021.50 ms 1952.00 ms 1873.20 ms 77.15 ms (-4.15 ms / -5% )
1 56 49.03 MB 917.00 ms 135.00 ms 1058.50 ms 2020.00 ms (-42.00 ms / -2% ) 1946.75 ms (-39.85 ms / -2% ) 73.90 ms

@superdav42 superdav42 merged commit 640b5b9 into main Apr 22, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

origin:interactive Created by interactive user session

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant