Prevent self deletion in users page - issue 309 #241
Conversation
suratdas
changed the title
src/services/users.service.ts
Outdated
|
|
||
| async function remove(ids: (string | number)[]): Promise<boolean> { | ||
|
|
||
| const userString = localStorage.getItem("user"); |
There was a problem hiding this comment.
service is responsible only for sending requests
could you move this into component as validation step?
src/services/users.service.ts
Outdated
| const userString = localStorage.getItem("user"); | ||
| if (userString) { | ||
| const user: User = JSON.parse(userString); | ||
| if (ids.includes(user.id)) { |
There was a problem hiding this comment.
current user id could be taken from user.context
| payload: userList.filter((user) => !ids.includes(user.id)), | ||
| }); | ||
| .then((value) => { | ||
| if (value.toString().trim().length === 0) { |
There was a problem hiding this comment.
could you elaborate why we need this condition?
| payload: userList.filter((user) => !ids.includes(user.id)), | ||
| }); | ||
| } else { | ||
| enqueueSnackbar(`You cannot delete yourself.`, { |
There was a problem hiding this comment.
consider showing error in case ids contains current user id and not sending request with error or filtering it out from array showing warning that current user id was not taken into request
There was a problem hiding this comment.
I have kept showing an error in case ids contains the current user id. That way, we are giving immediate feedback by explicitly telling the user to remove the current user. Please let me know what you think.
|
Fixed review comments. |
Addresses Visual-Regression-Tracker/Visual-Regression-Tracker#309