Retire dead lead/opportunity rules, stamp first response, guard CPQ lifecycles (#575 group B)

.changeset/lifecycle-guards-and-dead-fields.md

@@ -0,0 +1,49 @@
+---
+'hotcrm': patch
+---
+
+Retire two rules that could never fire, give `crm_case.first_response_date` its
+missing writer, and constrain the quote and contract lifecycles (#575 group B).
+
+`crm_lead`'s `cannot_edit_converted` validation is deleted. The code described
+it as the friendly, recoverable half of a two-layer converted-lead lock, but the
+`beforeUpdate` throw in `lead.hook.ts` aborts the write first, so the validation
+never produced that error on any field — the same dead-configuration shape as
+the `revenue_positive` rule removed in #571. The hook is now the single guard,
+and its message (which names the offending fields) is the one users see.
+
+`crm_opportunity.created_date` is deleted. It duplicated the platform's own
+`created_at` and had no writer at all, so it was null on every row while
+`deal_timeline` used it as `startDateField` — the timeline had no start dates.
+The view now reads `created_at` (the spelling the lead activity calendar already
+used) and the four locale packs no longer label a field that does not exist.
+`crm_case.created_date` is a different field with a real writer and is untouched.
+
+`crm_case.first_response_date` was the only member of the case SLA family with
+no writer — `sla_due_date` and `resolution_time_hours` come from `case.hook`,
+`is_sla_violated` from the `case_sla_monitor` flow — so the most standard
+service-desk metric was permanently null. It is now stamped by the shared
+`logActivityAction` body, on the first `sys_activity` a case receives: the
+industry definition (Salesforce `FirstResponseDateTime`, Zendesk first reply
+time) is when the customer first heard back, so a logged call or meeting is the
+event, deliberately NOT a status change — an agent can move a case to "in
+progress" and investigate for an hour while the customer hears nothing. The
+field drops `readonly`, which would otherwise silently discard the write
+(#2948), and the body reads the stored value rather than the dispatched record
+so a projected record cannot turn "first response" into "last response".
+
+`crm_quote` and `crm_contract` gain `state_machine` validations. Neither had a
+transition table OR a status guard in its hook, so `draft → accepted` on a quote
+(binding numbers nobody reviewed or sent) and `draft → activated` on a contract
+(which stamps `signed_date`, promotes the account to `customer` and starts the
+renewal clock) were both legal. Warning severity, matching the lead / opportunity
+/ case machines. `crm_campaign` and `crm_task` deliberately get nothing — their
+status is descriptive, not a controlled lifecycle — and a new test pins that
+absence so it stays a decision rather than a gap.
+
+New guards live in `test/converted-lead-guard.test.ts`,
+`test/case-first-response.test.ts`, `test/opportunity-creation-date.test.ts` and
+`test/status-state-machines.test.ts`. The first-response tests run the shipped
+action body through the real QuickJS sandbox added in #575 A1, because the two
+things that can break the stamp — the `api.read` capability and the engine
+facade's `update(data, options)` signature — only exist there.

docs/developers/api_reference.md

@@ -36,7 +36,7 @@ Source: `src/objects/opportunity.object.ts`
 
 Key fields:
 
-`name`, `crm_account`, `primary_contact`, `owner`, `amount`, `expected_revenue`, `stage`, `probability`, `close_date`, `created_date`, `stage_entry_date`, `type`, `lead_source`, `competitors`, `crm_campaign`, `days_in_stage`, `next_step`, `is_private`, `forecast_category`, `approval_status`, `approved_date`, `win_reason`, `loss_reason`, `loss_details`
+`name`, `crm_account`, `primary_contact`, `owner`, `amount`, `expected_revenue`, `stage`, `probability`, `close_date`, `stage_entry_date`, `type`, `lead_source`, `competitors`, `crm_campaign`, `days_in_stage`, `next_step`, `is_private`, `forecast_category`, `approval_status`, `approved_date`, `win_reason`, `loss_reason`, `loss_details`
 
 ### `crm_opportunity_line_item` - Opportunity Line Item
 

src/actions/global.actions.ts

@@ -67,6 +67,11 @@ type LogActivitySpec = {
  * is the direction that keeps both forms submittable, and it is the one the
  * body was already written for — the `duration ? … : subject` summary branch
  * is unreachable while the field is mandatory.
+ *
+ * The shared body is also where `crm_case.first_response_date` is stamped
+ * (#575 B2) — because every activity twin routes through here, "the first
+ * outbound contact on a case" has exactly one implementation instead of one
+ * per action.
  */
 function logActivityAction(spec: LogActivitySpec): Action {
   const extras = Object.entries(spec.metadataExtras)
@@ -117,9 +122,47 @@ function logActivityAction(spec: LogActivitySpec): Action {
         record_label: record[nameField] ?? null,
         metadata: JSON.stringify({ kind: '${spec.kind}', duration_minutes: duration, notes, ${extras} }),
       });
+      // SLA first-response stamp (#575 B2). \`first_response_date\` was the one
+      // member of the case SLA family with no writer at all — \`sla_due_date\`
+      // and \`resolution_time_hours\` come from case.hook, \`is_sla_violated\`
+      // from the case_sla_monitor flow — so the metric was permanently null.
+      // A logged call or meeting is the only record of outbound contact a case
+      // carries, which makes the FIRST \`sys_activity\` on the case the moment
+      // the customer first heard back: the industry definition (Salesforce
+      // \`FirstResponseDateTime\`, Zendesk first reply time). A status change is
+      // deliberately NOT used — an agent can move a case to "in progress" and
+      // investigate for an hour while the customer hears nothing.
+      //
+      // CONVENTION: any future customer-facing path on a case (a reply-email
+      // action, an inbound portal reply) MUST stamp this too, or the metric
+      // silently under-reports.
+      //
+      // The stored value is read rather than taken from \`ctx.record\`: the
+      // list_item / record_related dispatch paths hand the body a PROJECTED
+      // record, and a field missing from that projection reads as blank — which
+      // would re-stamp on every log and turn "first response" into "last".
+      if (objectName === 'crm_case' && recordId) {
+        const raw = await ctx.api.object('crm_case').find({
+          where: { id: recordId },
+          fields: ['first_response_date'],
+          top: 1,
+        });
+        const found = Array.isArray(raw) ? raw : (raw?.records ?? []);
+        const stored = found.length ? found[0].first_response_date : record.first_response_date;
+        if (!stored) {
+          // \`update(data, options)\` — \`ctx.api\` is the engine repo facade,
+          // whose update takes a DOCUMENT, not an id (mass_update_stage is the
+          // action that got this wrong; test/action-sandbox.test.ts pins the
+          // contract against a real kernel).
+          await ctx.api.object('crm_case').update(
+            { id: recordId, first_response_date: new Date().toISOString() },
+            { where: { id: recordId } },
+          );
+        }
+      }
       return { activityId: activity?.id };
     `,
-      capabilities: ['api.write'],
+      capabilities: ['api.read', 'api.write'],
       timeoutMs: 5000,
     },
     locations: ['record_header', 'list_item', 'record_related'],

src/objects/case.object.ts

@@ -165,10 +165,19 @@ export const Case = ObjectSchema.create({
       readonly: true,
     }),
 
+    // NOT `readonly`: stamped by the `log_call` / `log_meeting` activity
+    // actions (`src/actions/global.actions.ts`), and 16.x drops writes to
+    // readonly fields on user-context writes (#2948) — an action body runs as
+    // the acting user, so `readonly` here silently disabled the stamp. Same
+    // reason `is_sla_violated` and `escalated_date` below are not readonly.
+    //
+    // Definition: the moment the customer first heard back from us, matching
+    // Salesforce `FirstResponseDateTime` / Zendesk first reply time — NOT an
+    // internal status change, which would report "responded" while the
+    // customer is still waiting (#575 B2).
     first_response_date: Field.datetime({
       label: 'First Response Date',
       group: 'sla',
-      readonly: true,
     }),
 
     resolution_time_hours: Field.number({

src/objects/contract.object.ts

@@ -230,6 +230,31 @@ export const Contract = ObjectSchema.create({
     // dropped in the 9.8.0 upgrade — its CEL predicate used `monthsBetween()`,
     // which 9.8.0's aligned CEL stdlib (ADR-0032) no longer provides. It was a
     // non-blocking warning; the hard end-date>start-date rule above remains.
+    {
+      // #575 B4. Like `crm_quote`, this object shipped a lifecycle vocabulary
+      // with no transition constraint and no status guard in `contract.hook.ts`
+      // — so `draft → activated` was a legal move, activating an agreement that
+      // never passed approval. Activation is the consequential edge here:
+      // `contract_on_activation` stamps `signed_date`, promotes the account to
+      // `customer`, and the renewal flow starts counting from it.
+      name: 'contract_status_progression',
+      type: 'state_machine',
+      severity: 'warning',
+      message: 'Invalid contract status transition',
+      field: 'status',
+      transitions: {
+        draft: ['in_approval', 'terminated'],
+        // Approval either lands (`activated`) or sends the paperwork back.
+        in_approval: ['draft', 'activated', 'terminated'],
+        // `→ expired` is the contract_expiration flow, whose own sweep filter
+        // is `status: 'activated'` — nothing else ages out.
+        activated: ['expired', 'terminated'],
+        // Both ends are terminal: a renewal is a NEW contract (the
+        // contract_renewal flow files a task, it does not revive the old row).
+        expired: [],
+        terminated: [],
+      },
+    },
   ],
 
   // Workflow Rules

src/objects/lead.hook.ts

@@ -157,10 +157,15 @@ const leadHook: Hook = {
     // Converted-lead lock — USER edits only (`ctx.user?.id` is this repo's
     // system-write signal, cf. opportunity/quote/account hooks): a blanket
     // throw also rejected system writes (demo-bootstrap owner claims, flow
-    // backfills) and blocked ALL fields, far beyond the schema's own
-    // `cannot_edit_converted` validation (identity fields only). Narrative
-    // notes and framework-managed columns stay editable; identity and
-    // conversion fields stay locked.
+    // backfills). Narrative notes and framework-managed columns stay editable;
+    // identity and conversion fields stay locked.
+    //
+    // This is the ONLY converted-lead guard. `crm_lead` also carried a
+    // `cannot_edit_converted` script validation over the four identity fields,
+    // documented as the friendlier half of a two-layer design; #575 B1 removed
+    // it because this throw always won the race, so the second layer was a
+    // second implementation that could only drift. The message below therefore
+    // has to carry the whole story — hence the attempted-field list.
     if (event === 'beforeUpdate' && ctx.user?.id) {
       const previous = ctx.previous;
       const wasConverted = previous?.is_converted === true || previous?.status === 'converted';

src/objects/lead.object.ts

@@ -157,10 +157,15 @@ export const Lead = ObjectSchema.create({
     // Conversion tracking.
     // NOT `readonly`: since 16.x the platform drops writes to readonly fields
     // outright (#2948), including the lead_conversion flow's mark_converted
-    // update. Edit-protection comes from two places: the `cannot_edit_converted`
-    // validation below (4 identity fields, recoverable error) and the broader
-    // beforeUpdate guard in lead.hook.ts, which rejects ANY edit to a
-    // converted lead.
+    // update. Edit-protection is the beforeUpdate guard in lead.hook.ts, which
+    // rejects any USER edit to a converted lead outside a small allow-list.
+    // A `cannot_edit_converted` validation used to sit beside it covering the
+    // four identity fields, described as the friendlier recoverable half of a
+    // two-layer design. It was dead configuration and was removed in #575 B1:
+    // the hook's beforeUpdate throws first, so the validation never produced
+    // the error it promised (measured on 16.1.0 — a `PATCH company` on a
+    // converted lead returns the hook's message). Same shape as the
+    // `revenue_positive` rule removed in #571.
     is_converted: Field.boolean({
       label: 'Converted',
       defaultValue: false,
@@ -316,13 +321,6 @@ export const Lead = ObjectSchema.create({
       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',
-      severity: 'error',
-      message: 'Cannot edit a converted lead',
-      condition: P`record.is_converted == true && (record.company != previous.company || record.email != previous.email || record.first_name != previous.first_name || record.last_name != previous.last_name)`,
-    },
     {
       // Migrated from the removed top-level `stateMachines` key (LeadStateMachine).
       name: 'lead_status_progression',

src/objects/opportunity.object.ts

@@ -120,12 +120,12 @@ export const Opportunity = ObjectSchema.create({
       trackHistory: true,
     }),
 
-    created_date: Field.datetime({
-      label: 'Created Date',
-      readonly: true,
-      group: 'sales_process',
-    }),
-
+    // NO `created_date` here: the platform already injects `created_at` on
+    // every object, and this duplicate had no writer at all — not the seed
+    // data, not a hook, not a flow — so it was permanently null while
+    // `created_at` carried the real value (#575 B2). Surfaces that need the
+    // creation instant read `created_at` (see the deal_timeline view).
+    //
     // Stage-age clock (#489). This is the STORED half of the pair: a real,
     // indexed date column, so it is what automation and views may filter and
     // sort on. `days_in_stage` below is a formula derived from it.

src/objects/quote.object.ts

@@ -235,6 +235,35 @@ export const Quote = ObjectSchema.create({
       message: 'Discount cannot exceed 100%',
       condition: P`record.discount != null && record.discount > 100`,
     },
+    {
+      // #575 B4. `crm_quote` had a full lifecycle vocabulary and no transition
+      // constraint whatsoever — nor a status guard in `quote.hook.ts` — so a
+      // quote could go straight from `draft` to `accepted`, which in CPQ terms
+      // is a signed number nobody reviewed or sent. `warning` severity matches
+      // the lead / opportunity / case machines: it flags the jump on the record
+      // without blocking a support-driven correction.
+      name: 'quote_status_progression',
+      type: 'state_machine',
+      severity: 'warning',
+      message: 'Invalid quote status transition',
+      field: 'status',
+      transitions: {
+        // `→ expired` is legal from every UNSETTLED state: the quote_expiration
+        // flow sweeps on `expiration_date` alone and expires drafts that were
+        // never sent as readily as presented ones.
+        draft: ['in_review', 'expired'],
+        in_review: ['draft', 'presented', 'rejected', 'expired'],
+        presented: ['accepted', 'rejected', 'expired'],
+        // `accepted` and `expired` are terminal, and not only by convention —
+        // `quote_pricing_guard` in quote.hook.ts freezes both, allowing edits
+        // to `internal_notes` and nothing else.
+        accepted: [],
+        expired: [],
+        // A rejected quote is not frozen: the rep revises the numbers and
+        // re-issues, which starts again at `draft`.
+        rejected: ['draft'],
+      },
+    },
   ],
 
   // Workflow Rules

src/translations/en.ts

@@ -310,7 +310,6 @@ export const en: TranslationData = {
         },
         description: { label: 'Description' },
         next_step: { label: 'Next Step' },
-        created_date: { label: 'Created Date' },
         lead_source: {
           label: 'Lead Source',
           options: {

src/translations/es-ES.ts

@@ -288,7 +288,6 @@ export const esES: TranslationData = {
         },
         description: { label: 'Descripción' },
         next_step: { label: 'Próximo Paso' },
-        created_date: { label: 'Fecha de Creación' },
         lead_source: {
           label: 'Origen del Prospecto',
           options: {

src/translations/ja-JP.ts

@@ -288,7 +288,6 @@ export const jaJP: TranslationData = {
         },
         description: { label: '説明' },
         next_step: { label: '次のステップ' },
-        created_date: { label: '作成日' },
         lead_source: {
           label: 'リードソース',
           options: {

src/translations/zh-CN.ts

@@ -725,7 +725,6 @@ export const zhCN: TranslationData = {
         },
         description: { label: '描述' },
         next_step: { label: '下一步' },
-        created_date: { label: '创建日期' },
         lead_source: {
           label: '线索来源',
           options: {

src/views/opportunity.view.ts

@@ -146,7 +146,11 @@ export const OpportunityViews = defineView({
       data: { provider: 'object', object: 'crm_opportunity' },
       columns: ['name', 'crm_account', 'amount'],
       timeline: {
-        startDateField: 'created_date',
+        // `created_at`, the platform's own creation stamp — the object's
+        // duplicate `created_date` was removed in #575 B2 because nothing ever
+        // wrote it, so this timeline started every bar at null. Same spelling
+        // as the lead activity calendar.
+        startDateField: 'created_at',
         endDateField: 'close_date',
         titleField: 'name',
         groupByField: 'owner',