mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-20 22:16:26 +00:00
fix(db): repositories mirror the statement's column set and guard their row types
This commit is contained in:
@@ -1,5 +1,5 @@
|
||||
import type { DayNotes } from '../entities/DayNotes.entity';
|
||||
import { toRow } from './_shared/rows';
|
||||
import { toRow, type AssertRowKeys } from './_shared/rows';
|
||||
import { EntityRepository } from '@mikro-orm/sql';
|
||||
|
||||
export interface DayNoteRow {
|
||||
@@ -14,37 +14,42 @@ export interface DayNoteRow {
|
||||
color: string | null;
|
||||
}
|
||||
|
||||
const _dayNoteRowKeys: AssertRowKeys<DayNoteRow, DayNotes> = true;
|
||||
|
||||
export class DayNotesRepository extends EntityRepository<DayNotes> {
|
||||
/**
|
||||
* `INSERT INTO day_notes (day_id, trip_id, text, time, icon, sort_order, color)`
|
||||
* — `created_at` is left to `DEFAULT CURRENT_TIMESTAMP`, then the row is
|
||||
* re-read so the returned row is the stored one.
|
||||
* The column set of the legacy INSERT:
|
||||
* `INSERT INTO day_notes (day_id, trip_id, text, time, icon, sort_order, color)`.
|
||||
*
|
||||
* `created_at` alone would not need the re-read: a `defaultRaw` column the
|
||||
* insert does not name is in its `returning` clause, so it is already the
|
||||
* stored text by the time `flush()` resolves. The re-read is there for the
|
||||
* columns that are not — a nullable column this insert never named would
|
||||
* otherwise be `undefined` on the entity and missing from the row.
|
||||
* The caller passes already-coerced values — every field is required and is
|
||||
* written verbatim. The legacy defaults (`icon || '📝'`, `sortOrder ?? 9999`,
|
||||
* the trimmed text, `normalizeNoteColor(color)`) stay in
|
||||
* `nest/day-notes/day-notes.service.ts`, which is where that rule lives.
|
||||
*
|
||||
* `created_at` is not in the column set: it is left to the column's
|
||||
* `DEFAULT CURRENT_TIMESTAMP`. It alone would not need the re-read either —
|
||||
* a `defaultRaw` column the insert does not name is in its `returning`
|
||||
* clause, so it is already the stored text by the time `flush()` resolves.
|
||||
* The re-read is there for the columns that are not: one this insert never
|
||||
* names would otherwise be `undefined` on the entity and missing from the row.
|
||||
*/
|
||||
async createNote(input: {
|
||||
day_id: number;
|
||||
trip_id: number;
|
||||
text: string;
|
||||
time?: string | null;
|
||||
icon?: string | null;
|
||||
sort_order?: number | null;
|
||||
color?: string | null;
|
||||
time: string | null;
|
||||
icon: string | null;
|
||||
sort_order: number | null;
|
||||
color: string | null;
|
||||
}): Promise<DayNoteRow> {
|
||||
const note = this.create({
|
||||
day: input.day_id,
|
||||
day_id: input.day_id,
|
||||
trip: input.trip_id,
|
||||
trip_id: input.trip_id,
|
||||
text: input.text,
|
||||
time: input.time ?? null,
|
||||
icon: input.icon ?? '📝',
|
||||
sort_order: input.sort_order ?? 0,
|
||||
color: input.color ?? null,
|
||||
time: input.time,
|
||||
icon: input.icon,
|
||||
sort_order: input.sort_order,
|
||||
color: input.color,
|
||||
});
|
||||
await this.getEntityManager().persist(note).flush();
|
||||
await this.getEntityManager().refresh(note);
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import type { Days } from '../entities/Days.entity';
|
||||
import { toRow } from './_shared/rows';
|
||||
import { toRow, type AssertRowKeys } from './_shared/rows';
|
||||
import { EntityRepository } from '@mikro-orm/sql';
|
||||
|
||||
/** A `days` row as the API emits it. */
|
||||
@@ -13,6 +13,8 @@ export interface DayRow {
|
||||
default_transport_mode: string | null;
|
||||
}
|
||||
|
||||
const _dayRowKeys: AssertRowKeys<DayRow, Days> = true;
|
||||
|
||||
export class DaysRepository extends EntityRepository<Days> {
|
||||
/** `SELECT * FROM days WHERE trip_id = ? ORDER BY day_number ASC` */
|
||||
async listByTrip(trip_id: number): Promise<DayRow[]> {
|
||||
@@ -45,27 +47,31 @@ export class DaysRepository extends EntityRepository<Days> {
|
||||
}
|
||||
|
||||
/**
|
||||
* `INSERT INTO days (trip_id, day_number, date, notes) VALUES (?, ?, ?, ?)`
|
||||
* followed by the legacy re-select, so the caller gets the stored row (with
|
||||
* every column default applied) and not what it asked to store.
|
||||
* The column set of the legacy INSERT:
|
||||
* `INSERT INTO days (trip_id, day_number, date, notes) VALUES (?, ?, ?, ?)`.
|
||||
*
|
||||
* The re-select is not optional: the insert's `returning` clause carries only
|
||||
* the generated and `defaultRaw` columns, so a column this insert never named
|
||||
* (`title`, `default_transport_mode`) would still be `undefined` on the
|
||||
* entity and would simply be missing from the row.
|
||||
* The caller passes already-coerced values — every field is required and is
|
||||
* written verbatim, because a default or a coercion belongs to the service
|
||||
* that owns the rule, not to the statement that stores it.
|
||||
*
|
||||
* The insert is followed by the legacy re-select, so the caller gets the
|
||||
* stored row rather than what it asked to store. That re-select is not
|
||||
* optional: the insert's `returning` clause carries only the generated and
|
||||
* `defaultRaw` columns, so a column this insert never names (`title`,
|
||||
* `default_transport_mode`) would still be `undefined` on the entity and
|
||||
* would simply be missing from the row.
|
||||
*/
|
||||
async createDay(input: {
|
||||
trip_id: number;
|
||||
day_number: number;
|
||||
date?: string | null;
|
||||
notes?: string | null;
|
||||
date: string | null;
|
||||
notes: string | null;
|
||||
}): Promise<DayRow> {
|
||||
const day = this.create({
|
||||
trip: input.trip_id,
|
||||
trip_id: input.trip_id,
|
||||
day_number: input.day_number,
|
||||
date: input.date ?? null,
|
||||
notes: input.notes ?? null,
|
||||
date: input.date,
|
||||
notes: input.notes,
|
||||
});
|
||||
await this.getEntityManager().persist(day).flush();
|
||||
await this.getEntityManager().refresh(day);
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { wrap, type EntityDTO } from '@mikro-orm/core';
|
||||
import { wrap, type Collection, type EntityDTO, type Reference } from '@mikro-orm/core';
|
||||
|
||||
/**
|
||||
* The plain row a service (and, through it, the API) sees.
|
||||
@@ -12,3 +12,22 @@ import { wrap, type EntityDTO } from '@mikro-orm/core';
|
||||
export function toRow<T extends object>(entity: T): EntityDTO<T> {
|
||||
return wrap(entity).toObject() as EntityDTO<T>;
|
||||
}
|
||||
|
||||
/** The scalar (column-backed) keys of an entity class: everything that is not a relation or a collection. */
|
||||
export type ScalarKeys<T> = {
|
||||
[K in keyof T]-?: NonNullable<T[K]> extends Collection<object> | Reference<object> ? never : K;
|
||||
}[keyof T];
|
||||
|
||||
/**
|
||||
* Compile-time parity between a hand-written row interface and the entity's
|
||||
* scalar keys: a column added to or removed from the entity fails `tsc` here
|
||||
* instead of leaking silently through `toRow`'s cast. Types (nullability)
|
||||
* stay hand-maintained in the interface on purpose — that is where the API
|
||||
* shape is documented.
|
||||
*/
|
||||
export type AssertRowKeys<Row, Entity> = [
|
||||
Exclude<keyof Row, ScalarKeys<Entity>>,
|
||||
Exclude<ScalarKeys<Entity>, keyof Row>,
|
||||
] extends [never, never]
|
||||
? true
|
||||
: never;
|
||||
|
||||
@@ -22,19 +22,19 @@ describe('DayNotesRepository timestamps', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const day = createDay(testDb, trip.id);
|
||||
const row = await notes.createNote({ day_id: day.id, trip_id: trip.id, text: 'Lunch' });
|
||||
const row = await notes.createNote({ day_id: day.id, trip_id: trip.id, text: 'Lunch', time: null, icon: '📝', sort_order: 0, color: null });
|
||||
const stored = testDb.prepare('SELECT created_at, typeof(created_at) AS kind FROM day_notes WHERE id = ?').get(row.id) as { created_at: string; kind: string };
|
||||
expect(stored.kind).toBe('text');
|
||||
expect(stored.created_at).toMatch(DB_TIMESTAMP_RE);
|
||||
expect(row.created_at).toBe(stored.created_at);
|
||||
});
|
||||
|
||||
it('NOTEREPO-002: the returned row is the SELECT * row and the defaults are the legacy ones', async () => {
|
||||
it('NOTEREPO-002: the returned row is the SELECT * row, with the values the caller passed written verbatim', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const day = createDay(testDb, trip.id);
|
||||
const row = await notes.createNote({ day_id: day.id, trip_id: trip.id, text: 'Lunch' });
|
||||
expect(row).toEqual(testDb.prepare('SELECT * FROM day_notes WHERE id = ?').get(row.id));
|
||||
const row = await notes.createNote({ day_id: day.id, trip_id: trip.id, text: 'Lunch', time: null, icon: '📝', sort_order: 0, color: null });
|
||||
expect(row).toStrictEqual(testDb.prepare('SELECT * FROM day_notes WHERE id = ?').get(row.id));
|
||||
expect(row.icon).toBe('📝');
|
||||
expect(row.sort_order).toBe(0);
|
||||
});
|
||||
@@ -43,7 +43,7 @@ describe('DayNotesRepository timestamps', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const day = createDay(testDb, trip.id);
|
||||
const row = await notes.createNote({ day_id: day.id, trip_id: trip.id, text: 'Lunch' });
|
||||
const row = await notes.createNote({ day_id: day.id, trip_id: trip.id, text: 'Lunch', time: null, icon: '📝', sort_order: 0, color: null });
|
||||
const d = testDb.prepare('SELECT date(created_at) AS d FROM day_notes WHERE id = ?').get(row.id) as { d: string };
|
||||
expect(d.d).toMatch(/^\d{4}-\d{2}-\d{2}$/);
|
||||
});
|
||||
|
||||
@@ -26,7 +26,7 @@ describe('DaysRepository', () => {
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const day = createDay(testDb, trip.id, { date: '2026-07-01', title: 'Arrival' });
|
||||
const [row] = await days.listByTrip(trip.id);
|
||||
expect(row).toEqual(rawDay(day.id));
|
||||
expect(row).toStrictEqual(rawDay(day.id));
|
||||
});
|
||||
|
||||
it('DAYREPO-002: the scalar FK hydrates without loading the trip', async () => {
|
||||
@@ -60,15 +60,18 @@ describe('DaysRepository', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
expect(await days.maxDayNumber(trip.id)).toBe(0);
|
||||
createDay(testDb, trip.id, { day_number: 3 });
|
||||
expect(await days.maxDayNumber(trip.id)).toBe(3);
|
||||
createDay(testDb, trip.id, { day_number: 1 });
|
||||
createDay(testDb, trip.id, { day_number: 10 });
|
||||
createDay(testDb, trip.id, { day_number: 2 });
|
||||
// 10, not '2': the descending order has to be numeric, not lexicographic.
|
||||
expect(await days.maxDayNumber(trip.id)).toBe(10);
|
||||
});
|
||||
|
||||
it('DAYREPO-006: createDay writes the legacy column set and returns the re-read row', async () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const row = await days.createDay({ trip_id: trip.id, day_number: 1, date: '2026-07-01' });
|
||||
expect(row).toEqual(rawDay(row.id));
|
||||
const row = await days.createDay({ trip_id: trip.id, day_number: 1, date: '2026-07-01', notes: null });
|
||||
expect(row).toStrictEqual(rawDay(row.id));
|
||||
expect(row.notes).toBeNull();
|
||||
expect(row.trip_id).toBe(trip.id);
|
||||
});
|
||||
|
||||
@@ -68,6 +68,6 @@ describe('TripsRepository.findAccessible — parity with canAccessTrip', () => {
|
||||
const entity = await t.repo(Trips).findOneOrFail({ id: trip.id });
|
||||
const raw = testDb.prepare('SELECT * FROM trips WHERE id = ?').get(trip.id) as Record<string, unknown>;
|
||||
const { wrap } = await import('@mikro-orm/core');
|
||||
expect(wrap(entity).toObject()).toEqual(raw);
|
||||
expect(wrap(entity).toObject()).toStrictEqual(raw);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user