mirror of
https://github.com/jorenn92/Maintainerr.git
synced 2026-06-01 18:48:13 +02:00
fix(rules): scope exclusions to their rule group under TypeORM 1.0 (#2991)
* fix(rules): scope exclusions to their rule group under TypeORM 1.0 Adopt TypeORM 1.0 where-clause null semantics (default throw; IsNull() to match NULL) instead of 0.3.x's silent-ignore footgun, fixing latent exclusion bugs surfaced by the typeorm 1.0.0 bump (#2983): - getExclusions: a group-scoped exclusion no longer leaks into other rule groups (and no longer returns duplicates); global exclusions still apply everywhere. - setExclusion: global exclusions subsume scoped ones — an item is global or scoped, never both. - removeExclusion: removing a global exclusion no longer writes a spurious collection-log entry; logs media + ownership. - getCollection: a null/blank id returns undefined instead of an arbitrary first collection. Bumps engines.node to TypeORM 1.0's floor and adds getter-property and exclusion-scoping regression coverage. * feat(ui): warn before a global exclusion replaces rule-group exclusions When adding a global exclusion for an item that already has rule-group exclusions, show a confirmation listing each "<item> — <rule group>" (the group links to its collection, reusing the backdrop's maintainerr-status data so labels/links match and stay fresh) before proceeding, since going global removes those scoped exclusions. Only triggers on the Add action, never on Remove. * test(ui): cover the global-exclusion warning in AddModal * fix(ui): clarify Add/Remove action labels in the media modal * fix(ui): don't block global exclusion when warning prefetch fails
This commit is contained in:
@@ -357,6 +357,24 @@ initialization`.
|
||||
`node_modules/jest/bin/jest.js` exists locally. For direct Jest-under-Node
|
||||
entrypoints, use `yarn node $(yarn bin jest) ...`.
|
||||
|
||||
### TypeORM repository conventions
|
||||
|
||||
The server is on **TypeORM 1.x** (`better-sqlite3`). Two conventions to follow
|
||||
when writing repository code:
|
||||
|
||||
- **`relations` and `select` use the object form** —
|
||||
`relations: { ruleGroup: true }`, not the array form. `find`/`findOne` only
|
||||
accept objects.
|
||||
- **Never put a bare `null`/`undefined` in a `where`.** TypeORM 1.x throws on
|
||||
them (the default `invalidWhereValuesBehavior` is `throw`, and we keep that
|
||||
default rather than masking it). To match SQL `NULL`, use `IsNull()` —
|
||||
e.g. `where: { ruleGroupId: IsNull() }`, `where: { sizeBytes: Not(IsNull()) }`.
|
||||
For an optional value that may be absent, omit the key (conditional spread:
|
||||
`...(x !== undefined ? { x } : {})`) or guard before querying — don't pass it
|
||||
through. `where: {}` (no keys) is fine for the settings-singleton lookups.
|
||||
Note: a bare `null` does **not** mean "ignore the filter" — older code that
|
||||
relied on that was a latent bug (it matched everything); use `IsNull()`.
|
||||
|
||||
### Writing DATA migrations (backfills, no schema change)
|
||||
|
||||
- `migration:generate` **cannot** produce them — it diffs entity metadata vs DB
|
||||
|
||||
@@ -54,7 +54,7 @@ This is a **TypeScript monorepo** managed with **Turborepo** and **Yarn workspac
|
||||
### Backend (`apps/server/`)
|
||||
|
||||
- **Framework**: Nest.js v11+ with TypeScript
|
||||
- **Database**: TypeORM with SQLite
|
||||
- **Database**: TypeORM 1.x with SQLite (`better-sqlite3` driver)
|
||||
- **Testing**: Jest with @suites for dependency mocking and unit testing
|
||||
- **API Documentation**: Swagger/OpenAPI
|
||||
- **Validation**: Zod v4+ schemas with nestjs-zod
|
||||
@@ -329,7 +329,7 @@ degrade gracefully.
|
||||
|
||||
### Environment Setup
|
||||
|
||||
- **Node.js**: Version 20.19.0+ or 22.12.0+
|
||||
- **Node.js**: Version 20.19.0+, 22.13.0+, or 24.11.0+ (the floor is set by TypeORM 1.0.0's engine requirement; the Docker image ships Node 26)
|
||||
- **Package Manager**: Yarn 4.11 (managed via corepack)
|
||||
- **Data Directory**: Requires `data/` folder with proper permissions for development
|
||||
|
||||
|
||||
@@ -127,9 +127,11 @@ export class CollectionsService {
|
||||
try {
|
||||
if (title) {
|
||||
return await this.collectionRepo.findOne({ where: { title: title } });
|
||||
} else {
|
||||
}
|
||||
if (id != null) {
|
||||
return await this.collectionRepo.findOne({ where: { id: id } });
|
||||
}
|
||||
return undefined;
|
||||
} catch (error) {
|
||||
this.logger.warn('An error occurred while performing collection actions');
|
||||
this.logger.debug(error);
|
||||
|
||||
@@ -250,20 +250,22 @@ export class NotificationService implements OnModuleInit {
|
||||
notificationId: number;
|
||||
}) {
|
||||
try {
|
||||
const ruleGroup = await this.ruleGroupRepo.findOne({
|
||||
where: { id: payload.rulegroupId },
|
||||
});
|
||||
if (payload.rulegroupId && payload.notificationId) {
|
||||
const ruleGroup = await this.ruleGroupRepo.findOne({
|
||||
where: { id: payload.rulegroupId },
|
||||
});
|
||||
|
||||
const notificationConfig = await this.notificationRepo.findOne({
|
||||
where: { id: payload.notificationId },
|
||||
});
|
||||
const notificationConfig = await this.notificationRepo.findOne({
|
||||
where: { id: payload.notificationId },
|
||||
});
|
||||
|
||||
if (ruleGroup && notificationConfig) {
|
||||
ruleGroup.notifications = ruleGroup.notifications.filter(
|
||||
(c) => c.id !== payload.notificationId,
|
||||
);
|
||||
await this.ruleGroupRepo.save(ruleGroup);
|
||||
return { code: 1, result: 'success' };
|
||||
if (ruleGroup && notificationConfig) {
|
||||
ruleGroup.notifications = ruleGroup.notifications.filter(
|
||||
(c) => c.id !== payload.notificationId,
|
||||
);
|
||||
await this.ruleGroupRepo.save(ruleGroup);
|
||||
return { code: 1, result: 'success' };
|
||||
}
|
||||
}
|
||||
|
||||
return { code: 0, result: 'failed' };
|
||||
|
||||
@@ -0,0 +1,174 @@
|
||||
import { MediaItem } from '@maintainerr/contracts';
|
||||
import { TestBed } from '@suites/unit';
|
||||
import { writeFileSync } from 'fs';
|
||||
import {
|
||||
Application,
|
||||
Property,
|
||||
RuleConstants,
|
||||
} from '../constants/rules.constants';
|
||||
import { EmbyGetterService } from './emby-getter.service';
|
||||
import { JellyfinGetterService } from './jellyfin-getter.service';
|
||||
import { PlexGetterService } from './plex-getter.service';
|
||||
import { RadarrGetterService } from './radarr-getter.service';
|
||||
import { SeerrGetterService } from './seerr-getter.service';
|
||||
import { SonarrGetterService } from './sonarr-getter.service';
|
||||
import { StreamystatsGetterService } from './streamystats-getter.service';
|
||||
import { TautulliGetterService } from './tautulli-getter.service';
|
||||
|
||||
// Per-property getter coverage harness.
|
||||
//
|
||||
// Purpose: prove that EVERY rule property (all ~166 across all applications)
|
||||
// has a getter path that resolves to a RuleValueType without throwing, and
|
||||
// that the result is independent of the TypeORM version. Run on both 0.3.x and
|
||||
// 1.0.x with the same code and diff the emitted JSON (GETTER_MATRIX_OUT) — it
|
||||
// must be byte-identical, since getters derive values from (mocked) API
|
||||
// responses in memory, not from the database.
|
||||
//
|
||||
// Deterministic by construction: dependencies are auto-mocked (return
|
||||
// undefined) and the input media item is a fixed literal — no faker, no clock.
|
||||
|
||||
const libItem: MediaItem = {
|
||||
id: 'fixed-item-1',
|
||||
title: 'Fixed Item',
|
||||
guid: 'plex://movie/fixed',
|
||||
type: 'movie',
|
||||
addedAt: new Date('2020-01-01T00:00:00.000Z'),
|
||||
updatedAt: new Date('2020-06-01T00:00:00.000Z'),
|
||||
providerIds: { tvdb: ['1'], tmdb: ['2'], imdb: ['tt3'] },
|
||||
mediaSources: [],
|
||||
library: { id: 'lib-1', title: 'Movies' },
|
||||
index: 1,
|
||||
summary: 'fixed summary',
|
||||
year: 2020,
|
||||
} as MediaItem;
|
||||
|
||||
// Minimal rule group stub so getters that read ruleGroup.* don't hit a bare
|
||||
// TypeError before their own try/catch.
|
||||
const ruleGroup = {
|
||||
id: 1,
|
||||
collection: { id: 1 },
|
||||
dataType: 1,
|
||||
useRules: true,
|
||||
rules: [],
|
||||
} as any;
|
||||
|
||||
const buildGetter = async <T>(cls: new (...args: any[]) => T): Promise<T> => {
|
||||
const { unit } = await TestBed.solitary(cls as any).compile();
|
||||
return unit as T;
|
||||
};
|
||||
|
||||
// Per-application: how to construct the getter and how to invoke .get() with
|
||||
// the correct positional arguments for that getter's signature.
|
||||
const apps: Array<{
|
||||
app: Application;
|
||||
build: () => Promise<{ get: (id: number) => Promise<unknown> }>;
|
||||
}> = [
|
||||
{
|
||||
app: Application.PLEX,
|
||||
build: async () => {
|
||||
const g = await buildGetter(PlexGetterService);
|
||||
return { get: (id) => g.get(id, libItem, undefined, ruleGroup) };
|
||||
},
|
||||
},
|
||||
{
|
||||
app: Application.RADARR,
|
||||
build: async () => {
|
||||
const g = await buildGetter(RadarrGetterService);
|
||||
return { get: (id) => g.get(id, libItem, ruleGroup) };
|
||||
},
|
||||
},
|
||||
{
|
||||
app: Application.SONARR,
|
||||
build: async () => {
|
||||
const g = await buildGetter(SonarrGetterService);
|
||||
return { get: (id) => g.get(id, libItem, undefined, ruleGroup) };
|
||||
},
|
||||
},
|
||||
{
|
||||
app: Application.SEERR,
|
||||
build: async () => {
|
||||
const g = await buildGetter(SeerrGetterService);
|
||||
return { get: (id) => g.get(id, libItem, undefined) };
|
||||
},
|
||||
},
|
||||
{
|
||||
app: Application.TAUTULLI,
|
||||
build: async () => {
|
||||
const g = await buildGetter(TautulliGetterService);
|
||||
return { get: (id) => g.get(id, libItem, undefined, ruleGroup) };
|
||||
},
|
||||
},
|
||||
{
|
||||
app: Application.JELLYFIN,
|
||||
build: async () => {
|
||||
const g = await buildGetter(JellyfinGetterService);
|
||||
return { get: (id) => g.get(id, libItem, undefined, ruleGroup) };
|
||||
},
|
||||
},
|
||||
{
|
||||
app: Application.EMBY,
|
||||
build: async () => {
|
||||
const g = await buildGetter(EmbyGetterService);
|
||||
return { get: (id) => g.get(id, libItem, undefined, ruleGroup) };
|
||||
},
|
||||
},
|
||||
{
|
||||
app: Application.STREAMYSTATS,
|
||||
build: async () => {
|
||||
const g = await buildGetter(StreamystatsGetterService);
|
||||
return { get: (id) => g.get(id, libItem) };
|
||||
},
|
||||
},
|
||||
];
|
||||
|
||||
// Deterministic, version-independent summary of a getter result (no raw values
|
||||
// like random ids/dates — we record shape, which is what must stay stable).
|
||||
const describeResult = (v: unknown): string => {
|
||||
if (v === undefined) return 'undefined';
|
||||
if (v === null) return 'null';
|
||||
if (Array.isArray(v)) return `array(${v.length})`;
|
||||
if (v instanceof Date) return 'date';
|
||||
return typeof v;
|
||||
};
|
||||
|
||||
const propsFor = (app: Application): Property[] =>
|
||||
new RuleConstants().applications.find((a) => a.id === app)?.props ?? [];
|
||||
|
||||
describe('Getter property coverage matrix (all rule properties)', () => {
|
||||
const matrix: Record<string, string> = {};
|
||||
const threw: string[] = [];
|
||||
|
||||
it('every rule property resolves to a RuleValueType without throwing', async () => {
|
||||
for (const { app, build } of apps) {
|
||||
const getter = await build();
|
||||
for (const prop of propsFor(app)) {
|
||||
const key = `${Application[app]}.${prop.id} ${prop.humanName}`;
|
||||
try {
|
||||
const result = await getter.get(prop.id);
|
||||
matrix[key] = describeResult(result);
|
||||
} catch (e) {
|
||||
matrix[key] = `THREW:${(e as Error).constructor.name}`;
|
||||
threw.push(`${key} -> ${(e as Error).message.split('\n')[0]}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Emit the full matrix for cross-version diffing.
|
||||
const sorted = Object.fromEntries(
|
||||
Object.entries(matrix).sort(([a], [b]) => a.localeCompare(b)),
|
||||
);
|
||||
const out = process.env.GETTER_MATRIX_OUT ?? '/tmp/getter-matrix.json';
|
||||
writeFileSync(out, JSON.stringify(sorted, null, 2));
|
||||
|
||||
// The invariant: no property throws out of its getter.
|
||||
expect(threw).toEqual([]);
|
||||
});
|
||||
|
||||
it('covers every property defined in the rule constants', () => {
|
||||
const expected = apps.reduce(
|
||||
(sum, { app }) => sum + propsFor(app).length,
|
||||
0,
|
||||
);
|
||||
expect(Object.keys(matrix)).toHaveLength(expected);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,118 @@
|
||||
import { DataSource, EntitySchema, Repository } from 'typeorm';
|
||||
import { createMockLogger } from '../../../test/utils/data';
|
||||
import {
|
||||
buildExclusionCascadeSets,
|
||||
isMediaItemExcluded,
|
||||
} from './helpers/exclusion-cascade.helper';
|
||||
import { RulesService } from './rules.service';
|
||||
|
||||
// Deterministic, real-DB integration test for cross-group exclusion scoping.
|
||||
//
|
||||
// This composes the two real pieces the rule executor uses:
|
||||
// 1. RulesService.getExclusions(rulegroupId) — against a real SQLite repo
|
||||
// 2. the executor's gate: `if (!isMediaItemExcluded(cascade, item)) add(item)`
|
||||
// (rule-executor.service.ts handleCollection)
|
||||
//
|
||||
// It proves end-to-end that an item excluded *in another group* IS added to a
|
||||
// group that didn't exclude it, while global exclusions still apply everywhere.
|
||||
// No cron/scheduler/media-server — fully reproducible (unlike a live dev run).
|
||||
|
||||
// EntitySchema mirror of the Exclusion entity (avoids decorator/metadata setup).
|
||||
const ExclusionSchema = new EntitySchema<any>({
|
||||
name: 'Exclusion',
|
||||
tableName: 'exclusion',
|
||||
columns: {
|
||||
id: { type: 'integer', primary: true, generated: true },
|
||||
mediaServerId: { type: 'varchar', nullable: true },
|
||||
ruleGroupId: { type: 'integer', nullable: true },
|
||||
parent: { type: 'varchar', nullable: true },
|
||||
type: { type: 'varchar', nullable: true },
|
||||
},
|
||||
});
|
||||
|
||||
const GROUP_A = 168; // owns a scoped exclusion for MOVIE_X
|
||||
const GROUP_B = 169; // a different group
|
||||
const MOVIE_X = 'movie-x'; // scoped-excluded in A only
|
||||
const MOVIE_G = 'movie-global'; // globally excluded
|
||||
|
||||
describe('Exclusion scoping (real DB) — excluded-in-A item is added in B', () => {
|
||||
let ds: DataSource;
|
||||
let repo: Repository<any>;
|
||||
let service: RulesService;
|
||||
|
||||
const makeService = (exclusionRepo: Repository<any>) =>
|
||||
new RulesService(
|
||||
{} as any, // rulesRepository
|
||||
{} as any, // ruleGroupRepository
|
||||
{} as any, // collectionMediaRepository
|
||||
{} as any, // communityRuleKarmaRepository
|
||||
exclusionRepo as any,
|
||||
{} as any, // settingsRepo
|
||||
{} as any, // radarrSettingsRepo
|
||||
{} as any, // sonarrSettingsRepo
|
||||
{} as any, // collectionService
|
||||
{} as any, // mediaServerFactory
|
||||
{} as any, // connection
|
||||
{} as any, // ruleYamlService
|
||||
{} as any, // ruleComparatorServiceFactory
|
||||
{} as any, // ruleMigrationService
|
||||
{ emit: jest.fn() } as any, // eventEmitter
|
||||
createMockLogger() as any,
|
||||
);
|
||||
|
||||
beforeAll(async () => {
|
||||
ds = new DataSource({
|
||||
type: 'better-sqlite3',
|
||||
database: ':memory:',
|
||||
entities: [ExclusionSchema],
|
||||
synchronize: true,
|
||||
});
|
||||
await ds.initialize();
|
||||
repo = ds.getRepository('Exclusion');
|
||||
await repo.save([
|
||||
{ mediaServerId: MOVIE_X, ruleGroupId: GROUP_A, type: 'movie' },
|
||||
{ mediaServerId: MOVIE_G, ruleGroupId: null, type: 'movie' },
|
||||
]);
|
||||
service = makeService(repo);
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await ds.destroy();
|
||||
});
|
||||
|
||||
// Mirrors the executor gate: item is ADDED when not excluded.
|
||||
const isAddedTo = async (rulegroupId: number, itemId: string) => {
|
||||
const exclusions = await service.getExclusions(rulegroupId);
|
||||
const cascade = buildExclusionCascadeSets(exclusions as any);
|
||||
return !isMediaItemExcluded(cascade, { id: itemId });
|
||||
};
|
||||
|
||||
it('group B (no own exclusion for MOVIE_X) ADDS it — the A-scoped exclusion does not leak', async () => {
|
||||
expect(await isAddedTo(GROUP_B, MOVIE_X)).toBe(true);
|
||||
});
|
||||
|
||||
it('group A (owns the exclusion) keeps MOVIE_X excluded', async () => {
|
||||
expect(await isAddedTo(GROUP_A, MOVIE_X)).toBe(false);
|
||||
});
|
||||
|
||||
it('a global exclusion still applies in every group', async () => {
|
||||
expect(await isAddedTo(GROUP_A, MOVIE_G)).toBe(false);
|
||||
expect(await isAddedTo(GROUP_B, MOVIE_G)).toBe(false);
|
||||
});
|
||||
|
||||
it('getExclusions(B) returns only global + B-owned (no A-scoped, no duplicates)', async () => {
|
||||
const result = await service.getExclusions(GROUP_B);
|
||||
expect(result.map((e: any) => e.mediaServerId).sort()).toEqual([MOVIE_G]);
|
||||
});
|
||||
|
||||
it('contrast: the OLD behaviour (global part = all rows) WOULD leak A into B', async () => {
|
||||
// 0.3.x `find({ where: { ruleGroupId: null } })` ignored the key and
|
||||
// returned ALL rows. Reproduce that "global part" and show MOVIE_X leaks.
|
||||
const oldGlobalPart = await repo.find(); // all exclusions
|
||||
const bSpecific = await repo.find({ where: { ruleGroupId: GROUP_B } });
|
||||
const oldResult = [...bSpecific, ...oldGlobalPart];
|
||||
const cascade = buildExclusionCascadeSets(oldResult as any);
|
||||
// Under OLD, MOVIE_X (scoped to A) would be excluded from B too:
|
||||
expect(isMediaItemExcluded(cascade, { id: MOVIE_X })).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,182 @@
|
||||
import { FindOperator } from 'typeorm';
|
||||
import { createMockLogger } from '../../../test/utils/data';
|
||||
import { RulesService } from './rules.service';
|
||||
|
||||
// Regression coverage for global-exclusion handling (ruleGroupId IS NULL).
|
||||
// TypeORM 1.x throws on a bare `null` in a `where` clause, so these paths must
|
||||
// use IsNull() and must not feed a null id into a lookup.
|
||||
describe('RulesService exclusions — global (null ruleGroupId) handling', () => {
|
||||
const logger = createMockLogger();
|
||||
|
||||
const createService = (overrides?: {
|
||||
exclusionRepo?: any;
|
||||
ruleGroupRepository?: any;
|
||||
collectionService?: any;
|
||||
mediaServerFactory?: any;
|
||||
}) => {
|
||||
const exclusionRepo = overrides?.exclusionRepo ?? {
|
||||
find: jest.fn().mockResolvedValue([]),
|
||||
findOne: jest.fn().mockResolvedValue(undefined),
|
||||
delete: jest.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
const ruleGroupRepository = overrides?.ruleGroupRepository ?? {
|
||||
findOne: jest.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
const collectionService = overrides?.collectionService ?? {
|
||||
CollectionLogRecordForChild: jest.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
const mediaServerFactory = overrides?.mediaServerFactory ?? {
|
||||
getService: jest.fn(),
|
||||
};
|
||||
|
||||
const service = new RulesService(
|
||||
{} as any, // rulesRepository
|
||||
ruleGroupRepository as any,
|
||||
{} as any, // collectionMediaRepository
|
||||
{} as any, // communityRuleKarmaRepository
|
||||
exclusionRepo as any,
|
||||
{} as any, // settingsRepo
|
||||
{} as any, // radarrSettingsRepo
|
||||
{} as any, // sonarrSettingsRepo
|
||||
collectionService as any,
|
||||
mediaServerFactory as any,
|
||||
{} as any, // connection
|
||||
{} as any, // ruleYamlService
|
||||
{} as any, // ruleComparatorServiceFactory
|
||||
{} as any, // ruleMigrationService
|
||||
{} as any, // eventEmitter
|
||||
logger as any,
|
||||
);
|
||||
|
||||
return {
|
||||
service,
|
||||
exclusionRepo,
|
||||
ruleGroupRepository,
|
||||
collectionService,
|
||||
mediaServerFactory,
|
||||
};
|
||||
};
|
||||
|
||||
const isNullOperator = (value: unknown) =>
|
||||
value instanceof FindOperator && value.type === 'isNull';
|
||||
|
||||
beforeEach(() => jest.clearAllMocks());
|
||||
|
||||
it('getExclusions(rulegroupId) fetches global exclusions with IsNull(), not bare null', async () => {
|
||||
const { service, exclusionRepo } = createService();
|
||||
|
||||
await service.getExclusions(5);
|
||||
|
||||
// first call: the rule-group-specific exclusions
|
||||
expect(exclusionRepo.find).toHaveBeenNthCalledWith(1, {
|
||||
where: { ruleGroupId: 5 },
|
||||
});
|
||||
// second call: the global exclusions — must use IsNull(), never `null`
|
||||
const globalCallWhere = exclusionRepo.find.mock.calls[1][0].where;
|
||||
expect(isNullOperator(globalCallWhere.ruleGroupId)).toBe(true);
|
||||
});
|
||||
|
||||
it('removeExclusion skips the rule-group lookup for a global exclusion (null ruleGroupId)', async () => {
|
||||
const exclusionRepo = {
|
||||
findOne: jest
|
||||
.fn()
|
||||
.mockResolvedValue({ id: 1, ruleGroupId: null, mediaServerId: 'a' }),
|
||||
delete: jest.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
const ruleGroupRepository = { findOne: jest.fn() };
|
||||
const { service } = createService({ exclusionRepo, ruleGroupRepository });
|
||||
|
||||
const result = await service.removeExclusion(1);
|
||||
|
||||
expect(ruleGroupRepository.findOne).not.toHaveBeenCalled();
|
||||
expect(exclusionRepo.delete).toHaveBeenCalledWith(1);
|
||||
expect(result.code).toBe(1);
|
||||
});
|
||||
|
||||
it('removeExclusion looks up the rule group for a scoped exclusion', async () => {
|
||||
const exclusionRepo = {
|
||||
findOne: jest
|
||||
.fn()
|
||||
.mockResolvedValue({ id: 2, ruleGroupId: 7, mediaServerId: 'b' }),
|
||||
delete: jest.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
const ruleGroupRepository = {
|
||||
findOne: jest.fn().mockResolvedValue({ id: 7, collectionId: 9 }),
|
||||
};
|
||||
const { service } = createService({ exclusionRepo, ruleGroupRepository });
|
||||
|
||||
await service.removeExclusion(2);
|
||||
|
||||
expect(ruleGroupRepository.findOne).toHaveBeenCalledWith({
|
||||
where: { id: 7 },
|
||||
});
|
||||
});
|
||||
|
||||
it('setExclusion(global) looks up with IsNull(), saves a null ruleGroupId, and removes redundant scoped exclusions', async () => {
|
||||
const exclusionRepo = {
|
||||
findOne: jest.fn().mockResolvedValue(undefined),
|
||||
save: jest.fn().mockResolvedValue(undefined),
|
||||
delete: jest.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
const mediaServer = {
|
||||
getMetadata: jest.fn().mockResolvedValue({ type: 'movie' }),
|
||||
getAllIdsForContextAction: jest.fn().mockResolvedValue(['movie-1']),
|
||||
};
|
||||
const mediaServerFactory = {
|
||||
getService: jest.fn().mockResolvedValue(mediaServer),
|
||||
};
|
||||
const { service } = createService({ exclusionRepo, mediaServerFactory });
|
||||
|
||||
const result = await service.setExclusion({ mediaId: 'movie-1' } as any);
|
||||
|
||||
const findOneWhere = exclusionRepo.findOne.mock.calls[0][0].where;
|
||||
expect(isNullOperator(findOneWhere.ruleGroupId)).toBe(true);
|
||||
expect(exclusionRepo.save).toHaveBeenCalledWith([
|
||||
expect.objectContaining({
|
||||
mediaServerId: 'movie-1',
|
||||
ruleGroupId: null,
|
||||
parent: 'movie-1',
|
||||
type: 'movie',
|
||||
}),
|
||||
]);
|
||||
// global subsumes scoped: any rule-group exclusions for this item are dropped
|
||||
const deleteCriteria = exclusionRepo.delete.mock.calls[0][0];
|
||||
expect(deleteCriteria.mediaServerId).toBe('movie-1');
|
||||
expect(deleteCriteria.ruleGroupId).toBeInstanceOf(FindOperator);
|
||||
expect(deleteCriteria.ruleGroupId.type).toBe('not');
|
||||
expect(result.code).toBe(1);
|
||||
});
|
||||
|
||||
it('setExclusion(scoped) is a no-op when the item is already globally excluded', async () => {
|
||||
const exclusionRepo = {
|
||||
// the first lookup (existing-global check) finds a global exclusion
|
||||
findOne: jest.fn().mockResolvedValue({
|
||||
id: 9,
|
||||
mediaServerId: 'movie-1',
|
||||
ruleGroupId: null,
|
||||
}),
|
||||
save: jest.fn().mockResolvedValue(undefined),
|
||||
delete: jest.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
const mediaServer = {
|
||||
getMetadata: jest.fn().mockResolvedValue({ type: 'movie' }),
|
||||
getAllIdsForContextAction: jest.fn().mockResolvedValue(['movie-1']),
|
||||
};
|
||||
const mediaServerFactory = {
|
||||
getService: jest.fn().mockResolvedValue(mediaServer),
|
||||
};
|
||||
const { service } = createService({ exclusionRepo, mediaServerFactory });
|
||||
|
||||
const result = await service.setExclusion({
|
||||
mediaId: 'movie-1',
|
||||
ruleGroupId: 5,
|
||||
} as any);
|
||||
|
||||
// the existing-global check used IsNull(), and the scoped row was skipped
|
||||
expect(
|
||||
isNullOperator(exclusionRepo.findOne.mock.calls[0][0].where.ruleGroupId),
|
||||
).toBe(true);
|
||||
expect(exclusionRepo.save).not.toHaveBeenCalled();
|
||||
expect(result.code).toBe(1);
|
||||
});
|
||||
});
|
||||
@@ -9,7 +9,7 @@ import { EventEmitter2 } from '@nestjs/event-emitter';
|
||||
import { InjectRepository } from '@nestjs/typeorm';
|
||||
import axios from 'axios';
|
||||
import _ from 'lodash';
|
||||
import { DataSource, Repository } from 'typeorm';
|
||||
import { DataSource, IsNull, Not, Repository } from 'typeorm';
|
||||
import cacheManager from '../api/lib/cache';
|
||||
import { MediaServerFactory } from '../api/media-server/media-server.factory';
|
||||
import { IMediaServerService } from '../api/media-server/media-server.interface';
|
||||
@@ -673,12 +673,29 @@ export class RulesService {
|
||||
for (const media of handleMedia) {
|
||||
const metaData = await mediaServer.getMetadata(media.mediaServerId);
|
||||
|
||||
// Global subsumes scoped: skip a rule-group exclusion when the item is
|
||||
// already globally excluded (an item is global or scoped, never both).
|
||||
if (data.ruleGroupId !== undefined) {
|
||||
const existingGlobal = await this.exclusionRepo.findOne({
|
||||
where: {
|
||||
mediaServerId: media.mediaServerId,
|
||||
ruleGroupId: IsNull(),
|
||||
},
|
||||
});
|
||||
if (existingGlobal) {
|
||||
this.logger.log(
|
||||
`Media ${media.mediaServerId} is already globally excluded; skipped rule group ${data.ruleGroupId} exclusion`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
const old = await this.exclusionRepo.findOne({
|
||||
where: {
|
||||
mediaServerId: media.mediaServerId,
|
||||
...(data.ruleGroupId !== undefined
|
||||
? { ruleGroupId: data.ruleGroupId }
|
||||
: { ruleGroupId: null }),
|
||||
: { ruleGroupId: IsNull() }),
|
||||
},
|
||||
});
|
||||
|
||||
@@ -697,6 +714,15 @@ export class RulesService {
|
||||
},
|
||||
]);
|
||||
|
||||
// Global subsumes scoped: a new global exclusion drops the item's
|
||||
// now-redundant rule-group exclusions.
|
||||
if (data.ruleGroupId === undefined) {
|
||||
await this.exclusionRepo.delete({
|
||||
mediaServerId: media.mediaServerId,
|
||||
ruleGroupId: Not(IsNull()),
|
||||
});
|
||||
}
|
||||
|
||||
// add collection log record if needed
|
||||
if (data.collectionId) {
|
||||
await this.collectionService.CollectionLogRecordForChild(
|
||||
@@ -740,8 +766,8 @@ export class RulesService {
|
||||
return this.createReturnStatus(true, 'Success');
|
||||
}
|
||||
|
||||
// add collection log record if needed
|
||||
if (exclcusion.ruleGroupId !== undefined) {
|
||||
// global exclusions (null ruleGroupId) have no rule group to log against
|
||||
if (exclcusion.ruleGroupId != null) {
|
||||
const rulegroup = await this.ruleGroupRepository.findOne({
|
||||
where: {
|
||||
id: exclcusion.ruleGroupId,
|
||||
@@ -759,7 +785,13 @@ export class RulesService {
|
||||
|
||||
// do delete
|
||||
await this.exclusionRepo.delete(id);
|
||||
this.logger.log(`Removed exclusion with id ${id}`);
|
||||
this.logger.log(
|
||||
`Removed exclusion ${id} for media ${exclcusion.mediaServerId} (${
|
||||
exclcusion.ruleGroupId != null
|
||||
? `rule group ${exclcusion.ruleGroupId}`
|
||||
: 'global'
|
||||
})`,
|
||||
);
|
||||
return this.createReturnStatus(true, 'Success');
|
||||
} catch (error) {
|
||||
this.logger.warn(`Removing exclusion with id ${id} failed.`);
|
||||
@@ -898,7 +930,7 @@ export class RulesService {
|
||||
? exclusions.concat(
|
||||
await this.exclusionRepo.find({
|
||||
where: {
|
||||
ruleGroupId: null,
|
||||
ruleGroupId: IsNull(),
|
||||
},
|
||||
}),
|
||||
)
|
||||
|
||||
@@ -0,0 +1,140 @@
|
||||
import {
|
||||
cleanup,
|
||||
fireEvent,
|
||||
render,
|
||||
screen,
|
||||
waitFor,
|
||||
} from '@testing-library/react'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import GetApiHandler, { PostApiHandler } from '../../utils/ApiHandler'
|
||||
import AddModal from './index'
|
||||
|
||||
const invalidateQueries = vi.fn()
|
||||
|
||||
vi.mock('@tanstack/react-query', async () => {
|
||||
const actual = await vi.importActual<typeof import('@tanstack/react-query')>(
|
||||
'@tanstack/react-query',
|
||||
)
|
||||
return { ...actual, useQueryClient: () => ({ invalidateQueries }) }
|
||||
})
|
||||
|
||||
vi.mock('../../utils/ApiHandler', () => ({
|
||||
default: vi.fn(),
|
||||
PostApiHandler: vi.fn(),
|
||||
}))
|
||||
|
||||
describe('AddModal — global exclusion warning', () => {
|
||||
const getApiHandlerMock = vi.mocked(GetApiHandler)
|
||||
const postApiHandlerMock = vi.mocked(PostApiHandler)
|
||||
|
||||
const scopedStatus = {
|
||||
excludedFrom: [
|
||||
{ label: 'Archive Queue', targetPath: '/collections/9/exclusions' },
|
||||
{ label: 'Stale Movies', targetPath: '/collections/7/exclusions' },
|
||||
],
|
||||
manuallyAddedTo: [],
|
||||
}
|
||||
|
||||
// Route GetApiHandler by URL; `fetchMaintainerrStatusDetails` calls through
|
||||
// this same mock, so no separate mock is needed for the status helper.
|
||||
const stubApi = (status: unknown) => {
|
||||
getApiHandlerMock.mockImplementation(((url: string) => {
|
||||
if (url.includes('/maintainerr-status')) return Promise.resolve(status)
|
||||
if (url.startsWith('/media-server/meta/'))
|
||||
return Promise.resolve({ title: 'Mock Charlie' })
|
||||
if (url.startsWith('/collections')) return Promise.resolve([])
|
||||
return Promise.resolve(undefined)
|
||||
}) as typeof GetApiHandler)
|
||||
postApiHandlerMock.mockResolvedValue(undefined as never)
|
||||
}
|
||||
|
||||
const renderExclude = () =>
|
||||
render(
|
||||
<AddModal
|
||||
mediaServerId="m1"
|
||||
type="movie"
|
||||
modalType="exclude"
|
||||
onCancel={vi.fn()}
|
||||
onSubmit={vi.fn()}
|
||||
/>,
|
||||
)
|
||||
|
||||
const exclusionPost = () =>
|
||||
postApiHandlerMock.mock.calls.find(
|
||||
(call) => call[0] === '/rules/exclusion',
|
||||
)?.[1] as { collectionId?: number } | undefined
|
||||
|
||||
beforeEach(() => {
|
||||
getApiHandlerMock.mockReset()
|
||||
postApiHandlerMock.mockReset()
|
||||
})
|
||||
afterEach(() => cleanup())
|
||||
|
||||
it('Add + all collections, item has scoped exclusions: warns with item — rule-group links, then Proceed submits a global exclusion', async () => {
|
||||
stubApi(scopedStatus)
|
||||
renderExclude()
|
||||
|
||||
fireEvent.click(await screen.findByRole('button', { name: 'Submit' }))
|
||||
|
||||
await screen.findByText('Confirmation Required')
|
||||
// each scoped exclusion is listed as "<item> — <linked rule group>"
|
||||
expect(
|
||||
screen.getByRole('link', { name: 'Archive Queue' }).getAttribute('href'),
|
||||
).toBe('/collections/9/exclusions')
|
||||
expect(
|
||||
screen.getByRole('link', { name: 'Stale Movies' }).getAttribute('href'),
|
||||
).toBe('/collections/7/exclusions')
|
||||
expect(screen.getAllByText(/Mock Charlie/).length).toBeGreaterThan(0)
|
||||
// not submitted until confirmed
|
||||
expect(postApiHandlerMock).not.toHaveBeenCalled()
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Proceed' }))
|
||||
|
||||
await waitFor(() => expect(exclusionPost()).toBeDefined())
|
||||
expect(exclusionPost()?.collectionId).toBeUndefined() // global
|
||||
})
|
||||
|
||||
it('Remove + all collections: no warning, submits directly', async () => {
|
||||
stubApi(scopedStatus)
|
||||
renderExclude()
|
||||
|
||||
fireEvent.change(await screen.findByRole('combobox', { name: 'Action' }), {
|
||||
target: { value: '1' },
|
||||
})
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Submit' }))
|
||||
|
||||
await waitFor(() => expect(exclusionPost()).toBeDefined())
|
||||
expect(screen.queryByText('Confirmation Required')).toBeNull()
|
||||
})
|
||||
|
||||
it('Add + all collections, no scoped exclusions: no warning, submits', async () => {
|
||||
stubApi({ excludedFrom: [{ label: 'Global' }], manuallyAddedTo: [] })
|
||||
renderExclude()
|
||||
|
||||
fireEvent.click(await screen.findByRole('button', { name: 'Submit' }))
|
||||
|
||||
await waitFor(() => expect(exclusionPost()).toBeDefined())
|
||||
expect(screen.queryByText('Confirmation Required')).toBeNull()
|
||||
})
|
||||
|
||||
it('Add + all collections, warning prefetch fails: submits instead of blocking', async () => {
|
||||
// The status read rejects; the warning can't be built, but the exclusion
|
||||
// the user asked for must still go through.
|
||||
getApiHandlerMock.mockImplementation(((url: string) => {
|
||||
if (url.includes('/maintainerr-status'))
|
||||
return Promise.reject(new Error('boom'))
|
||||
if (url.startsWith('/media-server/meta/'))
|
||||
return Promise.resolve({ title: 'Mock Charlie' })
|
||||
if (url.startsWith('/collections')) return Promise.resolve([])
|
||||
return Promise.resolve(undefined)
|
||||
}) as typeof GetApiHandler)
|
||||
postApiHandlerMock.mockResolvedValue(undefined as never)
|
||||
renderExclude()
|
||||
|
||||
fireEvent.click(await screen.findByRole('button', { name: 'Submit' }))
|
||||
|
||||
await waitFor(() => expect(exclusionPost()).toBeDefined())
|
||||
expect(exclusionPost()?.collectionId).toBeUndefined() // global
|
||||
expect(screen.queryByText('Confirmation Required')).toBeNull()
|
||||
})
|
||||
})
|
||||
@@ -3,6 +3,7 @@ import { useQueryClient } from '@tanstack/react-query'
|
||||
import { useEffect, useMemo, useState } from 'react'
|
||||
import GetApiHandler, { PostApiHandler } from '../../utils/ApiHandler'
|
||||
import Alert from '../Common/Alert'
|
||||
import { fetchMaintainerrStatusDetails } from '../Common/MediaCard/maintainerrStatus'
|
||||
import Button from '../Common/Button'
|
||||
import FormItem from '../Common/FormItem'
|
||||
import Modal from '../Common/Modal'
|
||||
@@ -17,6 +18,10 @@ const AddModal = (props: IAddModal) => {
|
||||
const [loading, setLoading] = useState(true)
|
||||
const [alert, setAlert] = useState(false)
|
||||
const [forceRemovalcheck, setForceRemovalCheck] = useState(false)
|
||||
const [globalWarning, setGlobalWarning] = useState(false)
|
||||
const [affectedExclusions, setAffectedExclusions] = useState<
|
||||
{ title: string; label: string; targetPath: string }[]
|
||||
>([])
|
||||
const [submitting, setSubmitting] = useState(false)
|
||||
const [selectedAction, setSelectedAction] = useState<number>(0)
|
||||
// For show only
|
||||
@@ -76,44 +81,90 @@ const AddModal = (props: IAddModal) => {
|
||||
props.onCancel()
|
||||
}
|
||||
|
||||
const submitMedia = async () => {
|
||||
if (submitting) return
|
||||
setSubmitting(true)
|
||||
const mediaDto: IAlterableMediaDto = {
|
||||
id: selectedMediaId,
|
||||
type: selectedContext,
|
||||
}
|
||||
|
||||
try {
|
||||
if (props.modalType === 'add') {
|
||||
await PostApiHandler(`/collections/media/add`, {
|
||||
mediaId: props.mediaServerId,
|
||||
context: mediaDto,
|
||||
collectionId: currentCollectionId,
|
||||
action: selectedAction,
|
||||
})
|
||||
|
||||
await queryClient.invalidateQueries({
|
||||
queryKey: ['calendar', 'collections', 'overlay-data'],
|
||||
})
|
||||
} else {
|
||||
await PostApiHandler('/rules/exclusion', {
|
||||
mediaId: props.mediaServerId,
|
||||
context: mediaDto,
|
||||
collectionId:
|
||||
currentCollectionId !== -1 ? currentCollectionId : undefined,
|
||||
action: selectedAction,
|
||||
})
|
||||
}
|
||||
|
||||
props.onSubmit()
|
||||
} catch {
|
||||
setSubmitting(false)
|
||||
}
|
||||
}
|
||||
|
||||
const handleOk = async () => {
|
||||
if (submitting) return
|
||||
if (currentCollectionId !== undefined) {
|
||||
setSubmitting(true)
|
||||
const mediaDto: IAlterableMediaDto = {
|
||||
id: selectedMediaId,
|
||||
type: selectedContext,
|
||||
}
|
||||
|
||||
try {
|
||||
if (props.modalType === 'add') {
|
||||
await PostApiHandler(`/collections/media/add`, {
|
||||
mediaId: props.mediaServerId,
|
||||
context: mediaDto,
|
||||
collectionId: currentCollectionId,
|
||||
action: selectedAction,
|
||||
})
|
||||
|
||||
await queryClient.invalidateQueries({
|
||||
queryKey: ['calendar', 'collections', 'overlay-data'],
|
||||
})
|
||||
} else {
|
||||
await PostApiHandler('/rules/exclusion', {
|
||||
mediaId: props.mediaServerId,
|
||||
context: mediaDto,
|
||||
collectionId:
|
||||
currentCollectionId !== -1 ? currentCollectionId : undefined,
|
||||
action: selectedAction,
|
||||
})
|
||||
}
|
||||
|
||||
props.onSubmit()
|
||||
} catch {
|
||||
setSubmitting(false)
|
||||
}
|
||||
} else {
|
||||
if (currentCollectionId === undefined) {
|
||||
setAlert(true)
|
||||
return
|
||||
}
|
||||
|
||||
// Only ADDING a global exclusion clears the item's rule-group exclusions.
|
||||
// If it has any, warn and list each as "item — rule group", reusing the
|
||||
// backdrop's status data (no-cache fetch) so labels/links match and stay
|
||||
// fresh. (selectedAction 0 = Add, 1 = Remove.)
|
||||
if (
|
||||
props.modalType === 'exclude' &&
|
||||
selectedAction === 0 &&
|
||||
currentCollectionId === -1
|
||||
) {
|
||||
// Best-effort: if either read fails we can't build the warning, so fall
|
||||
// through and submit rather than blocking the exclusion the user asked for.
|
||||
try {
|
||||
const [meta, status] = await Promise.all([
|
||||
GetApiHandler<{ title?: string }>(
|
||||
`/media-server/meta/${props.mediaServerId}`,
|
||||
),
|
||||
fetchMaintainerrStatusDetails({
|
||||
id: props.mediaServerId,
|
||||
getApiHandler: GetApiHandler,
|
||||
}),
|
||||
])
|
||||
const scoped = status.excludedFrom.filter((e) => e.targetPath)
|
||||
|
||||
if (scoped.length > 0) {
|
||||
const title = meta?.title ?? String(props.mediaServerId)
|
||||
setAffectedExclusions(
|
||||
scoped.map((e) => ({
|
||||
title,
|
||||
label: e.label,
|
||||
targetPath: e.targetPath as string,
|
||||
})),
|
||||
)
|
||||
setGlobalWarning(true)
|
||||
return
|
||||
}
|
||||
} catch {
|
||||
// Warning data unavailable — proceed without it.
|
||||
}
|
||||
}
|
||||
|
||||
await submitMedia()
|
||||
}
|
||||
|
||||
const handleForceRemoval = async () => {
|
||||
@@ -265,6 +316,44 @@ const AddModal = (props: IAddModal) => {
|
||||
</Modal>
|
||||
) : undefined}
|
||||
|
||||
{globalWarning ? (
|
||||
<Modal
|
||||
loading={loading}
|
||||
backgroundClickable={false}
|
||||
onCancel={() => setGlobalWarning(false)}
|
||||
title={'Confirmation Required'}
|
||||
footerActions={
|
||||
<Button
|
||||
buttonType="primary"
|
||||
className="ml-3"
|
||||
onClick={() => {
|
||||
setGlobalWarning(false)
|
||||
submitMedia()
|
||||
}}
|
||||
>
|
||||
Proceed
|
||||
</Button>
|
||||
}
|
||||
>
|
||||
Making this a global exclusion removes the following rule-group
|
||||
exclusions, and they will not return if you later remove the global
|
||||
exclusion:
|
||||
<ul className="mt-2 list-disc pl-5">
|
||||
{affectedExclusions.map((e) => (
|
||||
<li key={`${e.title}-${e.targetPath}`}>
|
||||
{e.title} —{' '}
|
||||
<a
|
||||
href={e.targetPath}
|
||||
className="text-maintainerr underline hover:text-maintainerr-400"
|
||||
>
|
||||
{e.label}
|
||||
</a>
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
</Modal>
|
||||
) : undefined}
|
||||
|
||||
{alert ? (
|
||||
<Alert title="Please select a collection" type="warning" />
|
||||
) : undefined}
|
||||
@@ -279,8 +368,16 @@ const AddModal = (props: IAddModal) => {
|
||||
setSelectedAction(+e.target.value)
|
||||
}}
|
||||
>
|
||||
<option value={0}>Add</option>
|
||||
<option value={1}>Remove</option>
|
||||
<option value={0}>
|
||||
{props.modalType === 'add'
|
||||
? 'Add to collection'
|
||||
: 'Add exclusion'}
|
||||
</option>
|
||||
<option value={1}>
|
||||
{props.modalType === 'add'
|
||||
? 'Remove from collection'
|
||||
: 'Remove exclusion'}
|
||||
</option>
|
||||
</Select>
|
||||
</FormItem>
|
||||
|
||||
|
||||
+1
-1
@@ -27,7 +27,7 @@
|
||||
"packages/*"
|
||||
],
|
||||
"engines": {
|
||||
"node": "^20.19.0 || >=22.12.0"
|
||||
"node": "^20.19.0 || ^22.13.0 || >=24.11.0"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@semantic-release/changelog": "^6.0.3",
|
||||
|
||||
@@ -1,3 +1,7 @@
|
||||
// TypeORM 1.x (better-sqlite3). Repository code uses object-form
|
||||
// `relations`/`select` ({ rel: true }) and IsNull() to match SQL NULL — see
|
||||
// project-notes.instructions.md.
|
||||
|
||||
// Generate migration after entity changes
|
||||
yarn workspace @maintainerr/server migration:generate src/database/migrations/<migr_name>
|
||||
|
||||
|
||||
Reference in New Issue
Block a user