Closed jxckUK closed 1 month ago
@jxckUK just to make sure, you want support for steamId
and discordId
on admin/manage/users
?
also, from looking at the code for admin/manage/users
it looks like it does not return all users, but only the first one... or there is some magic going on i dont see
@jxckUK just to make sure, you want support for
steamId
anddiscordId
onadmin/manage/users
?
This is already supported it seems just without the null?discordId= string, should be consistent on all three routes, either way is personally fine with me?
Users should only ever return one result, it shouldn't allow multiple accounts to share the same Discord or Steam identifiers.
users is plural so i assumed it included many, my apologies im trying to get familiar with the codebase its been a while.
i see what you mean now, with admin/manage/users/id
you dont have to put the id as null and specify the id version, but with the others you do.
i could go ahead and make them follow the same pattern as admin/manage/users
as that is more clean imo, but that would be a semi breaking change to any third party apps.
@MarshyMelloow im down to do this fullstack but want you opinion as you are the primary maintainer
The path /admin/manager/users/:id
would fetch the user based on the User ID instead of the Discord ID. A new route could be created to fetched based on Discord ID, without doing anything that would be a breaking change
cc @jxckUK @subtosharki
The path
/admin/manager/users/:id
would fetch the user based on the User ID instead of the Discord ID. A new route could be created to fetched based on Discord ID, without doing anything that would be a breaking changecc @jxckUK @subtosharki
Hmm odd - I tested locally and that route will correctly filter on Discord ID at present without any changes so I assume it has logic to filter both without explicit strings in the request.
This is what all the routes should do and for legacy compatibility the null?discordId= string should remain a possible filter for the routes.
@MarshyMelloow im down to do this fullstack but want you opinion as you are the primary maintainer
Go for it @subtosharki! The more contributions the better!
Go for it @subtosharki! The more contributions the better!
sounds good, its 11 pm so ill do it tomorrow
What version of Node.js are you using?
18
What operating system are you using?
Ubuntu
What version of SnailyCAD are you currently using?
v1.78.4 aae52da
Describe the Bug
When trying to apply the same filter that works on the
admin/manage/units
andadmin/manage/citizens
routes on theadmin/manage/users
it returns a 404 error.I am aware that this can be achieved by v1/admin/manage/users/{discordId} but the behaviour not being consistent leads to confusion.
Expected Behavior
It should filter the users based on the filter query specified.
To Reproduce
Run a GET query against https://CADURL/v1/admin/manage/users/null?discordId=DISCORDID.
Running queries against https://CADURL/v1/admin/manage/units/null?discordId=DISCORDID or https://CADURL/v1/admin/manage/citizens/null?discordId=DISCORDID will work fine.
Original commit that adding this functionality: https://github.com/SnailyCAD/snaily-cadv4/commit/43a63238063bf80d290f08f1b0afc3195d2ed8bc