fix(overriderules): allows every user to be added to the override rules #1333
Conversation
gauthier-th
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const user: User = await res.json(); | ||
| return user; | ||
| }) | ||
| const distinctUsers = new Array( |
There was a problem hiding this comment.
Set to distinct, back to array to rejoin. Probably unnecessary as the repository query is set to distinct already, but it doesn't hurt.
There was a problem hiding this comment.
It doesn't hurt but it's not necessary at all, because the User Select will not allow you to add an user twice, and because the /user endpoint will not return an user twice even if specified twice.
It would be more clean without it.
There was a problem hiding this comment.
This is getting all the rules on the page, not just a single rule. I've implemented the fix, but the request will be making duplicates:
| const user: User = await res.json(); | ||
| return user; | ||
| }) | ||
| const distinctUsers = new Array( |
There was a problem hiding this comment.
It doesn't hurt but it's not necessary at all, because the User Select will not allow you to add an user twice, and because the /user endpoint will not return an user twice even if specified twice.
It would be more clean without it.
| (distinctUsers | ||
| ? `?includeIds=${encodeURIComponent(distinctUsers)}` | ||
| : '') |
There was a problem hiding this comment.
You can do a if (rule.users) before the const res = ... instead of this ternary. It would be more clear and avoid an unnecessary api call if the override rule has no rule with an user.
| const includeIds = req.query.includeIds | ||
| ? req.query.includeIds.toString().split(',') | ||
| : []; | ||
| const pageSize = req.query.take |
There was a problem hiding this comment.
Without this, if you had more than ten users in a single rule, it would only display the first ten. There may be better ways of doing this (i.e. from the Tile rather than in here), but this felt cleanest to me.
ETA - this does introduce the small issue where if you pass in duplicates, your pageSize is actually too large. i.e. I pass in eleven userIds but they're all the same, this will pageSize to 11. This seems minor but wanted to call it out.
There was a problem hiding this comment.
Then you could make the ids unique to make sure this doesn't happen? It would easily remove the issue
| const includeIds = req.query.includeIds | ||
| ? req.query.includeIds.toString().split(',') | ||
| : []; | ||
| const pageSize = req.query.take |
There was a problem hiding this comment.
Then you could make the ids unique to make sure this doesn't happen? It would easily remove the issue
Description
Screenshot (if UI-related)
To-Dos
pnpm buildpnpm i18n:extractIssues Fixed or Closed