Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Ensure input postcode is always parsed #229

Merged
merged 4 commits into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 35 additions & 12 deletions app/api/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { POST } from "../api/route";
import * as calculationService from "../services/calculationService";
import calculateFairhold from "../models/testClasses";
import { NextResponse } from "next/server";
import { calculationSchema, Calculation } from "../schemas/calculationSchema";

// Mock dependencies
jest.mock("../services/calculationService");
Expand All @@ -18,7 +17,7 @@ const callResponse = (res: unknown) => {
};

describe("POST API Route", () => {
const mockRequest = (data: Calculation | string) => ({
const mockRequest = (data: unknown) => ({
json: jest.fn().mockResolvedValueOnce(data),
});

Expand All @@ -27,14 +26,14 @@ describe("POST API Route", () => {
});

it("should return processed data for valid apiSchema input", async () => {
const validApiInput = calculationSchema.parse({
const validApiInput = {
housePostcode: "SE17 1PE",
houseSize: 100,
houseAge: 3,
houseBedrooms: 2,
houseType: "D",
maintenancePercentage: 0.02,
});
};

const householdData = {
/* mock household data */
Expand All @@ -54,9 +53,21 @@ describe("POST API Route", () => {
const res = await POST(req as unknown as Request);

// Assertions
expect(calculationService.getHouseholdData).toHaveBeenCalledWith(
validApiInput
);
expect(calculationService.getHouseholdData).toHaveBeenCalledWith({
...validApiInput,
// Parsed postcode object
housePostcode: {
area: "SE",
district: "SE17",
incode: "1PE",
outcode: "SE17",
postcode: "SE17 1PE",
sector: "SE17 1",
subDistrict: null,
unit: "PE",
valid: true,
},
});
expect(calculateFairhold).toHaveBeenCalledWith(householdData);
expect(res).toEqual(NextResponse.json(processedData));
});
Expand All @@ -77,14 +88,14 @@ describe("POST API Route", () => {
});

it("should handle service errors", async () => {
const validApiInput = calculationSchema.parse({
const validApiInput = {
housePostcode: "SE17 1PE",
houseSize: 100,
houseAge: 3,
houseBedrooms: 2,
houseType: "D",
maintenancePercentage: 0.02,
});
};

const errorMessage = "Service error";

Expand All @@ -98,9 +109,21 @@ describe("POST API Route", () => {
callResponse(res);

// Assertions
expect(calculationService.getHouseholdData).toHaveBeenCalledWith(
validApiInput
);
expect(calculationService.getHouseholdData).toHaveBeenCalledWith({
...validApiInput,
// Parsed postcode object
housePostcode: {
area: "SE",
district: "SE17",
incode: "1PE",
outcode: "SE17",
postcode: "SE17 1PE",
sector: "SE17 1",
subDistrict: null,
unit: "PE",
valid: true,
},
});
expect(NextResponse.json).toHaveBeenCalledWith(
{ error: errorMessage },
{ status: 500 }
Expand Down
8 changes: 1 addition & 7 deletions app/api/route.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { NextResponse } from "next/server";
import { api, apiSchema } from "../schemas/apiSchema";
import { calculationSchema } from "../schemas/calculationSchema";
import * as calculationService from "../services/calculationService";
import calculateFairhold from "../models/testClasses";
Expand All @@ -8,12 +7,7 @@ export async function POST(req: Request) {
try {
// Parse and validate user input
const data = await req.json();
let input: api;
if (!apiSchema.safeParse(data).success) {
input = calculationSchema.parse(data);
} else {
input = data;
}
const input = calculationSchema.parse(data);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Main change in this PR - falling back to parsing the input.

const householdData = await calculationService.getHouseholdData(input);
const processedData = calculateFairhold(householdData);
return NextResponse.json(processedData);
Expand Down
3 changes: 1 addition & 2 deletions app/data/itlRepo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ const getItl3ByPostcodeDistrict = async (
},
});

// Cast to string as 'not: null' clause in Prisma query does not type narrow
return itl3 as string;
return itl3;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small unrelated tidy up sorry - this column in now non-nullable.

} catch (error) {
throw new Error(
`Data error: Unable get get itl3 for postcode district ${postcodeDistrict}`
Expand Down
2 changes: 1 addition & 1 deletion app/models/testClasses.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ValidPostcode } from "../schemas/apiSchema";
import { ValidPostcode } from './../schemas/calculationSchema';
import { createForecastParameters } from "./ForecastParameters";
import { Household } from "./Household";
import { HouseType, MaintenancePercentage, Property } from "./Property";
Expand Down
30 changes: 0 additions & 30 deletions app/schemas/apiSchema.ts

This file was deleted.

9 changes: 6 additions & 3 deletions app/schemas/calculationSchema.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import { z } from "zod";
import { parse as parsePostcode, fix as fixPostcode } from "postcode";
import {
parse as parsePostcode,
isValid as isValidPostcode,
} from "postcode";
import { HOUSE_TYPES } from "../models/Property";
import { MAINTENANCE_LEVELS } from "../models/constants";

// Type not exported by postcode lib directly
type ValidPostcode = Extract<ReturnType<typeof parsePostcode>, { valid: true }>;
export type ValidPostcode = Extract<ReturnType<typeof parsePostcode>, { valid: true }>;

const HouseTypeEnum = z.enum(HOUSE_TYPES);

Expand All @@ -23,7 +26,7 @@ export const calculationSchema = z.object({
housePostcode: z
.string()
.min(1, "housePostcode is required")
.refine(fixPostcode, "Invalid postcode")
.refine(isValidPostcode, "Invalid postcode")
.transform(parsePostcode)
.refine((postcode): postcode is ValidPostcode => postcode.valid),
houseSize: z.coerce.number().positive("houseSize must be a positive integer"),
Expand Down
14 changes: 7 additions & 7 deletions app/schemas/formSchema.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { z } from "zod";
import { fix as fixPostcode } from "postcode";
import { isValid as isValidPostcode } from "postcode";
import { HOUSE_TYPES } from "../models/Property";
import { maintenancePercentageSchema } from "../schemas/calculationSchema";

Expand All @@ -11,17 +11,17 @@ const HouseTypeEnum = z.enum(HOUSE_TYPES);
export const formSchema = z.object({
housePostcode: z
.string()
.min(1, "housePostcode is required")
.refine(fixPostcode, "Invalid postcode"),
houseSize: z.coerce.number().positive("houseSize must be a positive integer"),
houseAge: z.coerce.number().positive("houseAge must be a positive integer"),
.min(1, "Postcode is required")
.refine(isValidPostcode, "Invalid postcode"),
houseSize: z.coerce.number().positive("House size must be a positive number"),
houseAge: z.coerce.number().positive("House age must be a positive number"),
houseBedrooms: z.coerce
.number()
.positive("houseBedrooms must be a positive integer"),
.positive("House bedrooms must be a positive number"),
houseType: HouseTypeEnum.refine(
(value) => HouseTypeEnum.options.includes(value),
{
message: `houseType is required and must be one of ${HOUSE_TYPES}`,
message: `House type is required`,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error messages here are using facing and more a bit more user-friendly.

}
),
maintenancePercentage: maintenancePercentageSchema,
Expand Down
2 changes: 1 addition & 1 deletion app/services/calculationService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { socialRentAdjustmentsService } from "./socialRentAdjustmentsService";
import { socialRentEarningsService } from "./socialRentEarningsService";
import { rentService } from "./rentService";
import { parse } from "postcode";
import { ValidPostcode } from "../schemas/apiSchema";
import { ValidPostcode } from "../schemas/calculationSchema";

jest.mock("./itlService");
jest.mock("./gdhiService");
Expand Down
Loading