Enforce lead disqualification reason, group campaign/task fields, unify priority_rank

.changeset/lead-disqualification-field-groups-priority-rank.md

@@ -0,0 +1,50 @@
+---
+'hotcrm': patch
+---
+
+Enforce `crm_lead.disqualification_reason`, group the campaign and task fields,
+and unify the `priority_rank` sentinel (#575 A2/A3/A4).
+
+**`disqualification_reason` promised required and enforced nothing.** The field
+description has read "Required when status is Unqualified" since it was added,
+with no validation, no hook, and no form that rendered the field at all — a
+lead could sit in `unqualified` with no recorded reason, and the seeded demo
+leads did. A `disqualification_reason_required` script validation now enforces
+it, modelled on `crm_case.escalation_reason_required`. Because a rule with no
+writer is worse than no rule — the save fails with an error the form gives the
+user no way to clear — the field is now on every `crm_lead` form that exposes an
+editable `status`, shown only when the status is `unqualified`, and on the lead
+detail page beside the status. The four generated `unqualified` seed leads
+carry a reason (rotated across four values, each with a matching note) instead
+of a budget-flavoured note and nothing else.
+
+**`crm_campaign` and `crm_task` had no `fieldGroups`.** They were the last two
+business objects with a full detail page and zero groups, so both rendered as
+one flat grid — the campaign's ROI formulas inline with its name, the task's
+five polymorphic `related_to_*` lookups and recurrence machinery inline with its
+subject. Both now declare groups mirroring the sections their forms already use,
+and every field is assigned to one. The two line-item objects stay ungrouped:
+they are edited inline in the parent's grid and have no detail page to section.
+
+**`priority_rank` diverged between the two objects that use it.** The ordinal
+exists because sorting on the `priority` select compares raw strings and inverts
+urgency. Its unknown-priority fallback was `1` on `crm_case` and `2` on
+`crm_task`, with field defaults to match, so the same unrecognised priority
+sorted differently on the two objects — and on each it was indistinguishable
+from a genuine priority (`low` / `normal` respectively). Both now use `0`, an
+unranked sentinel that sorts below every real rank on the `priority_rank desc`
+queues. The known ranks (1–4) are unchanged, so no seeded or stored row moves.
+
+The two rank maps stay hand-copied on purpose: L2 hook bodies run body-only in
+the QuickJS sandbox, so a shared module constant resolves at authoring time and
+arrives as `undefined` (see `_line-item-price-fill.ts`). Since the duplication
+is forced, `test/priority-rank-parity.test.ts` is what keeps the copies in
+agreement — it drives both hooks and asserts they rank an unknown priority
+identically and consistently with each object's field default.
+
+New guards live in three new test files rather than being appended to
+`test/metadata-references.test.ts`. Beyond the parity check above,
+`test/field-groups-coverage.test.ts` requires every detail-page object to
+declare groups and additionally catches the silent failures around them — a
+field pointing at an undeclared group key vanishes from the layout, and a
+declared group with no fields renders an empty section header.

src/data/index.ts

@@ -220,6 +220,23 @@ const contacts = defineSeed(Contact, {
 });
 
 // ─── Leads ────────────────────────────────────────────────────────────
+/**
+ * Disqualification reasons for the generated `unqualified` demo leads, paired
+ * with the note that explains them.
+ *
+ * `crm_lead.disqualification_reason_required` rejects an unqualified lead that
+ * carries no reason, so every seeded unqualified row must supply one — the rows
+ * used to carry a budget-flavoured note and no reason at all. Rotated over four
+ * values so the demo's disqualification breakdown has more than one bar, and
+ * the note always matches the reason it sits next to.
+ */
+const DISQUALIFICATION_REASONS = [
+  { reason: 'no_budget',     note: 'No budget approved for this fiscal year — revisit next planning cycle.' },
+  { reason: 'not_a_fit',     note: 'Requirements sit outside what the product covers today.' },
+  { reason: 'wrong_persona', note: 'Contact has no say in the buying decision and no path to an owner.' },
+  { reason: 'unreachable',   note: 'Six touches across email and phone over four weeks, no response.' },
+] as const;
+
 const leads = defineSeed(Lead, {
   mode: 'upsert',
   externalId: 'email',
@@ -312,7 +329,10 @@ const leads = defineSeed(Lead, {
           `Inbound via ${l.src.replace(/_/g, ' ')}. Evaluating a CRM to replace spreadsheets ` +
           `across their ${l.ind.replace(/_/g, ' ')} operation.`,
         ...(status === 'unqualified'
-          ? { notes: 'No budget approved for this fiscal year — revisit next planning cycle.' }
+          ? {
+              disqualification_reason: DISQUALIFICATION_REASONS[(i >> 2) % DISQUALIFICATION_REASONS.length].reason,
+              notes: DISQUALIFICATION_REASONS[(i >> 2) % DISQUALIFICATION_REASONS.length].note,
+            }
           : {}),
       };
     }),

src/objects/campaign.object.ts

@@ -26,16 +26,32 @@ export const Campaign = ObjectSchema.create({
   // search silently return zero. These are real, indexed columns.
   searchableFields: ['name', 'campaign_code'],
   highlightFields: ['campaign_code', 'name', 'type', 'status', 'start_date'],
-
+
+  // Every other business object with a detail page groups its fields; campaign
+  // was one of the two that did not, so its detail page fell back to one flat
+  // 30-field grid with the ROI formulas sitting next to the campaign name. The
+  // keys mirror the sections the campaign form already uses (Overview /
+  // Schedule & Budget / Performance) so the two surfaces agree.
+  fieldGroups: [
+    { key: 'basic',      label: 'Campaign Information', icon: 'megaphone' },
+    { key: 'schedule',   label: 'Schedule',             icon: 'calendar' },
+    { key: 'budget',     label: 'Budget & ROI',         icon: 'dollar-sign' },
+    { key: 'metrics',    label: 'Performance',          icon: 'bar-chart' },
+    { key: 'assignment', label: 'Ownership',            icon: 'user' },
+    { key: 'assets',     label: 'Campaign Assets',      icon: 'link', defaultExpanded: false },
+  ],
+
   fields: {
     // AutoNumber field
     campaign_code: Field.autonumber({
+      group: 'basic',
       label: 'Campaign Code',
       format: 'CPG-{0000}',
     }),
 
     // Basic Information
     name: Field.text({
+      group: 'basic',
       label: 'Campaign Name',
       required: true,
       searchable: true,
@@ -44,16 +60,19 @@ export const Campaign = ObjectSchema.create({
 
     // ADR-0079 record title (was titleFormat '{campaign_code} - {name}').
     display_title: Field.formula({
+      group: 'basic',
       label: 'Display Title',
       expression: F`record.campaign_code + " - " + record.name`,
     }),
 
     description: Field.markdown({
+      group: 'basic',
       label: 'Description',
     }),
 
     // Type & Channel
     type: Field.select({
+      group: 'basic',
       label: 'Campaign Type',
       options: [
         { label: 'Email', value: 'email', default: true },
@@ -68,6 +87,7 @@ export const Campaign = ObjectSchema.create({
     }),
 
     channel: Field.select({
+      group: 'basic',
       label: 'Primary Channel',
       options: [
         { label: 'Digital', value: 'digital' },
@@ -80,6 +100,7 @@ export const Campaign = ObjectSchema.create({
 
     // Status
     status: Field.select({
+      group: 'basic',
       label: 'Status',
       options: [
         { label: 'Planning', value: 'planning', color: '#999999', default: true },
@@ -93,29 +114,34 @@ export const Campaign = ObjectSchema.create({
 
     // Dates
     start_date: Field.date({
+      group: 'schedule',
       label: 'Start Date',
       required: true,
     }),
 
     end_date: Field.date({
+      group: 'schedule',
       label: 'End Date',
       required: true,
     }),
 
     // Budget & ROI
     budgeted_cost: Field.currency({ 
+      group: 'budget',
       label: 'Budgeted Cost',
       scale: 2,
       min: 0,
     }),
 
     actual_cost: Field.currency({ 
+      group: 'budget',
       label: 'Actual Cost',
       scale: 2,
       min: 0,
     }),
 
     expected_revenue: Field.currency({ 
+      group: 'budget',
       label: 'Expected Revenue',
       scale: 2,
       min: 0,
@@ -127,13 +153,15 @@ export const Campaign = ObjectSchema.create({
     // already opened up. With readonly on, the completion snapshot silently
     // persisted nothing but num_sent.
     actual_revenue: Field.currency({ 
+      group: 'budget',
       label: 'Actual Revenue',
       scale: 2,
       min: 0,
     }),
 
     // Metrics
     target_size: Field.number({
+      group: 'metrics',
       label: 'Target Size',
       description: 'Target number of leads/contacts',
       min: 0,
@@ -142,67 +170,78 @@ export const Campaign = ObjectSchema.create({
     // NOT `readonly`: the campaign_snapshot_metrics hook writes this rollup
     // (16.x drops readonly writes, #2948). Definition: total members enrolled.
     num_sent: Field.number({
+      group: 'metrics',
       label: 'Number Sent',
       min: 0,
     }),
 
     num_responses: Field.number({
+      group: 'metrics',
       label: 'Number of Responses',
       min: 0,
     }),
 
     num_leads: Field.number({
+      group: 'metrics',
       label: 'Number of Leads',
       min: 0,
     }),
 
     num_converted_leads: Field.number({
+      group: 'metrics',
       label: 'Converted Leads',
       min: 0,
     }),
 
     num_opportunities: Field.number({
+      group: 'metrics',
       label: 'Opportunities Created',
       min: 0,
     }),
 
     num_won_opportunities: Field.number({
+      group: 'metrics',
       label: 'Won Opportunities',
       min: 0,
     }),
 
     // Calculated Metrics (Formula Fields)
     response_rate: Field.formula({
+      group: 'metrics',
       label: 'Response Rate %',
       expression: F`coalesce(record.num_sent, 0) > 0 ? (coalesce(record.num_responses, 0) * 100.0) / record.num_sent : 0.0`,
       scale: 2,
     }),
 
     roi: Field.formula({
+      group: 'budget',
       label: 'ROI %',
       expression: F`coalesce(record.actual_cost, 0) > 0 ? ((coalesce(record.actual_revenue, 0) - record.actual_cost) * 100.0) / record.actual_cost : 0.0`,
       scale: 2,
     }),
 
     // Relationships
     parent_campaign: Field.lookup('crm_campaign', {
+      group: 'basic',
       label: 'Parent Campaign',
       description: 'Parent campaign in hierarchy',
     }),
 
     owner: Field.lookup('sys_user', {
-
+      group: 'assignment',
       defaultValue: cel`os.user.id`,
       label: 'Campaign Owner',
       trackHistory: true,
     }),
 
     // Campaign Assets
     landing_page_url: Field.url({
+      group: 'assets',
       label: 'Landing Page',
     }),
 
     is_active: Field.boolean({
+      group: 'assets',
       label: 'Active',
       defaultValue: true,
     }),

src/objects/case.hook.ts

@@ -42,9 +42,17 @@ const caseValidation: Hook = {
     // Materialise the urgency ordinal so queue views can sort by it. Sorting
     // on `priority` itself compares raw strings and inverts urgency
     // (medium > low > high > critical).
+    //
+    // The map is declared INLINE and duplicated in task.hook.ts on purpose: L2
+    // hook bodies run body-only in the QuickJS sandbox, so a shared module
+    // constant resolves at authoring time and arrives as `undefined` (see
+    // _line-item-price-fill.ts). The two maps key off different vocabularies
+    // (medium vs normal), but the UNKNOWN fallback must stay identical on both
+    // objects — `0`, the unranked sentinel that sorts below every real rank.
+    // `test/priority-rank-parity.test.ts` pins that agreement.
     if (priority) {
       const rank: Record<string, number> = { low: 1, medium: 2, high: 3, critical: 4 };
-      input.priority_rank = rank[priority] ?? 1;
+      input.priority_rank = rank[priority] ?? 0;
     }
 
     if (priority === 'critical' && !input.sla_due_date && !ctx.previous?.sla_due_date) {

src/objects/case.object.ts

@@ -109,10 +109,16 @@ export const Case = ObjectSchema.create({
     // buried every critical case at the bottom of the agent's queue. Select
     // options carry no ordinal in the spec, so the rank is materialised here
     // and stamped by `case_sla_defaults`; queue views sort on it instead.
+    // `0` is the UNRANKED sentinel, shared with crm_task.priority_rank: it is
+    // what a record carries when no recognised priority has been stamped, and
+    // it sorts below every real rank (1–4) on the `priority_rank desc` queues.
+    // It used to be `1` here and `2` on crm_task, so the same unknown priority
+    // ordered differently on the two objects — and on this object it was
+    // indistinguishable from a genuine `low`.
     priority_rank: Field.number({
       label: 'Priority Rank',
       readonly: true,
-      defaultValue: 1,
+      defaultValue: 0,
     }),
 
     type: Field.select({

src/objects/lead.object.ts

@@ -303,6 +303,19 @@ export const Lead = ObjectSchema.create({
       message: 'Email is required',
       condition: P`isBlank(record.email)`,
     },
+    {
+      // The field description has promised "Required when status is
+      // Unqualified" since the field was added, and nothing enforced it — a
+      // lead could sit in `unqualified` with no recorded reason, which is the
+      // one datum a disqualification review needs. Same shape as
+      // `crm_case.escalation_reason_required` (the repo's existing
+      // "required when state is X" idiom).
+      name: 'disqualification_reason_required',
+      type: 'script',
+      severity: 'error',
+      message: 'Disqualification reason is required when a lead is Unqualified',
+      condition: P`record.status == "unqualified" && isBlank(record.disqualification_reason)`,
+    },
     {
       name: 'cannot_edit_converted',
       type: 'script',

src/objects/task.hook.ts

@@ -36,9 +36,16 @@ const taskValidation: Hook = {
       (typeof input.priority === 'string' && input.priority) ||
       (typeof previous?.priority === 'string' && (previous.priority as string)) ||
       undefined;
+    //
+    // Inline and duplicated in case.hook.ts on purpose — L2 hook bodies run
+    // body-only in the QuickJS sandbox, so a shared module constant resolves at
+    // authoring time and arrives as `undefined` (see _line-item-price-fill.ts).
+    // The vocabularies differ (normal vs medium), but the UNKNOWN fallback must
+    // match crm_case: `0`, the unranked sentinel that sorts below every real
+    // rank. `test/priority-rank-parity.test.ts` pins that agreement.
     if (effPriority) {
       const rank: Record<string, number> = { low: 1, normal: 2, high: 3, urgent: 4 };
-      input.priority_rank = rank[effPriority] ?? 2;
+      input.priority_rank = rank[effPriority] ?? 0;
     }
 
     if (input.status === 'completed' && previous?.status !== 'completed') {