refactor: deduplicate formatters and improve test mocks (#81)

- Consolidate duplicate date formatting utilities
- Use shared formatters across backend and frontend
- Clean up test mocks to use consistent test data
- Remove redundant formatting functions
This commit is contained in:
Daniel Volz
2026-01-30 18:37:24 +01:00
committed by GitHub
parent fcd1b79c56
commit aed0b20875
13 changed files with 568 additions and 888 deletions
+5 -3
View File
@@ -4,6 +4,7 @@
*/
import { useTranslation } from "react-i18next";
import type { FieldErrors, FormBlister, FormState, Medication } from "../types";
import { deriveTotal } from "../utils";
// Field limits for validation
const FIELD_LIMITS = {
@@ -53,12 +54,13 @@ export interface MobileEditModalProps {
onSaveMedication: (e: React.FormEvent) => void;
}
function deriveTotal(form: FormState) {
/** Calculate total pills from form state */
function deriveTotalFromForm(form: FormState) {
const packCount = Number(form.packCount) || 0;
const blistersPerPack = Number(form.blistersPerPack) || 0;
const pillsPerBlister = Number(form.pillsPerBlister) || 1;
const looseTablets = Number(form.looseTablets) || 0;
return packCount * blistersPerPack * pillsPerBlister + looseTablets;
return deriveTotal(packCount, blistersPerPack, pillsPerBlister, looseTablets);
}
export function MobileEditModal({
@@ -216,7 +218,7 @@ export function MobileEditModal({
</label>
<div className="full">
<p className="sub">
<strong>{t("form.total")}:</strong> {deriveTotal(form)} {t("common.pills")}
<strong>{t("form.total")}:</strong> {deriveTotalFromForm(form)} {t("common.pills")}
</p>
</div>
<label className="full">
@@ -14,6 +14,15 @@ vi.mock("react-router-dom", async () => {
};
});
// Mock useUnsavedChanges
vi.mock("../../context", () => ({
useUnsavedChanges: () => ({
setHasUnsavedChanges: vi.fn(),
hasUnsavedChanges: false,
confirmNavigation: vi.fn().mockReturnValue(true),
}),
}));
describe("AppHeader", () => {
beforeEach(() => {
vi.clearAllMocks();
@@ -99,8 +99,7 @@ describe("MobileEditModal", () => {
it("calls onClose when close button clicked", () => {
const onClose = vi.fn();
const onResetForm = vi.fn();
render(<MobileEditModal {...defaultProps} onClose={onClose} onResetForm={onResetForm} />);
render(<MobileEditModal {...defaultProps} onClose={onClose} />);
const closeBtn = document.querySelector(".modal-close");
if (closeBtn) {
@@ -108,7 +107,6 @@ describe("MobileEditModal", () => {
}
expect(onClose).toHaveBeenCalledTimes(1);
expect(onResetForm).toHaveBeenCalledTimes(1);
});
it("renders form element", () => {
@@ -310,8 +308,9 @@ describe("MobileEditModal form submission", () => {
it("calls onSaveMedication when form submitted", () => {
const onSaveMedication = vi.fn((e: Event) => e.preventDefault());
const validForm = { ...defaultForm, name: "TestMed" };
render(<MobileEditModal {...defaultProps} onSaveMedication={onSaveMedication} />);
render(<MobileEditModal {...defaultProps} form={validForm} onSaveMedication={onSaveMedication} />);
const form = document.querySelector("form");
if (form) {
File diff suppressed because it is too large Load Diff
@@ -42,7 +42,7 @@ describe("useMedications", () => {
expect(result.current.meds).toEqual(mockMeds);
});
expect(fetch).toHaveBeenCalledWith("/api/medications");
expect(fetch).toHaveBeenCalledWith("/api/medications", { credentials: "include" });
});
it("handles API error gracefully", async () => {
@@ -104,7 +104,7 @@ describe("useMedications", () => {
await result.current.deleteMed(1, 1, mockResetForm);
});
expect(fetch).toHaveBeenCalledWith("/api/medications/1", { method: "DELETE" });
expect(fetch).toHaveBeenCalledWith("/api/medications/1", { method: "DELETE", credentials: "include" });
expect(mockResetForm).toHaveBeenCalled();
});
@@ -168,7 +168,7 @@ describe("useMedications", () => {
await result.current.deleteMedImage(1);
});
expect(fetch).toHaveBeenCalledWith("/api/medications/1/image", { method: "DELETE" });
expect(fetch).toHaveBeenCalledWith("/api/medications/1/image", { method: "DELETE", credentials: "include" });
});
it("allows setting meds directly", () => {
@@ -482,6 +482,17 @@ describe("DashboardPage with medications", () => {
});
it("renders schedule timeline with future doses", () => {
// Need showFutureDays: true for day-blocks to render
mockContextValue = createMockAppContext({
meds: mockMeds,
coverage: mockCoverage,
coverageByMed: {
Aspirin: mockCoverage.all[0],
},
futureDays: mockFutureDays,
showFutureDays: true,
});
render(
<MemoryRouter>
<DashboardPage />
@@ -548,6 +559,7 @@ describe("DashboardPage with email notifications", () => {
settings: {
...createMockAppContext().settings,
emailEnabled: true,
emailStockReminders: true,
notificationEmail: "test@example.com",
},
});
@@ -587,6 +599,7 @@ describe("DashboardPage with shoutrrr notifications", () => {
settings: {
...createMockAppContext().settings,
shoutrrrEnabled: true,
shoutrrrStockReminders: true,
},
});
});
@@ -110,11 +110,17 @@ let mockFormHookValue = createMockFormHook();
// Mock the hooks
vi.mock("../../hooks", () => ({
useMedicationForm: () => mockFormHookValue,
useUnsavedChangesWarning: () => ({}),
}));
// Mock the context
vi.mock("../../context", () => ({
useAppContext: () => mockContextValue,
useUnsavedChanges: () => ({
setHasUnsavedChanges: vi.fn(),
hasUnsavedChanges: false,
confirmNavigation: vi.fn().mockReturnValue(true),
}),
}));
describe("MedicationsPage", () => {
@@ -5,9 +5,7 @@ import {
compareSemver,
deriveTotal,
formatDateTime,
formatFullBlisters,
formatNumber,
formatOpenBlisterAndLoose,
getBlisterStock,
getExpiryClass,
pad2,
@@ -227,22 +225,6 @@ describe("getBlisterStock", () => {
});
});
describe("formatFullBlisters", () => {
it("formats count without pill info", () => {
expect(formatFullBlisters({ fullBlisters: 5, openBlisterPills: 3, loosePills: 3 })).toBe("5");
});
it("formats count with pill info", () => {
expect(formatFullBlisters({ fullBlisters: 5, openBlisterPills: 3, loosePills: 3 }, 10)).toBe("5 (50)");
});
});
describe("formatOpenBlisterAndLoose", () => {
it("formats open blister pills count", () => {
expect(formatOpenBlisterAndLoose({ fullBlisters: 5, openBlisterPills: 7, loosePills: 7 })).toBe("7");
});
});
describe("compareSemver", () => {
it("returns 0 for equal versions", () => {
expect(compareSemver("1.2.3", "1.2.3")).toBe(0);
-18
View File
@@ -232,24 +232,6 @@ export function getBlisterStock(med: Medication): BlisterStock {
return { fullBlisters, openBlisterPills, loosePills: openBlisterPills };
}
/**
* Format full blisters count with optional pills per blister
*/
export function formatFullBlisters(stock: BlisterStock, pillsPerBlister?: number): string {
const count = stock.fullBlisters;
if (pillsPerBlister !== undefined) {
return `${count} (${count * pillsPerBlister})`;
}
return String(count);
}
/**
* Format open blister and loose pills
*/
export function formatOpenBlisterAndLoose(stock: BlisterStock): string {
return String(stock.openBlisterPills);
}
/**
* Compare semantic version strings
* Returns: negative if a < b, positive if a > b, 0 if equal