jpkleemans / attribute-events

🔥 Fire events on attribute changes of your Eloquent model
https://attribute.events
MIT License
309 stars 20 forks source link

support PHP Enums #16

Closed patrocle closed 11 months ago

patrocle commented 1 year ago

support PHP Enums hasn't been merged

patrocle commented 1 year ago
 if (
                $expected === '*'
                || $value instanceof \UnitEnum && ($value->name === $expected) // <--- if false here
                || $expected === 'true' && $value === true
                || $expected === 'false' && $value === false
                || is_numeric($expected) && Str::contains($expected, '.') && $value === (float) $expected
                || is_numeric($expected) && $value === (int) $expected
                || (string) $value === $expected // <--- then it checks here
            ) {
                $this->fireModelEvent($change, false);
            }

if $value is an instance of \UnitEnum but $value->name different from $expected then it will check if (string) $value equals $expected, but we can't cast (string) $value from a Enum

jpkleemans commented 1 year ago

Hi, thanks for your PR! Sorry for my late reply. Could you add a unit test that illustrates the problem, so the unit test shows that your code fixes that problem?

patrocle commented 1 year ago

Hi, I update the test it_works_with_enumerated_casts to show that when we update an enum attribute that doesn't have an event match with $value->name === $expected, it try to cast to (string) $value === $expected Thanks

obrunsmann commented 1 year ago

Any chance to get this merged? Solved exception on my side perfectly fine - thanks @patrocle

obrunsmann commented 1 year ago

@jpkleemans ? 🥴🥴

patrocle commented 1 year ago

@jpkleemans Hi, can you merge this ? Thanks

jpkleemans commented 1 year ago

@patrocle thanks for adding the test. But it looks like it fails?

image

Could you have a look at it?

jpkleemans commented 11 months ago

Fixed in https://github.com/jpkleemans/attribute-events/pull/18