Skip to content

fix: correctly fail geocoding jersey postcodes #273

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.

Sign up for GitHub

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

Merged
joaquimds merged 1 commit into from
Jan 15, 2026
Merged

fix: correctly fail geocoding jersey postcodes #273

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 4 additions & 1 deletion src/server/mapping/geocode.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,10 @@ const geocodeRecordByPostcode = async (
!(typeof postcodesData.result === "object") ||
!("postcode" in postcodesData.result) ||
!("latitude" in postcodesData.result) ||
!("longitude" in postcodesData.result)
!("longitude" in postcodesData.result) ||
!postcodesData.result.postcode ||
!postcodesData.result.latitude ||
!postcodesData.result.longitude
Comment on lines +96 to +97
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

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

These checks will fail for valid latitude/longitude values of 0. For example, postcodes on the Prime Meridian have longitude 0, and postcodes on the Equator have latitude 0. Consider using explicit null/undefined checks instead: postcodesData.result.latitude == null || postcodesData.result.longitude == null.

Suggested change
!postcodesData.result.latitude ||
!postcodesData.result.longitude
postcodesData.result.latitude == null ||
postcodesData.result.longitude == null

Copilot uses AI. Check for mistakes.
) {
throw new Error(
`Bad postcodes.io response: ${JSON.stringify(postcodesData)}`,
Expand Down
9 changes: 9 additions & 0 deletions tests/feature/importDataSource.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,15 @@ describe("importDataSource tests", () => {
Postcode: "BT15 3ES",
},
},
{
externalId: "6",
geocodePoint: null,
geocodeResult: null,
json: {
Name: "Mysterion",
Postcode: "JE3 2DF",
},
},
]);
});

Expand Down
13 changes: 7 additions & 6 deletions tests/resources/members.csv
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
Name,Postcode
John,TN4 0PP
Paul,N76AS
George,G115RD
Ringo,HP20 2QB
Jane,BT15 3ES
Name,Postcode,Notes
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

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

The test expectations in tests/feature/importDataSource.test.ts still expect only 5 records (lines 81-171), but adding a 7th row to members.csv means the test will now import 6 records. The test expectations need to be updated to handle the new Jersey postcode record, or the test should explicitly verify that the Jersey record is excluded due to geocoding failure.

Copilot uses AI. Check for mistakes.
John,TN4 0PP,
Paul,N76AS,
George,G115RD,
Ringo,HP20 2QB,
Jane,BT15 3ES,Test Belfast
Mysterion,JE3 2DF, Test Jersey (currently does not geocode as is missing in the database and postcodes.io)
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

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

There is an extra space after the comma before 'Test Jersey'. This should be 'JE3 2DF,Test Jersey' to be consistent with line 6 where 'BT15 3ES,Test Belfast' has no space after the comma.

Copilot uses AI. Check for mistakes.