Skip to content

Commit 2391580

Browse files
committed
fix(requests): stop editing a request from re-requesting covered seasons
Creating a request skips seasons the media already has, but editing one only looked at seasons held by other requests, so an edit could add a season that was already available and charge the owner's quota for it. The availability check applies to seasons being added, not to the ones the request already holds, otherwise editing an approved request would drop its own seasons the moment they started processing. The quota arithmetic now counts the seasons the edit actually ends up with rather than assuming that is every season it asked for.
1 parent 62f6a45 commit 2391580

2 files changed

Lines changed: 139 additions & 7 deletions

File tree

server/routes/request.test.ts

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
import { getRepository } from '@server/datasource';
1010
import Media from '@server/entity/Media';
1111
import { MediaRequest } from '@server/entity/MediaRequest';
12+
import Season from '@server/entity/Season';
1213
import SeasonRequest from '@server/entity/SeasonRequest';
1314
import { User } from '@server/entity/User';
1415
import { getSettings } from '@server/lib/settings';
@@ -243,6 +244,19 @@ async function seedTvMedia(tmdbId: number) {
243244
);
244245
}
245246

247+
async function seedMediaSeasons(
248+
tmdbId: number,
249+
seasons: { seasonNumber: number; status: MediaStatus }[]
250+
) {
251+
const media = await seedTvMedia(tmdbId);
252+
media.seasons = seasons.map(
253+
({ seasonNumber, status }) =>
254+
new Season({ seasonNumber, status, status4k: MediaStatus.UNKNOWN })
255+
);
256+
257+
return getRepository(Media).save(media);
258+
}
259+
246260
async function seedTvRequest(
247261
requestedBy: User,
248262
seasons: number[],
@@ -303,6 +317,102 @@ describe('PUT /request/:requestId (tv)', () => {
303317
});
304318
});
305319

320+
describe('PUT /request/:requestId (season availability)', () => {
321+
it('does not add a season the media already has', async () => {
322+
const requestRepo = getRepository(MediaRequest);
323+
const owner = await seedUser('friend@seerr.dev');
324+
const mediaRequest = await seedTvRequest(owner, [1]);
325+
await seedMediaSeasons(67890, [
326+
{ seasonNumber: 2, status: MediaStatus.AVAILABLE },
327+
]);
328+
329+
const agent = await loginAs('friend@seerr.dev', 'test1234');
330+
const res = await agent.put(`/request/${mediaRequest.id}`).send({
331+
mediaType: MediaType.TV,
332+
seasons: [1, 2, 3],
333+
});
334+
335+
assert.strictEqual(res.status, 200);
336+
337+
const saved = await requestRepo.findOneOrFail({
338+
where: { id: mediaRequest.id },
339+
});
340+
assert.deepStrictEqual(
341+
saved.seasons.map((s) => s.seasonNumber).sort(),
342+
[1, 3]
343+
);
344+
});
345+
346+
it('returns 202 when every requested season is already covered', async () => {
347+
const owner = await seedUser('friend@seerr.dev');
348+
const mediaRequest = await seedTvRequest(owner, [1]);
349+
await seedMediaSeasons(67890, [
350+
{ seasonNumber: 2, status: MediaStatus.AVAILABLE },
351+
]);
352+
353+
const agent = await loginAs('friend@seerr.dev', 'test1234');
354+
const res = await agent.put(`/request/${mediaRequest.id}`).send({
355+
mediaType: MediaType.TV,
356+
seasons: [2],
357+
});
358+
359+
assert.strictEqual(res.status, 202);
360+
});
361+
362+
it('keeps the seasons it already holds once they are available', async () => {
363+
const requestRepo = getRepository(MediaRequest);
364+
const owner = await seedUser('friend@seerr.dev');
365+
const mediaRequest = await seedTvRequest(owner, [1, 2]);
366+
await seedMediaSeasons(67890, [
367+
{ seasonNumber: 1, status: MediaStatus.AVAILABLE },
368+
{ seasonNumber: 2, status: MediaStatus.PROCESSING },
369+
]);
370+
371+
const agent = await loginAs('friend@seerr.dev', 'test1234');
372+
const res = await agent.put(`/request/${mediaRequest.id}`).send({
373+
mediaType: MediaType.TV,
374+
seasons: [1, 2],
375+
serverId: 3,
376+
});
377+
378+
assert.strictEqual(res.status, 200);
379+
380+
const saved = await requestRepo.findOneOrFail({
381+
where: { id: mediaRequest.id },
382+
});
383+
assert.deepStrictEqual(
384+
saved.seasons.map((s) => s.seasonNumber).sort(),
385+
[1, 2]
386+
);
387+
assert.strictEqual(saved.serverId, 3);
388+
});
389+
390+
it('does not charge quota for a season the media already has', async () => {
391+
const requestRepo = getRepository(MediaRequest);
392+
const owner = await seedUser('friend@seerr.dev', { tvQuotaLimit: 1 });
393+
const mediaRequest = await seedTvRequest(owner, [1]);
394+
await seedMediaSeasons(67890, [
395+
{ seasonNumber: 2, status: MediaStatus.AVAILABLE },
396+
]);
397+
398+
const agent = await loginAs('friend@seerr.dev', 'test1234');
399+
const res = await agent.put(`/request/${mediaRequest.id}`).send({
400+
mediaType: MediaType.TV,
401+
seasons: [1, 2],
402+
});
403+
404+
assert.strictEqual(res.status, 200);
405+
406+
const saved = await requestRepo.findOneOrFail({
407+
where: { id: mediaRequest.id },
408+
});
409+
assert.deepStrictEqual(
410+
saved.seasons.map((s) => s.seasonNumber),
411+
[1]
412+
);
413+
});
414+
});
415+
306416
describe('PUT /request/:requestId (quota)', () => {
307417
it('rejects adding seasons beyond the season limit', async () => {
308418
const requestRepo = getRepository(MediaRequest);

server/routes/request.ts

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -594,25 +594,47 @@ requestRoutes.put<{ requestId: string }>(
594594
return [...seasons, ...combinedSeasons];
595595
}, [] as number[]);
596596

597+
const currentSeasons = request.seasons.map((s) => s.seasonNumber);
598+
599+
// Seasons the media already covers cannot be requested again, while
600+
// the ones this request holds stay on it
601+
const coveredSeasons = (media.seasons ?? [])
602+
.filter(
603+
(season) =>
604+
season[request.is4k ? 'status4k' : 'status'] !==
605+
MediaStatus.UNKNOWN &&
606+
season[request.is4k ? 'status4k' : 'status'] !==
607+
MediaStatus.DELETED
608+
)
609+
.map((season) => season.seasonNumber)
610+
.filter((sn) => !currentSeasons.includes(sn));
611+
597612
const filteredSeasons = requestedSeasons.filter(
598613
(rs) => !existingSeasons.includes(rs)
599614
);
600615

601-
if (filteredSeasons.length === 0) {
616+
const keptSeasons = filteredSeasons.filter((sn) =>
617+
currentSeasons.includes(sn)
618+
);
619+
620+
const newSeasons = filteredSeasons.filter(
621+
(sn) =>
622+
!currentSeasons.includes(sn) && !coveredSeasons.includes(sn)
623+
);
624+
625+
const resultingSeasonCount = keptSeasons.length + newSeasons.length;
626+
627+
if (resultingSeasonCount === 0) {
602628
return next({
603629
status: 202,
604630
message: 'No seasons available to request',
605631
});
606632
}
607633

608-
const newSeasons = filteredSeasons.filter(
609-
(sn) => !request.seasons.map((s) => s.seasonNumber).includes(sn)
610-
);
611-
612634
// The seasons this request already holds are counted in the owner's
613635
// usage, so they only need paying for again on reassignment
614636
const priorSeasonCount = ownerChanging ? 0 : request.seasons.length;
615-
const requiredSeasons = filteredSeasons.length - priorSeasonCount;
637+
const requiredSeasons = resultingSeasonCount - priorSeasonCount;
616638

617639
if (!request.ignoreQuota && requiredSeasons > 0) {
618640
const quotas = await requestUser.getQuota();
@@ -629,7 +651,7 @@ requestRoutes.put<{ requestId: string }>(
629651
}
630652

631653
request.seasons = request.seasons.filter((rs) =>
632-
filteredSeasons.includes(rs.seasonNumber)
654+
keptSeasons.includes(rs.seasonNumber)
633655
);
634656

635657
if (newSeasons.length > 0) {

0 commit comments

Comments
 (0)