RFC - Store user record as array instead of entity
Closed this issue · 5 comments
Is there any particular benefit of maintaining the user record as entity instead of just array provided by Auth? It just adds unnecessary overhead of type check and array to entity conversion imo. I would prefer the user record maintained by plugin to be in same format as Auth itself.
IIRC I had done it like this because using crud-users/dev-rbac, there are things one might use (from the CrudUsers\Model\Entity\UserInterface). I could be totally wrong too, I mainly took the code out from an app I had developed a little time ago.
Hmm okay, I guess we can keep entity in that case.
Still having all the user record getter/setter/converter methods in trait doesn't feel right to me. Perhaps they should be in the listener instead? I would prefer the trait to be as lean as possible and just do the job of attaching the listener to the models. Also instead of having the trait listen for Auth.afterIdentify make the listener itself listen for it.
If you don't think those methods could be re-used by a class using the trait, then am ok with it not being part of the trait. It was initially put there to allow control (i.e. over-riding the current logged in user - from shell for example).
As for the Auth.afterIdentify moving to the listener, I totally agree.
Making listener directly listen to Auth.afterIdentify cannot be done without moving record conversion methods too as the listener needs entity while Auth will send an array.
Current setup works and I have got 90%+ coverage, so let's just keep it this way until we come up with stronger reasons to change :)