Skip to content

fix: Entity::normalizeValue() must handle UnitEnum before toArray()#10137

Open
maniaba wants to merge 1 commit intocodeigniter4:developfrom
maniaba:bug-entity
Open

fix: Entity::normalizeValue() must handle UnitEnum before toArray()#10137
maniaba wants to merge 1 commit intocodeigniter4:developfrom
maniaba:bug-entity

Conversation

@maniaba
Copy link
Copy Markdown
Contributor

@maniaba maniaba commented Apr 24, 2026

Description

Moves the UnitEnum instanceof check before JsonSerializable and method_exists($data, 'toArray') in Entity::normalizeValue(), so that enums implementing toArray() are always normalized as enums rather than as generic objects.

Fixes #10136

  • fix test: correct toRawArray() assertion for injected enum

toRawArray() returns raw $this->attributes, so an injected enum object stays as an enum object — not the backing string value. The real regression is that hasChanged() must return false (normalizeValue() handles UnitEnum before toArray()).

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value (without duplication)
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@patel-vansh
Copy link
Copy Markdown
Contributor

You need to add this fix in v4.7.3 changelog

Comment thread tests/_support/Enum/StateEnum.php
Comment thread tests/system/Entity/EntityTest.php
@github-actions github-actions Bot added the stale Pull requests with conflicts label Apr 26, 2026
@github-actions

This comment was marked as resolved.

@maniaba maniaba requested a review from michalsn April 26, 2026 15:14
@paulbalandan paulbalandan added bug Verified issues on the current code behavior or pull requests that will fix them and removed stale Pull requests with conflicts labels Apr 26, 2026
Comment thread system/Entity/Entity.php
Comment on lines +475 to +476
} elseif (method_exists($data, 'toArray')) {
$objectData = $data->toArray();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This change creates a change in behavior for classes implementing both toArray() and either Traversable or DateTimeInterface. Previously, toArray() won. But now, this changes the picture. If this change is desirable, we should document this in the changelog as well. A couple of tests locking this new behavior would be nice, too.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's a good point. I think toArray() should stay where it was previously.

Comment thread system/Entity/Entity.php
// Check for Entity instance (use raw values, recursive)
if ($data instanceof self) {
$objectData = $data->toRawArray(false, true);
} elseif ($data instanceof UnitEnum) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since ordering has changed, it might be worthwhile to add in the phpdoc the order of resolution?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure if we should promise a specific normalization order or payload shape. This is purely an internal state.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Then, I guess, we can instead pin down the behavior via tests.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, that would work.

Comment thread system/Entity/Entity.php
} elseif ($data instanceof UnitEnum) {
return [
'__class' => $data::class,
'__enum' => $data instanceof BackedEnum ? $data->value : $data->name,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For completeness, can we additionally test a UnitEnum with toArray()? Currently, it's just a backed enum with toArray().

@paulbalandan paulbalandan changed the title fix: Entity::normalizeValue() must handle UnitEnum before toArray() fix: Entity::normalizeValue() must handle UnitEnum before toArray() Apr 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Verified issues on the current code behavior or pull requests that will fix them

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Entity::normalizeValue incorrectly handles enums when they implement toArray()

4 participants