Skip to content
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
41 changes: 41 additions & 0 deletions .changeset/related-list-columns-are-highlightfields.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
---
'hotcrm': patch
---

Correct what `src/views/event_attendee.view.ts` claims a detail-page related
list reads, and pin the metadata that actually curates one.

The file header justified the attendee grid and form with a mechanism it stated
as fact — "the related list renders THIS view's columns, and the quick-create
modal renders THIS form … for the same reason `crm_campaign_member` has them" —
and it was the only written account of that rendering path in this repo.
`crm_campaign_member` has no view metadata at all, so the cited precedent was a
counter-example, and #944 asked which half was wrong. Measured against the
shipped Console (17.0.0-rc.3), the answer is the mechanism:

- **A related list never reads the child's `list` view.** It takes its columns
from the child's lookup field (`relatedListColumns`, authored nowhere in this
app), then falls back to the child object's `highlightFields` minus the lookup
the panel is scoped by, capped at six, with columns that are empty on every
fetched row dropped. Only an object with no `highlightFields` reaches a
heuristic over the whole field map — which is title-ish names first and audit
columns last, not "every column in declaration order".
- **The `form` half is real.** The Console merges a view bundle's `form` onto
the object definition, and the drawer a related list opens renders its
sections. What it buys is the section split and the field order: with no form
the drawer already drops `autonumber`, `formula` and `summary` fields in
create mode and sections the rest by the object's own `fieldGroups`, so the
raw autonumber was never on offer.

So the campaign detail page's **Campaign Members** panel is not degraded and
never was: it renders Lead / Contact / Status / Response Date off
`crm_campaign_member.highlightFields`. Adding a member view would not have
changed one column of it. Nothing user-visible changes here — the header is
rewritten to what was measured, and `test/view-references.test.ts` now pins the
load-bearing metadata for every object reached only through a parent (the two
junctions and the two line items): `highlightFields` must exist, resolve, and
survive dropping the panel's own scope field, so that deleting it — the one
change that really would leave a panel leading with `CM-00001` — fails a test
instead of shipping.

Refs #944.
49 changes: 42 additions & 7 deletions src/views/event_attendee.view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,49 @@ import { defineView } from '@objectstack/spec/ui';
* Event Attendee Views (#592)
*
* A junction object is normally edited inside its parent's related list, and
* `crm_event_attendee` is no exception — but it still needs a grid and a form,
* for the same reason `crm_campaign_member` has them: the related list renders
* THIS view's columns, and the quick-create modal renders THIS form. Without
* them the panel falls back to every column in declaration order and the
* create modal offers the raw autonumber.
* `crm_event_attendee` is no exception. It carries no `tabs` and no navigation
* entry: an attendee is never something you go looking for on its own, you
* reach it through the meeting.
*
* It carries no `tabs` and no navigation entry: an attendee is never something
* you go looking for on its own, you reach it through the meeting.
* # What each half of this bundle actually renders — measured (#944)
*
* This header used to justify both halves in one sentence: the related list
* renders THIS view's columns, the quick-create modal renders THIS form, and
* without them the panel falls back to every column in declaration order while
* the modal offers the raw autonumber — "for the same reason
* `crm_campaign_member` has them". `crm_campaign_member` has no view metadata
* at all, which is what sent #944 looking; re-measuring the paragraph against
* the shipped Console (17.0.0-rc.3) left only the form half standing.
*
* **`form` — live.** The Console merges a view bundle's `form` onto the object
* definition it holds, and the record drawer a related list opens ("New" on
* the panel, edit on a row) renders that form's `sections`. What the authored
* form buys is the section split and the field order — not rescue from a raw
* autonumber, because the no-form fallback is not raw either: in `create` mode
* the drawer drops `autonumber`, `formula` and `summary` fields (and anything
* hidden or readonly) and sections whatever is left by the object's own
* `fieldGroups`. `attendee_number` was never on offer.
*
* **`list` — not what the related list reads.** A detail-page related list
* takes its columns from the CHILD's lookup field — `relatedListColumns` on
* `crm_event`, which this app authors nowhere — and falls back to the child
* object's `highlightFields` minus the parent lookup, capped at six, with
* columns that are empty on every fetched row dropped. Only an object with no
* `highlightFields` reaches the last resort, and that is a heuristic over the
* field map (title-ish names first, audit columns last, `rich_text` / `html` /
* `json` excluded) — not declaration order, and never this view. So the
* attendee panel on a meeting shows `attendee_type`, `response` and
* `is_organizer`, straight off `crm_event_attendee.highlightFields`, whatever
* the columns below say.
*
* The grid is therefore reachable only at this object's own list URL, which no
* navigation entry links to. It is kept as the curated answer to "what do you
* show about an attendee", and it is what a future `relatedListColumns` would
* be copied from — but nothing in this repo may cite it as what makes a
* related list readable. The load-bearing metadata is `highlightFields`, and
* `test/view-references.test.ts` pins it for every object reached only through
* a parent: strip it and the panel really does fall to the heuristic, which
* leads with the autonumber these objects use as their `nameField`.
*/
export const EventAttendeeViews = defineView({
list: {
Expand Down
108 changes: 108 additions & 0 deletions test/view-references.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -455,3 +455,111 @@ describe('every named list view is reachable', () => {
expect(bad, `dangling tabs:\n ${bad.join('\n ')}`).toEqual([]);
});
});

/**
* ── What curates a related list, when it is NOT a view (#944) ────────────────
*
* Four objects in this app are reached only through a parent's detail page —
* the two junctions and the two line items. It is a standing temptation to
* believe that panel renders the child's list view, and the header of
* `src/views/event_attendee.view.ts` said so in as many words until #944.
*
* It does not. MEASURED against the shipped Console (17.0.0-rc.3): a
* detail-page related list takes its columns from the CHILD's lookup field
* (`relatedListColumns`, which this app authors nowhere), then falls back to
* the child object's `highlightFields` minus the lookup the panel is scoped
* by — capped at six, with columns empty on every fetched row dropped. Only
* with neither of those does it reach a heuristic over the whole field map.
* The child's `list` view is never consulted, which is why
* `crm_campaign_member` ships no view at all and its panel on a campaign is
* still curated (Lead / Contact / Status / Response Date), while
* `crm_event_attendee` ships one whose columns that panel ignores.
*
* So the thing worth guarding here is not the views — it is `highlightFields`
* on the objects that have no other way to be read. Delete it and the panel
* drops to the heuristic, which leads with the autonumber these objects carry
* as their `nameField`: `CM-00001`, `EA-00001`, one useless column where the
* person's name should be. That is exactly the degradation #944 was filed
* about, and deleting this metadata is the only way to reach it.
*
* Derived, not listed: an object qualifies when it holds a REQUIRED lookup to
* another `crm_` object and has no navigation entry of its own. A fifth such
* object added tomorrow is held to the same bar without anyone extending a
* fixture.
*/
describe('objects reached only through a parent curate that related list', () => {
const apps: AnyRec[] = (stack as any).apps ?? [];
const navObjects = new Set(
apps.flatMap((app) =>
(app.navigation ?? []).flatMap(function walk(n: AnyRec): string[] {
return [...(n.objectName ? [n.objectName] : []), ...(n.children ?? []).flatMap(walk)];
})),
);

/** The required `crm_` lookups on an object — each one is a panel it appears in. */
const parentLookupsOf = (obj: AnyRec): string[] =>
Object.entries<AnyRec>(obj.fields ?? {})
.filter(([, def]) =>
(def?.type === 'lookup' || def?.type === 'master_detail') &&
def?.required === true &&
String(def?.reference_to ?? def?.reference ?? '').startsWith('crm_'))
.map(([name]) => name);

const parentOnly = objects.filter(
(o) => String(o.name).startsWith('crm_') && !navObjects.has(o.name) && parentLookupsOf(o).length > 0,
);

it('the derivation finds the objects it is meant to cover', () => {
// Guard the guard. Every assertion below iterates `parentOnly`, and all of
// them would pass by checking nothing if the navigation walk stopped
// yielding object names or the lookup filter stopped matching — the way
// the navigation guard in `test/action-references.test.ts` spent its life
// green. The two junctions are named because they are what #944 is about;
// the count is a floor, not a roster.
expect(navObjects.size, 'no navigation object entries parsed out of the app').toBeGreaterThan(5);
const names = parentOnly.map((o) => o.name);
expect(names).toContain('crm_campaign_member');
expect(names).toContain('crm_event_attendee');
expect(names.length).toBeGreaterThanOrEqual(4);
});

it.each(parentOnly.map((o) => o.name))('%s authors highlightFields', (name) => {
const obj = objects.find((o) => o.name === name) as AnyRec;
const highlight: string[] = Array.isArray(obj.highlightFields) ? obj.highlightFields : [];
expect(
highlight.length,
`${name} authors no highlightFields — nothing curates its related list, and the ` +
'fallback heuristic leads with whatever the field map offers first',
).toBeGreaterThan(0);

const known = new Set(Object.keys(obj.fields ?? {}));
const dangling = highlight.filter((f) => !known.has(f));
expect(dangling, `${name}.highlightFields names fields that do not exist: ${dangling.join(', ')}`).toEqual([]);
});

it.each(parentOnly.map((o) => o.name))('%s still has columns once the parent lookup is dropped', (name) => {
const obj = objects.find((o) => o.name === name) as AnyRec;
const highlight: string[] = Array.isArray(obj.highlightFields) ? obj.highlightFields : [];
const nameField = typeof obj.nameField === 'string' ? obj.nameField : undefined;
const isAutonumber = !!nameField && obj.fields?.[nameField]?.type === 'autonumber';

const bad: string[] = [];
for (const parent of parentLookupsOf(obj)) {
// What the panel on `parent`'s detail page is left with: the related list
// drops the field it scoped the query by, since every row shows the same
// value for it.
const surviving = highlight.filter((f) => f !== parent);
if (surviving.length === 0) {
bad.push(
highlight.length === 0
? `${name} in the ${parent} panel: no highlightFields at all — the panel falls to the heuristic`
: `${name} in the ${parent} panel: every highlightField is the panel's own scope — no columns left`,
);
}
if (isAutonumber && surviving.includes(nameField as string)) {
bad.push(`${name} in the ${parent} panel: leads with the autonumber "${nameField}"`);
}
}
expect(bad, `related lists with nothing to show:\n ${bad.join('\n ')}`).toEqual([]);
});
});
Loading