From 65e99d3a513a3d31db99fb4c3e51e2383a679c73 Mon Sep 17 00:00:00 2001 From: Harold Ikechukwu Date: Sat, 29 Aug 2026 13:05:06 +0100 Subject: [PATCH 1/2] Schedule and monitor stale draft cleanup in the backend --- backend/.env.example | 16 +++ backend/src/app.module.ts | 17 ++- backend/src/config/draft.config.ts | 31 ++++++ .../src/invoices/draft-cleanup.job.spec.ts | 104 ++++++++++++++++++ backend/src/invoices/draft-cleanup.job.ts | 82 ++++++++++++++ backend/src/invoices/draft.service.ts | 15 ++- backend/src/invoices/invoices.module.ts | 5 +- 7 files changed, 266 insertions(+), 4 deletions(-) create mode 100644 backend/src/config/draft.config.ts create mode 100644 backend/src/invoices/draft-cleanup.job.spec.ts create mode 100644 backend/src/invoices/draft-cleanup.job.ts diff --git a/backend/.env.example b/backend/.env.example index a894cb70..114f0ae3 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -54,3 +54,19 @@ ADMIN_SECRET_KEY=SXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX # DEPRECATED: Use ADMIN_SECRET_KEY instead # SOROBAN_SECRET_KEY is kept for backward compatibility but will be removed # SOROBAN_SECRET_KEY=SXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX + +# ── Draft cleanup ───────────────────────────────────────────────────────────── +# Number of days a draft can go without being auto-saved before the cleanup +# job permanently deletes it. Measured against lastAutoSavedAt. +# Minimum: 1. Default: 30. +DRAFT_RETENTION_DAYS=30 + +# Cron expression controlling when the stale-draft cleanup job fires (UTC). +# Standard 5-field cron syntax. The process must be restarted for a new +# schedule to take effect. +# +# Useful values: +# "0 3 * * *" – daily at 03:00 UTC (production default) +# "* * * * *" – every minute (local smoke-testing only) +# "0 3 * * 0" – weekly, Sundays at 03:00 UTC +DRAFT_CLEANUP_CRON="0 3 * * *" diff --git a/backend/src/app.module.ts b/backend/src/app.module.ts index b8cbaf87..1485cffe 100644 --- a/backend/src/app.module.ts +++ b/backend/src/app.module.ts @@ -6,6 +6,7 @@ import appConfig from "./config/app.config"; import stellarConfig from "./config/stellar.config"; import throttlerConfig from "./config/throttler.config"; import observabilityConfig from "./config/observability.config"; +import draftConfig from "./config/draft.config"; import { ObservabilityModule } from "./observability/observability.module"; // Modules @@ -40,7 +41,7 @@ import { Module } from "@nestjs/common"; ConfigModule.forRoot({ isGlobal: true, envFilePath: ".env", - load: [appConfig, stellarConfig, throttlerConfig, observabilityConfig], + load: [appConfig, stellarConfig, throttlerConfig, observabilityConfig, draftConfig], validationSchema: Joi.object({ PORT: Joi.number().default(3001), CORS_ORIGIN: Joi.string().default("http://localhost:3000"), @@ -117,6 +118,20 @@ import { Module } from "@nestjs/common"; }), // SOROBAN_SECRET_KEY (deprecated - kept for backward compatibility) SOROBAN_SECRET_KEY: Joi.string().optional().allow(""), + // Draft cleanup configuration + DRAFT_RETENTION_DAYS: Joi.number() + .integer() + .min(1) + .default(30) + .description( + "Days a draft can go without an auto-save before the cleanup job deletes it (default: 30)", + ), + DRAFT_CLEANUP_CRON: Joi.string() + .optional() + .allow("") + .description( + "Cron expression for the stale-draft cleanup job (default: \"0 3 * * *\" — daily at 03:00 UTC)", + ), }), validationOptions: { abortEarly: false, diff --git a/backend/src/config/draft.config.ts b/backend/src/config/draft.config.ts new file mode 100644 index 00000000..8560eab1 --- /dev/null +++ b/backend/src/config/draft.config.ts @@ -0,0 +1,31 @@ +import { registerAs } from "@nestjs/config"; + +/** + * Draft configuration + * + * Controls the stale-draft cleanup job behaviour. + * All values are read from environment variables at startup, so a process + * restart is sufficient to apply changes — no code change required. + * + * Environment variables + * --------------------- + * DRAFT_RETENTION_DAYS (number, default 30) + * Number of days a draft can go without being auto-saved before it becomes + * eligible for deletion by the cleanup job. The cutoff is measured against + * `lastAutoSavedAt` (set to `createdAt` on first save). + * + * DRAFT_CLEANUP_CRON (cron expression, default "0 3 * * *") + * When the cleanup job fires. The expression is in standard 5-field cron + * syntax and is interpreted in the server's local timezone (UTC in + * production). The default runs once a day at 03:00 UTC, offset from the + * overdue-invoice job (02:00 UTC) to avoid DB lock contention. + * + * Examples: + * "0 3 * * *" – daily at 03:00 UTC (production default) + * "* * * * *" – every minute (useful for local smoke-testing) + * "0 3 * * 0" – weekly, Sundays at 03:00 UTC + */ +export default registerAs("draft", () => ({ + retentionDays: parseInt(process.env.DRAFT_RETENTION_DAYS ?? "30", 10), + cleanupCron: process.env.DRAFT_CLEANUP_CRON ?? "0 3 * * *", +})); diff --git a/backend/src/invoices/draft-cleanup.job.spec.ts b/backend/src/invoices/draft-cleanup.job.spec.ts new file mode 100644 index 00000000..c064e843 --- /dev/null +++ b/backend/src/invoices/draft-cleanup.job.spec.ts @@ -0,0 +1,104 @@ +import { Test, TestingModule } from "@nestjs/testing"; +import { DraftCleanupJob } from "./draft-cleanup.job"; +import { DraftService } from "./draft.service"; +import { ConfigService } from "@nestjs/config"; + +/** + * Unit tests for DraftCleanupJob. + * + * The job must: + * 1. Log started + completed when drafts are deleted. + * 2. Log started + "no stale drafts" when count is 0. + * 3. Catch and log unexpected errors WITHOUT re-throwing so the scheduler + * continues running on subsequent intervals. + */ +describe("DraftCleanupJob", () => { + let job: DraftCleanupJob; + + const mockDraftService = { + cleanupOldDrafts: jest.fn(), + }; + + const mockConfigService = { + get: jest.fn().mockReturnValue(30), + }; + + beforeEach(async () => { + const module: TestingModule = await Test.createTestingModule({ + providers: [ + DraftCleanupJob, + { provide: DraftService, useValue: mockDraftService }, + { provide: ConfigService, useValue: mockConfigService }, + ], + }).compile(); + + job = module.get(DraftCleanupJob); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe("run()", () => { + it("should log completion when stale drafts are deleted", async () => { + mockDraftService.cleanupOldDrafts.mockResolvedValueOnce({ + deletedCount: 5, + }); + + const logSpy = jest + .spyOn((job as any).logger, "log") + .mockImplementation(() => {}); + + await job.run(); + + expect(mockDraftService.cleanupOldDrafts).toHaveBeenCalledTimes(1); + // Two log lines: started + complete-with-count + expect(logSpy).toHaveBeenCalledTimes(2); + expect(logSpy).toHaveBeenNthCalledWith( + 1, + expect.stringContaining("retentionDays=30"), + ); + expect(logSpy).toHaveBeenNthCalledWith( + 2, + expect.stringContaining("deleted 5 stale draft(s)"), + ); + }); + + it("should log 'no stale drafts found' when deletedCount is 0", async () => { + mockDraftService.cleanupOldDrafts.mockResolvedValueOnce({ + deletedCount: 0, + }); + + const logSpy = jest + .spyOn((job as any).logger, "log") + .mockImplementation(() => {}); + + await job.run(); + + expect(logSpy).toHaveBeenCalledTimes(2); + expect(logSpy).toHaveBeenNthCalledWith( + 2, + expect.stringContaining("no stale drafts found"), + ); + }); + + it("should catch errors and log them without re-throwing", async () => { + const boom = new Error("DB connection lost"); + mockDraftService.cleanupOldDrafts.mockRejectedValueOnce(boom); + + const errorSpy = jest + .spyOn((job as any).logger, "error") + .mockImplementation(() => {}); + // Suppress the start log to keep the test focused on the error path + jest.spyOn((job as any).logger, "log").mockImplementation(() => {}); + + // Must NOT throw — the scheduler must stay alive + await expect(job.run()).resolves.toBeUndefined(); + + expect(errorSpy).toHaveBeenCalledWith( + expect.stringContaining("unexpected error"), + expect.stringContaining("DB connection lost"), + ); + }); + }); +}); diff --git a/backend/src/invoices/draft-cleanup.job.ts b/backend/src/invoices/draft-cleanup.job.ts new file mode 100644 index 00000000..99055269 --- /dev/null +++ b/backend/src/invoices/draft-cleanup.job.ts @@ -0,0 +1,82 @@ +import { Injectable, Logger } from "@nestjs/common"; +import { Cron } from "@nestjs/schedule"; +import { ConfigService } from "@nestjs/config"; +import { DraftService } from "./draft.service"; + +/** + * DraftCleanupJob + * + * Scheduled job that purges stale drafts by delegating to + * {@link DraftService.cleanupOldDrafts}. The cleanup is considered "stale" + * when a draft has not been auto-saved for more than DRAFT_RETENTION_DAYS + * days (default: 30). + * + * Schedule + * ──────── + * The cron expression is controlled by the DRAFT_CLEANUP_CRON environment + * variable (default: "0 3 * * *" — 03:00 UTC daily). A restart is required + * to pick up a new schedule. + * + * Observability + * ───────────── + * Three log events are emitted so ops can verify the job without a code change: + * + * draft_cleanup_job_started – emitted at the top of every run + * drafts_cleaned_up – emitted (by DraftService) when count > 0 + * draft_cleanup_job_complete – emitted at the end of every successful run + * draft_cleanup_job_error – emitted when an unexpected error is caught + * + * Errors are logged but not re-thrown so that a single bad run does not + * prevent the scheduler from firing again on the next interval. + */ +@Injectable() +export class DraftCleanupJob { + private readonly logger = new Logger(DraftCleanupJob.name); + + constructor( + private readonly draftService: DraftService, + private readonly configService: ConfigService, + ) {} + + /** + * Entry point invoked by the NestJS scheduler. + * + * The `@Cron` expression is evaluated at module initialisation time. + * Changing DRAFT_CLEANUP_CRON therefore requires a process restart. + */ + @Cron( + // ConfigService is not yet available as a static value when the decorator + // is evaluated, so we read the env var directly here. The same pattern + // is safe because ConfigModule.forRoot() processes dotenv before any + // provider is instantiated, meaning process.env is already populated. + process.env.DRAFT_CLEANUP_CRON ?? "0 3 * * *", + { name: "draft-cleanup" }, + ) + async run(): Promise { + const retentionDays = + this.configService.get("draft.retentionDays") ?? 30; + + this.logger.log( + `Draft cleanup job started (retentionDays=${retentionDays})`, + ); + + try { + const { deletedCount } = await this.draftService.cleanupOldDrafts(); + + if (deletedCount === 0) { + this.logger.log("Draft cleanup complete — no stale drafts found"); + } else { + this.logger.log( + `Draft cleanup complete — deleted ${deletedCount} stale draft(s)`, + ); + } + } catch (err) { + this.logger.error( + "Draft cleanup job encountered an unexpected error", + err instanceof Error ? err.stack : String(err), + ); + // Do NOT re-throw: an uncaught exception would crash the scheduler + // context and prevent the next invocation from firing. + } + } +} diff --git a/backend/src/invoices/draft.service.ts b/backend/src/invoices/draft.service.ts index 00eb057c..90082a1b 100644 --- a/backend/src/invoices/draft.service.ts +++ b/backend/src/invoices/draft.service.ts @@ -3,6 +3,7 @@ import { NotFoundException, BadRequestException, } from "@nestjs/common"; +import { ConfigService } from "@nestjs/config"; import { PrismaService } from "../prisma/prisma.service"; import { StructuredLogger } from "../observability/structured-logger.service"; import { ActivityFeedService } from "../activity-feed/activity-feed.service"; @@ -23,15 +24,25 @@ import { Invoice } from "./entities/invoice.entity"; @Injectable() export class DraftService { private readonly AUTO_SAVE_INTERVAL_MS = 30_000; // 30 seconds - private readonly DRAFT_EXPIRY_DAYS = 30; // 30 days before cleanup constructor( private readonly prisma: PrismaService, private readonly stellarService: StellarService, private readonly structuredLogger: StructuredLogger, private readonly activityFeed: ActivityFeedService, + private readonly configService: ConfigService, ) {} + /** + * How many days a draft may go without an auto-save before it is + * considered stale and eligible for cleanup. + * Reads DRAFT_RETENTION_DAYS at call time so a restart picks up changes + * immediately without a code deploy. + */ + private get retentionDays(): number { + return this.configService.get("draft.retentionDays") ?? 30; + } + /** * Create a new draft invoice */ @@ -427,7 +438,7 @@ export class DraftService { */ async cleanupOldDrafts(): Promise<{ deletedCount: number }> { const cutoff = new Date(); - cutoff.setDate(cutoff.getDate() - this.DRAFT_EXPIRY_DAYS); + cutoff.setDate(cutoff.getDate() - this.retentionDays); const result = await this.prisma.invoice.deleteMany({ where: { diff --git a/backend/src/invoices/invoices.module.ts b/backend/src/invoices/invoices.module.ts index ab805612..c9d96438 100644 --- a/backend/src/invoices/invoices.module.ts +++ b/backend/src/invoices/invoices.module.ts @@ -1,7 +1,9 @@ import { Module } from "@nestjs/common"; +import { ConfigModule } from "@nestjs/config"; import { InvoicesController } from "./invoices.controller"; import { InvoicesService } from "./invoices.service"; import { DraftService } from "./draft.service"; +import { DraftCleanupJob } from "./draft-cleanup.job"; import { PaymentReviewsService } from "./payment-reviews.service"; import { StellarModule } from "../stellar/stellar.module"; import { SorobanModule } from "../soroban/soroban.module"; @@ -23,6 +25,7 @@ import { ActivityFeedModule } from "../activity-feed/activity-feed.module"; */ @Module({ imports: [ + ConfigModule, StellarModule, SorobanModule, PrismaModule, @@ -34,7 +37,7 @@ import { ActivityFeedModule } from "../activity-feed/activity-feed.module"; ActivityFeedModule, ], controllers: [InvoicesController], - providers: [InvoicesService, PaymentReviewsService, DraftService], + providers: [InvoicesService, PaymentReviewsService, DraftService, DraftCleanupJob], exports: [InvoicesService, DraftService], }) export class InvoicesModule {} From 12cbbee36a391d754eb664b02486d18a7e01d848 Mon Sep 17 00:00:00 2001 From: Harold Ikechukwu Date: Sun, 30 Aug 2026 21:05:23 +0100 Subject: [PATCH 2/2] fix(ci): add ConfigService mock to draft.service.spec and bump Node to 22.x - draft.service.spec.ts was not updated when ConfigService was added to DraftService's constructor in 65e99d3, causing NestJS DI to throw at compile time and all tests in that suite to fail with exit code 1 - Bump CI matrix node-version from 20.x (deprecated) to 22.x (LTS) to resolve the 'Node.js 20 is deprecated, being forced to Node.js 24' warning and ensure a predictable runtime --- .github/workflows/backend.yml | 2 +- backend/src/invoices/draft.service.spec.ts | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/backend.yml b/.github/workflows/backend.yml index a515f0e2..306bc068 100644 --- a/.github/workflows/backend.yml +++ b/.github/workflows/backend.yml @@ -53,7 +53,7 @@ jobs: strategy: matrix: - node-version: [20.x] + node-version: [22.x] steps: - uses: actions/checkout@v4 diff --git a/backend/src/invoices/draft.service.spec.ts b/backend/src/invoices/draft.service.spec.ts index 0b9f1437..ace86b81 100644 --- a/backend/src/invoices/draft.service.spec.ts +++ b/backend/src/invoices/draft.service.spec.ts @@ -1,5 +1,6 @@ import { Test, TestingModule } from "@nestjs/testing"; import { BadRequestException, NotFoundException } from "@nestjs/common"; +import { ConfigService } from "@nestjs/config"; import { DraftService } from "./draft.service"; import { PrismaService } from "../prisma/prisma.service"; import { StructuredLogger } from "../observability/structured-logger.service"; @@ -26,6 +27,10 @@ describe("DraftService — placeholder validation on conversion", () => { recordEvent: jest.fn(), }; + const mockConfigService = { + get: jest.fn().mockReturnValue(30), + }; + /** * Build a mock PrismaService with a controllable invoice store. */ @@ -133,6 +138,7 @@ describe("DraftService — placeholder validation on conversion", () => { { provide: StellarService, useValue: mockStellarService }, { provide: StructuredLogger, useValue: mockStructuredLogger }, { provide: ActivityFeedService, useValue: mockActivityFeed }, + { provide: ConfigService, useValue: mockConfigService }, ], }).compile();