Silence phpstan errors
Details
- Reviewers
mollekopf - Group Reviewers
Restricted Project - Commits
- rK3d7439962063: StatusPropertyTrait, and some more code de-duplication
./phpunit
Diff Detail
- Repository
- rK kolab
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
I'm wondering whether there is a cleaner solution to relying on constants being defined in the class from the trait and the objectState function, both seems a bit magic. But since the object states do not mean the same thing between different objects, I can't think of a better solution either.
Check if isDegraded/degrade/undegrade can be incorporated into the trait as well, otherwise it looks good to me.
src/app/Jobs/WalletCheck.php | ||
---|---|---|
206 | I suppose the call won't hurt, but checking for the method doesn't check if the entitleable is suspendable due to the trait (the method exists even if the constant doesn't). | |
src/app/User.php | ||
420 | Couldn't this function be split into isDegraded (which could be included in the trait), and isAccountDegraded(), which can check isDegraded first? |
Since traits can't define constants yet, one possible improvement would be to define interfaces, and use them together with trait(s). And we could unify the constant values to be the same for all objects at some point too. Still, User has STATUS_DEGRADED, Domain has STATUS_CONFIRMED, etc. So, for example:
interface StatusSuspendedInterface { const STATUS_SUSPENDED = 4; function isSuspended(); function suspend(); function unsuspend(); }
For now I prefer to not define interfaces and use my simple trait with the common status functionality.
src/app/Jobs/WalletCheck.php | ||
---|---|---|
206 | That's right, but the method does nothing for objects that do not support suspend, so we're good.. I find this approach more generic and simpler. | |
src/app/User.php | ||
420 | It probably could, but separately from this diff. |
I agree that we shouldn't go down the interface route for this, and that the change is too invasive for now (so doesn't block this patch).
FWIW, I think statics could also be used to work around the trait limitation, but that too doesn't seem like quite what we want and would result in a slight syntax change.
Regarding the allowed state per object, we'd have to specify for each object somehow which states are supported (separate traits/an array of allowed states/...)
Anyways, that's for another patch.