Skip to content

Added API routes to create and delete users - #294

Open
oliverbates94 wants to merge 5 commits into
tchapi:mainfrom
oliverbates94:feature/api_user_creation_and_deletion
Open

oliverbates94 wants to merge 5 commits into
tchapi:mainfrom
oliverbates94:feature/api_user_creation_and_deletion

Conversation

@oliverbates94

Copy link
Copy Markdown

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!

@tchapi tchapi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 extend Utils (e.g. createUserWithDefaultObjects(username, displayName, email, password, isAdmin) and a deleteUserAndObjects(User) or something like that) and call it from both controllers.

  • No email validation. The admin path gets EmailType + Assert\Email on Principal::$email via 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

Comment thread src/Controller/Api/ApiController.php Outdated
Comment thread src/Controller/Api/ApiController.php Outdated
Comment thread src/Controller/Api/ApiController.php Outdated
Comment thread src/Controller/Api/ApiController.php Outdated
Comment thread src/Controller/Api/ApiController.php Outdated
Comment thread src/Controller/Api/ApiController.php Outdated
@oliverbates94

Copy link
Copy Markdown
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants