fix(features): address Task 3 code review feedback
- Add navItemFactory to navigation (spec §5.1 — was missing) - Refactor blog/router.test.ts to use articleFactory (eliminate new Date()) - headerFactory uses sequence for logoId (deterministic buildList output) - Align media/tsconfig.json with other features (jsx + tests/ include) - Refactor auth/container.test.ts to use userFactory Reviewer: superpowers:code-reviewer (Task 3 of Plan 7). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -6,6 +6,7 @@ import { MockUsersRepository } from "@/infrastructure/repositories/mock-users.re
|
||||
import { MockAuthenticationService } from "@/infrastructure/services/mock-authentication.service";
|
||||
import type { IUsersRepository } from "@/application/repositories/users-repository.interface";
|
||||
import type { IAuthenticationService } from "@/application/services/authentication-service.interface";
|
||||
import { userFactory } from "@/__factories__/user.factory";
|
||||
|
||||
describe("authContainer", () => {
|
||||
beforeEach(() => {
|
||||
@@ -36,11 +37,8 @@ describe("authContainer", () => {
|
||||
AUTH_SYMBOLS.IAuthenticationService,
|
||||
);
|
||||
// The service should be able to validate against the seeded users
|
||||
const { session, cookie } = await service.createSession({
|
||||
id: "1",
|
||||
username: "alice",
|
||||
passwordHash: "hashed_password_alice",
|
||||
});
|
||||
const user = userFactory.build({ id: "1", username: "alice", passwordHash: "hashed_password_alice" });
|
||||
const { session, cookie } = await service.createSession(user);
|
||||
expect(session.userId).toBe("1");
|
||||
expect(cookie.value).toBe(session.id);
|
||||
|
||||
|
||||
@@ -4,6 +4,7 @@ import { BLOG_SYMBOLS } from "../../di/symbols";
|
||||
import { MockArticlesRepository } from "../../infrastructure/repositories/mock-articles.repository";
|
||||
import type { IArticlesRepository } from "../../application/repositories/articles-repository.interface";
|
||||
import { blogRouter } from "./router";
|
||||
import { articleFactory } from "../../__factories__/article.factory.js";
|
||||
|
||||
describe("blogRouter", () => {
|
||||
let repo: MockArticlesRepository;
|
||||
@@ -26,17 +27,9 @@ describe("blogRouter", () => {
|
||||
});
|
||||
|
||||
it("articleBySlug returns the article when present", async () => {
|
||||
const now = new Date();
|
||||
await repo.createArticle({
|
||||
id: "1",
|
||||
title: "T",
|
||||
slug: "t",
|
||||
content: null,
|
||||
status: "draft",
|
||||
authorId: "u1",
|
||||
createdAt: now,
|
||||
updatedAt: now,
|
||||
});
|
||||
await repo.createArticle(
|
||||
articleFactory.build({ id: "1", title: "T", slug: "t", authorId: "u1" }),
|
||||
);
|
||||
|
||||
const caller = blogRouter.createCaller({});
|
||||
const result = await caller.articleBySlug({ slug: "t" });
|
||||
|
||||
@@ -4,10 +4,11 @@
|
||||
"outDir": "dist",
|
||||
"rootDir": ".",
|
||||
"lib": ["ES2022", "DOM"],
|
||||
"jsx": "preserve",
|
||||
"paths": {
|
||||
"@/*": ["./src/*"]
|
||||
}
|
||||
},
|
||||
"include": ["src/**/*"],
|
||||
"include": ["src/**/*", "tests/**/*"],
|
||||
"exclude": ["node_modules", "dist"]
|
||||
}
|
||||
|
||||
@@ -7,7 +7,7 @@ describe("headerFactory", () => {
|
||||
it("returns a Header with stable defaults", () => {
|
||||
const h = headerFactory.build();
|
||||
expect(h.items).toHaveLength(0);
|
||||
expect(h.logoId).toBeUndefined();
|
||||
expect(h.logoId).toBe("logo-1");
|
||||
});
|
||||
|
||||
it("applies overrides", () => {
|
||||
@@ -26,4 +26,11 @@ describe("headerFactory", () => {
|
||||
expect(a.items).toHaveLength(1);
|
||||
expect(b.items).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("logoId reflects sequence for deterministic buildList output", () => {
|
||||
const first = headerFactory.build();
|
||||
const second = headerFactory.build();
|
||||
expect(first.logoId).toBe("logo-1");
|
||||
expect(second.logoId).toBe("logo-2");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { defineFactory } from "@repo/core-testing/factory";
|
||||
import type { Header } from "../entities/header.js";
|
||||
|
||||
export const headerFactory = defineFactory<Header>(() => ({
|
||||
export const headerFactory = defineFactory<Header>(({ sequence }) => ({
|
||||
logoId: `logo-${sequence}`,
|
||||
items: [],
|
||||
}));
|
||||
|
||||
@@ -1 +1,2 @@
|
||||
export { headerFactory } from "./header.factory.js";
|
||||
export { navItemFactory } from "./nav-item.factory.js";
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
import { describe, it, expect, beforeEach } from "vitest";
|
||||
import { navItemFactory } from "@/__factories__/nav-item.factory";
|
||||
|
||||
describe("navItemFactory", () => {
|
||||
beforeEach(() => navItemFactory.reset());
|
||||
|
||||
it("returns a HeaderItem with stable defaults", () => {
|
||||
const item = navItemFactory.build();
|
||||
expect(item.label).toBe("Item 1");
|
||||
expect(item.href).toBe("/item-1");
|
||||
expect(item.external).toBe(false);
|
||||
});
|
||||
|
||||
it("applies overrides", () => {
|
||||
const item = navItemFactory.build({ label: "Home", href: "/", external: true });
|
||||
expect(item.label).toBe("Home");
|
||||
expect(item.href).toBe("/");
|
||||
expect(item.external).toBe(true);
|
||||
});
|
||||
|
||||
it("increments sequence per build", () => {
|
||||
const a = navItemFactory.build();
|
||||
const b = navItemFactory.build();
|
||||
expect(a.label).toBe("Item 1");
|
||||
expect(b.label).toBe("Item 2");
|
||||
expect(a.href).toBe("/item-1");
|
||||
expect(b.href).toBe("/item-2");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,8 @@
|
||||
import { defineFactory } from "@repo/core-testing/factory";
|
||||
import type { HeaderItem } from "../entities/header.js";
|
||||
|
||||
export const navItemFactory = defineFactory<HeaderItem>(({ sequence }) => ({
|
||||
label: `Item ${sequence}`,
|
||||
href: `/item-${sequence}`,
|
||||
external: false,
|
||||
}));
|
||||
Reference in New Issue
Block a user