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
Open

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

Show file tree
Hide file tree
Changes from 6 commits
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
12 changes: 11 additions & 1 deletion server/routes/user/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,12 @@ const router = Router();

router.get('/', async (req, res, next) => {
try {
const pageSize = req.query.take ? Number(req.query.take) : 10;
const includeIds = req.query.includeIds
? req.query.includeIds.toString().split(',')
: [];
const pageSize = req.query.take
Copy link
Author

@RankWeis RankWeis Feb 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Then you could make the ids unique to make sure this doesn't happen? It would easily remove the issue

? Number(req.query.take)
: Math.max(10, includeIds.length);
const skip = req.query.skip ? Number(req.query.skip) : 0;
const q = req.query.q ? req.query.q.toString().toLowerCase() : '';
let query = getRepository(User).createQueryBuilder('user');
Expand All @@ -44,6 +49,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 +93,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
5 changes: 4 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,10 @@ export const UserSelector = ({

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

const res = await fetch(`/api/v1/user`);
const res = await fetch(
`/api/v1/user?includeIds=${encodeURIComponent(defaultValue)}`
);

if (!res.ok) {
throw new Error('Network response was not ok');
}
Expand Down
25 changes: 12 additions & 13 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,19 +73,18 @@ 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;
})
);
setUsers(users);
const distinctUsers = rules
.map((rule) => rule.users)
.filter((users) => users)
.join(',');
if (distinctUsers) {
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