Skip to content

Commit b864382

Browse files
committed
fix(endpoint-form): a location group can come from a field-name prefix, so relocation stops asking visitors for coordinates
/embed/relocation-wheel rendered four raw latitude/longitude number inputs plus a decimal-hours timezone, and it was the only widget that did. Never a missing feature: the city-search substitution was already generic, it just could not see this one shape. A group used to come from object nesting alone, and groupHasLocation required all three of LOCATION_TRIO by exact field name. Five of the six two-location operations nest per person (person1/person2, personA/personB) so each already got its own city box. generateRelocationChart is the only operation in the spec that puts both coordinate pairs at the TOP level, told apart by a name prefix, so no field was named latitude, the flat group failed the test, and nothing was suppressed. Three rules now, all derived from spec shape, no per-operation list anywhere: - splitCoordinateName groups a flat *Latitude/*Longitude pair under its prefix, so a future partnerLatitude needs no code change. The prefix is harvested from coordinates ONLY, so birthDate does not invent a birth group holding a lone date. - The field key stays the original wire name. A prefixed coordinate reports name: latitude but keeps key: birthLatitude, because the key is the serialisation identity and birth.latitude POSTs a body the API rejects. The fill handler now looks keys up from the model instead of building group.name, which is what made that assumption safe to break. - LOCATION_PAIR: a group qualifies on lat+lon, timezone optional. Relocation has one top-level timezone and it is the BIRTH timezone, since relocating does not move the birth moment, so the relocation group owns none. Only the first location group claims an unowned flat timezone, else two city boxes overwrite each other with the wrong offset. The browser audit then caught two things no model-level test could: the placeholder was hardcoded "City of birth" so both boxes read the same, and the help text hardcoded "Required: latitude, longitude, timezone", which is a visible lie on the block that owns no timezone. Both now derive. Rendered result: Birth location / Relocation location, Birth city / Relocation city, "Fills latitude, longitude, timezone" vs "Fills latitude, longitude", zero number inputs. Verified: all 6 multi-location operations produce the right picker count, a sweep of all 73 coordinate operations leaves 0 raw coordinates and creates 0 phantom single-field groups, new unit tests are sabotage-verified, a new Playwright test renders relocation in a real browser, 474 unit and 27 e2e green, axe still passes with the second picker. Also in here, both found while auditing rather than assumed: - e2e specs renamed to *.e2e.ts with testMatch pinned. Bun globs *.spec.ts as well as *.test.ts, so a bare `bun test` swept up the Playwright files and reported 4 failures. The enumerated test script hid it, but a red suite that is not red trains you to ignore the number. Bare bun test now 0 fail, and the test script is a glob instead of 16 hand-listed files, which was a real drift trap since a new test file silently never ran. - release.yml: the git tag + git push --follow-tags pair is INERT here, since --follow-tags pushes only annotated tags. Tagging actually happens server-side in action-gh-release, which is why every tag on this remote is lightweight. Annotated in place after the identical two lines were found losing every tag in a sibling repo with no Release step. Not restructured on a release day.
1 parent 5d184ff commit b864382

11 files changed

Lines changed: 286 additions & 15 deletions

File tree

.github/workflows/release.yml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,15 @@ jobs:
183183
# and the committed codegen (registry, manifest, docs tables).
184184
git add packages/ registry/ apps/docs/manifest.js README.md AGENTS.md
185185
git commit -m "release: v$VERSION"
186+
# These two lines are INERT and the tag you see on the remote is not from here.
187+
# `git tag` makes a LIGHTWEIGHT tag and `git push --follow-tags` pushes only
188+
# ANNOTATED ones, so this pushes the commit and silently drops the tag. Tagging
189+
# actually happens server-side in the `action-gh-release` step below, via
190+
# `tag_name`, which is why every tag on this remote is lightweight (no `^{}`
191+
# dereference line in `git ls-remote --tags`). Verified 2026-08-05 after the same
192+
# two lines were found losing every tag in a sibling repo that has no Release step.
193+
# So do NOT remove `action-gh-release` or replace it with a plain `git push`
194+
# believing this covers tagging. Cleanup tracked; not touched on a release day.
186195
git tag "v$VERSION"
187196
git push --follow-tags
188197

AGENTS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,8 @@ Every snippet below follows this rule.
141141

142142
Every chart endpoint (Western, Vedic, KP, synastry, transits, dasha, dosha, panchang) needs `latitude`, `longitude`, and `timezone`. Never ask the user to type coordinates. Call `/location/search` first, then feed the result into the chart endpoint.
143143

144+
Two endpoints take TWO locations, and they name them differently. `POST /astrology/synastry`, `/astrology/composite-chart`, `/vedic-astrology/compatibility` and `/human-design/connection` nest a full location per person (`person1` / `person2`, or `personA` / `personB`). `POST /astrology/relocation-chart` instead takes `birthLatitude`, `birthLongitude`, `relocationLatitude`, `relocationLongitude` at the top level with a single `timezone`, which is the BIRTH timezone: relocating does not move the birth moment. `<roxy-endpoint-form>` renders a separate city search for each location automatically, so self-fetch mode needs no extra work.
145+
144146
```ts
145147
// Right
146148
const { data: cities } = await roxy.location.searchCities({ query: { q: 'Mumbai' } });

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
"check:previews": "bun run scripts/check-previews.ts",
3333
"audit": "bun run scripts/audit.ts",
3434
"typecheck": "tsc --noEmit",
35-
"test": "bun test packages/ui/tests/utils.test.ts packages/ui/tests/components.test.ts packages/ui/tests/base-element.test.ts packages/ui/tests/bindings.test.ts packages/ui/tests/chart-width.test.ts packages/ui/tests/bodygraph.test.ts packages/ui/tests/theming.test.ts packages/ui/tests/themes.test.ts packages/ui/tests/field-schema.test.ts packages/ui/tests/taxonomy.test.ts packages/ui/tests/endpoint-form.test.ts packages/ui/tests/key-guard.test.ts packages/ui/tests/build-schemas.test.ts packages/ui/tests/widgets.test.ts packages/ui/tests/embed-tab.test.ts packages/ui/tests/check-sizes.test.ts --timeout 10000",
35+
"test": "bun test --timeout 10000",
3636
"test:e2e": "playwright test",
3737
"check": "biome check . --fix",
3838
"brand:check": "bun run scripts/brand-grep.ts",

packages/ui/src/components/endpoint-form.ts

Lines changed: 75 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
type FieldDef,
1111
type FormModel,
1212
isZodiacEnum,
13+
LOCATION_PAIR,
1314
LOCATION_TRIO,
1415
type OpenApiDoc,
1516
type OperationSchema,
@@ -500,6 +501,14 @@ export class RoxyEndpointForm extends LitElement {
500501
this.groupHasLocation(f.group)
501502
)
502503
return 'location';
504+
// A flat timezone in a group with no coordinates of its own is still suppressed when a
505+
// location group has claimed it, otherwise the prefixed shape shows a raw decimal-hours
506+
// box beside two city searches that already know the answer.
507+
if (
508+
f.name === 'timezone' &&
509+
this.locationGroups().some((g) => this.timezoneFieldFor(g)?.key === f.key)
510+
)
511+
return 'location';
503512
return 'normal';
504513
}
505514

@@ -516,10 +525,39 @@ export class RoxyEndpointForm extends LitElement {
516525
return keys;
517526
}
518527

519-
/** True when the fields in `group` (or the flat top level) carry latitude+longitude+timezone, so a location-search can autofill them. */
528+
/** True when the fields in `group` (or the flat top level) carry a latitude+longitude pair, so a location-search can autofill them. Timezone is NOT required here, see {@link LOCATION_PAIR}. */
520529
private groupHasLocation(group?: string): boolean {
521530
const inGroup = this.fields.filter((f) => f.group === group);
522-
return LOCATION_TRIO.every((n) => inGroup.some((f) => f.name === n));
531+
return LOCATION_PAIR.every((n) => inGroup.some((f) => f.name === n));
532+
}
533+
534+
/**
535+
* Location groups in field order. Order is load-bearing: when a request carries coordinate
536+
* groups but only ONE unprefixed top-level `timezone`, the FIRST group owns it, because that
537+
* timezone belongs to the primary moment (`generateRelocationChart` has one `timezone` and it is
538+
* the birth timezone). Without an owner both city boxes would write the same key and the second
539+
* pick would silently overwrite the first with the wrong offset.
540+
*/
541+
private locationGroups(): (string | undefined)[] {
542+
return this.groupKeys().filter((g) => this.groupHasLocation(g));
543+
}
544+
545+
/**
546+
* The timezone field a group's city search should fill, or undefined when there is none.
547+
*
548+
* Prefers a timezone inside the group (the nested `person1`/`person2` shape has its own). Falls
549+
* back to an unowned flat `timezone` for the first location group only, which is what makes the
550+
* prefixed shape work without leaving a decimal-hours box for a visitor to guess at.
551+
*/
552+
private timezoneFieldFor(group?: string): FieldDef | undefined {
553+
const own = this.fields.find(
554+
(f) => f.group === group && f.name === 'timezone',
555+
);
556+
if (own) return own;
557+
if (this.locationGroups()[0] !== group) return undefined;
558+
return this.fields.find(
559+
(f) => f.group === undefined && f.name === 'timezone',
560+
);
523561
}
524562

525563
/**
@@ -561,9 +599,16 @@ export class RoxyEndpointForm extends LitElement {
561599
return this.lang || undefined;
562600
}
563601

564-
/** Location-select handler bound to a group: fills that group's lat/lng/timezone keys (flat top level when group is undefined). */
602+
/**
603+
* Location-select handler bound to a group: fills that group's coordinate keys plus whichever
604+
* timezone field it owns.
605+
*
606+
* Keys are LOOKED UP from the model rather than built as `group.name`. That assumption held only
607+
* while every group came from object nesting; a prefixed group stores under the original wire
608+
* name (`birthLatitude`), so constructing `birth.latitude` would write a key the request builder
609+
* never reads and the coordinates would silently stay empty.
610+
*/
565611
private onLocationFor(group?: string) {
566-
const prefix = group ? `${group}.` : '';
567612
return (e: Event) => {
568613
const detail = (e as CustomEvent).detail as {
569614
latitude?: number;
@@ -572,12 +617,16 @@ export class RoxyEndpointForm extends LitElement {
572617
utcOffset?: number;
573618
};
574619
if (!detail) return;
575-
this.values = {
576-
...this.values,
577-
[`${prefix}latitude`]: detail.latitude,
578-
[`${prefix}longitude`]: detail.longitude,
579-
[`${prefix}timezone`]: detail.timezone ?? detail.utcOffset,
580-
};
620+
const keyOf = (name: string) =>
621+
this.fields.find((f) => f.group === group && f.name === name)?.key;
622+
const next: Record<string, unknown> = { ...this.values };
623+
const lat = keyOf('latitude');
624+
const lon = keyOf('longitude');
625+
if (lat) next[lat] = detail.latitude;
626+
if (lon) next[lon] = detail.longitude;
627+
const tz = this.timezoneFieldFor(group);
628+
if (tz) next[tz.key] = detail.timezone ?? detail.utcOffset;
629+
this.values = next;
581630
};
582631
}
583632

@@ -848,6 +897,20 @@ export class RoxyEndpointForm extends LitElement {
848897
}
849898
}
850899

900+
/**
901+
* The fields this group's city search actually fills, named in the help text.
902+
*
903+
* Not the hardcoded "latitude, longitude, timezone" it used to say: a group does not always own a
904+
* timezone. `generateRelocationChart` has one top-level `timezone` that belongs to the birth
905+
* moment, so the relocation block fills coordinates only, and promising a timezone there would be
906+
* a visible lie on the one form that made this method necessary.
907+
*/
908+
private locationFillList(group?: string): string {
909+
const names = [...LOCATION_PAIR] as string[];
910+
if (this.timezoneFieldFor(group)) names.push('timezone');
911+
return names.join(', ');
912+
}
913+
851914
private locationBlock(group?: string) {
852915
return html`<div class="location-block">
853916
<label
@@ -860,10 +923,10 @@ export class RoxyEndpointForm extends LitElement {
860923
<roxy-location-search
861924
publishable-key=${ifDefined(this.publishableKey)}
862925
@roxy-location-select=${this.onLocationFor(group)}
863-
placeholder="City of birth"
926+
placeholder=${group ? `${humanize(group)} city` : 'City of birth'}
864927
></roxy-location-search>
865928
<small class="help">
866-
Required: latitude, longitude, timezone. Pick a city to autofill.
929+
Fills ${this.locationFillList(group)}. Pick a city to autofill.
867930
</small>
868931
</div>`;
869932
}

packages/ui/src/utils/field-schema.ts

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,47 @@ export const TILE_MAX = 12;
109109
/** The latitude+longitude+timezone trio the form suppresses in favour of a city search. Centralised so the render path and tests agree. */
110110
export const LOCATION_TRIO = ['latitude', 'longitude', 'timezone'] as const;
111111

112+
/**
113+
* The coordinate pair that DEFINES a location group. `timezone` is deliberately absent.
114+
*
115+
* @remarks
116+
* A group qualifies for a city search on latitude+longitude alone, because a timezone is not always
117+
* part of the group that owns the coordinates: `generateRelocationChart` carries two coordinate
118+
* pairs and exactly ONE top-level `timezone`, which is the BIRTH timezone (relocating does not move
119+
* the birth moment), so the relocation pair correctly has no timezone of its own. Requiring all
120+
* three would leave that operation rendering raw number inputs, which is the bug this exists to
121+
* prevent.
122+
*/
123+
export const LOCATION_PAIR = ['latitude', 'longitude'] as const;
124+
125+
/**
126+
* Split a flat coordinate property into its group prefix and canonical leaf name, or `null` when the
127+
* name is not a prefixed coordinate.
128+
*
129+
* @remarks
130+
* **Two shapes carry two locations in one request and this handles the second one.** Most
131+
* multi-location operations nest per person (`person1`/`person2`, `personA`/`personB`), so object
132+
* nesting alone already groups them. `generateRelocationChart` is the only operation in the spec
133+
* that instead puts both pairs at the TOP level and distinguishes them by a name prefix
134+
* (`birthLatitude` / `relocationLatitude`). Keying the grouping off the prefix as well as off the
135+
* nesting means both shapes converge on the same group machinery, and a future `partnerLatitude`
136+
* needs no code change.
137+
*
138+
* Matching is deliberately restricted to the coordinate pair. A prefix is NOT harvested from any
139+
* other field name, so `birthDate` stays an ordinary field in the flat group and does not invent a
140+
* phantom `birth` group with a lone date in it.
141+
*/
142+
export function splitCoordinateName(
143+
name: string,
144+
): { group: string; leaf: (typeof LOCATION_PAIR)[number] } | null {
145+
const m = name.match(/^(.+?)(Latitude|Longitude)$/);
146+
if (!m) return null;
147+
return {
148+
group: m[1],
149+
leaf: m[2].toLowerCase() as (typeof LOCATION_PAIR)[number],
150+
};
151+
}
152+
112153
/** Canonical lowercase zodiac set, derived from {@link SIGNS_ORDER} so sign detection and the glyph map can never disagree. */
113154
const ZODIAC_LOWER = SIGNS_ORDER.map((s) => s.toLowerCase());
114155

@@ -232,8 +273,23 @@ export function buildFormModel(
232273
);
233274
}
234275
} else {
276+
// A prefixed coordinate joins a group named after its prefix and reports the
277+
// canonical leaf name, so the trio logic and the city search match it unchanged.
278+
// `key` stays the ORIGINAL property name because it is the storage and wire
279+
// identity: rewriting it to `birth.latitude` would serialise a body the API
280+
// rejects.
281+
const coord = splitCoordinateName(name);
235282
fields.push(
236-
toField(name, resolved, { key: name, required: required.has(name) }),
283+
coord
284+
? toField(coord.leaf, resolved, {
285+
key: name,
286+
group: coord.group,
287+
required: required.has(name),
288+
})
289+
: toField(name, resolved, {
290+
key: name,
291+
required: required.has(name),
292+
}),
237293
);
238294
}
239295
}
Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ const FORMS = [
1919
{ id: 'e2e-tarot-draw', endpoint: 'tarot/draw', method: 'POST' },
2020
{ id: 'e2e-natal', endpoint: 'astrology/natal-chart', method: 'POST' },
2121
{ id: 'e2e-synastry', endpoint: 'astrology/synastry', method: 'POST' },
22+
{
23+
id: 'e2e-relocation',
24+
endpoint: 'astrology/relocation-chart',
25+
method: 'POST',
26+
},
2227
];
2328

2429
async function mountForms(page: Page): Promise<void> {
@@ -45,7 +50,10 @@ async function mountForms(page: Page): Promise<void> {
4550
const sy = document.getElementById('e2e-synastry');
4651
return (
4752
!!ho?.shadowRoot?.querySelector('[role="radiogroup"]') &&
48-
!!sy?.shadowRoot?.querySelector('fieldset.person-group')
53+
!!sy?.shadowRoot?.querySelector('fieldset.person-group') &&
54+
!!document
55+
.getElementById('e2e-relocation')
56+
?.shadowRoot?.querySelector('roxy-location-search')
4957
);
5058
});
5159
}
@@ -214,3 +222,40 @@ test.describe('practitioner theme preset', () => {
214222
await scanThemed();
215223
});
216224
});
225+
226+
/**
227+
* The two-locations-in-one-request case, rendered rather than modelled.
228+
*
229+
* `generateRelocationChart` is the only operation in the spec that carries two coordinate pairs at
230+
* the TOP level, told apart by a name prefix (`birthLatitude` / `relocationLatitude`) instead of by
231+
* per-person object nesting. Grouping used to come from nesting alone, so this form fell through to
232+
* four raw number inputs and a decimal-hours timezone box, which no visitor of an embedder's site
233+
* can answer. The unit tests assert the form MODEL; this asserts what a browser actually paints,
234+
* which is the only claim that matters to the person filling it in.
235+
*/
236+
test.describe('two-location form', () => {
237+
test('relocation renders two city searches and no raw coordinate inputs', async ({
238+
page,
239+
}) => {
240+
await mountForms(page);
241+
242+
const pickers = page.locator('#e2e-relocation roxy-location-search');
243+
await expect(pickers).toHaveCount(2);
244+
245+
// Each block is labelled from its group, so the two are tellable apart.
246+
const labels = await page
247+
.locator('#e2e-relocation .location-block label')
248+
.allInnerTexts();
249+
expect(labels.join(' | ').toLowerCase()).toContain('birth');
250+
expect(labels.join(' | ').toLowerCase()).toContain('relocation');
251+
252+
// No coordinate or timezone field may survive as a typed input.
253+
const names = await page
254+
.locator('#e2e-relocation input')
255+
.evaluateAll((els) =>
256+
els.map((e) => (e as HTMLInputElement).name || e.id || ''),
257+
);
258+
const raw = names.filter((n) => /latitude|longitude|timezone/i.test(n));
259+
expect(raw, `raw inputs still rendered: ${raw.join(', ')}`).toEqual([]);
260+
});
261+
});

0 commit comments

Comments
 (0)