Details
- Reviewers
machniak - Group Reviewers
Restricted Project - Commits
- rKcdfdbedb8393: Use a UuidTrait instead of an observer
Diff Detail
- Repository
- rK kolab
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 36376 Build 14392: arc lint + arc unit
Event Timeline
If it works, I like it. We should apply it to other models too. I'm not sure about the name, maybe UuidKeyTrait would be better?
Yes, the idea would be to apply this across the board.
UuidKeyTrait is indeed a better name I think, and for the int variant I would create another trait BinaryUuidKeyTrait or IntUuidKeyTrait (I like the former better).
I think I would prefer UuidIntKeyTrait and UuidStrKeyTrait. Other than that it looks good.
| src/app/Traits/BinaryUuidKeyTrait.php | ||
|---|---|---|
| 14 | We didn't check that before. | |
| src/app/Traits/BinaryUuidKeyTrait.php | ||
|---|---|---|
| 14 | But we need to if we want to make the trait compatible with softdelete models, right? | |
| src/app/Traits/BinaryUuidKeyTrait.php | ||
|---|---|---|
| 14 | I was referring to if (empty($model->{$model->getKeyName()})) {. There was no such check before. | |
I found one problem with this approach. The handlers registered by these traits are executed before observer's event handlers. I.e. we generate uuid before we do 'creating' pre-checks. For example EntitlementObserver. I think it makes sense to keep pre-checks in the observer, and do "simple" stuff via traits, but the order should be reversed so observers run first. Well, at least for these cases at hand it would be a proper way.