CRUD ClanRoles / UI-UX #1202
CRUD ClanRoles / UI-UX #1202
Conversation
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
YouriDK
force-pushed
the
901---Clan-roles-(with-editable-name/permissions)
branch
from
b411df7 to
803dfbd
Compare
There was a problem hiding this comment.
There's no clan role creation for the existing clans in the migration anymore?
There was a problem hiding this comment.
What do you mean ? Master of clan will be able to create clan to delegate, there was no initial role before except the master one
There was a problem hiding this comment.
Huh?
Previously, you had SQL queries in your migration file that created default roles for the existing clans, which led to me telling you not to use FOR clauses in your queries, but now, there's nothing like that in your current migration file
server/prisma/schema.prisma
Outdated
| name String @db.VarChar(50) | ||
| // Permissions | ||
| permissions ClanPermission[] @default([]) | ||
| members ClanMemberRole[] |
There was a problem hiding this comment.
I might be missing something, by why don't we simply have an array of brutes here instead?
I don't see the use for an additional ClanMemberRole table
There was a problem hiding this comment.
Not adding another table will implying we are sure that we won't need other datas ( like who assign who and other detail ). Adding datas will imply a roll back to a new table if we switch. i though that might a better way and prevent for next features
There was a problem hiding this comment.
To be honest I don't see the use for it, I'd rather keep everything simpler
There was a problem hiding this comment.
I'll remove the table ClanMemberRole and adapt the rest of the permissions with an array of brutes for members. I'll have some test to do
server/src/controllers/Clans.ts
Outdated
| // Update the role | ||
| const role = await prisma.clanRole.update({ | ||
| where: { id: roleId }, | ||
| data: req.body, |
There was a problem hiding this comment.
I would rather set the fields to update manually instead of trusting the body here.
You could abuse this to update tables linked to clan roles this way otherwise
There was a problem hiding this comment.
I assumme that you want a more precise datas instead of the whole body like data: {
name: req.body.name,
permissions: req.body.permissions,
},
There was a problem hiding this comment.
Exactly, with some minor checks against the variables, to check if they're the correct type, etc
| select: { | ||
| id: true, | ||
| masterOfClan: { select: { id: true } }, | ||
| clanRoles: { | ||
| select: { | ||
| role: { | ||
| select: { permissions: true }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
? It's not used anywhere in the method
There was a problem hiding this comment.
Some logic was missing because we will need this to check for the role canSelectWarFighters , so this part is kinda use now
server/src/utils/clan/permissions.ts
Outdated
| }; | ||
|
|
||
|
|
||
| export const getBruteRoles = async ( |
There was a problem hiding this comment.
Huh? No I mean I don't see the method getBruteRoles used anywhere in your code
Co-authored-by: Franck <perso@franckdemoute.fr>
2 participants
Implementation of Clan roles a lil a bit late xD