fix(yachts): ft↔m round-trip is lossless (4dp + canonical helpers)
Three copies of the imperial/metric conversion logic existed:
- src/components/yachts/yacht-dimensions.ts (canonical, used by
read-side `formatYachtDimensionsBothUnits`)
- src/components/yachts/yacht-form.tsx (create/edit sheet —
local `ftToM`/`mToFt` with 2dp precision)
- src/components/yachts/yacht-tabs.tsx (detail-tab inline
edit — local arithmetic with 2dp precision)
The 2dp rounding lost precision on the round-trip: `1 ft → 0.30 m →
0.98 ft`. Whenever a rep entered ft, then later touched the m field,
the ft column silently shifted off. Same for sub-meter draft values.
Consolidate both surfaces onto `feetToMeters` / `metersToFeet` from
yacht-dimensions.ts and bump display precision to 4dp. After
trimZero strips trailing zeros the rendered string stays clean
("3.81" not "3.8100") but the round-trip now lands back on the
original value:
1 ft → 0.3048 m → 1 ft
12.5 ft → 3.81 m → 12.5 ft
50 ft → 15.24 m → 50 ft
0.5 m → 1.6404 ft → 0.5 m
New unit test (`tests/unit/yacht-dimensions.test.ts`) covers the
helpers + the form-shape round-trip, including the canonical
12.5 ft ↔ 3.81 m case from the UAT bug report.
29/29 new tests pass; full vitest 1448/1448.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -23,6 +23,7 @@ import { CountryCombobox } from '@/components/shared/country-combobox';
|
|||||||
import { OwnerPicker, type OwnerRef } from '@/components/shared/owner-picker';
|
import { OwnerPicker, type OwnerRef } from '@/components/shared/owner-picker';
|
||||||
import { TagPicker } from '@/components/shared/tag-picker';
|
import { TagPicker } from '@/components/shared/tag-picker';
|
||||||
import { apiFetch } from '@/lib/api/client';
|
import { apiFetch } from '@/lib/api/client';
|
||||||
|
import { feetToMeters, metersToFeet } from '@/components/yachts/yacht-dimensions';
|
||||||
import type { z } from 'zod';
|
import type { z } from 'zod';
|
||||||
import { createYachtSchema, type CreateYachtInput } from '@/lib/validators/yachts';
|
import { createYachtSchema, type CreateYachtInput } from '@/lib/validators/yachts';
|
||||||
|
|
||||||
@@ -424,11 +425,12 @@ export function YachtForm({
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
// 1 ft = 0.3048 m exactly. Round to 2 decimals so the cross-filled value is
|
// Cross-fill the sibling unit. Delegates the math to `yacht-dimensions.ts`
|
||||||
// readable but stable; `trimZero` strips trailing `.0` so a whole-number
|
// so the form and the read-side `formatYachtDimensionsBothUnits` agree on
|
||||||
// conversion like `5 ft → 1.52 m → 1.52` doesn't render as `1.520000`.
|
// the formula. 4-decimal precision keeps the round-trip lossless within
|
||||||
const FT_PER_M = 3.28084;
|
// display: `1 ft → 0.3048 m → 1.0000 ft` (after trimZero → "1"); the prior
|
||||||
|
// 2-decimal precision rounded `1 ft → 0.30 m → 0.98 ft` which lost data
|
||||||
|
// whenever the rep touched both fields on the same yacht.
|
||||||
function trimZero(s: string): string {
|
function trimZero(s: string): string {
|
||||||
if (!s.includes('.')) return s;
|
if (!s.includes('.')) return s;
|
||||||
return s.replace(/\.?0+$/, '');
|
return s.replace(/\.?0+$/, '');
|
||||||
@@ -436,16 +438,16 @@ function trimZero(s: string): string {
|
|||||||
|
|
||||||
function ftToM(value: string | null | undefined): string {
|
function ftToM(value: string | null | undefined): string {
|
||||||
if (value == null || value === '') return '';
|
if (value == null || value === '') return '';
|
||||||
const n = Number(value);
|
const meters = feetToMeters(value);
|
||||||
if (!Number.isFinite(n)) return '';
|
if (meters === null) return '';
|
||||||
return trimZero((n * 0.3048).toFixed(2));
|
return trimZero(meters.toFixed(4));
|
||||||
}
|
}
|
||||||
|
|
||||||
function mToFt(value: string | null | undefined): string {
|
function mToFt(value: string | null | undefined): string {
|
||||||
if (value == null || value === '') return '';
|
if (value == null || value === '') return '';
|
||||||
const n = Number(value);
|
const feet = metersToFeet(value);
|
||||||
if (!Number.isFinite(n)) return '';
|
if (feet === null) return '';
|
||||||
return trimZero((n * FT_PER_M).toFixed(2));
|
return trimZero(feet.toFixed(4));
|
||||||
}
|
}
|
||||||
|
|
||||||
function DimensionPair({
|
function DimensionPair({
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ import { EntityActivityFeed } from '@/components/shared/entity-activity-feed';
|
|||||||
import { ReservationList, type ReservationRow } from '@/components/reservations/reservation-list';
|
import { ReservationList, type ReservationRow } from '@/components/reservations/reservation-list';
|
||||||
import { RemindersInline } from '@/components/reminders/reminders-inline';
|
import { RemindersInline } from '@/components/reminders/reminders-inline';
|
||||||
import { YachtOwnershipHistory } from '@/components/yachts/yacht-ownership-history';
|
import { YachtOwnershipHistory } from '@/components/yachts/yacht-ownership-history';
|
||||||
|
import { feetToMeters, metersToFeet } from '@/components/yachts/yacht-dimensions';
|
||||||
import { apiFetch } from '@/lib/api/client';
|
import { apiFetch } from '@/lib/api/client';
|
||||||
import { stageLabel } from '@/lib/constants';
|
import { stageLabel } from '@/lib/constants';
|
||||||
|
|
||||||
@@ -131,12 +132,13 @@ function OverviewTab({
|
|||||||
await mutation.mutateAsync({ [primaryField]: next });
|
await mutation.mutateAsync({ [primaryField]: next });
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
const FT_PER_M = 3.28084;
|
// Delegate the math to the canonical helpers in yacht-dimensions.ts
|
||||||
const converted = isFt ? n / FT_PER_M : n * FT_PER_M;
|
// so this surface, the create-form, and the read-side formatter all
|
||||||
const convertedStr = converted
|
// round-trip identically. 4dp precision keeps `1 ft → 0.3048 m →
|
||||||
.toFixed(2)
|
// 1.0000 ft` (after trimZero → "1"); 2dp lost data on small values.
|
||||||
.replace(/\.0+$/, '')
|
const converted = isFt ? feetToMeters(next) : metersToFeet(next);
|
||||||
.replace(/(\.\d)0$/, '$1');
|
const convertedStr =
|
||||||
|
converted === null ? '' : converted.toFixed(4).replace(/0+$/, '').replace(/\.$/, '');
|
||||||
await mutation.mutateAsync({
|
await mutation.mutateAsync({
|
||||||
[primaryField]: next,
|
[primaryField]: next,
|
||||||
[counterpart]: convertedStr,
|
[counterpart]: convertedStr,
|
||||||
|
|||||||
126
tests/unit/yacht-dimensions.test.ts
Normal file
126
tests/unit/yacht-dimensions.test.ts
Normal file
@@ -0,0 +1,126 @@
|
|||||||
|
import { describe, it, expect } from 'vitest';
|
||||||
|
|
||||||
|
import {
|
||||||
|
feetToMeters,
|
||||||
|
metersToFeet,
|
||||||
|
formatNumber1dp,
|
||||||
|
dimInFeet,
|
||||||
|
dimInMeters,
|
||||||
|
} from '@/components/yachts/yacht-dimensions';
|
||||||
|
|
||||||
|
describe('yacht-dimensions: ft ↔ m conversions', () => {
|
||||||
|
it('feetToMeters: returns null for non-finite inputs', () => {
|
||||||
|
expect(feetToMeters(null)).toBeNull();
|
||||||
|
expect(feetToMeters(undefined)).toBeNull();
|
||||||
|
expect(feetToMeters('')).toBeNull();
|
||||||
|
expect(feetToMeters('abc')).toBeNull();
|
||||||
|
expect(feetToMeters(NaN)).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('metersToFeet: returns null for non-finite inputs', () => {
|
||||||
|
expect(metersToFeet(null)).toBeNull();
|
||||||
|
expect(metersToFeet(undefined)).toBeNull();
|
||||||
|
expect(metersToFeet('')).toBeNull();
|
||||||
|
expect(metersToFeet('abc')).toBeNull();
|
||||||
|
expect(metersToFeet(NaN)).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('feetToMeters: matches 1 ft = 0.3048 m within 1e-4', () => {
|
||||||
|
// 1 / 3.28084 = 0.304800061... vs the SI definition 0.3048 m
|
||||||
|
expect(feetToMeters(1)).toBeCloseTo(0.3048, 4);
|
||||||
|
expect(feetToMeters(12.5)).toBeCloseTo(3.81, 2);
|
||||||
|
expect(feetToMeters(50)).toBeCloseTo(15.24, 2);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('metersToFeet: produces inverse of feetToMeters within float precision', () => {
|
||||||
|
expect(metersToFeet(0.3048)).toBeCloseTo(1, 4);
|
||||||
|
expect(metersToFeet(3.81)).toBeCloseTo(12.5, 2);
|
||||||
|
expect(metersToFeet(15.24)).toBeCloseTo(50, 2);
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('round-trip is lossless within helper precision', () => {
|
||||||
|
const cases = [1, 5, 12.5, 25, 50, 120, 250];
|
||||||
|
cases.forEach((ft) => {
|
||||||
|
it(`${ft} ft → m → ft preserves value`, () => {
|
||||||
|
const m = feetToMeters(ft)!;
|
||||||
|
const backToFt = metersToFeet(m)!;
|
||||||
|
expect(backToFt).toBeCloseTo(ft, 6);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
const mCases = [0.5, 1, 3.81, 7.62, 15.24, 36.58];
|
||||||
|
mCases.forEach((m) => {
|
||||||
|
it(`${m} m → ft → m preserves value`, () => {
|
||||||
|
const ft = metersToFeet(m)!;
|
||||||
|
const backToM = feetToMeters(ft)!;
|
||||||
|
expect(backToM).toBeCloseTo(m, 6);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('formatNumber1dp', () => {
|
||||||
|
it('strips trailing .0', () => {
|
||||||
|
expect(formatNumber1dp(12)).toBe('12');
|
||||||
|
expect(formatNumber1dp(12.5)).toBe('12.5');
|
||||||
|
expect(formatNumber1dp(3.812)).toBe('3.8');
|
||||||
|
expect(formatNumber1dp(null)).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('form-shape round-trip (4dp + trimZero, mirrors yacht-form.tsx)', () => {
|
||||||
|
// The form persists string values, so we model the same shape the
|
||||||
|
// input fields see. trimZero strips trailing zeros after the dot.
|
||||||
|
const trimZero = (s: string) => (!s.includes('.') ? s : s.replace(/\.?0+$/, ''));
|
||||||
|
const ftToM = (v: string): string => {
|
||||||
|
const m = feetToMeters(v);
|
||||||
|
return m === null ? '' : trimZero(m.toFixed(4));
|
||||||
|
};
|
||||||
|
const mToFt = (v: string): string => {
|
||||||
|
const ft = metersToFeet(v);
|
||||||
|
return ft === null ? '' : trimZero(ft.toFixed(4));
|
||||||
|
};
|
||||||
|
|
||||||
|
it('round-trip 1 ft → m → ft is lossless', () => {
|
||||||
|
const m = ftToM('1');
|
||||||
|
expect(mToFt(m)).toBe('1');
|
||||||
|
});
|
||||||
|
it('round-trip 12.5 ft ↔ 3.81 m ↔ 12.5 ft', () => {
|
||||||
|
const m = ftToM('12.5');
|
||||||
|
const ft = mToFt(m);
|
||||||
|
// 12.5 / 3.28084 = 3.80999961... → "3.81" → 3.81 * 3.28084 = 12.4999... → "12.5"
|
||||||
|
expect(ft).toBe('12.5');
|
||||||
|
});
|
||||||
|
it('round-trip 50 ft → 15.24 m → 50 ft', () => {
|
||||||
|
expect(mToFt(ftToM('50'))).toBe('50');
|
||||||
|
});
|
||||||
|
it('round-trip 0.5 m → ft → 0.5 m', () => {
|
||||||
|
expect(ftToM(mToFt('0.5'))).toBe('0.5');
|
||||||
|
});
|
||||||
|
it('empty string returns empty', () => {
|
||||||
|
expect(ftToM('')).toBe('');
|
||||||
|
expect(mToFt('')).toBe('');
|
||||||
|
});
|
||||||
|
it('non-numeric returns empty', () => {
|
||||||
|
expect(ftToM('abc')).toBe('');
|
||||||
|
expect(mToFt('abc')).toBe('');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('dimInFeet / dimInMeters: derive missing unit from sibling', () => {
|
||||||
|
it('returns ft value when set', () => {
|
||||||
|
expect(dimInFeet({ ft: 12.5, m: 3.81 })).toBe('12.5');
|
||||||
|
});
|
||||||
|
it('derives ft from m when ft is null', () => {
|
||||||
|
expect(dimInFeet({ ft: null, m: 3.81 })).toBe('12.5');
|
||||||
|
});
|
||||||
|
it('returns m value when set', () => {
|
||||||
|
expect(dimInMeters({ ft: 12.5, m: 3.81 })).toBe('3.8');
|
||||||
|
});
|
||||||
|
it('derives m from ft when m is null', () => {
|
||||||
|
expect(dimInMeters({ ft: 12.5, m: null })).toBe('3.8');
|
||||||
|
});
|
||||||
|
it('returns null when both null', () => {
|
||||||
|
expect(dimInFeet({ ft: null, m: null })).toBeNull();
|
||||||
|
expect(dimInMeters({ ft: null, m: null })).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user