updates on the changes on warning and lat and long fixed to make it good
This commit is contained in:
@@ -10,6 +10,7 @@
|
||||
// so a test that passes is a statement about what the page does, not about a
|
||||
// copy of it that happens to agree today.
|
||||
|
||||
import { latitude, longitude } from '@/lib/coords';
|
||||
import { PICKUP_SOURCE, buildFlowFields, flowForDraft } from '@/lib/orderFlow';
|
||||
|
||||
export const BULK_SERVICE_OPTION = 'Normal';
|
||||
@@ -83,15 +84,40 @@ export const buildBulkBookingPayload = ({
|
||||
pickupaddress: origin.address || '',
|
||||
pickuppincode: String(origin.pincode || origin.postcode || ''),
|
||||
pickupcity: origin.city || '',
|
||||
pickuplatitude: Number(origin.latitude) || 0,
|
||||
pickuplongitude: Number(origin.longitude) || 0,
|
||||
// A coordinate we do not have is sent as null, never as 0.
|
||||
//
|
||||
// `Number(x) || 0` turned every missing pin into (0, 0), and the backend
|
||||
// cannot tell that apart from a deliberate pin in the Gulf of Guinea. For
|
||||
// the PICKUP end it currently recovers — adminController falls back to the
|
||||
// hub or tenant location when pickup is 0,0 — but relying on that means
|
||||
// relying on a rescue for data we should not have sent.
|
||||
pickuplatitude: latitude(origin.latitude),
|
||||
pickuplongitude: longitude(origin.longitude),
|
||||
customer_phone: row?.contactno != null ? String(row.contactno) : '',
|
||||
customer_name: row?.firstname || '',
|
||||
deliveryaddress: row?.address || '',
|
||||
deliverypincode: row?.postcode != null ? String(row.postcode) : '',
|
||||
deliverycity: row?.city || '',
|
||||
deliverylatitude: Number(row?.latitude) || 0,
|
||||
deliverylongitude: Number(row?.longitude) || 0,
|
||||
// The DELIVERY end has no rescue, which is where the money went. With
|
||||
// delivery at 0,0 the backend skips its distance calculation entirely
|
||||
// (`if booking.Deliverylatitude != 0 && ...`) and prices the order at base
|
||||
// rate with no distance component — or, when the console did manage to
|
||||
// quote a charge from those same 0,0 coordinates, bills an 11,000 km run.
|
||||
// One defect, opposite errors.
|
||||
//
|
||||
// Be clear about what null buys and what it does not. It does NOT make the
|
||||
// server refuse: adminController declares these as `float64`, not
|
||||
// `*float64`, and Go unmarshals JSON null into a float64 as 0 with no
|
||||
// error — verified, not assumed. So by the time a null reaches the handler
|
||||
// it is indistinguishable from the 0 this used to send.
|
||||
//
|
||||
// The submission guard in MultipleOrders is therefore the actual fix; this
|
||||
// is defence in depth and an honest local shape. It stops the console
|
||||
// itself from treating a missing pin as a position — the distance
|
||||
// calculation and the zone filters read these values too — and it means a
|
||||
// future `*float64` on the server side starts working with no change here.
|
||||
deliverylatitude: latitude(row?.latitude),
|
||||
deliverylongitude: longitude(row?.longitude),
|
||||
deliverytime,
|
||||
service_option: BULK_SERVICE_OPTION,
|
||||
finalprice: Number(row?.totalcharge) || 0,
|
||||
|
||||
116
src/lib/coords.js
Normal file
116
src/lib/coords.js
Normal file
@@ -0,0 +1,116 @@
|
||||
// ==============================|| Coordinates ||============================== //
|
||||
//
|
||||
// One place that decides whether a latitude/longitude is real.
|
||||
//
|
||||
// ── Why this file exists ──
|
||||
//
|
||||
// The console validated coordinates with `Number.isFinite(Number(value))` in
|
||||
// about twenty places, and that guard does not do what it looks like it does:
|
||||
//
|
||||
// Number(null) === 0 → finite → passes
|
||||
// Number('') === 0 → finite → passes
|
||||
// Number(false) === 0 → finite → passes
|
||||
//
|
||||
// A missing coordinate therefore became latitude 0, longitude 0 — a point in
|
||||
// the Gulf of Guinea — and every downstream consumer treated it as a real
|
||||
// place. The cost was not a blank field, it was a wrong price: OSRM refuses to
|
||||
// route to open ocean, `calculateDrivingRoute` falls back to Haversine, and a
|
||||
// Coimbatore pickup measured against 0,0 comes out at roughly 11,000 km. That
|
||||
// distance reaches `calculateTotalCharge`, becomes the row's `finalprice`, and
|
||||
// the backend honours it verbatim (`adminController.go`: `if req.Finalprice > 0`).
|
||||
// The same defect bills the opposite way when the console fails to quote at
|
||||
// all: the backend then measures for itself, skips the calculation because
|
||||
// delivery is 0,0, and charges base price with no distance component.
|
||||
//
|
||||
// So a zero coordinate is rejected here rather than merely reported. On Earth
|
||||
// (0, 0) is a valid point; in this product it is only ever the residue of a
|
||||
// failed geocode, and treating it as data has never once been correct.
|
||||
//
|
||||
// Range is checked too, because nothing in the console checked it before. It
|
||||
// is worth being precise about what that does and does not buy, since the
|
||||
// axes genuinely do swap inside this codebase — OSRM takes lon,lat while
|
||||
// Leaflet takes lat,lng — so a transposed call site is a live risk:
|
||||
//
|
||||
// Caught: a longitude past 90 read as a latitude. Dibrugarh (27.4728,
|
||||
// 94.9120) transposed gives latitude 94.9, past the pole.
|
||||
// NOT caught: anything west of 90°E. Coimbatore is (11.0168, 76.9558), and
|
||||
// 76.9558 is a legal latitude — northern Siberia — so a
|
||||
// transposed Coimbatore pin satisfies every rule here.
|
||||
//
|
||||
// Detecting that second case needs a plausibility check against the operating
|
||||
// region rather than a bounds check; `geocodingService.isWithinIndia` is the
|
||||
// right tool and is deliberately not folded in here, because this module is
|
||||
// about whether a number is a coordinate at all, not about where we operate.
|
||||
//
|
||||
// The logic is lifted from `hubForm.js`, which has always got this right. It
|
||||
// was correct in one form and nowhere else; now it is correct in one place.
|
||||
|
||||
/** Latitude is bounded at the poles, longitude at the antimeridian. */
|
||||
const BOUNDS = Object.freeze({ lat: 90, lng: 180 });
|
||||
|
||||
/**
|
||||
* One coordinate, or null when there isn't one.
|
||||
*
|
||||
* @param {unknown} value raw cell, form field, or API field
|
||||
* @param {'lat'|'lng'} axis which bound to apply
|
||||
* @returns {number|null} a usable coordinate, or null
|
||||
*/
|
||||
export const coord = (value, axis) => {
|
||||
if (value === null || value === undefined || value === '') return null;
|
||||
// A boolean coerces to 0/1 and an empty array to 0. Neither is a coordinate,
|
||||
// and both reach here from spreadsheet cells.
|
||||
if (typeof value === 'boolean') return null;
|
||||
|
||||
const n = Number(value);
|
||||
if (!Number.isFinite(n)) return null;
|
||||
if (n === 0) return null;
|
||||
|
||||
const max = BOUNDS[axis];
|
||||
if (max === undefined) return null;
|
||||
return n < -max || n > max ? null : n;
|
||||
};
|
||||
|
||||
export const latitude = (value) => coord(value, 'lat');
|
||||
export const longitude = (value) => coord(value, 'lng');
|
||||
|
||||
/**
|
||||
* True when both halves of a pair are usable.
|
||||
*
|
||||
* Both or neither: a point with one axis is not half a location, it is no
|
||||
* location, and passing it on lets the missing half default to 0 somewhere
|
||||
* further downstream — which is the bug this module exists to end.
|
||||
*/
|
||||
export const hasCoords = (lat, lng) =>
|
||||
coord(lat, 'lat') !== null && coord(lng, 'lng') !== null;
|
||||
|
||||
/**
|
||||
* A `{ latitude, longitude }` pair, or null.
|
||||
*
|
||||
* Reads the shapes this codebase actually passes around — `{latitude,longitude}`
|
||||
* from the API and the forms, `{lat,lng}` from Leaflet and the map pickers —
|
||||
* so callers stop re-deriving which spelling they were handed.
|
||||
*/
|
||||
export const coordsOf = (source) => {
|
||||
if (!source || typeof source !== 'object') return null;
|
||||
const lat = coord(source.latitude ?? source.lat, 'lat');
|
||||
const lng = coord(source.longitude ?? source.lng, 'lng');
|
||||
if (lat === null || lng === null) return null;
|
||||
return { latitude: lat, longitude: lng };
|
||||
};
|
||||
|
||||
/**
|
||||
* The message shown when a coordinate is missing or wrong, or null when it is
|
||||
* fine. Kept next to the rule so the form text and the validation cannot drift.
|
||||
*/
|
||||
export const coordError = (value, axis) => {
|
||||
const axisName = axis === 'lat' ? 'Latitude' : 'Longitude';
|
||||
if (value === null || value === undefined || value === '') {
|
||||
return 'Navigate cannot work without a pin';
|
||||
}
|
||||
const n = Number(value);
|
||||
if (typeof value === 'boolean' || !Number.isFinite(n)) return 'Enter a number';
|
||||
if (n === 0) return 'Navigate cannot work without a pin';
|
||||
const max = BOUNDS[axis];
|
||||
if (n < -max || n > max) return `${axisName} must be between ${-max} and ${max}`;
|
||||
return null;
|
||||
};
|
||||
@@ -4,6 +4,8 @@
|
||||
* and falls back seamlessly to Haversine with a 1.3 road winding multiplier.
|
||||
*/
|
||||
|
||||
import { coordsOf } from '@/lib/coords';
|
||||
|
||||
let lastRouteDurationMin = null;
|
||||
|
||||
export const getLastRouteDurationMin = () => lastRouteDurationMin;
|
||||
@@ -15,15 +17,27 @@ export const getLastRouteDurationMin = () => lastRouteDurationMin;
|
||||
* @returns {Promise<{ distance: number, minutes: number, polyline: Array<[number, number]>, resolved: boolean }>}
|
||||
*/
|
||||
export const calculateDrivingRoute = async (origin, destination) => {
|
||||
const lat1 = Number(origin?.latitude);
|
||||
const lon1 = Number(origin?.longitude);
|
||||
const lat2 = Number(destination?.latitude);
|
||||
const lon2 = Number(destination?.longitude);
|
||||
|
||||
if (!Number.isFinite(lat1) || !Number.isFinite(lon1) || !Number.isFinite(lat2) || !Number.isFinite(lon2)) {
|
||||
// Validated through lib/coords rather than with Number.isFinite.
|
||||
//
|
||||
// The previous guard was `Number.isFinite(Number(value))`, which accepts
|
||||
// null, '' and false — all three coerce to 0, and 0 is finite. A missing pin
|
||||
// therefore reached OSRM as (0, 0). OSRM answers HTTP 200 with no `routes`
|
||||
// for a point in open ocean, so control fell through to the Haversine
|
||||
// fallback below and a Coimbatore pickup measured against 0,0 returned
|
||||
// roughly 11,000 km. That number becomes the row's charge and the backend
|
||||
// honours it verbatim, so the guard being wrong here was a billing defect,
|
||||
// not a display one.
|
||||
const from = coordsOf(origin);
|
||||
const to = coordsOf(destination);
|
||||
if (!from || !to) {
|
||||
throw new Error("Invalid coordinates");
|
||||
}
|
||||
|
||||
const lat1 = from.latitude;
|
||||
const lon1 = from.longitude;
|
||||
const lat2 = to.latitude;
|
||||
const lon2 = to.longitude;
|
||||
|
||||
// 1. Try OSRM API with full GeoJSON geometry for live polyline map preview
|
||||
const osrmBaseUrl =
|
||||
(typeof import.meta !== 'undefined' && import.meta?.env?.VITE_OSRM_URL) ||
|
||||
|
||||
@@ -145,10 +145,25 @@ export const standardizePlace = ({
|
||||
longitude: Number.isFinite(lngNum) ? lngNum : null,
|
||||
provider,
|
||||
raw,
|
||||
// Google-Maps-shaped accessors, for the call sites that still read a place
|
||||
// the way the Places API returned one.
|
||||
//
|
||||
// These return null — not 0 — when there is no coordinate, and that is a
|
||||
// fix rather than a style choice. Returning 0 made this object contradict
|
||||
// itself: `.latitude` above says null while `.geometry.location.lat()`
|
||||
// said 0, so which answer a caller got depended on which spelling it
|
||||
// happened to read. MultipleOrders read the accessor, so a suggestion with
|
||||
// no coordinates became a drop pinned at (0, 0); the pre-submit guard
|
||||
// tested `== null` and waved it through; the distance calculation then
|
||||
// routed Coimbatore to the Gulf of Guinea, fell back to Haversine, and
|
||||
// billed roughly 11,000 km — a number the backend honours verbatim
|
||||
// because the console sent it as `finalprice`.
|
||||
//
|
||||
// null propagates as "no pin", which every guard already handles.
|
||||
geometry: {
|
||||
location: {
|
||||
lat: () => (Number.isFinite(latNum) ? latNum : 0),
|
||||
lng: () => (Number.isFinite(lngNum) ? lngNum : 0)
|
||||
lat: () => (Number.isFinite(latNum) && latNum !== 0 ? latNum : null),
|
||||
lng: () => (Number.isFinite(lngNum) && lngNum !== 0 ? lngNum : null)
|
||||
}
|
||||
},
|
||||
address_components: [
|
||||
|
||||
@@ -32,7 +32,22 @@ import {
|
||||
getAdminTenants,
|
||||
getTenantLocations
|
||||
} from '@/api/doormile/endpoints';
|
||||
import { calculateDrivingRoute, calculateTotalCharge, getLastRouteDurationMin } from '@/lib/distance';
|
||||
// calculateDrivingDistance was used by the deadhead-leg effect below without
|
||||
// ever being imported — a ReferenceError on every customer-pickup order that
|
||||
// had a pinned collection address, thrown synchronously inside the effect so
|
||||
// the `.catch()` on that promise chain could never see it. Neither the build
|
||||
// nor the lint config catches an unbound identifier in a .jsx file, which is
|
||||
// why it survived. Found while auditing the coordinate changes; unrelated to
|
||||
// them.
|
||||
//
|
||||
// getLastRouteDurationMin is gone from this list: it was imported and never
|
||||
// called, which the project's own lint reports as an error. Dropped while
|
||||
// rewriting the statement rather than left behind in it.
|
||||
import {
|
||||
calculateDrivingDistance,
|
||||
calculateDrivingRoute,
|
||||
calculateTotalCharge
|
||||
} from '@/lib/distance';
|
||||
import { saveRecentAddress } from '@/lib/geocodingService';
|
||||
import { useHubs } from '@/lib/doormileHooks';
|
||||
import {
|
||||
|
||||
@@ -37,6 +37,7 @@ import {
|
||||
normalizeHeader,
|
||||
requiredSheetColumns
|
||||
} from '@/lib/bulkOrderColumns';
|
||||
import { coord, hasCoords } from '@/lib/coords';
|
||||
import { calculateDrivingDistance, calculateTotalCharge } from '@/lib/distance';
|
||||
import { useHubs } from '@/lib/doormileHooks';
|
||||
import { PICKUP_SOURCE, buildAnchors } from '@/lib/orderFlow';
|
||||
@@ -465,8 +466,20 @@ export default function MultipleOrders() {
|
||||
});
|
||||
});
|
||||
|
||||
const latitude = place.geometry.location.lat();
|
||||
const longitude = place.geometry.location.lng();
|
||||
// Read the place's own fields first and fall back to the Google-shaped
|
||||
// accessors, matching how line 343 already does it. Reading the accessor
|
||||
// alone was the entry point for the 0,0 defect: geocodingService used to
|
||||
// return 0 from `geometry.location.lat()` for a suggestion with no
|
||||
// coordinates, this assignment stored that 0 verbatim, and the pre-submit
|
||||
// guard's `== null` test could not see it.
|
||||
const latitude = coord(
|
||||
place?.latitude ?? place?.geometry?.location?.lat?.(),
|
||||
'lat'
|
||||
);
|
||||
const longitude = coord(
|
||||
place?.longitude ?? place?.geometry?.location?.lng?.(),
|
||||
'lng'
|
||||
);
|
||||
const address = place.formatted_address || '';
|
||||
|
||||
setDropCust((prev) =>
|
||||
@@ -477,6 +490,20 @@ export default function MultipleOrders() {
|
||||
)
|
||||
);
|
||||
|
||||
// No pin, no quote. Pricing an unpinned row is what produced the wrong
|
||||
// charges: the distance came back from a fallback measurement to 0,0 and
|
||||
// the row carried it as `totalcharge`, which the backend then honours as
|
||||
// `finalprice`. Clearing the charge leaves the row visibly incomplete, and
|
||||
// the submit guard below refuses it.
|
||||
if (latitude === null || longitude === null) {
|
||||
setDropCust((prev) =>
|
||||
prev.map((c) =>
|
||||
c._rowkey === rowkey ? { ...c, distance: null, totalcharge: null } : c
|
||||
)
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
const { roundedDistance, totalcharge } = await calculateDistance({ latitude, longitude });
|
||||
setDropCust((prev) =>
|
||||
@@ -532,7 +559,17 @@ export default function MultipleOrders() {
|
||||
return;
|
||||
}
|
||||
|
||||
const incomplete = dropCust.find((c) => c.latitude == null || c.longitude == null || !c.address);
|
||||
// Checked with hasCoords, not `== null`.
|
||||
//
|
||||
// `== null` matches null and undefined only, so a row carrying latitude 0
|
||||
// — which is what a suggestion with no coordinates used to produce —
|
||||
// passed this guard and was submitted. It then priced either as an
|
||||
// 11,000 km run or, with no charge quoted, at base rate with no distance
|
||||
// component. hasCoords also rejects '' and unparseable sheet cells, which
|
||||
// reach here from an uploaded file.
|
||||
const incomplete = dropCust.find(
|
||||
(c) => !hasCoords(c.latitude, c.longitude) || !c.address
|
||||
);
|
||||
if (incomplete) {
|
||||
OpenToast(
|
||||
`Please search and complete address for ${incomplete.firstname || 'one of the drops'}`,
|
||||
@@ -542,6 +579,31 @@ export default function MultipleOrders() {
|
||||
return;
|
||||
}
|
||||
|
||||
// The collection end, which was never checked here at all. CreateOrder
|
||||
// gained this check already; the bulk form did not, so a hand-typed
|
||||
// pickup that failed to geocode submitted every row in the file against a
|
||||
// pickup the backend has to rescue from the hub record.
|
||||
//
|
||||
// Checked per row through rowPickup, not once against the shared pickup:
|
||||
// a row that resolved its own sender address collects from THAT, and
|
||||
// buildBulkBookingPayload reads the same `row.__pickup || sharedPickup`.
|
||||
// Validating only the shared one would leave a per-row collection point
|
||||
// unchecked, which is the half of the fan-out that varies.
|
||||
const unpinnedPickup = dropCust.find((c) => {
|
||||
const origin = rowPickup(c);
|
||||
return !hasCoords(origin?.latitude, origin?.longitude);
|
||||
});
|
||||
if (unpinnedPickup) {
|
||||
OpenToast(
|
||||
rowPickup(unpinnedPickup) === pickCust
|
||||
? 'The collection address has no coordinates — pick it from the suggestions or drop a map pin'
|
||||
: `The sender address for ${unpinnedPickup.firstname || 'one of the drops'} has no coordinates`,
|
||||
'warning',
|
||||
4000
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
const deliverytime = dayjs(`${startDate} ${selectedSlotTime}`, 'YYYY-MM-DD HH:mm').format(
|
||||
'YYYY-MM-DD HH:mm:ss'
|
||||
);
|
||||
@@ -1048,7 +1110,14 @@ export default function MultipleOrders() {
|
||||
</thead>
|
||||
<tbody className="divide-y divide-slate-100 bg-white">
|
||||
{dropCust.map((row, idx) => {
|
||||
const needsAddress = row.latitude == null || row.longitude == null || !row.address;
|
||||
// Same rule as the submit guard, deliberately. When this
|
||||
// used `== null` and the guard used hasCoords, a row
|
||||
// carrying latitude 0 rendered as complete and was then
|
||||
// refused on dispatch — an error with nothing on screen
|
||||
// pointing at the row that caused it. Both ends ask the
|
||||
// same question now, so an unusable row always shows its
|
||||
// address search.
|
||||
const needsAddress = !hasCoords(row.latitude, row.longitude) || !row.address;
|
||||
return (
|
||||
<tr key={row._rowkey || idx} className="hover:bg-slate-50/70 transition-colors">
|
||||
<td className="py-3 pl-4 pr-2 text-slate-600 font-mono text-xs font-bold">{idx + 1}</td>
|
||||
|
||||
@@ -6,6 +6,8 @@
|
||||
* @param {Object} destination - { latitude, longitude }
|
||||
* @returns {Promise<number>} - Round distance in KM
|
||||
*/
|
||||
import { coordsOf } from '@/lib/coords';
|
||||
|
||||
// Duration from the most recent OSRM route, in minutes, or null when the
|
||||
// Haversine fallback was used (a straight-line estimate has no honest
|
||||
// duration — better to show nothing than a made-up ETA).
|
||||
@@ -14,15 +16,25 @@ let lastRouteDurationMin = null;
|
||||
export const getLastRouteDurationMin = () => lastRouteDurationMin;
|
||||
|
||||
export const calculateDrivingDistance = async (origin, destination) => {
|
||||
const lat1 = origin?.latitude;
|
||||
const lon1 = origin?.longitude;
|
||||
const lat2 = destination?.latitude;
|
||||
const lon2 = destination?.longitude;
|
||||
|
||||
if (lat1 == null || lon1 == null || lat2 == null || lon2 == null) {
|
||||
// Validated through lib/coords rather than with `== null`.
|
||||
//
|
||||
// `== null` matches null and undefined only, so 0, '' and a garbage sheet
|
||||
// cell all passed and reached OSRM as (0, 0). OSRM has no route to open
|
||||
// ocean, so control fell through to the Haversine fallback and a Coimbatore
|
||||
// pickup came back at roughly 11,000 km — which becomes the row's charge and
|
||||
// is honoured verbatim by the backend. Same defect as lib/distance.js; this
|
||||
// is the second live copy of this module (both trees have importers).
|
||||
const from = coordsOf(origin);
|
||||
const to = coordsOf(destination);
|
||||
if (!from || !to) {
|
||||
throw new Error("Invalid coordinates");
|
||||
}
|
||||
|
||||
const lat1 = from.latitude;
|
||||
const lon1 = from.longitude;
|
||||
const lat2 = to.latitude;
|
||||
const lon2 = to.longitude;
|
||||
|
||||
// 1. Try OSRM API (either self-hosted or public demo server for fallback)
|
||||
const osrmBaseUrl = (typeof import.meta !== 'undefined' && import.meta.env?.VITE_OSRM_URL) || (typeof process !== 'undefined' && process.env?.REACT_APP_OSRM_URL) || "https://router.project-osrm.org";
|
||||
try {
|
||||
|
||||
136
tests/lib/bulkOrderPayload.test.js
Normal file
136
tests/lib/bulkOrderPayload.test.js
Normal file
@@ -0,0 +1,136 @@
|
||||
import { buildBulkBookingPayload, buildBulkBookingPayloads } from '@/lib/bulkOrderPayload';
|
||||
|
||||
/**
|
||||
* Coordinates on the bulk booking payload.
|
||||
*
|
||||
* This is the regression guard for the billing defect. The builder used
|
||||
* `Number(value) || 0` on all four coordinates, so a row with no usable pin
|
||||
* was submitted as (0, 0) — and the backend cannot tell that apart from a
|
||||
* deliberate pin in the Gulf of Guinea.
|
||||
*
|
||||
* Both failure modes came from that one substitution:
|
||||
*
|
||||
* • With a charge quoted, the console had measured the distance from those
|
||||
* same 0,0 coordinates. OSRM refuses to route to open ocean, so the
|
||||
* Haversine fallback returned roughly 11,000 km, and adminController
|
||||
* honours a supplied `finalprice` verbatim. Massive overcharge.
|
||||
* • With no charge quoted, the backend measured for itself — and skipped the
|
||||
* calculation entirely, because its guard is
|
||||
* `if booking.Deliverylatitude != 0 && ...`. Base rate, no distance
|
||||
* component. Undercharge.
|
||||
*
|
||||
* A coordinate we do not have is now null, which the server can refuse.
|
||||
*/
|
||||
|
||||
const PICKUP = {
|
||||
address: 'Doormile Base, Peelamedu',
|
||||
pincode: '641004',
|
||||
city: 'Coimbatore',
|
||||
latitude: 11.0168,
|
||||
longitude: 76.9558,
|
||||
};
|
||||
|
||||
const ROW = {
|
||||
firstname: 'Priya',
|
||||
contactno: '9876543210',
|
||||
address: '45 Cross Cut Rd, Gandhipuram',
|
||||
postcode: '641012',
|
||||
city: 'Coimbatore',
|
||||
latitude: 11.0272,
|
||||
longitude: 76.9722,
|
||||
totalcharge: 180,
|
||||
};
|
||||
|
||||
const build = (row = {}, pickup = PICKUP) =>
|
||||
buildBulkBookingPayload({
|
||||
row: { ...ROW, ...row },
|
||||
sharedPickup: pickup,
|
||||
tenantId: 3,
|
||||
tenantLocationId: 7,
|
||||
deliverytime: '2026-09-18 10:00:00',
|
||||
notes: '',
|
||||
anchors: [],
|
||||
});
|
||||
|
||||
describe('buildBulkBookingPayload — coordinates', () => {
|
||||
it('passes real coordinates through untouched', () => {
|
||||
const p = build();
|
||||
expect(p.pickuplatitude).toBe(11.0168);
|
||||
expect(p.pickuplongitude).toBe(76.9558);
|
||||
expect(p.deliverylatitude).toBe(11.0272);
|
||||
expect(p.deliverylongitude).toBe(76.9722);
|
||||
});
|
||||
|
||||
it('sends null, NOT 0, for a delivery pin that is missing', () => {
|
||||
const p = build({ latitude: null, longitude: null });
|
||||
expect(p.deliverylatitude).toBeNull();
|
||||
expect(p.deliverylongitude).toBeNull();
|
||||
// The specific regression: 0 would have been priced as an 11,000 km run.
|
||||
expect(p.deliverylatitude).not.toBe(0);
|
||||
});
|
||||
|
||||
it('sends null for an empty sheet cell', () => {
|
||||
// An untouched Excel cell arrives as '' rather than null, and
|
||||
// `Number('') || 0` is 0.
|
||||
const p = build({ latitude: '', longitude: '' });
|
||||
expect(p.deliverylatitude).toBeNull();
|
||||
expect(p.deliverylongitude).toBeNull();
|
||||
});
|
||||
|
||||
it('sends null for an unparseable sheet cell', () => {
|
||||
const p = build({ latitude: 'N/A', longitude: '--' });
|
||||
expect(p.deliverylatitude).toBeNull();
|
||||
expect(p.deliverylongitude).toBeNull();
|
||||
});
|
||||
|
||||
it('refuses to forward a literal zero it was handed', () => {
|
||||
// A row that already carried 0 — which is what the geocoding shim used to
|
||||
// return for a suggestion with no coordinates — must not be laundered
|
||||
// into a valid-looking payload.
|
||||
const p = build({ latitude: 0, longitude: 0 });
|
||||
expect(p.deliverylatitude).toBeNull();
|
||||
expect(p.deliverylongitude).toBeNull();
|
||||
});
|
||||
|
||||
it('sends null for a coordinate outside its axis range', () => {
|
||||
const p = build({ latitude: 94.912, longitude: 200 });
|
||||
expect(p.deliverylatitude).toBeNull();
|
||||
expect(p.deliverylongitude).toBeNull();
|
||||
});
|
||||
|
||||
it('nulls the pickup end too, rather than relying on the backend rescue', () => {
|
||||
// adminController falls back to the hub or tenant location when pickup is
|
||||
// 0,0. That rescue exists, but sending data that needs rescuing is not the
|
||||
// same as sending correct data.
|
||||
const p = build({}, { ...PICKUP, latitude: null, longitude: undefined });
|
||||
expect(p.pickuplatitude).toBeNull();
|
||||
expect(p.pickuplongitude).toBeNull();
|
||||
});
|
||||
|
||||
it('treats the two ends independently', () => {
|
||||
const p = build({ latitude: null, longitude: null });
|
||||
expect(p.pickuplatitude).toBe(11.0168);
|
||||
expect(p.deliverylatitude).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildBulkBookingPayloads — batch', () => {
|
||||
it('nulls only the rows that are missing a pin', () => {
|
||||
const payloads = buildBulkBookingPayloads({
|
||||
rows: [ROW, { ...ROW, latitude: 0, longitude: 0 }, { ...ROW, latitude: '' }],
|
||||
sharedPickup: PICKUP,
|
||||
tenantId: 3,
|
||||
tenantLocationId: 7,
|
||||
deliverytime: '2026-09-18 10:00:00',
|
||||
notes: '',
|
||||
anchors: [],
|
||||
});
|
||||
|
||||
expect(payloads).toHaveLength(3);
|
||||
expect(payloads[0].deliverylatitude).toBe(11.0272);
|
||||
expect(payloads[1].deliverylatitude).toBeNull();
|
||||
expect(payloads[2].deliverylatitude).toBeNull();
|
||||
// The good row keeps its longitude even though a sibling lost its latitude.
|
||||
expect(payloads[2].deliverylongitude).toBe(76.9722);
|
||||
});
|
||||
});
|
||||
202
tests/lib/coords.test.js
Normal file
202
tests/lib/coords.test.js
Normal file
@@ -0,0 +1,202 @@
|
||||
import {
|
||||
coord,
|
||||
coordError,
|
||||
coordsOf,
|
||||
hasCoords,
|
||||
latitude,
|
||||
longitude,
|
||||
} from '@/lib/coords';
|
||||
|
||||
/**
|
||||
* Coordinate validation.
|
||||
*
|
||||
* The first block is the whole reason this module exists: the guard it replaces
|
||||
* was `Number.isFinite(Number(value))`, and every case below passed it, because
|
||||
* null, '' and false all coerce to 0 and 0 is finite. A missing pin became
|
||||
* latitude 0 / longitude 0 and was priced as an 11,000 km run.
|
||||
*/
|
||||
|
||||
const COIMBATORE = { latitude: 11.0168, longitude: 76.9558 };
|
||||
|
||||
describe('coord', () => {
|
||||
describe('the empty values that used to pass as zero', () => {
|
||||
it.each([
|
||||
['null', null],
|
||||
['undefined', undefined],
|
||||
['empty string', ''],
|
||||
['false', false],
|
||||
['true', true],
|
||||
['an empty array', []],
|
||||
])('rejects %s', (_label, value) => {
|
||||
// Number(value) is 0 for every one of these, and 0 is finite — which is
|
||||
// exactly how they got through before.
|
||||
expect(coord(value, 'lat')).toBeNull();
|
||||
expect(coord(value, 'lng')).toBeNull();
|
||||
});
|
||||
|
||||
it('confirms the coercion that caused the bug', () => {
|
||||
expect(Number(null)).toBe(0);
|
||||
expect(Number.isFinite(Number(null))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('zero', () => {
|
||||
it('rejects a literal zero on both axes', () => {
|
||||
// (0, 0) is a real point in the Gulf of Guinea. In this product it is
|
||||
// only ever the residue of a failed geocode.
|
||||
expect(coord(0, 'lat')).toBeNull();
|
||||
expect(coord(0, 'lng')).toBeNull();
|
||||
expect(coord('0', 'lat')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('unparseable values', () => {
|
||||
it.each(['abc', 'N/A', '--', 'NaN', {}])('rejects %p', (value) => {
|
||||
expect(coord(value, 'lat')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('range', () => {
|
||||
it('accepts the poles and the antimeridian exactly', () => {
|
||||
expect(coord(90, 'lat')).toBe(90);
|
||||
expect(coord(-90, 'lat')).toBe(-90);
|
||||
expect(coord(180, 'lng')).toBe(180);
|
||||
expect(coord(-180, 'lng')).toBe(-180);
|
||||
});
|
||||
|
||||
it('rejects a latitude past the poles', () => {
|
||||
expect(coord(90.1, 'lat')).toBeNull();
|
||||
expect(coord(-91, 'lat')).toBeNull();
|
||||
});
|
||||
|
||||
it('rejects a longitude past the antimeridian', () => {
|
||||
expect(coord(180.5, 'lng')).toBeNull();
|
||||
expect(coord(-181, 'lng')).toBeNull();
|
||||
});
|
||||
|
||||
it('catches a transposition only when the longitude exceeds 90', () => {
|
||||
// The honest limit of a range check. Kolkata is (22.5726, 88.3639) and
|
||||
// Dibrugarh is (27.4728, 94.9120) — transpose the second and the
|
||||
// "latitude" becomes 94.9, past the pole, so it is caught.
|
||||
expect(coord(94.912, 'lat')).toBeNull();
|
||||
expect(hasCoords(94.912, 27.4728)).toBe(false);
|
||||
});
|
||||
|
||||
it('CANNOT catch a transposed Coimbatore pin — documented limitation', () => {
|
||||
// 76.9558 is a perfectly legal latitude (northern Siberia), so a
|
||||
// transposed Coimbatore pin passes every check in this module. Range
|
||||
// validation only catches a swap when one axis leaves the other's
|
||||
// domain, i.e. |longitude| > 90. Anything west of 90°E transposes
|
||||
// silently.
|
||||
//
|
||||
// Pinned as a test so nobody reads `coord` as swap-proof. Catching this
|
||||
// needs a plausibility check against the operating region (see
|
||||
// geocodingService.isWithinIndia), not a bounds check.
|
||||
expect(coord(COIMBATORE.longitude, 'lat')).toBe(76.9558);
|
||||
expect(hasCoords(COIMBATORE.longitude, COIMBATORE.latitude)).toBe(true);
|
||||
});
|
||||
|
||||
it('rejects an unknown axis rather than guessing a bound', () => {
|
||||
expect(coord(11.0168, 'elevation')).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('real coordinates', () => {
|
||||
it('passes a Coimbatore pin through unchanged', () => {
|
||||
expect(coord(COIMBATORE.latitude, 'lat')).toBe(11.0168);
|
||||
expect(coord(COIMBATORE.longitude, 'lng')).toBe(76.9558);
|
||||
});
|
||||
|
||||
it('parses a numeric string, which is how a sheet cell arrives', () => {
|
||||
expect(coord('11.0168', 'lat')).toBe(11.0168);
|
||||
expect(coord(' 76.9558 ', 'lng')).toBe(76.9558);
|
||||
});
|
||||
|
||||
it('accepts southern and western hemispheres', () => {
|
||||
expect(coord(-33.8688, 'lat')).toBe(-33.8688);
|
||||
expect(coord(-74.006, 'lng')).toBe(-74.006);
|
||||
});
|
||||
});
|
||||
|
||||
describe('axis helpers', () => {
|
||||
it('applies the latitude bound', () => {
|
||||
expect(latitude(91)).toBeNull();
|
||||
expect(latitude(11.0168)).toBe(11.0168);
|
||||
});
|
||||
|
||||
it('applies the longitude bound', () => {
|
||||
expect(longitude(76.9558)).toBe(76.9558);
|
||||
expect(longitude(181)).toBeNull();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('hasCoords', () => {
|
||||
it('needs both halves', () => {
|
||||
expect(hasCoords(11.0168, 76.9558)).toBe(true);
|
||||
expect(hasCoords(11.0168, null)).toBe(false);
|
||||
expect(hasCoords(null, 76.9558)).toBe(false);
|
||||
expect(hasCoords(11.0168, 0)).toBe(false);
|
||||
});
|
||||
|
||||
it('rejects the pair the geocoding shim used to produce', () => {
|
||||
// geocodingService returned 0 from geometry.location.lat() when a place
|
||||
// had no coordinates. This is that pair.
|
||||
expect(hasCoords(0, 0)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('coordsOf', () => {
|
||||
it('reads the API/form shape', () => {
|
||||
expect(coordsOf(COIMBATORE)).toEqual({ latitude: 11.0168, longitude: 76.9558 });
|
||||
});
|
||||
|
||||
it('reads the Leaflet/map-picker shape', () => {
|
||||
expect(coordsOf({ lat: 11.0168, lng: 76.9558 })).toEqual({
|
||||
latitude: 11.0168,
|
||||
longitude: 76.9558,
|
||||
});
|
||||
});
|
||||
|
||||
it('returns null when either half is unusable', () => {
|
||||
expect(coordsOf({ latitude: 11.0168, longitude: null })).toBeNull();
|
||||
expect(coordsOf({ latitude: 0, longitude: 0 })).toBeNull();
|
||||
expect(coordsOf({ latitude: '', longitude: '' })).toBeNull();
|
||||
});
|
||||
|
||||
it('returns null for a non-object', () => {
|
||||
expect(coordsOf(null)).toBeNull();
|
||||
expect(coordsOf(undefined)).toBeNull();
|
||||
expect(coordsOf('11.0168,76.9558')).toBeNull();
|
||||
});
|
||||
|
||||
it('prefers the explicit spelling when both are present', () => {
|
||||
expect(coordsOf({ latitude: 11.0168, lat: 99, longitude: 76.9558, lng: 99 })).toEqual({
|
||||
latitude: 11.0168,
|
||||
longitude: 76.9558,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('coordError', () => {
|
||||
it('names a missing pin rather than a number problem', () => {
|
||||
expect(coordError('', 'lat')).toBe('Navigate cannot work without a pin');
|
||||
expect(coordError(null, 'lng')).toBe('Navigate cannot work without a pin');
|
||||
expect(coordError(0, 'lat')).toBe('Navigate cannot work without a pin');
|
||||
});
|
||||
|
||||
it('names a number problem when it is one', () => {
|
||||
expect(coordError('abc', 'lat')).toBe('Enter a number');
|
||||
expect(coordError(true, 'lat')).toBe('Enter a number');
|
||||
});
|
||||
|
||||
it('names the axis and its bounds when out of range', () => {
|
||||
expect(coordError(91, 'lat')).toBe('Latitude must be between -90 and 90');
|
||||
expect(coordError(181, 'lng')).toBe('Longitude must be between -180 and 180');
|
||||
});
|
||||
|
||||
it('is null for a good coordinate', () => {
|
||||
expect(coordError(11.0168, 'lat')).toBeNull();
|
||||
expect(coordError(76.9558, 'lng')).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -35,11 +35,40 @@ describe('calculateDrivingDistance', () => {
|
||||
expect(global.fetch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should accept zero coordinates, which are a real position', async () => {
|
||||
it('should reject zero coordinates, which are only ever a failed geocode', async () => {
|
||||
// This assertion is INVERTED from what it used to say, and the reason is
|
||||
// that the old pair of expectations could not both hold.
|
||||
//
|
||||
// The case above requires a null latitude to be rejected. `Number(null)`
|
||||
// is 0, so to any guard built on Number() a null latitude and a zero
|
||||
// latitude are the same value — there is no implementation that rejects
|
||||
// one and accepts the other. The old code accepted both, which is why
|
||||
// "should reject a null latitude value" was failing on main.
|
||||
//
|
||||
// So it is a choice, and zero loses. Geographically (0, 0) is a real
|
||||
// point in the Gulf of Guinea; in this product it is not reachable —
|
||||
// Doormile operates inside the India bounding box (latitude 6.5 to 37.5,
|
||||
// longitude 68 to 97.5, see geocodingService.isWithinIndia) and every
|
||||
// zero coordinate ever seen in production was the residue of a geocode
|
||||
// that failed. Accepting them cost real money: OSRM has no route to open
|
||||
// ocean, so the Haversine fallback measured a Coimbatore pickup at
|
||||
// roughly 11,000 km, and the backend honours that as the order's price.
|
||||
global.fetch.mockResolvedValue(osrmOk(1000, 120));
|
||||
await expect(
|
||||
calculateDrivingDistance({ latitude: 0, longitude: 0 }, { latitude: 0, longitude: 1 })
|
||||
).resolves.toEqual(expect.any(Number));
|
||||
).rejects.toThrow('Invalid coordinates');
|
||||
expect(global.fetch).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should reject a coordinate outside its own axis range', async () => {
|
||||
// Nothing in the console checked this before. Note the limit: this
|
||||
// catches a longitude read as a latitude only when it exceeds 90, so a
|
||||
// transposed Coimbatore pin (11.0168, 76.9558) still passes. See
|
||||
// lib/coords for why, and what would actually catch it.
|
||||
await expect(
|
||||
calculateDrivingDistance({ latitude: 94.912, longitude: 27.4728 }, IND)
|
||||
).rejects.toThrow('Invalid coordinates');
|
||||
expect(global.fetch).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -61,7 +90,11 @@ describe('calculateDrivingDistance', () => {
|
||||
await calculateDrivingDistance(BLR, IND);
|
||||
const url = global.fetch.mock.calls[0][0];
|
||||
expect(url).toContain('/route/v1/driving/77.5946,12.9716;77.6245,12.9352');
|
||||
expect(url).toContain('overview=false');
|
||||
// The route geometry IS requested: `calculateDrivingRoute` returns a
|
||||
// polyline for the live map preview, so this asks for overview=full with
|
||||
// geojson geometry. It read `overview=false` until the polyline landed.
|
||||
expect(url).toContain('overview=full');
|
||||
expect(url).toContain('geometries=geojson');
|
||||
});
|
||||
|
||||
it('should use the public OSRM host by default', async () => {
|
||||
@@ -113,10 +146,13 @@ describe('calculateDrivingDistance', () => {
|
||||
|
||||
it('should apply the 1.3 road multiplier to the straight-line distance', async () => {
|
||||
// One degree of latitude is 111.19 km great-circle; x1.3 = 144.5 -> 145.
|
||||
// Measured along a meridian over India rather than from (0, 0): the
|
||||
// arithmetic is identical at any longitude, and zero coordinates are now
|
||||
// refused before the fetch.
|
||||
global.fetch.mockResolvedValue({ ok: false, json: async () => ({}) });
|
||||
const distance = await calculateDrivingDistance(
|
||||
{ latitude: 0, longitude: 0 },
|
||||
{ latitude: 1, longitude: 0 }
|
||||
{ latitude: 10, longitude: 77 },
|
||||
{ latitude: 11, longitude: 77 }
|
||||
);
|
||||
expect(distance).toBe(145);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user