k1nq/TREK
1
0
Fork 0
mirror of https://github.com/mauriceboe/TREK.git synced 2026-10-18 21:16:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(server): run the remaining multi-statement service writes in one transaction

Browse Source 
A sweep of the services for write sequences that committed step by step,
each one leaving half a change behind when the next statement failed:

- accommodations: the linked bookings' times moved after the stay's own
  transaction had committed
- airports: the boot backfill wrote a flight's two endpoints one at a
  time, and a flight left with only the first was never picked up again;
  each flight is its own transaction now
- auth: the admin settings form and a password reset (retire the old
  link, issue the new one) each land whole
- oauth: a refresh revoked the old pair before issuing the new one, so a
  failure in between logged the client out; rotating a client secret and
  revoking its tokens go together too
- oidc: the identity link, the role from the claims and the avatar of a
  returning user are one write
- addons, notification preferences: a settings save lands whole
- places: create with its tags, update, and the GPX/KML imports with
  their track colours
- trips: the trip row, the shifted leave entries and the rebuilt day grid
- vacay: a first visit provisions the plan in one transaction with a
  second look against a concurrent first visit; updatePlan fetches the
  public holidays between two transactions instead of inside its writes
- memories: a photo and its trip link, and the deletes with the orphan
  sweep behind them

AddonsService, AirportsService and OauthService take the UnitOfWork for
this. Network I/O stays outside every transaction. Comments that only
narrated the migration plan make room under the lint:size entries, which
drop with them.
This commit is contained in:
Maurice
2026-10-08 15:35:29 +02:00
parent f76538b2b2
commit e015e5e5e8
27 changed files with 523 additions and 350 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/scripts/size-baseline.json
+4 -4
View File
@@ -7,17 +7,17 @@
"src/db/repositories/Users.repository.ts": 1710,
"src/nest/atlas/atlas-geo.ts": 1005,
"src/nest/atlas/atlas.service.ts": 1182,
"src/nest/auth/auth.service.ts": 1166,
"src/nest/auth/auth.service.ts": 1165,
"src/nest/budget/budget.service.ts": 1703,
"src/nest/collections/collections.service.ts": 1488,
"src/nest/doc-sync/doc-sync.service.ts": 1060,
"src/nest/doc-sync/providers/synology-drive.client.ts": 1160,
"src/nest/journey/journey-domain.service.ts": 1804,
"src/nest/maps/maps.service.ts": 3252,
"src/nest/places/places.service.ts": 2022,
"src/nest/places/places.service.ts": 2020,
"src/nest/plugins/plugin-runtime.service.ts": 1463,
"src/nest/plugins/runtime/plugin-sdk.ts": 1094,
"src/nest/reservations/reservations.service.ts": 1248,
"src/nest/trips/trips.service.ts": 1014,
"src/nest/vacay/vacay.service.ts": 1442
"src/nest/trips/trips.service.ts": 1013,
"src/nest/vacay/vacay.service.ts": 1440
}
server/src/nest/accommodations/accommodations.service.ts
+17 -18
View File
@@ -619,8 +619,8 @@ export class AccommodationsService {
const newConfirmation = fields.confirmation !== undefined ? fields.confirmation : existing.confirmation;
const newNotes = fields.notes !== undefined ? fields.notes : existing.notes;
// The stay row and the day stop that mirrors it describe the same booking, so a
// move that wrote only one of the two must not survive.
// The stay row, the day stop that mirrors it and the linked bookings' times all
// describe the same booking, so a write that landed only some of them must not survive.
const mirror = await this.uow.transactional(async () => {
// AC36 — `existing.id`, the SAME id AC34's gate (`getAccommodation`)
// already resolved (rule 21): the caller always reads `existing` from
@@ -636,25 +636,24 @@ export class AccommodationsService {
confirmation: newConfirmation,
notes: newNotes,
});
return await this.remirrorStay(existing.id, newPlaceId, newStartDayId, newCheckIn, {
const moved = await this.remirrorStay(existing.id, newPlaceId, newStartDayId, newCheckIn, {
checkInChanged: fields.check_in !== undefined && (fields.check_in || null) !== (existing.check_in || null),
});
});
// Sync check-in/out/confirmation to every linked reservation. The booking form
// lets more than one hotel booking point at the same block and there is no
// unique constraint on reservations.accommodation_id, so a single .get() would
// silently leave the others on the old times.
// AC37 — runs AFTER the transaction above commits (§18.6, R5 class: pre-existing, flagged not fixed).
const linkedRes = await this.reservationsRepo.listIdMetadataByStay(existing.id);
for (const res of linkedRes) {
const meta = res.metadata ? JSON.parse(res.metadata) : {};
if (newCheckIn) meta.check_in_time = newCheckIn;
if (newCheckInEnd) meta.check_in_end_time = newCheckInEnd;
if (newCheckOut) meta.check_out_time = newCheckOut;
// AC38
await this.reservationsRepo.setMetadataAndConfirmation(res.id, JSON.stringify(meta), newConfirmation || null);
}
// Sync check-in/out/confirmation to every linked reservation (AC37/AC38). The
// booking form lets more than one hotel booking point at the same block and there
// is no unique constraint on reservations.accommodation_id, so a single .get()
// would silently leave the others on the old times.
const linkedRes = await this.reservationsRepo.listIdMetadataByStay(existing.id);
for (const res of linkedRes) {
const meta = res.metadata ? JSON.parse(res.metadata) : {};
if (newCheckIn) meta.check_in_time = newCheckIn;
if (newCheckInEnd) meta.check_in_end_time = newCheckInEnd;
if (newCheckOut) meta.check_out_time = newCheckOut;
await this.reservationsRepo.setMetadataAndConfirmation(res.id, JSON.stringify(meta), newConfirmation || null);
}
return moved;
});
return { accommodation: await this.getAccommodationWithPlace(existing.id), mirror };
}
server/src/nest/addons/addons.service.ts
+9 -11
View File
@@ -16,6 +16,7 @@ import { AppSettings } from '../../db/entities/AppSettings.entity';
import type { AppSettingsRepository } from '../../db/repositories/AppSettings.repository';
import { Users } from '../../db/entities/Users.entity';
import type { UsersRepository } from '../../db/repositories/Users.repository';
import { UnitOfWork } from '../database/unit-of-work';
/**
* Thin wrapper around the enabled-addons + photo-provider read that the legacy
@@ -45,6 +46,7 @@ export class AddonsService {
@InjectRepository(PhotoProviderFields) private readonly photoProviderFields: PhotoProviderFieldsRepository,
@InjectRepository(AppSettings) private readonly appSettings: AppSettingsRepository,
@InjectRepository(Users) private readonly users: UsersRepository,
private readonly uow: UnitOfWork,
) {}
async isAddonEnabled(addonId: string): Promise<boolean> {
@@ -88,13 +90,7 @@ export class AddonsService {
};
}
/**
* Ruling (plan §"Do not fix legacy behaviour"): the legacy handler wrote up
* to 5 `app_settings` rows one at a time, with no transaction around them —
* this stays exactly that way. `AppSettingsRepository.setValue` imposes no
* transaction of its own (Task 0 concern #5), so the loop below is still 0
* to 5 independent, individually-committed upserts.
*/
/** Up to five `app_settings` rows, written together: a save lands whole or not at all. */
async updateCollabFeatures(features: { chat?: boolean; notes?: boolean; links?: boolean; polls?: boolean; whatsnext?: boolean }) {
const mapping: Record<string, string> = {
chat: 'collab_chat_enabled',
@@ -104,10 +100,12 @@ export class AddonsService {
whatsnext: 'collab_whatsnext_enabled',
};
const before = await this.getCollabFeatures();
for (const [feat, key] of Object.entries(mapping)) {
const value = features[feat as keyof typeof features];
if (value !== undefined) await this.appSettings.setValue(key, value ? 'true' : 'false');
}
await this.uow.transactional(async () => {
for (const [feat, key] of Object.entries(mapping)) {
const value = features[feat as keyof typeof features];
if (value !== undefined) await this.appSettings.setValue(key, value ? 'true' : 'false');
}
});
const after = await this.getCollabFeatures();
// Collab flags gate MCP tool/resource registration, so callers must know
// whether anything actually flipped — a no-op save must not tear down every
server/src/nest/airports/airports.service.ts
+44 -28
View File
@@ -7,6 +7,7 @@ import { ReservationsRepository } from '../../db/repositories/Reservations.repos
import { ReservationEndpoints } from '../../db/entities/ReservationEndpoints.entity';
import { ReservationEndpointsRepository } from '../../db/repositories/ReservationEndpoints.repository';
import { CronRegistrarService } from '../scheduling/cron-registrar.service';
import { UnitOfWork } from '../database/unit-of-work';
/**
* The in-container face of the airport dataset, plus the flight-endpoint
@@ -42,6 +43,7 @@ export class AirportsService implements OnApplicationBootstrap {
@InjectRepository(Reservations) private readonly reservationsRepo: ReservationsRepository,
@InjectRepository(ReservationEndpoints) private readonly endpointsRepo: ReservationEndpointsRepository,
private readonly registrar: CronRegistrarService,
private readonly uow: UnitOfWork,
) {}
async onApplicationBootstrap(): Promise<void> {
@@ -66,6 +68,14 @@ export class AirportsService implements OnApplicationBootstrap {
return findByIata(code) as Airport | null;
}
/**
* Endpoints for every flight booked before they existed, from the IATA codes in
* its metadata; a flight without usable codes is flagged for review instead.
*
* @txIndependent one transaction per flight: its two endpoints land together or
* not at all (half a pair hid the flight from every later run), and a flight that
* failed is picked up again on the next boot.
*/
async backfillFlightEndpoints(): Promise<void> {
const pending = await this.reservationsRepo.listFlightsMissingEndpoints();
@@ -76,36 +86,42 @@ export class AirportsService implements OnApplicationBootstrap {
let filled = 0;
let flagged = 0;
for (const r of pending) {
if (!r.metadata) { await this.reservationsRepo.markNeedsReview(r.id); flagged++; continue; }
let meta: any;
try { meta = JSON.parse(r.metadata); } catch { await this.reservationsRepo.markNeedsReview(r.id); flagged++; continue; }
const dep = meta.departure_airport ? findByIata(String(meta.departure_airport).slice(0, 3)) : null;
const arr = meta.arrival_airport ? findByIata(String(meta.arrival_airport).slice(0, 3)) : null;
if (!dep || !arr) { await this.reservationsRepo.markNeedsReview(r.id); flagged++; continue; }
const split = (iso: string | null) => {
if (!iso) return { date: null as string | null, time: null as string | null };
const [date, time] = iso.split('T');
return { date: date || null, time: time ? time.slice(0, 5) : null };
};
const depParts = split(r.reservation_time);
const arrParts = split(r.reservation_end_time);
await this.endpointsRepo.insertEndpoint({
reservation_id: r.id, role: 'from', sequence: 0,
name: dep.city ? `${dep.city} (${dep.iata})` : dep.name, code: dep.iata,
lat: dep.lat, lng: dep.lng, timezone: dep.tz, local_time: depParts.time, local_date: depParts.date,
});
await this.endpointsRepo.insertEndpoint({
reservation_id: r.id, role: 'to', sequence: 1,
name: arr.city ? `${arr.city} (${arr.iata})` : arr.name, code: arr.iata,
lat: arr.lat, lng: arr.lng, timezone: arr.tz, local_time: arrParts.time, local_date: arrParts.date,
});
filled++;
if (await this.uow.transactional(() => this.backfillOne(r))) filled++;
else flagged++;
}
console.log(`[airports] Backfill: ${filled} filled, ${flagged} flagged for review`);
}
/** One flight's endpoints, or its review flag. True when it was filled. */
private async backfillOne(r: { id: number; metadata: string | null; reservation_time: string | null; reservation_end_time: string | null }): Promise<boolean> {
if (!r.metadata) { await this.reservationsRepo.markNeedsReview(r.id); return false; }
let meta: any;
try { meta = JSON.parse(r.metadata); } catch { await this.reservationsRepo.markNeedsReview(r.id); return false; }
const dep = meta.departure_airport ? findByIata(String(meta.departure_airport).slice(0, 3)) : null;
const arr = meta.arrival_airport ? findByIata(String(meta.arrival_airport).slice(0, 3)) : null;
if (!dep || !arr) { await this.reservationsRepo.markNeedsReview(r.id); return false; }
const split = (iso: string | null) => {
if (!iso) return { date: null as string | null, time: null as string | null };
const [date, time] = iso.split('T');
return { date: date || null, time: time ? time.slice(0, 5) : null };
};
const depParts = split(r.reservation_time);
const arrParts = split(r.reservation_end_time);
await this.endpointsRepo.insertEndpoint({
reservation_id: r.id, role: 'from', sequence: 0,
name: dep.city ? `${dep.city} (${dep.iata})` : dep.name, code: dep.iata,
lat: dep.lat, lng: dep.lng, timezone: dep.tz, local_time: depParts.time, local_date: depParts.date,
});
await this.endpointsRepo.insertEndpoint({
reservation_id: r.id, role: 'to', sequence: 1,
name: arr.city ? `${arr.city} (${arr.iata})` : arr.name, code: arr.iata,
lat: arr.lat, lng: arr.lng, timezone: arr.tz, local_time: arrParts.time, local_date: arrParts.date,
});
return true;
}
}
server/src/nest/auth/auth.service.ts
+28 -29
View File
@@ -437,11 +437,8 @@ export class AuthService {
if (!invite) return { error: 'Invalid invite link', status: 404 };
if (invite.max_uses > 0 && invite.used_count >= invite.max_uses) return { error: 'Invite link has been fully used', status: 410 };
if (invite.expires_at && new Date(invite.expires_at) < new Date()) return { error: 'Invite link has expired', status: 410 };
// Rule 16 (nullable columns stay null on the wire, docs/superpowers/plans/
// 2026-09-21-orm-migration-program.md:45; task-5-review-security.md F1 /
// task-5-review-template.md T1): pass the repository's `string | null`
// through unchanged — `?? undefined` made JSON.stringify drop the key
// entirely for a never-expiring invite instead of emitting `null`.
// A nullable column stays null on the wire: `?? undefined` would make
// JSON.stringify drop the key for a never-expiring invite.
return { valid: true, max_uses: invite.max_uses, used_count: invite.used_count, expires_at: invite.expires_at };
}
@@ -777,26 +774,28 @@ export class AuthService {
// the whole form in one request.
const { blocked } = splitManagedKeys(body as Record<string, unknown>, readEnv().managed.enabled);
for (const key of ADMIN_SETTINGS_KEYS) {
if (blocked.includes(key)) continue;
if (body[key] !== undefined) {
let val = String(body[key]);
if (key === 'require_mfa') {
val = body[key] === true || val === 'true' ? 'true' : 'false';
await this.uow.transactional(async () => { // the whole form lands, or none of it
for (const key of ADMIN_SETTINGS_KEYS) {
if (blocked.includes(key)) continue;
if (body[key] !== undefined) {
let val = String(body[key]);
if (key === 'require_mfa') {
val = body[key] === true || val === 'true' ? 'true' : 'false';
}
// An unknown provider name is dropped, not stored: the maps service
// degrades an unrecognised row to 'auto', so writing one would show the
// admin a saved setting that quietly does nothing.
if (key === 'places_provider' && !isPlacesProviderChoice(val)) continue;
if (key === 'smtp_pass' && val === '••••••••') continue;
if (key === 'smtp_pass') val = encrypt_api_key(val);
if (key === 'admin_webhook_url' && val === '••••••••') continue;
if (key === 'admin_webhook_url' && val) val = maybe_encrypt_api_key(val) ?? val;
if (key === 'admin_ntfy_token' && val === '••••••••') continue;
if (key === 'admin_ntfy_token' && val) val = maybe_encrypt_api_key(val) ?? val;
await this.appSettings.setValue(key, val);
}
// An unknown provider name is dropped, not stored: the maps service
// degrades an unrecognised row to 'auto', so writing one would show the
// admin a saved setting that quietly does nothing.
if (key === 'places_provider' && !isPlacesProviderChoice(val)) continue;
if (key === 'smtp_pass' && val === '••••••••') continue;
if (key === 'smtp_pass') val = encrypt_api_key(val);
if (key === 'admin_webhook_url' && val === '••••••••') continue;
if (key === 'admin_webhook_url' && val) val = maybe_encrypt_api_key(val) ?? val;
if (key === 'admin_ntfy_token' && val === '••••••••') continue;
if (key === 'admin_ntfy_token' && val) val = maybe_encrypt_api_key(val) ?? val;
await this.appSettings.setValue(key, val);
}
}
});
const changedKeys = ADMIN_SETTINGS_KEYS.filter(k => !blocked.includes(k) && body[k] !== undefined && !(k === 'smtp_pass' && String(body[k]) === '••••••••'));
@@ -1010,15 +1009,15 @@ export class AuthService {
return { tokenForDelivery: null, userId: user.id, userEmail: user.email, reason: 'oidc_only' };
}
// Invalidate any prior unconsumed tokens for this user so there is
// always at most one live reset link in flight.
await this.passwordResetTokens.consumeAllLiveForUser(user.id);
const raw = randomBytes(PASSWORD_RESET_TOKEN_BYTES).toString('base64url');
const token_hash = hashResetToken(raw);
const expires_at = new Date(Date.now() + PASSWORD_RESET_TTL_MS).toISOString();
await this.passwordResetTokens.insertToken({ user_id: user.id, token_hash, expires_at, created_ip: createdIp });
// Invalidate any prior unconsumed tokens for this user and issue the new one
// together, so there is always exactly one live reset link in flight.
await this.uow.transactional(async () => {
await this.passwordResetTokens.consumeAllLiveForUser(user.id);
await this.passwordResetTokens.insertToken({ user_id: user.id, token_hash, expires_at, created_ip: createdIp });
});
return { tokenForDelivery: raw, userId: user.id, userEmail: user.email, reason: 'issued' };
}
server/src/nest/memories/unified-memories.service.ts
+15 -14
View File
@@ -130,13 +130,16 @@ export class UnifiedMemoriesService {
return providerResult as ServiceResult<boolean>;
}
try {
const photoId = await this.photos.getOrCreate(provider, assetId, userId, passphrase);
const added = await this.tripPhotos.insertIgnore({
trip_id: tripId,
user_id: userId,
photo_id: photoId,
shared: shared ? 1 : 0,
album_link_id: albumLinkId || null,
// The photo row and its trip link together: no registered photo without its link.
const added = await this.uow.transactional(async () => {
const photoId = await this.photos.getOrCreate(provider, assetId, userId, passphrase);
return await this.tripPhotos.insertIgnore({
trip_id: tripId,
user_id: userId,
photo_id: photoId,
shared: shared ? 1 : 0,
album_link_id: albumLinkId || null,
});
});
return success(added);
}
@@ -222,9 +225,10 @@ export class UnifiedMemoriesService {
}
try {
await this.tripPhotos.deleteForUserPhoto(tripId, userId, photoId);
await this.photos.deleteIfOrphan(photoId);
await this.uow.transactional(async () => {
await this.tripPhotos.deleteForUserPhoto(tripId, userId, photoId);
await this.photos.deleteIfOrphan(photoId);
});
this.realtime.broadcast(tripId, 'memories:updated', { userId }, sid);
return success(true);
@@ -292,12 +296,9 @@ export class UnifiedMemoriesService {
await this.uow.transactional(async () => {
await this.tripPhotos.deleteForAlbumLink(tripId, linkId);
await this.tripAlbumLinks.deleteScoped(linkId, tripId, userId);
for (const photo_id of linkedPhotoIds) await this.photos.deleteIfOrphan(photo_id);
});
for (const photo_id of linkedPhotoIds) {
await this.photos.deleteIfOrphan(photo_id);
}
return success(true);
} catch (error) {
return mapDbError(error, 'Failed to remove album link');
server/src/nest/notifications/notification-preferences.service.ts
+9 -14
View File
@@ -370,21 +370,16 @@ export class NotificationPreferencesService {
}
}
// Apply global prefs outside the transaction (they write to app_settings).
// Mixed atomicity, preserved as-is (plan3f-sql-inventory.md §4b's flag, R10):
// these are single-key upserts with no cross-row invariant, so a partial
// failure here is low-risk — widening this into the transaction below is a
// ruling this task does not make on its own.
for (const [eventType, channels] of Object.entries(globalPrefs)) {
if (!channels) continue;
for (const [channel, enabled] of Object.entries(channels)) {
if (!isAdminGlobalChannel(channel)) continue;
await this.setAdminGlobalPref(eventType as NotifEventType, channel, enabled);
}
}
// Apply per-user (inapp) prefs in a transaction
// The global rows (app_settings) and the admin's own in-app rows are one save:
// a failure halfway used to keep the email/webhook half of a form.
await this.uow.transactional(async () => {
for (const [eventType, channels] of Object.entries(globalPrefs)) {
if (!channels) continue;
for (const [channel, enabled] of Object.entries(channels)) {
if (!isAdminGlobalChannel(channel)) continue;
await this.setAdminGlobalPref(eventType as NotifEventType, channel, enabled);
}
}
await this.applyUserChannelPrefs(userId, userPrefs);
});
}
server/src/nest/oauth/oauth.service.ts
+14 -12
View File
@@ -21,6 +21,7 @@ import { OauthTokens } from '../../db/entities/OauthTokens.entity';
import type { OauthTokenRefreshRow, OauthTokensRepository } from '../../db/repositories/OauthTokens.repository';
import { OauthConsents } from '../../db/entities/OauthConsents.entity';
import type { OauthConsentsRepository } from '../../db/repositories/OauthConsents.repository';
import { UnitOfWork } from '../database/unit-of-work';
import {
ACCESS_TOKEN_TTL_S,
CODE_CHALLENGE_RE,
@@ -89,6 +90,7 @@ export class OauthService {
@InjectRepository(OauthConsents) private readonly consents: OauthConsentsRepository,
private readonly addons: AddonsService,
private readonly audit: AuditService,
private readonly uow: UnitOfWork,
private readonly pendingCodes: PendingCodeStore = pendingCodesSlot.get(),
) {}
@@ -202,10 +204,12 @@ export class OauthService {
const rawSecret = 'trekcs_' + randomBytes(24).toString('hex');
const secretHash = hashToken(rawSecret);
await this.clients.updateSecretHash(clientRowId, secretHash);
// Revoke all existing tokens for this client so old sessions are invalidated
await this.tokens.revokeAllForClient(row.client_id);
// The new secret and the revocation of every token issued under the old one
// land together, so a failure never leaves old sessions alive on a new secret.
await this.uow.transactional(async () => {
await this.clients.updateSecretHash(clientRowId, secretHash);
await this.tokens.revokeAllForClient(row.client_id);
});
// Terminate active MCP sessions for this (user, client) pair
revokeUserSessionsForClient(userId, row.client_id);
@@ -499,14 +503,12 @@ export class OauthService {
// request. Killing the session on every routine hourly refresh broke long-lived
// MCP connections (#1475).
//
// Stays NON-transactional with the issueTokens() call below — legacy
// parity (Task 4 brief ruling, §9.6 of the inventory): revoke-old then
// issue-new are two separate statements, not one transaction. A crash
// between the two would leave the old token revoked with no successor —
// the same window the legacy code always had.
await this.tokens.revokeById(row.id);
const tokens = await this.issueTokens(clientId, row.user_id, JSON.parse(row.scopes), row.id, row.audience ?? null);
// Revoke-old and issue-new are one transaction: a failure between the two used
// to leave the old token revoked with no successor, logging the client out.
const tokens = await this.uow.transactional(async () => {
await this.tokens.revokeById(row.id);
return await this.issueTokens(clientId, row.user_id, JSON.parse(row.scopes), row.id, row.audience ?? null);
});
await this.audit.writeAudit({ userId: row.user_id, action: 'oauth.token.refresh', details: { client_id: clientId }, ip });
return { tokens };
server/src/nest/oidc/oidc.service.ts
+75 -58
View File
@@ -638,6 +638,78 @@ export class OidcService implements OnModuleDestroy {
return userInfo.name || userInfo.preferred_username;
}
/**
* A login by an account that already exists: link the OIDC identity when the
* verified email matched it, follow the role claim, keep the OIDC avatar current.
* Runs inside findOrCreateUser's transaction.
*/
private async refreshLinkedUser(
user: UserRow,
sub: string,
config: OidcConfig,
userInfo: OidcUserInfo,
picture: string | null,
): Promise<{ user: User; roleChange?: OidcRoleChange } | { error: string }> {
// Reaching here without an oidc_sub means we matched an existing local
// account by email. Only auto-link the OIDC identity when the IdP asserts
// the email is verified; an unverified email must not auto-link.
if (!user.oidc_sub) {
const emailVerified = userInfo.email_verified === true || userInfo.email_verified === 'true';
if (!emailVerified) {
return { error: 'email_not_verified' };
}
await this.usersRepo.linkOidcIdentity(user.id, sub, config.issuer);
user = { ...user, oidc_sub: sub, oidc_issuer: config.issuer };
} else if (user.oidc_issuer !== config.issuer || user.oidc_sub !== sub) {
// The admin pointed the instance at a different IdP. We got here through the
// verified-email lookup, so this is the same person arriving from the new
// provider; leaving the old sub and issuer on the row would keep the account
// pinned to a provider that no longer exists (#2110).
await this.usersRepo.linkOidcIdentity(user.id, sub, config.issuer);
user = { ...user, oidc_sub: sub, oidc_issuer: config.issuer };
}
// Update role based on OIDC claims on every login (if claim mapping is configured)
let roleChange: OidcRoleChange | undefined;
if (readEnv().oidc.adminValue) {
const resolution = this.resolveOidcRoleDetailed(userInfo, false);
const newRole = resolution.role;
if (resolution.claimMissing) {
this.warnMissingAdminClaim(resolution, { id: user.id, username: user.username, role: user.role === 'admin' ? 'admin' : 'user' });
} else if (user.role !== newRole) {
// Never let the claim-based downgrade strip the last admin. The bootstrap
// admin (first SSO user) usually doesn't carry the admin claim, so a forced
// re-login — e.g. after a JWT-secret rotation — would otherwise demote it and
// lock an OIDC-only instance out for good. #1274
const demotingLastAdmin =
user.role === 'admin' &&
newRole !== 'admin' &&
(await this.usersRepo.countAdmins()) <= 1;
if (demotingLastAdmin) {
console.warn(`[OIDC] Kept admin role for user ${user.id}: their OIDC claims map to '${newRole}', but they are the only admin — demoting would lock the instance out.`);
} else {
await this.usersRepo.setRole(user.id, newRole);
roleChange = { from: user.role === 'admin' ? 'admin' : 'user', to: newRole, claim: resolution.claimKey };
user = { ...user, role: newRole };
}
}
}
// Keep the avatar in sync with the OIDC picture, but never clobber a custom
// upload: only touch it when empty or when the current value is itself an OIDC
// picture URL, so the picture refreshes on each login without overriding an
// uploaded one. #1399
//
// "In sync" includes the provider having no picture for this user any more. That
// is what a provider switch looks like from here, and the old value points at a
// host this instance no longer talks to, so it renders as a broken image forever
// (#2110). An uploaded avatar is a bare filename and stays untouched either way.
const avatarIsOidc = !!user.avatar && /^https:\/\//i.test(user.avatar);
if (picture ? picture !== user.avatar && (!user.avatar || avatarIsOidc) : avatarIsOidc) {
await this.usersRepo.setAvatarRaw(user.id, picture);
user = { ...user, avatar: picture };
}
return { user: toClientUser(user), roleChange };
}
async findOrCreateUser(
userInfo: OidcUserInfo,
config: OidcConfig,
@@ -663,64 +735,9 @@ export class OidcService implements OnModuleDestroy {
}
if (user) {
// Reaching here without an oidc_sub means we matched an existing local
// account by email. Only auto-link the OIDC identity when the IdP asserts
// the email is verified; an unverified email must not auto-link.
if (!user.oidc_sub) {
const emailVerified = userInfo.email_verified === true || userInfo.email_verified === 'true';
if (!emailVerified) {
return { error: 'email_not_verified' };
}
await this.usersRepo.linkOidcIdentity(user.id, sub, config.issuer);
user = { ...user, oidc_sub: sub, oidc_issuer: config.issuer };
} else if (user.oidc_issuer !== config.issuer || user.oidc_sub !== sub) {
// The admin pointed the instance at a different IdP. We got here through the
// verified-email lookup, so this is the same person arriving from the new
// provider; leaving the old sub and issuer on the row would keep the account
// pinned to a provider that no longer exists (#2110).
await this.usersRepo.linkOidcIdentity(user.id, sub, config.issuer);
user = { ...user, oidc_sub: sub, oidc_issuer: config.issuer };
}
// Update role based on OIDC claims on every login (if claim mapping is configured)
let roleChange: OidcRoleChange | undefined;
if (readEnv().oidc.adminValue) {
const resolution = this.resolveOidcRoleDetailed(userInfo, false);
const newRole = resolution.role;
if (resolution.claimMissing) {
this.warnMissingAdminClaim(resolution, { id: user.id, username: user.username, role: user.role === 'admin' ? 'admin' : 'user' });
} else if (user.role !== newRole) {
// Never let the claim-based downgrade strip the last admin. The bootstrap
// admin (first SSO user) usually doesn't carry the admin claim, so a forced
// re-login — e.g. after a JWT-secret rotation — would otherwise demote it and
// lock an OIDC-only instance out for good. #1274
const demotingLastAdmin =
user.role === 'admin' &&
newRole !== 'admin' &&
(await this.usersRepo.countAdmins()) <= 1;
if (demotingLastAdmin) {
console.warn(`[OIDC] Kept admin role for user ${user.id}: their OIDC claims map to '${newRole}', but they are the only admin — demoting would lock the instance out.`);
} else {
await this.usersRepo.setRole(user.id, newRole);
roleChange = { from: user.role === 'admin' ? 'admin' : 'user', to: newRole, claim: resolution.claimKey };
user = { ...user, role: newRole };
}
}
}
// Keep the avatar in sync with the OIDC picture, but never clobber a custom
// upload: only touch it when empty or when the current value is itself an OIDC
// picture URL, so the picture refreshes on each login without overriding an
// uploaded one. #1399
//
// "In sync" includes the provider having no picture for this user any more. That
// is what a provider switch looks like from here, and the old value points at a
// host this instance no longer talks to, so it renders as a broken image forever
// (#2110). An uploaded avatar is a bare filename and stays untouched either way.
const avatarIsOidc = !!user.avatar && /^https:\/\//i.test(user.avatar);
if (picture ? picture !== user.avatar && (!user.avatar || avatarIsOidc) : avatarIsOidc) {
await this.usersRepo.setAvatarRaw(user.id, picture);
user = { ...user, avatar: picture };
}
return { user: toClientUser(user), roleChange };
// The identity link, the role from the claims and the avatar are one write.
const existing = user;
return await this.uow.transactional(() => this.refreshLinkedUser(existing, sub, config, userInfo, picture));
}
// --- New user registration ---
server/src/nest/places/places.service.ts
+59 -61
View File
@@ -393,43 +393,42 @@ export class PlacesService {
// the value as a stay, and a stop added from the corridor search brings its own
// figure — ten minutes for fuel, twenty for a rest area — so the kinds that would
// be misread as an hour never take the default in the first place.
const placeId = await this.placesRepo.insertPlace({
trip_id: tid,
name,
description: description || null,
lat: lat ?? null,
lng: lng ?? null,
address: address || null,
category_id: category_id || null,
price: price ?? null,
currency: currency || null,
place_time: place_time || null,
end_time: end_time || null,
duration_minutes: duration_minutes ?? 60,
notes: notes || null,
image_url: image_url || null,
google_place_id: google_place_id || null,
google_ftid: google_ftid || null,
osm_id: osm_id || null,
amap_poi_id: amap_poi_id || null,
website: website || null,
phone: phone || null,
transport_mode: transport_mode || 'walking',
route_geometry: route_geometry || null,
route_color: route_color || null,
stop_type: stop_type || null,
fill_percent: fill_percent ?? null,
email: email?.trim() || null,
opening_hours: opening_hours || null,
const placeId = await this.uow.transactional(async () => {
const id = await this.placesRepo.insertPlace({
trip_id: tid,
name,
description: description || null,
lat: lat ?? null,
lng: lng ?? null,
address: address || null,
category_id: category_id || null,
price: price ?? null,
currency: currency || null,
place_time: place_time || null,
end_time: end_time || null,
duration_minutes: duration_minutes ?? 60,
notes: notes || null,
image_url: image_url || null,
google_place_id: google_place_id || null,
google_ftid: google_ftid || null,
osm_id: osm_id || null,
amap_poi_id: amap_poi_id || null,
website: website || null,
phone: phone || null,
transport_mode: transport_mode || 'walking',
route_geometry: route_geometry || null,
route_color: route_color || null,
stop_type: stop_type || null,
fill_percent: fill_percent ?? null,
email: email?.trim() || null,
opening_hours: opening_hours || null,
});
// PL5: the row and its tags are one write.
if (tags && tags.length > 0) await this.tagsRepo.insertIgnore(id, await this.tagsOnTrip(tid, tags));
return id;
});
// PL5 — `INSERT OR IGNORE INTO place_tags (place_id, tag_id) VALUES (?, ?)`.
if (tags && tags.length > 0) {
await this.tagsRepo.insertIgnore(placeId, await this.tagsOnTrip(tid, tags));
}
// PL6 — `findWithTagsAndRatings` replaces the `getPlaceWithTags` delegation.
return (await this.placesRepo.findWithTagsAndRatings(placeId))!;
return (await this.placesRepo.findWithTagsAndRatings(placeId))!; // PL6
}
// -------------------------------------------------------------------------
@@ -453,8 +452,7 @@ export class PlacesService {
const tid = toRowId(tripId);
if (tid === null) return null;
if (!(await this.placesRepo.existsInTrip(id, tid))) return null;
// PL8 — `findWithTagsAndRatings` replaces the `getPlaceWithTags` delegation.
return await this.placesRepo.findWithTagsAndRatings(id);
return await this.placesRepo.findWithTagsAndRatings(id); // PL8
}
// -------------------------------------------------------------------------
@@ -467,7 +465,7 @@ export class PlacesService {
body: PlaceUpdateInput,
ifMatch?: string,
): Promise<PlaceWithTags | UpdateConflict | null> {
const { result, reclaim } = await this.applyUpdate(tripId, placeId, body, ifMatch);
const { result, reclaim } = await this.uow.transactional(() => this.applyUpdate(tripId, placeId, body, ifMatch));
if (reclaim !== undefined) await this.reclaimPlaceImage(reclaim);
return result;
}
@@ -571,12 +569,8 @@ export class PlacesService {
});
if (tags !== undefined) {
// PL12 — `DELETE FROM place_tags WHERE place_id = ?`.
await this.tagsRepo.deleteForPlace(id);
if (tags.length > 0) {
// PL13 — `INSERT OR IGNORE INTO place_tags (place_id, tag_id) VALUES (?, ?)`.
await this.tagsRepo.insertIgnore(id, await this.tagsOnTrip(tid, tags));
}
await this.tagsRepo.deleteForPlace(id); // PL12
if (tags.length > 0) await this.tagsRepo.insertIgnore(id, await this.tagsOnTrip(tid, tags)); // PL13
}
// A custom uploaded thumbnail (#1136) that was just replaced or cleared leaves
@@ -586,8 +580,7 @@ export class PlacesService {
? existingPlace.image_url
: undefined;
// PL14 — `findWithTagsAndRatings` replaces the `getPlaceWithTags` delegation.
return { result: await this.placesRepo.findWithTagsAndRatings(id), reclaim };
return { result: await this.placesRepo.findWithTagsAndRatings(id), reclaim }; // PL14
}
// -------------------------------------------------------------------------
@@ -864,9 +857,12 @@ export class PlacesService {
// -------------------------------------------------------------------------
async importGpx(tripId: string, fileBuffer: Buffer, opts: GpxImportOptions = {}): Promise<GpxImportResult | null> {
const result = await this.importGpxRows(tripId, fileBuffer, opts);
await this.colorizeImportedTracks(tripId, result);
return result;
// The rows and their track colours are one write, as for KML below.
return await this.uow.transactional(async () => {
const result = await this.importGpxRows(tripId, fileBuffer, opts);
await this.colorizeImportedTracks(tripId, result);
return result;
});
}
/**
@@ -1076,9 +1072,11 @@ export class PlacesService {
// -------------------------------------------------------------------------
async importMapFile(tripId: string, fileBuffer: Buffer, filename: string, opts: KmlImportOptions = {}): Promise<PlaceImportResult> {
const result = await this.importMapFileRows(tripId, fileBuffer, filename, opts);
await this.colorizeImportedTracks(tripId, result);
return result;
return await this.uow.transactional(async () => {
const result = await this.importMapFileRows(tripId, fileBuffer, filename, opts);
await this.colorizeImportedTracks(tripId, result);
return result;
});
}
private async importMapFileRows(tripId: string, fileBuffer: Buffer, filename: string, opts: KmlImportOptions = {}): Promise<PlaceImportResult> {
@@ -1242,15 +1240,13 @@ export class PlacesService {
const tracks = result?.places?.filter((p) => p.route_geometry && !p.route_color) ?? [];
if (tracks.length === 0) return;
// PL36+PL37 — read (`distinctRouteColors`) and write (`setRouteColor`) in
// ONE transaction so two concurrent imports cannot both read the same set
// of free colours.
// PL36+PL37: read and write in ONE transaction so two concurrent imports
// cannot both read the same set of free colours.
await this.uow.transactional(async () => {
const taken = new Set(await this.placesRepo.distinctRouteColors(tripId));
const free = TRACK_COLORS.filter((c) => !taken.has(c));
for (const [i, track] of tracks.entries()) {
// Free ones first, then wrap through the whole palette — never reuse a
// free colour twice within the same import.
// Free ones first, then wrap through the palette: never a free colour twice.
const color = i < free.length ? free[i] : TRACK_COLORS[(i - free.length) % TRACK_COLORS.length];
await this.placesRepo.setRouteColor(track.id, color);
track.route_color = color;
@@ -1854,6 +1850,7 @@ export class PlacesService {
* The Google pass for places a file brought in (#2536). Only its points: a track
* or a drawn path is a line, and looking a line up by its name finds a stranger.
* Detached like the list imports, and just as quietly a no-op without a key.
* @txStandalone detached, after the import has committed.
*/
enrichImportedFilePlaces(tripId: string, userId: number, places: ImportedPlace[]): void {
const points = (places as (ImportedPlace & EnrichablePlace)[]).filter(p => !p.route_geometry);
@@ -1973,21 +1970,22 @@ export class PlacesService {
// PL51 — the only explicit `ON CONFLICT … DO UPDATE` in the cluster; never touches `places.updated_at`.
await this.placeRatingsRepo.upsertRating(id, userId, rating);
}
// PL52 — `findWithTagsAndRatings` replaces the `getPlaceWithTags` delegation.
return await this.placesRepo.findWithTagsAndRatings(id);
return await this.placesRepo.findWithTagsAndRatings(id); // PL52
}
// Journey hooks — non-fatal, mirroring the route's try/catch wrappers.
/** @txStandalone a journey hook: a non-fatal sync after the place write, like the route's try/catch. */
async onCreated(tripId: string, placeId: number): Promise<void> {
try {
await this.journey.onPlaceCreated(Number(tripId), placeId);
} catch { /* non-fatal */ }
}
/** @txStandalone the same non-fatal journey hook. */
async onUpdated(placeId: number): Promise<void> {
try {
await this.journey.onPlaceUpdated(placeId);
} catch { /* non-fatal */ }
}
/** @txStandalone the same non-fatal journey hook. */
async onDeleted(placeId: number): Promise<void> {
try {
await this.journey.onPlaceDeleted(placeId);
server/src/nest/trips/trips.service.ts
+25 -26
View File
@@ -524,28 +524,30 @@ export class TripsService {
: oldReminder;
const tripIdNum = Number(tripId); // safe: `trip` above only resolved through the raw-bind seam on a real row (Task 6 review's own ruling on this exact conversion)
await this.tripsRepo.updateTripRow(tripIdNum, { // TP25
title: newTitle,
description: newDesc ?? null,
start_date: newStart || null,
end_date: newEnd || null,
currency: newCurrency,
// Task 7 security review L1 (absorbed here): `newArchived` is already
// `trip.is_archived` verbatim when `data.is_archived` was never sent —
// the legacy statement bound that value AS-IS, including a stored
// `NULL`. A `?? 0` fold here would write `0` in that case instead,
// silently un-nulling a column the caller never asked to change.
is_archived: newArchived,
cover_image: newCover ?? null,
reminder_days: newReminder,
});
if (trip.start_date && trip.end_date && newStart && newStart !== trip.start_date)
await this.vacay.shiftOwnerEntriesForTripWindow(trip.user_id, trip.start_date, trip.end_date, newStart);
// The row, the leave entries that follow its dates and the rebuilt day grid are
// one write: a failure halfway used to leave a trip whose days missed its dates.
let removedDays: DayGridRemoval[] = [];
if (regenerate) {
await this.uow.transactional(async () => {
await this.uow.transactional(async () => {
await this.tripsRepo.updateTripRow(tripIdNum, { // TP25
title: newTitle,
description: newDesc ?? null,
start_date: newStart || null,
end_date: newEnd || null,
currency: newCurrency,
// Task 7 security review L1 (absorbed here): `newArchived` is already
// `trip.is_archived` verbatim when `data.is_archived` was never sent —
// the legacy statement bound that value AS-IS, including a stored
// `NULL`. A `?? 0` fold here would write `0` in that case instead,
// silently un-nulling a column the caller never asked to change.
is_archived: newArchived,
cover_image: newCover ?? null,
reminder_days: newReminder,
});
if (trip.start_date && trip.end_date && newStart && newStart !== trip.start_date)
await this.vacay.shiftOwnerEntriesForTripWindow(trip.user_id, trip.start_date, trip.end_date, newStart);
if (regenerate) {
// Accommodations have no absolute date columns, so their pre-change dates must be
// snapshotted before generateDays re-dates the day rows in place.
const prevDays = await this.daysRepo.listOrderedForReorder(tripIdNum); // TP26
@@ -556,9 +558,6 @@ export class TripsService {
// so re-stamp reservation_time to follow — same rules as reorderDays/insertDay.
const newDays = await this.daysRepo.listOrderedForReorder(tripIdNum); // TP27
const newDateByDayId = new Map(newDays.map(d => [d.id, d.date]));
// `tripIdNum`, not `tripId` (Task 9 fix wave, H2): `restampReservationDates`
// now takes the parsed row id, the same value every other DaysService
// survivor takes — see its own docstring.
await this.days.restampReservationDates(tripIdNum, prevDateByDayId, newDateByDayId);
} else {
// Default: generateDays re-dates day rows positionally; re-anchor dated bookings to
@@ -567,8 +566,8 @@ export class TripsService {
await this.reservations.resyncReservationDays(tripId);
await this.days.resyncAccommodationDays(tripIdNum, prevDateByDayId);
}
});
}
}
});
const changes: Record<string, unknown> = {};
if (title && title !== trip.title) changes.title = title;
server/src/nest/vacay/vacay.service.ts
+58 -60
View File
@@ -209,27 +209,13 @@ function parseAsUtcMillis(value: string): number {
* legacy lazy require sat in; notifications stay fire-and-forget dynamic
* imports.
*
* Post-migration fixes on top of the relocated legacy behavior: the
* multi-statement writes (acceptInvite, dissolvePlan, deleteYear, updatePlan's
* carry-over recompute) run in uow.transactional(); every outbound fetch carries
* an AbortSignal timeout and the nager.at responses are ok-checked;
* applyHolidayCalendars honors the cache TTL; addYear no longer swallows real
* errors; the holiday cache is instance state instead of a module-level map.
* All consumers are in-container since the trip fold (TripsService injects
* this class); vacay.bridge.ts was deleted with its last outside-container
* consumer.
* Multi-statement writes run in uow.transactional(), never around the
* nager.at fetch (timed out and ok-checked, cached per instance for the TTL);
* addYear does not swallow real errors. TripsService injects this class.
*
* R6 (this task): `removeShare`/`setShareHidden` no longer read an UNSCOPED
* `vacay_shares` row and check ownership in JS — `VacaySharesRepository
* .findScopedForRemoval`/`.findScopedForHide` scope the SAME two checks IN
* the SQL statement itself (two distinct methods, not one with a mode flag —
* see that repository's docstrings). Behaviorally identical to the legacy
* shape on every input (the JS check ran before any mutation either way),
* flagged as a genuine SQL-shape tightening per the plan's "For the user"
* note. `getStats` (VC126) now wraps its per-request carry-over write in
* `uow.transactional`, matching every OTHER multi-row write loop in this
* file (plan3f-inputs.md correction #7's "wrap it, flag it" default) —
* flagged here rather than left silently un-transacted either way.
* `removeShare`/`setShareHidden` scope their ownership check in the SQL
* itself (`VacaySharesRepository.findScopedForRemoval`/`.findScopedForHide`),
* and `getStats` writes its per-request carry-over in `uow.transactional`.
*/
@Injectable()
export class VacayService {
@@ -382,19 +368,28 @@ export class VacayService {
// Plan management
// -------------------------------------------------------------------------
/**
* The caller's own plan, created with its first year, allowance and colour on
* first use, in one transaction whose second look keeps two first requests
* from creating two plans.
* @txStandalone an idempotent lazy provision, whatever the caller does next.
*/
async getOwnPlan(userId: number): Promise<VacayPlan> {
let plan = await this.plans.findByOwner(userId);
if (!plan) {
const existing = await this.plans.findByOwner(userId);
if (existing) return existing;
return await this.uow.transactional(async () => {
const raced = await this.plans.findByOwner(userId);
if (raced) return raced;
await this.plans.insertForOwner(userId);
plan = (await this.plans.findByOwner(userId))!;
const plan = (await this.plans.findByOwner(userId))!;
// Seed the period today falls into — with a shifted leave year (#737) that is
// not necessarily the current calendar year.
const yr = await this.currentPeriodYear(userId);
await this.years.insertIgnore(plan.id, yr);
await this.userYears.insertIgnore(userId, plan.id, yr, 30, 0);
await this.userColors.insertIgnore(userId, plan.id, '#6366f1');
}
return plan;
return plan;
});
}
async getActivePlan(userId: number): Promise<VacayPlan> {
@@ -498,10 +493,17 @@ export class VacayService {
}
async applyHolidayCalendars(planId: number): Promise<void> {
const dates = await this.holidayDatesToClear(planId);
if (dates.length > 0) await this.uow.transactional(() => this.clearHolidayDates(planId, dates));
}
/** The dates the plan's public-holiday calendars clear. Fetched first, outside any transaction. */
private async holidayDatesToClear(planId: number): Promise<string[]> {
const dates = new Set<string>();
const holidaysEnabled = await this.plans.getHolidaysEnabled(planId);
if (!holidaysEnabled) return;
if (!holidaysEnabled) return [];
const calendars = await this.holidayCalendars.listPublicForPlan(planId);
if (calendars.length === 0) return;
if (calendars.length === 0) return [];
const years = await this.years.listForPlan(planId);
// A shifted leave year (#737) runs into the next calendar year, so collect the
// calendar years the members' windows actually touch — not just the period ids.
@@ -520,16 +522,20 @@ export class VacayService {
if (!holidays) continue;
const hasRegions = holidays.some((h: Holiday) => h.counties && h.counties.length > 0);
if (hasRegions && !region) continue;
// Outside the fetch's catch: a failing delete is a real error, and
// swallowing it as "API error" left vacation days standing on holidays.
for (const h of holidays) {
if (h.global || !h.counties || (region && h.counties.includes(region))) {
await this.entries.deleteForPlanAndDate(planId, h.date);
await this.companyHolidays.deleteForPlanAndDate(planId, h.date);
}
if (h.global || !h.counties || (region && h.counties.includes(region))) dates.add(h.date);
}
}
}
return [...dates];
}
/** Outside the fetch's catch: a failing delete is a real error, not an unanswered API. */
private async clearHolidayDates(planId: number, dates: string[]): Promise<void> {
for (const date of dates) {
await this.entries.deleteForPlanAndDate(planId, date);
await this.companyHolidays.deleteForPlanAndDate(planId, date);
}
}
async migrateHolidayCalendars(planId: number, plan: VacayPlan): Promise<void> {
@@ -557,25 +563,22 @@ export class VacayService {
if (weekend_days !== undefined) patch.weekend_days = String(weekend_days);
if (week_start !== undefined) patch.week_start = week_start === 0 ? 0 : 1;
await this.plans.update(planId, patch);
// Two transactions with the holiday fetch between them: network I/O never
// holds the connection. The settings land first, the clean-up they imply second.
await this.uow.transactional(async () => {
await this.plans.update(planId, patch);
if (company_holidays_enabled === true) {
const companyDates = await this.companyHolidays.listForPlan(planId);
for (const { date, fraction } of companyDates) await this.makeRoomForCompanyHoliday(planId, date, fraction);
}
await this.migrateHolidayCalendars(planId, (await this.plans.findById(planId))!);
});
const holidayDates = await this.holidayDatesToClear(planId);
if (company_holidays_enabled === true) {
const companyDates = await this.companyHolidays.listForPlan(planId);
for (const { date, fraction } of companyDates) await this.makeRoomForCompanyHoliday(planId, date, fraction);
}
const updatedPlan = (await this.plans.findById(planId))!;
await this.migrateHolidayCalendars(planId, updatedPlan);
await this.applyHolidayCalendars(planId);
if (carry_over_enabled === false) {
await this.userYears.resetCarriedOverForPlan(planId);
}
if (carry_over_enabled === true) {
// The chained per-year/per-user recompute is atomic — a failure mid-chain
// would otherwise leave later years carrying stale balances.
await this.uow.transactional(async () => {
await this.uow.transactional(async () => {
await this.clearHolidayDates(planId, holidayDates);
if (carry_over_enabled === false) await this.userYears.resetCarriedOverForPlan(planId);
if (carry_over_enabled === true) {
const years = await this.years.listForPlan(planId);
const users = await this.getPlanUsers(planId);
for (let i = 0; i < years.length - 1; i++) {
@@ -584,20 +587,15 @@ export class VacayService {
for (const u of users) {
const used = await this.usedDays(u.id, planId, yr);
const config = await this.userYears.findForYear(u.id, planId, yr);
// L1 (task-7-review.md): legacy bound `config.vacation_days`/
// `config.carried_over` into this total AS-IS, letting a NULL
// column pass through and coerce to 0 in the `+` (JS: `null + n
// = n`) — never defaulting a NULL vacation_days to 30. `?? 0`
// (not `?? 30`) reproduces that exact coercion under strict
// nullable typing; only the OUTER `config ? … : 30` (no row at
// all) is a genuine default.
// No row at all defaults to 30 days; a NULL column counts as 0, the
// way the legacy statement's arithmetic coerced it.
const total = (config ? config.vacation_days ?? 0 : 30) + (config ? config.carried_over ?? 0 : 0);
const carry = Math.max(0, total - used);
await this.userYears.upsertCarriedOver(u.id, planId, nextYr, carry);
}
}
});
}
}
});
await this.notifyPlanUsers(planId, socketId, 'vacay:settings');
server/tests/helpers/test-addons.ts
+2
View File
@@ -1,6 +1,7 @@
import type Database from 'better-sqlite3';
import { AddonsService } from '../../src/nest/addons/addons.service';
import { sharedTestOrm } from './test-uow';
import { UnitOfWork } from '../../src/nest/database/unit-of-work';
import { Addons } from '../../src/db/entities/Addons.entity';
import { PhotoProviders } from '../../src/db/entities/PhotoProviders.entity';
import { PhotoProviderFields } from '../../src/db/entities/PhotoProviderFields.entity';
@@ -40,5 +41,6 @@ export async function createTestAddonsService(db: Database.Database): Promise<Ad
t.repo(PhotoProviderFields),
t.repo(AppSettings),
t.repo(Users),
new UnitOfWork(t.em),
);
}
server/tests/integration/mcp.test.ts
+2 -1
View File
@@ -37,6 +37,7 @@ import { setPluginMcpToolSource } from '../../src/plugin-mcp-tools';
import type { McpDynamicTool } from '../../src/nest-mcp';
import { getMcpSafeUrl } from '../../src/app-config';
import { OauthService } from '../../src/nest/oauth/oauth.service';
import { UnitOfWork } from '../../src/nest/database/unit-of-work';
import { createTestAddonsService } from '../helpers/test-addons';
import { AuditService } from '../../src/nest/audit/audit.service';
import { createTestOrm, type TestOrm } from '../helpers/test-orm';
@@ -76,7 +77,7 @@ beforeAll(async () => {
nestApp = await buildApp();
app = nestApp.getHttpAdapter().getInstance();
t = await createTestOrm(testDb);
oauthSvc = new OauthService(t.repo(OauthClients), t.repo(OauthTokens), t.repo(OauthConsents), await createTestAddonsService(testDb), new AuditService(t.repo(AuditLog), t.repo(Users)));
oauthSvc = new OauthService(t.repo(OauthClients), t.repo(OauthTokens), t.repo(OauthConsents), await createTestAddonsService(testDb), new AuditService(t.repo(AuditLog), t.repo(Users)), new UnitOfWork(t.em));
});
beforeEach(async () => {
server/tests/integration/oauth-authorize.test.ts
+2 -1
View File
@@ -41,6 +41,7 @@ import { buildApp } from '../../src/bootstrap';
import { resetTestDb, resetRateLimits } from '../helpers/test-db';
import { createUser } from '../helpers/factories';
import { OauthService } from '../../src/nest/oauth/oauth.service';
import { UnitOfWork } from '../../src/nest/database/unit-of-work';
import { createTestAddonsService } from '../helpers/test-addons';
import { AuditService } from '../../src/nest/audit/audit.service';
import { createTestOrm, type TestOrm } from '../helpers/test-orm';
@@ -86,7 +87,7 @@ beforeAll(async () => {
nestApp = await buildApp();
app = nestApp.getHttpAdapter().getInstance();
t = await createTestOrm(testDb);
containerSideOauth = new OauthService(t.repo(OauthClients), t.repo(OauthTokens), t.repo(OauthConsents), await createTestAddonsService(testDb), new AuditService(t.repo(AuditLog), t.repo(Users)));
containerSideOauth = new OauthService(t.repo(OauthClients), t.repo(OauthTokens), t.repo(OauthConsents), await createTestAddonsService(testDb), new AuditService(t.repo(AuditLog), t.repo(Users)), new UnitOfWork(t.em));
});
beforeEach(async () => {
server/tests/integration/oauth.test.ts
+2 -1
View File
@@ -38,6 +38,7 @@ import { createUser } from '../helpers/factories';
import { authCookie } from '../helpers/auth';
import { ALL_SCOPES } from '../../src/mcp/scopes';
import { OauthService } from '../../src/nest/oauth/oauth.service';
import { UnitOfWork } from '../../src/nest/database/unit-of-work';
import { createTestAddonsService } from '../helpers/test-addons';
import { AuditService } from '../../src/nest/audit/audit.service';
import { createTestOrm, type TestOrm } from '../helpers/test-orm';
@@ -84,7 +85,7 @@ beforeAll(async () => {
nestApp = await buildApp();
app = nestApp.getHttpAdapter().getInstance();
t = await createTestOrm(testDb);
containerSideOauth = new OauthService(t.repo(OauthClients), t.repo(OauthTokens), t.repo(OauthConsents), await createTestAddonsService(testDb), new AuditService(t.repo(AuditLog), t.repo(Users)));
containerSideOauth = new OauthService(t.repo(OauthClients), t.repo(OauthTokens), t.repo(OauthConsents), await createTestAddonsService(testDb), new AuditService(t.repo(AuditLog), t.repo(Users)), new UnitOfWork(t.em));
});
beforeEach(async () => {
server/tests/integration/plugins/boot-registry-order.test.ts
+1
View File
@@ -232,6 +232,7 @@ describe('plugin boot vs registry scan ordering', () => {
t2.repo(PhotoProviderFields),
t2.repo(AppSettings),
t2.repo(Users),
new UnitOfWork(t2.em),
);
const auditLogRepo2 = t2.repo(AuditLog);
const usersRepo2 = t2.repo(Users);
server/tests/unit/nest/accommodations.service.test.ts
+18 -1
View File
@@ -42,7 +42,7 @@ import { AccommodationsModule } from '../../../src/nest/accommodations/accommoda
import { AccommodationsDomainModule } from '../../../src/nest/accommodations/accommodations-domain.module';
import { AccommodationsController } from '../../../src/nest/accommodations/accommodations.controller';
import { expectRegisteredProvider, expectRegisteredController } from '../../helpers/module-providers';
import { createTestUnitOfWork, createTestAppSettingsRepo } from '../../helpers/test-uow';
import { createTestUnitOfWork, createTestAppSettingsRepo, createTestReservationsRepo } from '../../helpers/test-uow';
// Named `svc` so the moved cases read exactly as they did on DaysService.
let svc: Awaited<ReturnType<typeof makeAccommodationsService>>;
@@ -177,6 +177,23 @@ describe('getAccommodation', () => {
});
describe('updateAccommodation', () => {
it('DAY-SVC-TX-001: a failing booking sync rolls the stay edit back with it', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const day = createDay(testDb, trip.id);
const place = createPlace(testDb, trip.id, { name: 'Hotel' });
await svc.createAccommodation(trip.id, { place_id: place.id, start_day_id: day.id, end_day_id: day.id, check_in: '14:00' });
const { id } = testDb.prepare('SELECT id FROM day_accommodations WHERE trip_id = ?').get(trip.id) as { id: number };
const existing = (await svc.getAccommodation(id, trip.id))!;
const reservations = await createTestReservationsRepo(testDb);
const spy = vi.spyOn(reservations, 'setMetadataAndConfirmation').mockRejectedValueOnce(new Error('boom'));
await expect(svc.updateAccommodation(id, existing, { check_in: '18:00' })).rejects.toThrow('boom');
expect(testDb.prepare('SELECT check_in FROM day_accommodations WHERE id = ?').get(id)).toEqual({ check_in: '14:00' });
spy.mockRestore();
});
it('DAY-SVC-023 — updates check-in and check-out times', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
server/tests/unit/nest/addons.service.test.ts
+15 -1
View File
@@ -27,6 +27,7 @@ import { resetTestDb } from '../../helpers/test-db';
import { createUser } from '../../helpers/factories';
import { createTestOrm, type TestOrm } from '../../helpers/test-orm';
import { AddonsService } from '../../../src/nest/addons/addons.service';
import { UnitOfWork } from '../../../src/nest/database/unit-of-work';
import { PlaceShadowService } from '../../../src/nest/place-shadow/place-shadow.service';
import { Addons } from '../../../src/db/entities/Addons.entity';
import type { AddonsRepository } from '../../../src/db/repositories/Addons.repository';
@@ -169,7 +170,7 @@ beforeEach(() => {
t.clear();
getPhotoProviderConfig.mockReset();
getPhotoProviderConfig.mockReturnValue({});
svc = new AddonsService(addonsRepo, photoProvidersRepo, photoProviderFieldsRepo, appSettingsRepo, usersRepo);
svc = new AddonsService(addonsRepo, photoProvidersRepo, photoProviderFieldsRepo, appSettingsRepo, usersRepo, new UnitOfWork(t.em));
});
afterEach(() => {
@@ -415,6 +416,19 @@ describe('AddonsService addon/feature flags', () => {
setValueSpy.mockRestore();
});
it('updateCollabFeatures saves the flags together: a failing second write keeps the first one off disk', async () => {
const setValue = appSettingsRepo.setValue.bind(appSettingsRepo);
const setValueSpy = vi.spyOn(appSettingsRepo, 'setValue')
.mockImplementationOnce(setValue)
.mockRejectedValueOnce(new Error('disk full'));
await expect(svc.updateCollabFeatures({ chat: false, notes: false })).rejects.toThrow('disk full');
expect(rawAppSetting('collab_chat_enabled')?.value).not.toBe('false');
expect(await svc.getCollabFeatures()).toEqual({ chat: true, notes: true, links: true, polls: true, whatsnext: true });
setValueSpy.mockRestore();
});
});
/**
server/tests/unit/nest/airports/airports.service.test.ts
+34 -5
View File
@@ -19,7 +19,9 @@ import { CronRegistrarService } from '../../../../src/nest/scheduling/cron-regis
import type { RuntimeEnvService } from '../../../../src/nest/app-config/runtime-env.service';
import { createSnapshotTestDb } from '../../../helpers/db-mock';
import { createTestOrm, type TestOrm } from '../../../helpers/test-orm';
import { createTestReservationsRepo, createTestReservationEndpointsRepo } from '../../../helpers/test-uow';
import { createTestReservationsRepo, createTestReservationEndpointsRepo, createTestUnitOfWork } from '../../../helpers/test-uow';
import type { UnitOfWork } from '../../../../src/nest/database/unit-of-work';
import { createUser, createTrip } from '../../../helpers/factories';
import type { ReservationsRepository } from '../../../../src/db/repositories/Reservations.repository';
import type { ReservationEndpointsRepository } from '../../../../src/db/repositories/ReservationEndpoints.repository';
@@ -27,6 +29,7 @@ const testDb = createSnapshotTestDb();
let t: TestOrm;
let reservationsRepo: ReservationsRepository;
let endpointsRepo: ReservationEndpointsRepository;
let uow: UnitOfWork;
beforeAll(async () => {
// Global context disallowed — the production setting — so this genuinely
@@ -35,6 +38,7 @@ beforeAll(async () => {
t = await createTestOrm(testDb, { allowGlobalContext: false });
reservationsRepo = await createTestReservationsRepo(testDb);
endpointsRepo = await createTestReservationEndpointsRepo(testDb);
uow = await createTestUnitOfWork(testDb);
});
afterAll(async () => { await t.close(); testDb.close(); });
beforeEach(() => vi.clearAllMocks());
@@ -48,7 +52,7 @@ describe('AirportsService boot backfill', () => {
unregister: vi.fn(),
runOnBoot: vi.fn(async (name: string, fn: () => void | Promise<void>) => { registered.push([name, fn]); await fn(); }),
};
const svc = new AirportsService(reservationsRepo, endpointsRepo, registrar as unknown as CronRegistrarService);
const svc = new AirportsService(reservationsRepo, endpointsRepo, registrar as unknown as CronRegistrarService, uow);
const backfillSpy = vi.spyOn(svc, 'backfillFlightEndpoints').mockResolvedValue(undefined);
await svc.onApplicationBootstrap();
expect(registrar.runOnBoot).toHaveBeenCalledWith('airports-flight-endpoints-boot', expect.any(Function));
@@ -57,7 +61,7 @@ describe('AirportsService boot backfill', () => {
it('AIRPORTS-SVC-002: with MikroORM wired into the registrar, the boot-time backfill actually runs — no cannotUseGlobalContext, nothing swallowed', async () => {
const registrar = new CronRegistrarService(new SchedulerRegistry(), { isTest: () => false } as RuntimeEnvService, t.orm);
const svc = new AirportsService(reservationsRepo, endpointsRepo, registrar);
const svc = new AirportsService(reservationsRepo, endpointsRepo, registrar, uow);
const backfillSpy = vi.spyOn(svc, 'backfillFlightEndpoints');
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
try {
@@ -72,7 +76,7 @@ describe('AirportsService boot backfill', () => {
it('AIRPORTS-SVC-003: without MikroORM, the boot-time backfill never runs at all — logged with runOnBoot\'s own distinct message, never swallowed into "[DB] Flight endpoint backfill failed"', async () => {
const registrar = new CronRegistrarService(new SchedulerRegistry(), { isTest: () => false } as RuntimeEnvService); // no orm
const svc = new AirportsService(reservationsRepo, endpointsRepo, registrar);
const svc = new AirportsService(reservationsRepo, endpointsRepo, registrar, uow);
const backfillSpy = vi.spyOn(svc, 'backfillFlightEndpoints');
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
try {
@@ -92,7 +96,7 @@ describe('AirportsService boot backfill', () => {
unregister: vi.fn(),
runOnBoot: vi.fn(async () => { throw new Error('RequestContext.create blew up'); }),
};
const svc = new AirportsService(reservationsRepo, endpointsRepo, registrar as unknown as CronRegistrarService);
const svc = new AirportsService(reservationsRepo, endpointsRepo, registrar as unknown as CronRegistrarService, uow);
const backfillSpy = vi.spyOn(svc, 'backfillFlightEndpoints');
const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
try {
@@ -103,4 +107,29 @@ describe('AirportsService boot backfill', () => {
errSpy.mockRestore();
}
});
it('AIRPORTS-SVC-005: a flight gets both endpoints or neither: a failing second insert leaves it to the next run', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const meta = JSON.stringify({ departure_airport: 'FRA', arrival_airport: 'JFK' });
const half = Number(testDb.prepare("INSERT INTO reservations (trip_id, title, type, metadata) VALUES (?, 'Out', 'flight', ?)").run(trip.id, meta).lastInsertRowid);
const whole = Number(testDb.prepare("INSERT INTO reservations (trip_id, title, type, metadata) VALUES (?, 'Back', 'flight', ?)").run(trip.id, meta).lastInsertRowid);
const insert = endpointsRepo.insertEndpoint.bind(endpointsRepo);
// The first flight's 'to' endpoint fails, which ends this run.
const spy = vi.spyOn(endpointsRepo, 'insertEndpoint')
.mockImplementationOnce(insert)
.mockRejectedValueOnce(new Error('disk full'));
const svc = new AirportsService(reservationsRepo, endpointsRepo, {} as CronRegistrarService, uow);
await expect(svc.backfillFlightEndpoints()).rejects.toThrow('disk full');
const count = (id: number) => (testDb.prepare('SELECT COUNT(*) AS c FROM reservation_endpoints WHERE reservation_id = ?').get(id) as { c: number }).c;
expect(count(half)).toBe(0);
spy.mockRestore();
const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
await svc.backfillFlightEndpoints();
logSpy.mockRestore();
expect(count(half)).toBe(2);
expect(count(whole)).toBe(2);
});
});
server/tests/unit/nest/notification-preferences.service.test.ts
+9
View File
@@ -352,6 +352,15 @@ describe('setAdminPreferences', () => {
const row = testDb.prepare("SELECT value FROM app_settings WHERE key = ?").get('admin_notif_pref_version_available_email') as { value: string } | undefined;
expect(row?.value).toBe('1');
});
it('NPREF-025b: the global and the per-user rows are one save: a failing in-app row keeps the email pref unchanged', async () => {
const { user } = createAdmin(testDb);
const prefsRepo = await createTestNotificationChannelPreferencesRepo(testDb);
const spy = vi.spyOn(prefsRepo, 'upsertPreference').mockRejectedValueOnce(new Error('disk full'));
await expect(setAdminPreferences(user.id, { version_available: { email: false, inapp: false } })).rejects.toThrow('disk full');
expect(testDb.prepare("SELECT value FROM app_settings WHERE key = ?").get('admin_notif_pref_version_available_email')).toBeUndefined();
spy.mockRestore();
});
});
// ─────────────────────────────────────────────────────────────────────────────
server/tests/unit/nest/oauth.service.test.ts
+18 -2
View File
@@ -56,6 +56,7 @@ function makePkce() {
}
import { OauthService } from '../../../src/nest/oauth/oauth.service';
import { UnitOfWork } from '../../../src/nest/database/unit-of-work';
import { AuditService } from '../../../src/nest/audit/audit.service';
import type { AddonsService } from '../../../src/nest/addons/addons.service';
import { getMcpSafeUrl } from '../../../src/app-config';
@@ -119,7 +120,7 @@ beforeAll(async () => {
clientsRepo = t.repo(OauthClients);
tokensRepo = t.repo(OauthTokens);
consentsRepo = t.repo(OauthConsents);
svc = new OauthService(clientsRepo, tokensRepo, consentsRepo, addonsStub, new AuditService(auditLogRepo, usersRepo));
svc = new OauthService(clientsRepo, tokensRepo, consentsRepo, addonsStub, new AuditService(auditLogRepo, usersRepo), new UnitOfWork(t.em));
});
beforeEach(() => {
@@ -492,6 +493,21 @@ describe('refreshTokens', () => {
expect(result.tokens!.access_token.startsWith('trekoa_')).toBe(true);
});
it('revoking the old pair and issuing the new one are one write: a failing issue keeps the old pair', async () => {
const { user } = createUser(testDb);
const created = await makeClient(user.id);
const clientId = created.client!.client_id as string;
const rawSecret = created.client!.client_secret as string;
const { access_token, refresh_token } = await issueTokens(clientId, user.id, ['trips:read']);
const spy = vi.spyOn(tokensRepo, 'insertToken').mockRejectedValueOnce(new Error('disk full'));
await expect(refreshTokens(refresh_token, clientId, rawSecret)).rejects.toThrow('disk full');
spy.mockRestore();
// Before, the old pair was already revoked here and the client logged out.
expect(await getUserByAccessToken(access_token)).not.toBeNull();
});
it('old tokens are revoked after refresh (rotation)', async () => {
const { user } = createUser(testDb);
const created = await makeClient(user.id);
@@ -1308,7 +1324,7 @@ describe('module-scoped OAuth state', () => {
codeChallengeMethod: 'S256',
}))!;
const secondInstance = new OauthService(clientsRepo, tokensRepo, consentsRepo, addonsStub, new AuditService(auditLogRepo, usersRepo));
const secondInstance = new OauthService(clientsRepo, tokensRepo, consentsRepo, addonsStub, new AuditService(auditLogRepo, usersRepo), new UnitOfWork(t.em));
expect((await secondInstance.consumeAuthCode(code))?.userId).toBe(42);
});
});
server/tests/unit/nest/oidc.service.test.ts
+17
View File
@@ -793,6 +793,23 @@ describe('findOrCreateUser', () => {
expect(row.avatar).toBeNull();
});
it('OIDC-SVC-062: the identity switch and the avatar are one write: a failing avatar keeps the old link', async () => {
const { user } = createUser(testDb, { email: 'switch4@example.com' });
testDb.prepare('UPDATE users SET oidc_sub = ?, oidc_issuer = ?, avatar = ? WHERE id = ?')
.run('sub-old-4', 'https://old-idp.example.com', 'https://old-idp.example.com/u/me.png', user.id);
const users = await createTestUsersRepo(testDb);
const spy = vi.spyOn(users, 'setAvatarRaw').mockRejectedValueOnce(new Error('disk full'));
await expect(svc.findOrCreateUser(
{ sub: 'sub-new-4', email: 'switch4@example.com', name: 'Switcher', email_verified: true },
{ ...MOCK_CONFIG, issuer: 'https://new-idp.example.com' },
)).rejects.toThrow('disk full');
expect(testDb.prepare('SELECT oidc_sub, oidc_issuer FROM users WHERE id = ?').get(user.id))
.toEqual({ oidc_sub: 'sub-old-4', oidc_issuer: 'https://old-idp.example.com' });
spy.mockRestore();
});
it('OIDC-SVC-061: an uploaded avatar survives the same switch', async () => {
const { user } = createUser(testDb, { email: 'switch2@example.com' });
// A local upload is a bare filename, not a URL, and belongs to the user.
server/tests/unit/nest/places.service.test.ts
+13
View File
@@ -269,6 +269,19 @@ describe('create', () => {
expect(Array.isArray(place.tags)).toBe(true);
});
it('PLACE-SVC-007b: the place and its tags are one write: a failing tag insert leaves no place', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const tag = createTag(testDb, user.id, { name: 'Highlight' });
const tags = await createTestTagsRepo(testDb);
const spy = vi.spyOn(tags, 'insertIgnore').mockRejectedValueOnce(new Error('disk full'));
await expect(svc.create(String(trip.id), { name: 'Half Place', tags: [tag.id] })).rejects.toThrow('disk full');
expect(testDb.prepare("SELECT id FROM places WHERE name = 'Half Place'").all()).toEqual([]);
spy.mockRestore();
});
it('PLACE-SVC-008 — creates a place with tags', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
server/tests/unit/nest/process-state-ports.test.ts
+3 -3
View File
@@ -121,7 +121,7 @@ describe('OAuth pending codes', () => {
it('PORTS-006: OauthService issues and redeems codes through the injected store', async () => {
const store = new InMemoryPendingCodeStore();
const service = new OauthService({} as never, {} as never, {} as never, {} as never, {} as never, store);
const service = new OauthService({} as never, {} as never, {} as never, {} as never, {} as never, {} as never, store);
const issued = await service.createAuthCode({
clientId: 'c',
userId: 1,
@@ -138,7 +138,7 @@ describe('OAuth pending codes', () => {
it('PORTS-007: OauthModule provides the process-wide store, the one a hand-built service defaults to', () => {
expectRegisteredProvider(OauthModule, { provide: PendingCodeStore, useValue: processPendingCodes });
const service = new OauthService({} as never, {} as never, {} as never, {} as never, {} as never);
const service = new OauthService({} as never, {} as never, {} as never, {} as never, {} as never, {} as never);
expect((service as unknown as { pendingCodes: unknown }).pendingCodes).toBe(processPendingCodes);
});
@@ -150,7 +150,7 @@ describe('OAuth pending codes', () => {
expect(pendingCodesSlot.get()).toBe(fake);
await sweepPendingCodes(5);
expect(sweep).toHaveBeenCalledWith(5);
const service = new OauthService({} as never, {} as never, {} as never, {} as never, {} as never);
const service = new OauthService({} as never, {} as never, {} as never, {} as never, {} as never, {} as never);
expect((service as unknown as { pendingCodes: unknown }).pendingCodes).toBe(fake);
} finally {
oauthModule.onModuleDestroy();
server/tests/unit/nest/trips.service.test.ts
+13
View File
@@ -828,6 +828,19 @@ describe('resyncReservationDays (#1288)', () => {
expect(res.day_id).toBe(dayFor(trip.id, '2025-06-02'));
});
it('TRIP-SVC-018b: the trip row and its rebuilt days are one write: a failing rebuild keeps the old dates', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id, { start_date: '2025-06-01', end_date: '2025-06-05' });
const days = await createTestDaysRepo(testDb);
const spy = vi.spyOn(days, 'listOrderedForReorder').mockRejectedValueOnce(new Error('boom'));
await expect(svc.updateTrip(trip.id, user.id, { start_date: '2025-06-02', end_date: '2025-06-06' }, 'user')).rejects.toThrow('boom');
expect(testDb.prepare('SELECT start_date, end_date FROM trips WHERE id = ?').get(trip.id))
.toEqual({ start_date: '2025-06-01', end_date: '2025-06-05' });
spy.mockRestore();
});
it('TRIP-SVC-019: a reservation whose date falls outside the new range keeps its day_id (not nulled)', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id, { start_date: '2025-06-01', end_date: '2025-06-05' });
server/tests/unit/nest/unified-memories.service.test.ts
+17
View File
@@ -242,6 +242,23 @@ describe('addTripPhotos', () => {
});
});
describe('addTripPhotos is atomic per photo', () => {
it('MEM-UNIFIED-007b: the photo row and its trip link are one write: a failing link leaves no photo row', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
testDb.prepare(
'INSERT OR IGNORE INTO photo_providers (id, name, description, icon, enabled, sort_order) VALUES (?, ?, ?, ?, ?, ?)'
).run('tx-prov', 'Tx', 'Tx provider', 'Image', 1, 98);
const spy = vi.spyOn(tripPhotosRepo, 'insertIgnore').mockRejectedValueOnce(new Error('disk full'));
const result = await addTripPhotos(String(trip.id), user.id, false, [{ provider: 'tx-prov', asset_ids: ['asset-tx'] }], 'sid');
expect(result.success).toBe(false);
expect(testDb.prepare("SELECT id FROM trek_photos WHERE asset_id = 'asset-tx'").all()).toEqual([]);
spy.mockRestore();
});
});
// ── setTripPhotoSharing ───────────────────────────────────────────────────────
describe('setTripPhotoSharing', () => {