Page MenuHomePhorge

Use a UuidTrait instead of an observer
ClosedPublic

Authored by mollekopf on Aug 20 2021, 1:27 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Mar 23, 11:53 PM
Unknown Object (File)
Thu, Mar 21, 2:03 PM
Unknown Object (File)
Sun, Mar 17, 8:29 AM
Unknown Object (File)
Sun, Mar 17, 8:03 AM
Unknown Object (File)
Wed, Mar 6, 3:17 AM
Unknown Object (File)
Sat, Mar 2, 2:05 AM
Unknown Object (File)
Fri, Mar 1, 12:35 PM
Unknown Object (File)
Feb 15 2024, 12:40 PM
Subscribers

Details

Reviewers
machniak
Group Reviewers
Restricted Project
Commits
rKcdfdbedb8393: Use a UuidTrait instead of an observer

Diff Detail

Repository
rK kolab
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 36427
Build 14443: arc lint + arc unit

Event Timeline

mollekopf created this revision.
mollekopf added a reviewer: Restricted Project.Aug 20 2021, 3:54 PM

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?

We also have three models that use uuidInt. So, another trait?

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).

Just the string variant so far, not handling a withTrashed issue.

Now including the binary variant

I think I would prefer UuidIntKeyTrait and UuidStrKeyTrait. Other than that it looks good.

src/app/Traits/BinaryUuidKeyTrait.php
15

We didn't check that before.

Use the trait boot method so we can have multiple traits with boot methods

src/app/Traits/BinaryUuidKeyTrait.php
15

But we need to if we want to make the trait compatible with softdelete models, right?

src/app/Traits/BinaryUuidKeyTrait.php
15

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.

This revision is now accepted and ready to land.Aug 27 2021, 8:59 AM
This revision was automatically updated to reflect the committed changes.