fix(navigation): honest not-found contract; skip invalid header items
IHeaderRepository.getHeader() now returns Header | null, making the use case's HeaderNotFoundError branch honestly reachable instead of dead code behind a lying type (B12). The Payload repo also skips CMS rows with empty label/href rather than emitting items that violate headerItemSchema.min(1) and 500 at output validation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -35,13 +35,13 @@ export const headerRepositoryContract = defineContractSuite<IHeaderRepository>(
|
||||
|
||||
it("getHeader returns an object with an items array of the seeded length", async () => {
|
||||
const header = await repo.getHeader();
|
||||
expect(header).toBeDefined();
|
||||
expect(header.items).toBeInstanceOf(Array);
|
||||
expect(header.items).toHaveLength(3);
|
||||
expect(header).not.toBeNull();
|
||||
expect(header!.items).toBeInstanceOf(Array);
|
||||
expect(header!.items).toHaveLength(3);
|
||||
});
|
||||
|
||||
it("getHeader items appear in the seeded order with correct shape", async () => {
|
||||
const header = await repo.getHeader();
|
||||
const header = (await repo.getHeader())!;
|
||||
expect(header.items[0]?.label).toBe("Home");
|
||||
expect(header.items[0]?.href).toBe("/");
|
||||
expect(header.items[0]?.external).toBe(false);
|
||||
@@ -54,7 +54,7 @@ export const headerRepositoryContract = defineContractSuite<IHeaderRepository>(
|
||||
});
|
||||
|
||||
it("getHeader items have label, href, and external fields", async () => {
|
||||
const header = await repo.getHeader();
|
||||
const header = (await repo.getHeader())!;
|
||||
for (const item of header.items) {
|
||||
expect(typeof item.label).toBe("string");
|
||||
expect(item.label.length).toBeGreaterThan(0);
|
||||
@@ -65,7 +65,7 @@ export const headerRepositoryContract = defineContractSuite<IHeaderRepository>(
|
||||
});
|
||||
|
||||
it("getHeader logoId is string or undefined", async () => {
|
||||
const header = await repo.getHeader();
|
||||
const header = (await repo.getHeader())!;
|
||||
expect(
|
||||
header.logoId === undefined || typeof header.logoId === "string",
|
||||
).toBe(true);
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { Header } from "../../entities/models/header";
|
||||
|
||||
export interface IHeaderRepository {
|
||||
getHeader(): Promise<Header>;
|
||||
/** Returns the header, or `null` when no header is configured. */
|
||||
getHeader(): Promise<Header | null>;
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest";
|
||||
import { ZodError } from "zod";
|
||||
import { getHeaderUseCase } from "@/application/use-cases/get-header.use-case";
|
||||
import { MockHeaderRepository } from "@/infrastructure/repositories/header.repository.mock";
|
||||
import { HeaderNotFoundError } from "@/entities/errors/header";
|
||||
|
||||
describe("getHeaderUseCase", () => {
|
||||
it("returns the seeded header items", async () => {
|
||||
@@ -12,6 +13,12 @@ describe("getHeaderUseCase", () => {
|
||||
expect(result.items[0]?.label).toBe("Home");
|
||||
});
|
||||
|
||||
it("throws HeaderNotFoundError when no header is configured", async () => {
|
||||
const repo = new MockHeaderRepository(null);
|
||||
const useCase = getHeaderUseCase(repo);
|
||||
await expect(useCase({})).rejects.toBeInstanceOf(HeaderNotFoundError);
|
||||
});
|
||||
|
||||
it("throws ZodError when repository returns malformed header", async () => {
|
||||
const malformedRepo = {
|
||||
getHeader: async () =>
|
||||
|
||||
@@ -1,7 +1,10 @@
|
||||
import "reflect-metadata";
|
||||
import { afterEach, beforeEach, describe, expect, it } from "vitest";
|
||||
import { NoopTracer, NoopLogger } from "@repo/core-shared/instrumentation";
|
||||
import { RecordingEventBus, RecordingJobQueue } from "@repo/core-testing/instrumentation";
|
||||
import {
|
||||
RecordingEventBus,
|
||||
RecordingJobQueue,
|
||||
} from "@repo/core-testing/instrumentation";
|
||||
import { bindDevSeedNavigation } from "@/di/bind-dev-seed";
|
||||
import { navigationContainer } from "@/di/container";
|
||||
import { NAVIGATION_SYMBOLS } from "@/di/symbols";
|
||||
@@ -32,7 +35,11 @@ describe("bindDevSeedNavigation", () => {
|
||||
});
|
||||
|
||||
it("populates the header repository with the dev header", async () => {
|
||||
await bindDevSeedNavigation({ ...noop, bus: new RecordingEventBus(), queue: new RecordingJobQueue() });
|
||||
await bindDevSeedNavigation({
|
||||
...noop,
|
||||
bus: new RecordingEventBus(),
|
||||
queue: new RecordingJobQueue(),
|
||||
});
|
||||
|
||||
const repo = navigationContainer.get<IHeaderRepository>(
|
||||
NAVIGATION_SYMBOLS.IHeaderRepository,
|
||||
@@ -43,31 +50,44 @@ describe("bindDevSeedNavigation", () => {
|
||||
});
|
||||
|
||||
it("seeds a header with a non-empty items array", async () => {
|
||||
await bindDevSeedNavigation({ ...noop, bus: new RecordingEventBus(), queue: new RecordingJobQueue() });
|
||||
await bindDevSeedNavigation({
|
||||
...noop,
|
||||
bus: new RecordingEventBus(),
|
||||
queue: new RecordingJobQueue(),
|
||||
});
|
||||
|
||||
const repo = navigationContainer.get<IHeaderRepository>(
|
||||
NAVIGATION_SYMBOLS.IHeaderRepository,
|
||||
);
|
||||
const header = await repo.getHeader();
|
||||
|
||||
expect(header.items.length).toBeGreaterThan(0);
|
||||
const homeItem = header.items.find((item) => item.label === "Home");
|
||||
expect(header).not.toBeNull();
|
||||
expect(header!.items.length).toBeGreaterThan(0);
|
||||
const homeItem = header!.items.find((item) => item.label === "Home");
|
||||
expect(homeItem).toBeDefined();
|
||||
expect(homeItem?.href).toBe("/");
|
||||
});
|
||||
|
||||
it("is idempotent — calling twice rebuilds a fresh populated repo", async () => {
|
||||
await bindDevSeedNavigation({ ...noop, bus: new RecordingEventBus(), queue: new RecordingJobQueue() });
|
||||
await bindDevSeedNavigation({
|
||||
...noop,
|
||||
bus: new RecordingEventBus(),
|
||||
queue: new RecordingJobQueue(),
|
||||
});
|
||||
const before = navigationContainer.get<IHeaderRepository>(
|
||||
NAVIGATION_SYMBOLS.IHeaderRepository,
|
||||
);
|
||||
const beforeHeader = await before.getHeader();
|
||||
const beforeHeader = (await before.getHeader())!;
|
||||
|
||||
await bindDevSeedNavigation({ ...noop, bus: new RecordingEventBus(), queue: new RecordingJobQueue() });
|
||||
await bindDevSeedNavigation({
|
||||
...noop,
|
||||
bus: new RecordingEventBus(),
|
||||
queue: new RecordingJobQueue(),
|
||||
});
|
||||
const after = navigationContainer.get<IHeaderRepository>(
|
||||
NAVIGATION_SYMBOLS.IHeaderRepository,
|
||||
);
|
||||
const afterHeader = await after.getHeader();
|
||||
const afterHeader = (await after.getHeader())!;
|
||||
|
||||
expect(afterHeader.items.length).toBe(beforeHeader.items.length);
|
||||
// It's a fresh instance — not the previous one.
|
||||
|
||||
@@ -18,26 +18,31 @@ const DEFAULT_ITEMS: HeaderItem[] = [
|
||||
|
||||
@injectable()
|
||||
export class MockHeaderRepository implements IHeaderRepository {
|
||||
private readonly data: Header;
|
||||
private readonly data: Header | null;
|
||||
private tracer: ITracer;
|
||||
private logger: ILogger;
|
||||
|
||||
/**
|
||||
* Pass `null` as `initialData` to simulate an unconfigured header
|
||||
* (getHeader resolves to `null`); omit it for the default seed.
|
||||
*/
|
||||
constructor(
|
||||
initialData?: Header,
|
||||
initialData?: Header | null,
|
||||
tracer: ITracer = new NoopTracer(),
|
||||
logger: ILogger = new NoopLogger(),
|
||||
) {
|
||||
this.data = initialData ?? { items: DEFAULT_ITEMS };
|
||||
this.data =
|
||||
initialData === undefined ? { items: DEFAULT_ITEMS } : initialData;
|
||||
this.tracer = tracer;
|
||||
this.logger = logger;
|
||||
void this.logger; // currently unused; reserved for future mock-thrown captures
|
||||
}
|
||||
|
||||
async getHeader(): Promise<Header> {
|
||||
async getHeader(): Promise<Header | null> {
|
||||
return this.tracer.startSpan(
|
||||
{ name: "header.getHeader", op: "repository", attributes: {} },
|
||||
async (span) => {
|
||||
span.setAttribute("itemCount", this.data.items.length);
|
||||
span.setAttribute("itemCount", this.data?.items.length ?? 0);
|
||||
return this.data;
|
||||
},
|
||||
);
|
||||
|
||||
@@ -1,8 +1,12 @@
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { RecordingTracer } from "@repo/core-testing/instrumentation";
|
||||
import { HeaderRepository } from "@/infrastructure/repositories/header.repository";
|
||||
import { headerRepositoryContract, CONTRACT_HEADER_SEED } from "@/__contracts__/header-repository.contract";
|
||||
import {
|
||||
headerRepositoryContract,
|
||||
CONTRACT_HEADER_SEED,
|
||||
} from "@/__contracts__/header-repository.contract";
|
||||
import { stubPayloadConfig } from "@repo/core-testing/payload/stub-config";
|
||||
import { headerSchema } from "@/entities/models/header";
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// In-memory Payload stub for header (Global)
|
||||
@@ -75,10 +79,15 @@ describe("HeaderRepository", () => {
|
||||
expect(header.logoId).toBeUndefined();
|
||||
});
|
||||
|
||||
it("maps null item fields to empty strings and false", async () => {
|
||||
it("skips items with missing label or href instead of emitting invalid ones", async () => {
|
||||
const stub = {
|
||||
findGlobal: vi.fn(async () => ({
|
||||
items: [{ label: null, href: null, external: null }],
|
||||
items: [
|
||||
{ label: null, href: null, external: null },
|
||||
{ label: "Valid", href: "/valid", external: null },
|
||||
{ label: "", href: "/empty-label", external: false },
|
||||
{ label: "No href", href: "", external: false },
|
||||
],
|
||||
})),
|
||||
};
|
||||
const { getPayload } = await import("payload");
|
||||
@@ -87,8 +96,26 @@ describe("HeaderRepository", () => {
|
||||
const repo = new HeaderRepository(stubPayloadConfig);
|
||||
const header = await repo.getHeader();
|
||||
|
||||
expect(header.items[0]?.label).toBe("");
|
||||
expect(header.items[0]?.href).toBe("");
|
||||
expect(header.items[0]?.external).toBe(false);
|
||||
expect(header.items).toEqual([
|
||||
{ label: "Valid", href: "/valid", external: false },
|
||||
]);
|
||||
});
|
||||
|
||||
it("skipped items keep the result valid against headerSchema", async () => {
|
||||
const stub = {
|
||||
findGlobal: vi.fn(async () => ({
|
||||
items: [
|
||||
{ label: "", href: "", external: false },
|
||||
{ label: "Home", href: "/", external: false },
|
||||
],
|
||||
})),
|
||||
};
|
||||
const { getPayload } = await import("payload");
|
||||
(getPayload as ReturnType<typeof vi.fn>).mockResolvedValue(stub);
|
||||
|
||||
const repo = new HeaderRepository(stubPayloadConfig);
|
||||
const header = await repo.getHeader();
|
||||
|
||||
expect(() => headerSchema.parse(header)).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -58,11 +58,20 @@ export class HeaderRepository implements IHeaderRepository {
|
||||
? String(doc.logo)
|
||||
: undefined;
|
||||
|
||||
const items: HeaderItem[] = (doc.items ?? []).map((item) => ({
|
||||
label: item.label ?? "",
|
||||
href: item.href ?? "",
|
||||
// headerItemSchema requires non-empty label + href — skip
|
||||
// incomplete CMS rows instead of emitting invalid items that
|
||||
// would fail output validation downstream.
|
||||
const items: HeaderItem[] = (doc.items ?? []).flatMap((item) =>
|
||||
item.label && item.href
|
||||
? [
|
||||
{
|
||||
label: item.label,
|
||||
href: item.href,
|
||||
external: item.external ?? false,
|
||||
}));
|
||||
},
|
||||
]
|
||||
: [],
|
||||
);
|
||||
|
||||
span.setAttribute("itemCount", items.length);
|
||||
return { logoId, items };
|
||||
@@ -70,7 +79,10 @@ export class HeaderRepository implements IHeaderRepository {
|
||||
this.logger.captureException(err, {
|
||||
tags: { feature: FEATURE, repo: REPO, method: "getHeader" },
|
||||
});
|
||||
span.setStatus("error", err instanceof Error ? err.message : String(err));
|
||||
span.setStatus(
|
||||
"error",
|
||||
err instanceof Error ? err.message : String(err),
|
||||
);
|
||||
throw err;
|
||||
}
|
||||
},
|
||||
|
||||
@@ -58,7 +58,7 @@ describe("navigationRouter error mapping", () => {
|
||||
@injectable()
|
||||
class NullHeaderRepository {
|
||||
async getHeader() {
|
||||
return null as never;
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user