Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(overriderules): allows every user to be added to the override rules #1333

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from
+34 −15
Open

fix(overriderules): allows every user to be added to the override rules #1333

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c7e4a89
fix: allows every user to be added to the override rules
RankWeis Feb 5, 2025
d22b95f
fix: missed case in defaulting includeIds
RankWeis Feb 5, 2025
4d2ce12
chore: code cleanup
RankWeis Feb 5, 2025
1cac49c
fix: fixing bad commit
RankWeis Feb 5, 2025
cdb135c
fix(overriderules): removing unnecessary api call, cleaning code
RankWeis Feb 6, 2025
73608d9
fix(overriderules): displaying more than ten users at a time
RankWeis Feb 6, 2025
69ea315
fix(overriderules): pageSize unaffected by duplicate includeIds
RankWeis Feb 6, 2025
16474cf
chore(overriderules): renaming inaccurately named constant
RankWeis Feb 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions overseerr-api.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3781,6 +3781,11 @@ paths:
required: false
schema:
type: string
- in: query
name: includeIds
required: false
schema:
type: string
responses:
'200':
description: A JSON array of all users
Expand Down
8 changes: 8 additions & 0 deletions server/routes/user/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ router.get('/', async (req, res, next) => {
const pageSize = req.query.take ? Number(req.query.take) : 10;
const skip = req.query.skip ? Number(req.query.skip) : 0;
const q = req.query.q ? req.query.q.toString().toLowerCase() : '';
const includeIds = req.query.includeIds
? req.query.includeIds.toString().split(',')
: '';
let query = getRepository(User).createQueryBuilder('user');

if (q) {
Expand All @@ -44,6 +47,10 @@ router.get('/', async (req, res, next) => {
);
}

if (includeIds.length > 0) {
query.andWhereInIds(includeIds);
}

switch (req.query.sort) {
case 'updated':
query = query.orderBy('user.updatedAt', 'DESC');
Expand Down Expand Up @@ -84,6 +91,7 @@ router.get('/', async (req, res, next) => {
const [users, userCount] = await query
.take(pageSize)
.skip(skip)
.distinct(true)
.getManyAndCount();

return res.status(200).json({
Expand Down
7 changes: 6 additions & 1 deletion src/components/Selector/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,12 @@

const users = defaultValue.split(',');

const res = await fetch(`/api/v1/user`);
const res = await fetch(
`/api/v1/user${
defaultValue ? `?includeIds=${encodeURIComponent(defaultValue)}` : ''
Fixed Show fixed Hide fixed
}`
);

if (!res.ok) {
throw new Error('Network response was not ok');
}
Expand Down
24 changes: 13 additions & 11 deletions src/components/Settings/OverrideRule/OverrideRuleTile.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,18 +73,20 @@ const OverrideRuleTile = ({
})
);
setKeywords(keywords);
const users = await Promise.all(
rules
.map((rule) => rule.users?.split(','))
.flat()
.filter((userId) => userId)
.map(async (userId) => {
const res = await fetch(`/api/v1/user/${userId}`);
if (!res.ok) throw new Error();
const user: User = await res.json();
return user;
})
const distinctUsers = new Array(
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set to distinct, back to array to rejoin. Probably unnecessary as the repository query is set to distinct already, but it doesn't hurt.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

image

...new Set(
rules
.map((rule) => rule.users)
.filter((users) => users)
.flat()
.flatMap((userIds) => userIds?.split(','))
)
).join(',');
const res = await fetch(
`/api/v1/user?includeIds=${encodeURIComponent(distinctUsers)}`
);
if (!res.ok) throw new Error();
const users: User[] = (await res.json()).results;
setUsers(users);
})();
}, [rules]);
Expand Down
Loading