Skip to content

Conversation

@binaryfire
Copy link
Contributor

Summary

The getCasts() method in HasAttributes was only calling $this->casts() for incrementing models. For non-incrementing models (those using HasUuids or HasUlids), only the $casts property was returned, ignoring the casts() method entirely.

The Bug

public function getCasts(): array
{
    // ...cache check...

    if ($this->getIncrementing()) {
        // ✅ Incrementing: calls both property AND method
        return ... = array_merge([...], $this->casts, $this->casts());
    }

    // ❌ Non-incrementing: only returns property, ignores method!
    return ... = $this->casts;
}

Impact

Any model using HasUuids or HasUlids that defines casts via the casts() method (Laravel's modern approach) would have those casts ignored. This causes issues like:

  • Arrays not being JSON-encoded when saved to database
  • Dates not being cast to Carbon instances
  • Enums not being properly cast

The Fix

return static::$castsCache[static::class] = array_merge($this->casts, $this->casts());

Now the non-incrementing branch also merges both the property and method, matching the behavior of the incrementing branch.

Test Plan

  • Added HasAttributesTest with 3 test cases:
    • testGetCastsIncludesCastsMethodForIncrementingModels - existing behavior works
    • testGetCastsIncludesCastsMethodForNonIncrementingModels - UUID models get casts() method
    • testGetCastsMergesPropertyAndMethodForNonIncrementingModels - both property and method are merged

The getCasts() method was only calling $this->casts() for incrementing
models. For non-incrementing models (those using HasUuids or HasUlids),
only the $casts property was returned, ignoring the casts() method entirely.

This caused casts defined via the casts() method to be ignored for UUID/ULID
models, leading to issues like arrays not being JSON-encoded when saved.
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.

1 participant