feat(users): Created user managment & administration system.#16
feat(users): Created user managment & administration system.#16AdenMGB wants to merge 10 commits into
Conversation
DecDuck
left a comment
DecDuck
left a comment
There was a problem hiding this comment.
I think you might need to spend more time looking over how the backend is written.
I don't want to seem rude, but this isn't really well done.
| </div> | ||
|
|
||
| <!-- Save Button --> | ||
| <div class="flex justify-end pt-4"> |
There was a problem hiding this comment.
This whole form should be wrapped in a
element, and we should use@submit.prevent on the form, instead of @click on the button
| }) | ||
|
|
||
| // Save changes | ||
| async function saveChanges() { |
There was a problem hiding this comment.
This function shouldn't have a try-catch, instead using a saveChanges_wrapper that uses then, catch and finally.
Also should use an error component that renders in the form, rather than a modal. Check other forms (like signin or admin metadata import) for an example.
| name: "User Management", | ||
| enabled: true, | ||
| icon: UserGroupIcon, | ||
| route: "/admin/auth/users", |
There was a problem hiding this comment.
Can we refactor to /admin/users and then auth is under /admin/users/auth
| } | ||
|
|
||
| // Validate password | ||
| if (!body.password || typeof body.password !== 'string' || body.password.length < 14) { |
There was a problem hiding this comment.
Should probably be refactored as a single function in server/internal/security
| }, | ||
| }); | ||
|
|
||
| return { |
There was a problem hiding this comment.
Don't need to result a success message, just a 200/201 response (default)
There was a problem hiding this comment.
This should be split into separate user.get.ts, user.patch.ts and user.delete.ts. I can't review it properly if it's all in one file.
| import { defineEventHandler, readBody, createError } from "h3"; | ||
| import prisma from "~/server/internal/db/database"; | ||
|
|
||
| export default defineEventHandler(async (event) => { |
There was a problem hiding this comment.
We use h3 instead of event as this arg (I think this also needs to be updated elsewhere)
| }); | ||
| } | ||
|
|
||
| const body = await readBody(event); |
There was a problem hiding this comment.
We break out body into stuff like this:
const body = await readBody(event);
const displayName = body.displayName;
const email = body.email;
That lets us do multiple checks without repeating ourselves.
|
Ah well, don't be sorry there's a reason I'm mainly front end developer haha. At least I gave it a go, thanks for the feedback though. |
* Create account.vue with logout button * Update auth.rs to add signout command * Update lib.rs to pass sign_out command to frontend
2 participants
Authentication System
Note: Does NOT include sign out functionality, it is on a separate PR (#15)
What this does include: