Added API routes to create and delete users - #294
oliverbates94 wants to merge 5 commits into
Conversation
create and delete users
tchapi
left a comment
There was a problem hiding this comment.
Thanks for the work. I'm overall ok for these two new APIs, but we need to make them right 馃檹馃徏
Apart from the in-line comments above:
-
Most lines are copy-pasted from
Admin\UserController::userCreate/userDelete.Utils::createPasswordlessUserWithDefaultObjects()already exists and does 90% of the create path; I think the right move is to extendUtils(e.g.createUserWithDefaultObjects(username, displayName, email, password, isAdmin)and adeleteUserAndObjects(User)or something like that) and call it from both controllers. -
No email validation. The admin path gets EmailType + Assert\Email on
Principal::$emailvia the form validator but the API path never runs the validator, so any string is stored as an email (which then ends up in mailto: share hrefs). -
I'll need some functional test added under
tests/Functional+ update of docs/api
|
Hi, I addressed all your comments I have some doubts about the "userCreate" implementation in the UserController: maybe it would be cleaner to split the create and edit functionality in two functions, let me know |
Hi! I'm not sure if this is the right approach to this, I basically duplicated the code from the UserController, but I needed these two API routes
Tell me if there's something I need to change!