From b685aeab9b1d0395b80335db01947f82a8530113 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 19:48:42 +0000 Subject: [PATCH 1/2] feat(opportunity): require a win/loss reason at close, and measure win rate (#593) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `crm_opportunity.win_reason` / `loss_reason` / `loss_details` existed since the object was written, and `loss_reason` even carried the comment "required when stage moves to closed_*" — while nothing required anything. Both columns were empty on every record, seeded ones included, and no report or widget read them. Capture: both fields carry a `requiredWhen` predicate (`loss_reason` on closed_lost, `win_reason` on closed_won), evaluated by the engine on insert AND update and reported against the field. Measured against a real ObjectQL over both `InMemoryDriver` (sparse stored rows) and a real SQLite database: the write is REJECTED and the record stays in its previous stage — not a WARN, and not a dry-run refusal with the write landing anyway. `crm_case`'s `resolution_required_for_closed`, the pattern the issue names, was re-measured the same way before being copied and is pinned so it cannot rot silently. `win_reason` gains a `quote_accepted` option: `quote_on_accepted` closes the deal with no human in the write, so it names the automated path rather than stamping a fabricated rep answer, and the rule stays exception-free. Analytics: `opportunity_metrics` gains `won_count` / `lost_count` / `decided_count` / `won_amount` / `lost_amount` (each with its own measure filter), `win_rate` as a derived ratio, and `win_reason` / `loss_reason` dimensions. The Sales dashboard gains a Win Rate (12M) tile flanked by its two inputs, Win/Loss by Rep and by Lead Source tables, and a Why We Lose breakdown. Both halves of the ratio come from the measures, never from a widget filter — a widget-level stage filter narrows numerator and denominator alike, which is how #614 shipped. Tests perturb one deal at a time and assert the rate falls, rises, and does not move for open pipeline. Seeds: every settled deal carries its reason (the engine now rejects the boot otherwise), plus three more lost deals so the breakdown has five distinct reasons and three lead sources carry both a win and a loss. i18n: win_reason / loss_reason translated in all four locales — they were missing from ja-JP and es-ES, which would have put raw stored values into a required picklist and a chart legend. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019SS7C5SXpniKeCApxgARyf --- .../win-loss-reason-capture-and-analytics.md | 56 ++ content/docs/analytics/dashboards.mdx | 4 +- content/docs/analytics/dashboards.zh-Hans.mdx | 4 +- content/docs/analytics/dashboards.zh-Hant.mdx | 4 +- content/docs/sales/opportunities.mdx | 22 +- content/docs/sales/opportunities.zh-Hans.mdx | 22 +- content/docs/sales/opportunities.zh-Hant.mdx | 22 +- src/dashboards/sales.dashboard.ts | 180 +++- src/data/sales.seed.ts | 90 +- src/datasets/opportunity.dataset.ts | 49 + src/objects/opportunity.object.ts | 46 +- src/objects/quote.hook.ts | 8 + src/translations/en.ts | 3 +- src/translations/es-ES.ts | 21 + src/translations/ja-JP.ts | 20 + src/translations/zh-CN.ts | 3 +- test/flow-condition-totality.test.ts | 10 +- test/metadata-references.test.ts | 8 +- test/win-loss-capture.test.ts | 851 ++++++++++++++++++ 19 files changed, 1397 insertions(+), 26 deletions(-) create mode 100644 .changeset/win-loss-reason-capture-and-analytics.md create mode 100644 test/win-loss-capture.test.ts diff --git a/.changeset/win-loss-reason-capture-and-analytics.md b/.changeset/win-loss-reason-capture-and-analytics.md new file mode 100644 index 00000000..28931973 --- /dev/null +++ b/.changeset/win-loss-reason-capture-and-analytics.md @@ -0,0 +1,56 @@ +--- +'hotcrm': patch +--- + +Closing an opportunity now REQUIRES a win or loss reason, and the Sales dashboard finally shows what those reasons say. + +`crm_opportunity.win_reason` / `loss_reason` / `loss_details` have existed since +the object was written, and `loss_reason` even carried the comment *"required +when stage moves to closed_\*"* — but nothing required anything. Both columns +were empty on every record, including every seeded one, and no report or +dashboard widget read them. Declared, unenforced, unused. + +**Capture.** Both fields carry a `requiredWhen` predicate — `loss_reason` on +`closed_lost`, `win_reason` on `closed_won` — which the engine evaluates inside +`evaluateValidationRules` on insert AND update, and reports against the field so +the form marks the empty picklist. This is server-side: an API call, a data +import or a bulk update that closes a deal without the applicable reason is +rejected exactly like a rep's form is, and the record stays in its previous +stage. Capturing at close time is not a style choice — a closed opportunity is +frozen to its narrative fields, so a reason not recorded in the closing write can +never be added afterwards. + +`win_reason` gains one option, **Quote Accepted**. An accepted quote closes its +opportunity automatically (`quote_on_accepted`), and there is no human in that +write to attribute the win; naming the automated path keeps the rule +exception-free and keeps a CPQ close distinguishable from a rep's answer, rather +than stamping a fabricated "Better Product" on it. + +**Analytics.** `opportunity_metrics` gains `won_count`, `lost_count`, +`decided_count`, `won_amount`, `lost_amount` (each scoped by its own measure +filter) and `win_rate = won_count / decided_count` as a derived ratio, plus +`win_reason` / `loss_reason` dimensions. The Sales dashboard gains a **Win Rate +(12M)** tile flanked by **Deals Won** and **Deals Lost**, **Win / Loss by Rep** +and **Win / Loss by Lead Source** tables, and a **Why We Lose** loss-reason +breakdown. + +The ratio's two halves come from the *measures*, never from a widget filter: a +widget-level `stage` filter narrows numerator and denominator together, which is +how a ratio quietly becomes a division by itself. Every breakdown is a table +showing won, lost and settled counts beside the percentage, so the arithmetic +behind the number is always on screen — the check the quota table shipped +without in #614. Tests perturb one deal at a time (win a lost deal, lose a won +one, add open pipeline) and assert the rate moves, moves the other way, and does +not move, respectively. + +**Seeds.** Every settled seeded deal now carries its reason, and three more lost +deals were added so the loss-reason breakdown has five distinct reasons and three +lead sources carry both a win and a loss. Out of the box the demo shows a 62% +win rate over 8 won and 5 lost deals. + +**i18n.** `win_reason` and `loss_reason` are now translated in all four locales; +they had been missing from `ja-JP` and `es-ES`, which would have put raw stored +values (`no_budget`, `quote_accepted`) into a picklist a rep is forced to choose +from and into a chart legend. + +Fixes #593. diff --git a/content/docs/analytics/dashboards.mdx b/content/docs/analytics/dashboards.mdx index 3f20360a..637f984b 100644 --- a/content/docs/analytics/dashboards.mdx +++ b/content/docs/analytics/dashboards.mdx @@ -42,7 +42,9 @@ The sales team's daily operational dashboard. - **Pipeline by Stage** — funnel chart showing $ by stage. - **Forecast vs Quota** — gauge. - **Closed Won this Quarter** — running total with target line. -- **Win Rate (last 90 days)** — percentage with trend. +- **Win Rate (12M)** — deals won as a share of deals *settled* (won + lost) in the last 12 months, shown next to **Deals Won** and **Deals Lost** so the two numbers behind the percentage are always on screen. Open pipeline is not in the denominator. +- **Win / Loss by Rep** and **Win / Loss by Lead Source** — the same ratio broken down, as tables: won, lost, settled, win rate and won revenue per row. +- **Why We Lose** — lost deals by loss reason. Every closed-lost deal carries a reason (it is [required at close](/docs/sales/opportunities)), so this chart has no "unattributed" slice — an empty chart means no losses in the window, not an unfilled field. - **Average Deal Size** — current quarter vs last quarter. - **Average Sales Cycle** — days from creation to close (won deals). - **Top 10 Open Deals** — sortable table. diff --git a/content/docs/analytics/dashboards.zh-Hans.mdx b/content/docs/analytics/dashboards.zh-Hans.mdx index b704800c..add29db0 100644 --- a/content/docs/analytics/dashboards.zh-Hans.mdx +++ b/content/docs/analytics/dashboards.zh-Hans.mdx @@ -42,7 +42,9 @@ HotCRM 内置四个仪表盘。每个都为特定角色而构建,回答该角 - **Pipeline by Stage** — 显示各阶段 $ 的漏斗图。 - **Forecast vs Quota** — 仪表盘图。 - **Closed Won this Quarter** — 带目标线的累计总额。 -- **Win Rate (last 90 days)** — 带趋势的百分比。 +- **Win Rate (12M)** — 近 12 个月内**已结案**(赢单 + 丢单)商机中的赢单占比,旁边同时展示 **Deals Won** 与 **Deals Lost**,让分子分母始终可见。进行中的管道不计入分母。 +- **Win / Loss by Rep** 与 **Win / Loss by Lead Source** — 同一比值的拆分表:每行给出赢单数、丢单数、已结案数、赢率与赢单金额。 +- **Why We Lose** — 按丢单原因统计的丢单数。每条丢单商机都必须填写原因([关单时强制](/docs/sales/opportunities)),因此图中不存在“未归因”切片——图为空表示该时间窗内没有丢单,而不是没人填字段。 - **Average Deal Size** — 本季度对比上季度。 - **Average Sales Cycle** — 从创建到成交的天数(赢单交易)。 - **Top 10 Open Deals** — 可排序的表格。 diff --git a/content/docs/analytics/dashboards.zh-Hant.mdx b/content/docs/analytics/dashboards.zh-Hant.mdx index b17b35e5..6852f884 100644 --- a/content/docs/analytics/dashboards.zh-Hant.mdx +++ b/content/docs/analytics/dashboards.zh-Hant.mdx @@ -42,7 +42,9 @@ HotCRM 內建四個儀表板。每個都為特定角色而建構,回答該角 - **Pipeline by Stage** — 顯示各階段 $ 的漏斗圖。 - **Forecast vs Quota** — 量表圖。 - **Closed Won this Quarter** — 帶目標線的累計總額。 -- **Win Rate (last 90 days)** — 帶趨勢的百分比。 +- **Win Rate (12M)** — 近 12 個月內**已結案**(贏單 + 丟單)商機中的贏單占比,旁邊同時顯示 **Deals Won** 與 **Deals Lost**,讓分子分母始終可見。進行中的管道不計入分母。 +- **Win / Loss by Rep** 與 **Win / Loss by Lead Source** — 同一比值的拆分表:每列給出贏單數、丟單數、已結案數、贏率與贏單金額。 +- **Why We Lose** — 按丟單原因統計的丟單數。每筆丟單商機都必須填寫原因([結案時強制](/docs/sales/opportunities)),因此圖中不存在「未歸因」切片——圖為空表示該時間窗內沒有丟單,而不是沒人填欄位。 - **Average Deal Size** — 本季對比上一季。 - **Average Sales Cycle** — 從建立到成交的天數(贏單交易)。 - **Top 10 Open Deals** — 可排序的表格。 diff --git a/content/docs/sales/opportunities.mdx b/content/docs/sales/opportunities.mdx index 409cad7b..d8e84b85 100644 --- a/content/docs/sales/opportunities.mdx +++ b/content/docs/sales/opportunities.mdx @@ -23,6 +23,26 @@ Every opportunity moves through these stages. The system tracks where you are an You can move from any open stage directly to **Closed Lost** (deals can be lost at any point), but you can't skip forward — you have to walk through the funnel one stage at a time. The system blocks invalid jumps to keep your forecast honest. +## Closing a deal — the reason is required + +Closing an opportunity takes one extra field, and the system will not save the record without it: + +| Closing as | You must pick | +| --- | --- | +| 🟢 **Closed Won** | **Win Reason** — Better Product, Better Price, Existing Relationship, Better Support, Best Fit / Features, or Other | +| 🔴 **Closed Lost** | **Loss Reason** — Price Too High, Lost to Competitor, No Budget, No Decision, Bad Timing, Missing Features, or Other | + +**Win/Loss Details** is the free-text box next to it — use it for the sentence a teammate would actually need six months later ("marketing is locked into a 2-year HubSpot contract, revisit at renewal"). + +Two things to know: + +- **This is checked on the server**, not just in the form. An import, an API call or a bulk update that closes a deal without the applicable reason is rejected the same way a rep's form is. +- **Closed deals are locked.** Once a deal is closed, only the narrative fields (description, next steps) stay editable — so the reason cannot be filled in afterwards. Pick it when you close. + +If the deal closes because the customer accepted a quote, HotCRM closes it for you and records the win reason as **Quote Accepted** — see [Quotes](/docs/sales/quotes). + +Those two fields are what the Sales dashboard's **win rate** and **Why We Lose** widgets are built from; see [Dashboards](/docs/analytics/dashboards). + ## What an opportunity record stores The detail screen has 7 sections: @@ -32,7 +52,7 @@ The detail screen has 7 sections: | **Basic Information** | Name, account, primary contact, owner | | **Financials** | Amount, expected revenue *(auto)*, probability *(auto from stage)* | | **Sales Process** | Stage, close date, created date, stage entry date | -| **Classification** | Type (New Business / Upgrade / Renewal / Expansion), lead source | +| **Classification** | Type (New Business / Upgrade / Renewal / Expansion), lead source, win reason, loss reason, win/loss details | | **Competition & Campaigns** | Competitors, source campaign | | **Notes & Next Steps** | Description, next steps | | **Forecast & Metrics** | Line item totals, approval status | diff --git a/content/docs/sales/opportunities.zh-Hans.mdx b/content/docs/sales/opportunities.zh-Hans.mdx index 2eba84f6..f4c52787 100644 --- a/content/docs/sales/opportunities.zh-Hans.mdx +++ b/content/docs/sales/opportunities.zh-Hans.mdx @@ -23,6 +23,26 @@ description: 活跃的销售交易——销售管道的核心,包含 7 个阶 你可以从任何开放阶段直接移动到**输单**(交易随时可能失去),但你不能向前跳过——你必须一次一个阶段地走过漏斗。系统会拦截无效跳转,以保持你的预测真实可靠。 +## 关单必须填写原因 + +关闭商机时必须多填一个字段,否则记录保存不了: + +| 关单方式 | 必填字段 | +| --- | --- | +| 🟢 **赢单** | **赢单原因** —— 产品更优 / 价格更优 / 客户关系 / 支持更好 / 最佳契合 / 其他 | +| 🔴 **输单** | **丢单原因** —— 价格过高 / 输给竞争对手 / 无预算 / 未决策 / 时机不合适 / 功能缺失 / 其他 | + +旁边的 **赢/丢单详情** 是自由文本框,写下半年后同事真正需要的那句话("对方市场部被 HubSpot 两年合同锁定,续约时再谈")。 + +两点须知: + +- **这是服务端校验**,不只是表单提示。导入、API 调用或批量更新在缺少对应原因时同样会被拒绝。 +- **已关单的商机是锁定的**。关单之后只有叙述性字段(描述、后续步骤)还能编辑,原因无法事后补填——请在关单那一刻就选好。 + +如果商机是因为客户接受报价而成交,HotCRM 会自动关单并把赢单原因记为 **报价被接受**,详见[报价](/docs/sales/quotes)。 + +这两个字段正是销售仪表盘 **Win Rate** 与 **Why We Lose** 组件的数据来源,详见[仪表盘](/docs/analytics/dashboards)。 + ## 商机记录存储的内容 详情界面有 7 个区块: @@ -32,7 +52,7 @@ description: 活跃的销售交易——销售管道的核心,包含 7 个阶 | **基本信息** | 名称、客户、主要联系人、负责人 | | **财务** | 金额、预期营收 *(自动)*、概率 *(从阶段自动得出)* | | **销售流程** | 阶段、成交日期、创建日期、进入当前阶段日期 | -| **分类** | 类型(新业务 / 升级 / 续约 / 扩展)、线索来源 | +| **分类** | 类型(新业务 / 升级 / 续约 / 扩展)、线索来源、赢单原因、丢单原因、赢/丢单详情 | | **竞争与营销活动** | 竞争对手、来源营销活动 | | **备注与后续步骤** | 描述、后续步骤 | | **预测与指标** | 行项目合计、审批状态 | diff --git a/content/docs/sales/opportunities.zh-Hant.mdx b/content/docs/sales/opportunities.zh-Hant.mdx index 48420bb6..4fdaf0d6 100644 --- a/content/docs/sales/opportunities.zh-Hant.mdx +++ b/content/docs/sales/opportunities.zh-Hant.mdx @@ -23,6 +23,26 @@ description: 活躍的銷售交易——銷售管道的核心,包含 7 個階 你可以從任何開放階段直接移動到**輸單**(交易隨時可能失去),但你不能向前跳過——你必須一次一個階段地走過漏斗。系統會攔截無效跳轉,以保持你的預測真實可靠。 +## 結案必須填寫原因 + +關閉商機時必須多填一個欄位,否則記錄無法儲存: + +| 結案方式 | 必填欄位 | +| --- | --- | +| 🟢 **贏單** | **贏單原因** —— 產品更優 / 價格更優 / 客戶關係 / 支援更好 / 最佳契合 / 其他 | +| 🔴 **輸單** | **丟單原因** —— 價格過高 / 輸給競爭對手 / 無預算 / 未決策 / 時機不合適 / 功能缺失 / 其他 | + +旁邊的 **贏/丟單詳情** 是自由文字框,寫下半年後同事真正需要的那句話(「對方行銷部被 HubSpot 兩年合約鎖定,續約時再談」)。 + +兩點須知: + +- **這是伺服器端驗證**,不只是表單提示。匯入、API 呼叫或批次更新在缺少對應原因時同樣會被拒絕。 +- **已結案的商機是鎖定的**。結案之後只有敘述性欄位(描述、後續步驟)還能編輯,原因無法事後補填——請在結案當下就選好。 + +如果商機是因為客戶接受報價而成交,HotCRM 會自動結案並把贏單原因記為 **報價被接受**,詳見[報價](/docs/sales/quotes)。 + +這兩個欄位正是銷售儀表板 **Win Rate** 與 **Why We Lose** 元件的資料來源,詳見[儀表板](/docs/analytics/dashboards)。 + ## 商機記錄儲存的內容 詳情介面有 7 個區塊: @@ -32,7 +52,7 @@ description: 活躍的銷售交易——銷售管道的核心,包含 7 個階 | **基本資訊** | 名稱、客戶、主要聯絡人、負責人 | | **財務** | 金額、預期營收 *(自動)*、機率 *(從階段自動得出)* | | **銷售流程** | 階段、成交日期、建立日期、進入當前階段日期 | -| **分類** | 類型(新業務 / 升級 / 續約 / 擴展)、潛在客戶來源 | +| **分類** | 類型(新業務 / 升級 / 續約 / 擴展)、潛在客戶來源、贏單原因、丟單原因、贏/丟單詳情 | | **競爭與行銷活動** | 競爭對手、來源行銷活動 | | **備註與後續步驟** | 描述、後續步驟 | | **預測與指標** | 行項目合計、審批狀態 | diff --git a/src/dashboards/sales.dashboard.ts b/src/dashboards/sales.dashboard.ts index 7f1922d8..486e4145 100644 --- a/src/dashboards/sales.dashboard.ts +++ b/src/dashboards/sales.dashboard.ts @@ -120,8 +120,63 @@ export const SalesDashboard: Dashboard = { }, }), - // ─── Row 2: Pipeline & Trends ───────────────────────────────────── - pipelineByStageFunnelWidget({ x: 0, y: 2, w: 6, h: 4 }), + // ─── Row 2: Win / Loss KPIs ─────────────────────────────────────── + // + // The ratio and its two inputs sit side by side ON PURPOSE (#593). A win + // rate is the failure mode #614 shipped: a wrong denominator raises no + // error, it just prints a plausible percentage. Showing "62%" next to + // "8 won" and "5 lost" means a reader can check the arithmetic at a + // glance, and a half that silently loses its filter stops being invisible. + // + // All three windows are the same self-scoped 12 months (`dateRange: false` + // so the header's date picker cannot re-window one of them and not the + // others — three tiles over three different windows would be worse than + // no tiles at all). + { + // NOTE: no `stage` filter here, and that is deliberate. Both halves of + // the ratio come from the DATASET's own measure filters — `won_count` + // over closed_won, `decided_count` over closed_won + closed_lost — so + // open pipeline is excluded by the measures. A widget-level + // `stage: {$in: [...]}` filter would narrow numerator and denominator + // alike, which is how a ratio quietly becomes a division by itself. + id: 'win_rate_12m', + title: 'Win Rate (12M)', + description: 'Deals won as a share of all deals settled in the last 12 months', + type: 'metric', + filter: { close_date: { $gte: '{12_months_ago}' } }, + filterBindings: { dateRange: false }, + colorVariant: 'success', + dataset: 'opportunity_metrics', values: ['win_rate'], + layout: { x: 0, y: 2, w: 4, h: 2 }, + options: { icon: 'Percent', format: '0%' }, + }, + { + id: 'won_deals_12m', + title: 'Deals Won (12M)', + description: 'The numerator of the win rate', + type: 'metric', + filter: { close_date: { $gte: '{12_months_ago}' } }, + filterBindings: { dateRange: false }, + colorVariant: 'blue', + dataset: 'opportunity_metrics', values: ['won_count'], + layout: { x: 4, y: 2, w: 4, h: 2 }, + options: { icon: 'Trophy', format: '0,0' }, + }, + { + id: 'lost_deals_12m', + title: 'Deals Lost (12M)', + description: 'The other half of the win-rate denominator', + type: 'metric', + filter: { close_date: { $gte: '{12_months_ago}' } }, + filterBindings: { dateRange: false }, + colorVariant: 'orange', + dataset: 'opportunity_metrics', values: ['lost_count'], + layout: { x: 8, y: 2, w: 4, h: 2 }, + options: { icon: 'TrendingDown', format: '0,0' }, + }, + + // ─── Row 3: Pipeline & Trends ───────────────────────────────────── + pipelineByStageFunnelWidget({ x: 0, y: 4, w: 6, h: 4 }), { id: 'monthly_revenue_trend', title: 'Monthly Revenue Trend', @@ -131,7 +186,7 @@ export const SalesDashboard: Dashboard = { filterBindings: { dateRange: false }, // self-scoped to 12 months — the date picker must not narrow it colorVariant: 'success', dataset: 'opportunity_metrics', dimensions: ['close_date'], values: ['total_amount'], - layout: { x: 6, y: 2, w: 6, h: 4 }, + layout: { x: 6, y: 4, w: 6, h: 4 }, chartConfig: { type: 'area', showLegend: false, @@ -149,7 +204,7 @@ export const SalesDashboard: Dashboard = { options: { dateGranularity: 'month' }, }, - // ─── Row 3: Performance Breakdown ───────────────────────────────── + // ─── Row 4: Performance Breakdown ───────────────────────────────── { id: 'pipeline_by_forecast_category', title: 'Pipeline by Forecast Category', @@ -158,7 +213,7 @@ export const SalesDashboard: Dashboard = { filter: { stage: { $nin: ['closed_won', 'closed_lost'] } }, colorVariant: 'blue', dataset: 'opportunity_metrics', dimensions: ['forecast_category'], values: ['total_amount'], - layout: { x: 0, y: 6, w: 6, h: 4 }, + layout: { x: 0, y: 8, w: 6, h: 4 }, chartConfig: { type: 'horizontal-bar', showLegend: false, @@ -176,7 +231,7 @@ export const SalesDashboard: Dashboard = { filter: { stage: { $nin: ['closed_lost'] } }, colorVariant: 'purple', dataset: 'opportunity_metrics', dimensions: ['lead_source'], values: ['total_amount'], - layout: { x: 6, y: 6, w: 6, h: 4 }, + layout: { x: 6, y: 8, w: 6, h: 4 }, chartConfig: { type: 'donut', showLegend: true, @@ -185,7 +240,7 @@ export const SalesDashboard: Dashboard = { }, }, - // ─── Row 4: Rep Leaderboard ─────────────────────────────────────── + // ─── Row 5: Rep Leaderboard ─────────────────────────────────────── // A dashboard `table` binds to an analytics cube and aggregates; it cannot // list raw deals (ADR-0021). The previous "Top Open Opportunities" table // selected only `opp_count` with no dimension — one summary row, not a deal @@ -200,7 +255,7 @@ export const SalesDashboard: Dashboard = { filter: { stage: { $nin: ['closed_won', 'closed_lost'] } }, colorVariant: 'default', dataset: 'opportunity_metrics', dimensions: ['owner'], values: ['total_amount', 'opp_count', 'avg_probability'], - layout: { x: 0, y: 10, w: 12, h: 4 }, + layout: { x: 0, y: 12, w: 12, h: 4 }, options: { columns: [ { header: 'Owner', accessorKey: 'owner' }, @@ -216,7 +271,7 @@ export const SalesDashboard: Dashboard = { }, }, - // ─── Row 5: Quota Attainment ────────────────────────────────────── + // ─── Row 6: Quota Attainment ────────────────────────────────────── // The real per-rep quota data (crm_forecast.quota) surfaced as a table: // dashboard chart annotations cannot reference a dataset (static values // only), so quota vs. actual gets its own widget instead of a fake line. @@ -261,7 +316,7 @@ export const SalesDashboard: Dashboard = { // this one. filter: { period: 'quarter', period_start: '{current_quarter_start}' }, dataset: 'forecast_metrics', dimensions: ['owner'], values: ['quota_sum', 'closed_sum', 'attainment'], - layout: { x: 0, y: 14, w: 12, h: 4 }, + layout: { x: 0, y: 16, w: 12, h: 4 }, options: { columns: [ { header: 'Owner', accessorKey: 'owner' }, @@ -277,7 +332,108 @@ export const SalesDashboard: Dashboard = { }, }, - // ─── Row 6: Pivot — Stage × Lead Source ─────────────────────────── + // ─── Row 7: Win / Loss analysis ─────────────────────────────────── + // + // Three widgets over the settled half of the pipeline (#593). Each of the + // two win-rate breakdowns is a TABLE, not a bar chart, and that is the + // whole design: a bar of "67%" is unfalsifiable, whereas a row reading + // "Won 2 · Lost 1 · Settled 3 · 67%" carries its own proof. Perturb either + // half — win a deal that was lost, or lose one that was won — and three of + // those four numbers move. `test/win-loss-capture.test.ts` does exactly + // that against the real dataset executor and the real seeds. + // + // Both tables opt out of the header's `close_date` window + // (`dateRange: false`) for the reason the quota table does: a win rate over + // "this quarter" on a demo database is computed from whichever one or two + // deals happen to have settled inside it, which is noise wearing the + // costume of a KPI. Twelve months is the same window the row-2 tiles and + // the revenue trend use, so the dashboard tells one consistent story. + { + id: 'win_rate_by_owner', + title: 'Win / Loss by Rep', + description: 'Deals won, deals lost and win rate per rep — last 12 months', + type: 'table', + filter: { close_date: { $gte: '{12_months_ago}' } }, + filterBindings: { dateRange: false }, + colorVariant: 'default', + dataset: 'opportunity_metrics', + dimensions: ['owner'], + values: ['won_count', 'lost_count', 'decided_count', 'win_rate', 'won_amount'], + layout: { x: 0, y: 20, w: 6, h: 4 }, + options: { + columns: [ + { header: 'Owner', accessorKey: 'owner' }, + { header: 'Won', accessorKey: 'won_count' }, + { header: 'Lost', accessorKey: 'lost_count' }, + { header: 'Settled', accessorKey: 'decided_count' }, + { header: 'Win Rate', accessorKey: 'win_rate', format: '0%' }, + { header: 'Won Revenue', accessorKey: 'won_amount', format: '0,0' }, + ], + sortBy: 'decided_count', + sortOrder: 'desc', + limit: 10, + striped: true, + density: 'comfortable', + }, + }, + { + id: 'win_rate_by_lead_source', + title: 'Win / Loss by Lead Source', + description: 'Which sources produce deals that actually close — last 12 months', + type: 'table', + filter: { close_date: { $gte: '{12_months_ago}' } }, + filterBindings: { dateRange: false }, + colorVariant: 'default', + dataset: 'opportunity_metrics', + dimensions: ['lead_source'], + values: ['won_count', 'lost_count', 'decided_count', 'win_rate', 'won_amount'], + layout: { x: 6, y: 20, w: 6, h: 4 }, + options: { + columns: [ + { header: 'Lead Source', accessorKey: 'lead_source' }, + { header: 'Won', accessorKey: 'won_count' }, + { header: 'Lost', accessorKey: 'lost_count' }, + { header: 'Settled', accessorKey: 'decided_count' }, + { header: 'Win Rate', accessorKey: 'win_rate', format: '0%' }, + { header: 'Won Revenue', accessorKey: 'won_amount', format: '0,0' }, + ], + sortBy: 'decided_count', + sortOrder: 'desc', + limit: 12, + striped: true, + density: 'comfortable', + }, + }, + { + // The loss-reason breakdown the fields were declared for and never got. + // + // `loss_reason` is mandatory on every `closed_lost` record (see + // crm_opportunity), so this chart has no "unattributed" slice to + // apologise for — an empty chart here means no losses in the window, + // never "nobody filled the field in". + // + // Counts, not amounts: the question a loss review asks is "how often did + // we lose for this reason", and one large lost deal should not outweigh + // five recurring feature gaps. `lost_amount` is declared on the dataset + // for the revenue view of the same question. + id: 'loss_reason_breakdown', + title: 'Why We Lose', + description: 'Lost deals by reason — last 12 months', + type: 'donut', + filter: { stage: 'closed_lost', close_date: { $gte: '{12_months_ago}' } }, + filterBindings: { dateRange: false }, + colorVariant: 'orange', + dataset: 'opportunity_metrics', dimensions: ['loss_reason'], values: ['opp_count'], + layout: { x: 0, y: 24, w: 12, h: 4 }, + chartConfig: { + type: 'donut', + showLegend: true, + showDataLabels: true, + colors: ['#EF4444', '#F59E0B', '#8B5CF6', '#06B6D4', '#4F46E5', '#10B981', '#64748B'], + }, + }, + + // ─── Row 9: Pivot — Stage × Lead Source ─────────────────────────── { id: 'pipeline_stage_by_source', title: 'Pipeline by Stage × Lead Source', @@ -286,7 +442,7 @@ export const SalesDashboard: Dashboard = { filter: { stage: { $nin: ['closed_won', 'closed_lost'] } }, colorVariant: 'default', dataset: 'opportunity_metrics', dimensions: ['stage', 'lead_source'], values: ['total_amount'], - layout: { x: 0, y: 18, w: 12, h: 4 }, + layout: { x: 0, y: 28, w: 12, h: 4 }, options: { rowField: 'stage', columnField: 'lead_source', diff --git a/src/data/sales.seed.ts b/src/data/sales.seed.ts index e834a5f0..52fb51fe 100644 --- a/src/data/sales.seed.ts +++ b/src/data/sales.seed.ts @@ -535,6 +535,21 @@ export const OPPORTUNITY_LINES: Record = { { product: 'Field Service Mobile', quantity: 1, unit_price: 14000, description: 'Field service mobile for the device-servicing team.' }, { product: 'AI Agent Seat (Annual)', quantity: 6, unit_price: 1000, description: 'Agent seats for the service coordinators.' }, ], + 'Globex Line Expansion (Lost)': [ + { product: 'ObjectStack Platform', quantity: 2, unit_price: 50000, description: 'Two further production-line tenants.' }, + { product: 'Integration Connector Pack', quantity: 1, unit_price: 16000, description: 'Connector pack for the line-control systems.' }, + { product: 'AI Agent Seat (Annual)', quantity: 8, unit_price: 1000, description: 'Agent seats for the line supervisors.' }, + ], + 'Northwind Field Service Pilot (Lost)': [ + { product: 'Field Service Mobile', quantity: 1, unit_price: 14000, description: 'Field-service mobile for the metering crews.' }, + { product: 'Standard Support', quantity: 1, unit_price: 9000, description: 'Business-hours support during the pilot.' }, + { product: 'AI Agent Seat (Annual)', quantity: 10, unit_price: 1000, description: 'Agent seats for the dispatch desk.' }, + ], + 'Apex Compliance Reporting (Lost)': [ + { product: 'Analytics Add-on', quantity: 1, unit_price: 22000, description: 'Regulatory reporting pack for fleet compliance.' }, + { product: 'Sandbox Environment (Annual)', quantity: 1, unit_price: 7500, description: 'Sandbox for the compliance rule build-out.' }, + { product: 'AI Agent Seat (Annual)', quantity: 6, unit_price: 1000, description: 'Agent seats for the compliance analysts.' }, + ], 'Northwind Grid Modernization': [ { product: 'ObjectStack Platform', quantity: 3, unit_price: 50000, description: 'Enterprise edition for grid, field and customer operations.' }, { product: 'Implementation Services', quantity: 1, unit_price: 75000, description: 'Operational-data assessment and implementation.' }, @@ -683,6 +698,7 @@ analytics seats for the Ops org, (3) priority support SLA.`, type: 'existing_renewal', forecast_category: 'closed', lead_source: 'partner', + win_reason: 'relationship', description: `Annual renewal of the Acme Standard subscription (40 seats), signed two weeks ahead of the renewal date. 22% YoY uplift driven by seat expansion in the new EMEA team. Multi-year option declined this round — they want to see how the platform upgrade lands first.`, }, { @@ -696,6 +712,7 @@ analytics seats for the Ops org, (3) priority support SLA.`, type: 'new_business', forecast_category: 'closed', lead_source: 'event', + win_reason: 'best_fit', }, { name: 'Wayne Q1 Expansion', @@ -708,6 +725,7 @@ analytics seats for the Ops org, (3) priority support SLA.`, type: 'existing_upgrade', forecast_category: 'closed', lead_source: 'web', + win_reason: 'better_product', }, { name: 'Globex Training Package', @@ -720,6 +738,7 @@ analytics seats for the Ops org, (3) priority support SLA.`, type: 'new_business', forecast_category: 'closed', lead_source: 'referral', + win_reason: 'better_support', }, { name: 'Initech Phase 1', @@ -732,6 +751,7 @@ analytics seats for the Ops org, (3) priority support SLA.`, type: 'new_business', forecast_category: 'closed', lead_source: 'web', + win_reason: 'better_price', }, // ─── Campaign-attributed wins ─────────────────────────────────────── // `crm_campaign` is what `campaign_snapshot_metrics` counts when a campaign @@ -753,6 +773,7 @@ analytics seats for the Ops org, (3) priority support SLA.`, type: 'existing_expansion', forecast_category: 'closed', lead_source: 'email_campaign', + win_reason: 'best_fit', description: 'Analytics package for admissions and alumni engagement, sourced from the enterprise nurture track.', }, { @@ -769,6 +790,7 @@ analytics seats for the Ops org, (3) priority support SLA.`, forecast_category: 'closed', lead_source: 'content', description: 'Operations module for two manufacturing divisions, closed off the operations-platform launch program.', + win_reason: 'relationship', }, { name: 'Vertex Developer Platform Adoption', @@ -784,8 +806,23 @@ analytics seats for the Ops org, (3) priority support SLA.`, forecast_category: 'closed', lead_source: 'content', description: 'Three product teams adopted the developer platform after the technical content series.', - }, - // Closed Lost deals (powers win-rate analytics) + win_reason: 'better_product', + }, + // ─── Closed Lost deals (powers win-rate + loss-reason analytics) ──── + // + // Five losses across five distinct `loss_reason` values, and every one of + // them is now MANDATORY rather than decorative: `loss_reason` is + // `requiredWhen` stage is `closed_lost` (#593), so a seeded lost deal + // without one is rejected by the engine at seed time — the demo database + // cannot boot with the empty column that made the loss-reason widget + // unbuildable in the first place. + // + // Their `lead_source` values are chosen so that the sources carrying a loss + // ALSO carry a win (`web`, `referral`, `content`), plus two sources that + // only ever lost (`cold_call`, `advertisement`). That is what makes + // `win_rate_by_lead_source` a real measurement instead of a column of + // 100%s: three rows where both halves of the ratio move, and two rows that + // show what an all-loss source looks like. { name: 'Acme Add-on (Lost)', crm_account: 'Acme Corporation', @@ -797,6 +834,8 @@ analytics seats for the Ops org, (3) priority support SLA.`, type: 'existing_upgrade', forecast_category: 'omitted', lead_source: 'cold_call', + loss_reason: 'timing', + loss_details: 'Marketing is locked into a 2-year HubSpot contract; the buying window opens when that renews.', description: `Tried to bolt on the Marketing Cloud module via cold outbound. Lost because Acme's marketing org is already on a 2-year HubSpot contract. Revisit in Q3 when that contract is up for renewal.`, }, { @@ -810,6 +849,53 @@ analytics seats for the Ops org, (3) priority support SLA.`, type: 'new_business', forecast_category: 'omitted', lead_source: 'advertisement', + loss_reason: 'no_budget', + loss_details: 'Capital freeze on non-clinical systems for the rest of the fiscal year.', + }, + { + name: 'Globex Line Expansion (Lost)', + crm_account: 'Globex Industries', + ...dealValue('Globex Line Expansion (Lost)', 0), + stage: 'closed_lost', + probability: 0, + close_date: cel`daysAgo(35)`, + stage_entry_date: cel`daysAgo(35)`, + type: 'existing_expansion', + forecast_category: 'omitted', + lead_source: 'referral', + loss_reason: 'competitor', + loss_details: 'Incumbent MES vendor bundled the two additional lines into an existing master agreement.', + description: 'Expansion into two further production lines, lost to the incumbent MES vendor on bundling.', + }, + { + name: 'Northwind Field Service Pilot (Lost)', + crm_account: 'Northwind Energy', + ...dealValue('Northwind Field Service Pilot (Lost)', 0), + stage: 'closed_lost', + probability: 0, + close_date: cel`daysAgo(80)`, + stage_entry_date: cel`daysAgo(80)`, + type: 'new_business', + forecast_category: 'omitted', + lead_source: 'web', + loss_reason: 'price', + loss_details: 'Per-technician pricing landed ~30% above the budget the operations director had approved.', + description: 'Field-service pilot for the metering crews, lost on price against a cheaper point solution.', + }, + { + name: 'Apex Compliance Reporting (Lost)', + crm_account: 'Apex Logistics', + ...dealValue('Apex Compliance Reporting (Lost)', 0), + stage: 'closed_lost', + probability: 0, + close_date: cel`daysAgo(110)`, + stage_entry_date: cel`daysAgo(110)`, + type: 'new_business', + forecast_category: 'omitted', + lead_source: 'content', + loss_reason: 'features', + loss_details: 'No native hours-of-service audit trail; the compliance team could not sign off without it.', + description: 'Regulatory reporting pack for the fleet compliance team, lost on a missing audit-trail capability.', }, // ─── Curated active pipeline ───────────────────────────────────── // Two cards in each active stage make the kanban immediately legible in diff --git a/src/datasets/opportunity.dataset.ts b/src/datasets/opportunity.dataset.ts index 87cde54b..f801e91f 100644 --- a/src/datasets/opportunity.dataset.ts +++ b/src/datasets/opportunity.dataset.ts @@ -26,6 +26,11 @@ export const OpportunityDataset = defineDataset({ { name: 'forecast_category', label: 'Forecast Category', field: 'forecast_category', type: 'string' }, { name: 'type', label: 'Deal Type', field: 'type', type: 'string' }, { name: 'owner', label: 'Owner', field: 'owner', type: 'lookup' }, + // Win/loss attribution (#593). Both columns are now mandatory at close + // (`requiredWhen` on crm_opportunity), so grouping by either one over + // settled deals has no "unattributed" bucket to explain away. + { name: 'win_reason', label: 'Win Reason', field: 'win_reason', type: 'string' }, + { name: 'loss_reason', label: 'Loss Reason', field: 'loss_reason', type: 'string' }, // v17 fixes scoped aggregate execution and SQLite's epoch bucketing. // Trends and quarter coverage need different buckets over the same field, // so each one is its own semantic dimension. @@ -40,5 +45,49 @@ export const OpportunityDataset = defineDataset({ { name: 'total_amount', label: 'Total Amount', aggregate: 'sum', field: 'amount', format: '0,0' }, { name: 'avg_amount', label: 'Avg Deal Size', aggregate: 'avg', field: 'amount', format: '0,0' }, { name: 'avg_probability', label: 'Avg Probability', aggregate: 'avg', field: 'probability', format: '0%' }, + + // ─── Win rate (#593) ──────────────────────────────────────────────── + // + // A ratio is the most dangerous kind of measure: a wrong denominator does + // not error, it returns a plausible number (#614 rendered a 1,500,000 + // quota as 7,940,000 because a filter was missing one of its two halves). + // So the two halves are declared here, ONCE, as named measures — not + // improvised per widget — and every widget that shows `win_rate` also + // shows `won_count` and `lost_count` beside it, so the reader can check + // the arithmetic that produced the percentage. + // + // Each carries its OWN `filter`, which the executor honours by running + // that measure as its own sub-query and merging the result back on the + // selected dimensions. That is what lets one widget row hold "won 2, lost + // 1, 67%" — the counts are over different row sets, and only a + // measure-level filter can say so. A widget-level `filter` cannot: it + // narrows the whole row set at once, which is exactly how you end up + // dividing a number by itself. + // + // `decided_count` is deliberately its own filtered count and NOT + // `derived: { op: 'sum', of: ['won_count', 'lost_count'] }`. Measured on + // 17.0.0-rc.1: a filtered measure contributes NO row for a group its + // filter selects nothing in — the count comes back absent, not 0 — and a + // derived measure over an absent input evaluates to null. Summing the two + // would therefore blank the win rate of any rep who has never lost a deal + // (100%, the group you least want to hide). Counting the union directly + // keeps the denominator present whenever the group has any settled deal at + // all. The remaining asymmetry — a group with losses but NO wins reports a + // null win rate rather than 0% — is a platform behaviour, not something a + // widget can paper over; it is pinned by `test/win-loss-capture.test.ts` + // and filed upstream. The tables show `lost_count` next to `win_rate` + // precisely so that a blank rate reads as "0 wins out of 3", not as + // "no data". + { name: 'won_count', label: 'Won Deals', aggregate: 'count', filter: { stage: 'closed_won' } }, + { name: 'lost_count', label: 'Lost Deals', aggregate: 'count', filter: { stage: 'closed_lost' } }, + { + name: 'decided_count', + label: 'Settled Deals', + aggregate: 'count', + filter: { stage: { $in: ['closed_won', 'closed_lost'] } }, + }, + { name: 'won_amount', label: 'Won Revenue', aggregate: 'sum', field: 'amount', filter: { stage: 'closed_won' }, format: '0,0' }, + { name: 'lost_amount', label: 'Lost Revenue', aggregate: 'sum', field: 'amount', filter: { stage: 'closed_lost' }, format: '0,0' }, + { name: 'win_rate', label: 'Win Rate', derived: { op: 'ratio', of: ['won_count', 'decided_count'] }, format: '0%' }, ], }); diff --git a/src/objects/opportunity.object.ts b/src/objects/opportunity.object.ts index 4c153f65..ad469dad 100644 --- a/src/objects/opportunity.object.ts +++ b/src/objects/opportunity.object.ts @@ -263,23 +263,66 @@ export const Opportunity = ObjectSchema.create({ group: 'sales_process', }), - // Win / Loss analysis — required when stage moves to closed_* + // ─── Win / Loss analysis ──────────────────────────────────────────── + // + // The reason is captured AT CLOSE, and that is enforced, not requested + // (#593). This comment used to read "required when stage moves to + // closed_*" while nothing anywhere required anything: both fields were + // optional, so every seeded and user-closed deal landed with them empty + // and the win/loss widgets below had nothing to draw. + // + // `requiredWhen`, not a script validation, for the same reason + // `crm_lead.duplicate_of_lead` uses it (ADR-0113): "this field must hold a + // value when the record looks like X" is exactly a conditional write + // contract, the engine evaluates it on insert AND update inside + // `evaluateValidationRules`, and it reports against the FIELD — so the form + // marks the empty picklist instead of showing a record-level banner. It is + // ALSO the only shape the freeze below leaves usable: once a deal is + // closed, `opportunity.hook.ts` refuses every user edit outside the + // narrative fields, so a reason not captured in the closing write can never + // be added afterwards. Close time is the only chance. + // + // MEASURED, not assumed (the "declared ≠ enforced" family this repo keeps + // finding — #621 / #633 / #650 / #651): both predicates reject the write + // through a real ObjectQL over a real driver, on insert and on update, and + // the record stays at its previous stage. `crm_case`'s + // `resolution_required_for_closed` was re-measured the same way first and + // is genuinely blocking today. See `test/win-loss-capture.test.ts`. + // + // ⚠️ `has(...)` is load-bearing. A bare `record.stage == "closed_lost"` + // aborts with `No such key` on any merged record that simply omits the + // column, and the engine's answer to a predicate that cannot evaluate is to + // SKIP it ("requiredWhen for 'loss_reason' failed to evaluate — skipped"), + // which would make this read as enforced while requiring nothing at all. win_reason: Field.select({ label: 'Win Reason', + description: 'Why this deal was won. Required to close an opportunity as Won.', group: 'classification', + requiredWhen: P`has(record.stage) && record.stage == "closed_won"`, options: [ { label: 'Better Product', value: 'better_product' }, { label: 'Better Price', value: 'better_price' }, { label: 'Existing Relationship', value: 'relationship' }, { label: 'Better Support', value: 'better_support' }, { label: 'Best Fit / Features', value: 'best_fit' }, + // Written by the machine, not chosen by a rep: `quote_on_accepted` + // (quote.hook.ts) wins the deal the moment a customer accepts the + // quote, and no human is in that write to attribute it. Naming the + // automated path is this repo's existing idiom for exactly that split + // (`crm_lead.duplicate_status` = machine `suspected` vs human + // `confirmed`) and it keeps the rule exception-free: every closed_won + // row carries a reason, and an analyst can still tell a CPQ close from + // a rep's attribution instead of reading a fabricated "Better Product". + { label: 'Quote Accepted', value: 'quote_accepted' }, { label: 'Other', value: 'other' }, ], }), loss_reason: Field.select({ label: 'Loss Reason', + description: 'Why this deal was lost. Required to close an opportunity as Lost.', group: 'classification', + requiredWhen: P`has(record.stage) && record.stage == "closed_lost"`, options: [ { label: 'Price Too High', value: 'price' }, { label: 'Lost to Competitor', value: 'competitor' }, @@ -293,6 +336,7 @@ export const Opportunity = ObjectSchema.create({ loss_details: Field.textarea({ label: 'Loss/Win Details', + description: 'Free-text context behind the win or loss reason.', group: 'classification', }), }, diff --git a/src/objects/quote.hook.ts b/src/objects/quote.hook.ts index befda9fd..286aa7c8 100644 --- a/src/objects/quote.hook.ts +++ b/src/objects/quote.hook.ts @@ -135,6 +135,14 @@ const quoteAccepted: Hook = { id: opportunityId, stage: 'closed_won', close_date: today, + // `crm_opportunity.win_reason` is `requiredWhen` stage is + // closed_won (#593), and this write is the ONE close path with no + // human in it to attribute the win — so without a value here the + // CPQ leg would be rejected by the engine on every accepted quote. + // `quote_accepted` names the automated path rather than guessing a + // rep's answer; keep the reason the rep already recorded if there + // is one. + ...(opp.win_reason ? {} : { win_reason: 'quote_accepted' }), }, { where: { id: opportunityId } }, ); diff --git a/src/translations/en.ts b/src/translations/en.ts index e19cce1b..7e211148 100644 --- a/src/translations/en.ts +++ b/src/translations/en.ts @@ -357,7 +357,8 @@ export const en: TranslationData = { label: 'Win Reason', options: { better_product: 'Better Product', better_price: 'Better Price', relationship: 'Existing Relationship', - better_support: 'Better Support', best_fit: 'Best Fit / Features', other: 'Other', + better_support: 'Better Support', best_fit: 'Best Fit / Features', + quote_accepted: 'Quote Accepted', other: 'Other', }, }, loss_reason: { diff --git a/src/translations/es-ES.ts b/src/translations/es-ES.ts index 2dd13ab4..e7fde288 100644 --- a/src/translations/es-ES.ts +++ b/src/translations/es-ES.ts @@ -326,6 +326,27 @@ export const esES: TranslationData = { days_in_stage: { label: 'Días en Etapa Actual' }, stage_entry_date: { label: 'Fecha de Entrada a la Etapa' }, is_private: { label: 'Privado' }, + // #593 — obligatorias al cerrar la oportunidad, y el desglose de + // motivos de pérdida las muestra en el panel de ventas, así que el + // valor almacenado nunca debe llegar crudo al selector. + win_reason: { + label: 'Motivo de Ganancia', + options: { + better_product: 'Mejor Producto', better_price: 'Mejor Precio', + relationship: 'Relación Existente', better_support: 'Mejor Soporte', + best_fit: 'Mejor Ajuste / Funcionalidades', + quote_accepted: 'Presupuesto Aceptado', other: 'Otro', + }, + }, + loss_reason: { + label: 'Motivo de Pérdida', + options: { + price: 'Precio Demasiado Alto', competitor: 'Perdida ante Competidor', + no_budget: 'Sin Presupuesto', no_decision: 'Sin Decisión', + timing: 'Momento Inadecuado', features: 'Funcionalidades Faltantes', other: 'Otro', + }, + }, + loss_details: { label: 'Detalles de Ganancia/Pérdida' }, }, _views: { open_opportunities: { label: 'Oportunidades Abiertas' }, diff --git a/src/translations/ja-JP.ts b/src/translations/ja-JP.ts index 08728339..2068ff0e 100644 --- a/src/translations/ja-JP.ts +++ b/src/translations/ja-JP.ts @@ -326,6 +326,26 @@ export const jaJP: TranslationData = { days_in_stage: { label: '現ステージ滞在日数' }, stage_entry_date: { label: 'ステージ開始日' }, is_private: { label: '非公開' }, + // #593 — 商談クローズ時に必須。失注理由は営業ダッシュボードの + // 内訳ウィジェットにも出るため、保存値がそのまま画面に出てはいけない。 + win_reason: { + label: '受注理由', + options: { + better_product: '製品が優れている', better_price: '価格が優れている', + relationship: '既存の関係', better_support: 'サポートが優れている', + best_fit: '最適な機能・適合性', + quote_accepted: '見積承認', other: 'その他', + }, + }, + loss_reason: { + label: '失注理由', + options: { + price: '価格が高すぎる', competitor: '競合他社に敗北', + no_budget: '予算なし', no_decision: '意思決定なし', + timing: 'タイミング不適', features: '機能不足', other: 'その他', + }, + }, + loss_details: { label: '受注・失注の詳細' }, }, _views: { open_opportunities: { label: '進行中の商談' }, diff --git a/src/translations/zh-CN.ts b/src/translations/zh-CN.ts index ef4a03af..29ba1eaf 100644 --- a/src/translations/zh-CN.ts +++ b/src/translations/zh-CN.ts @@ -777,7 +777,8 @@ export const zhCN: TranslationData = { label: '赢单原因', options: { better_product: '产品更优', better_price: '价格更优', relationship: '客户关系', - better_support: '支持更好', best_fit: '最佳契合', other: '其他', + better_support: '支持更好', best_fit: '最佳契合', + quote_accepted: '报价被接受', other: '其他', }, }, loss_reason: { diff --git a/test/flow-condition-totality.test.ts b/test/flow-condition-totality.test.ts index 7749de9a..69a01be4 100644 --- a/test/flow-condition-totality.test.ts +++ b/test/flow-condition-totality.test.ts @@ -611,7 +611,15 @@ describe('conditions answer on a driver whose stored record omits the key', () = name: 'Big Deal', amount: 750_000, stage: 'negotiation', close_date: '2026-12-31', crm_account: account.id, }); - await api.object('crm_opportunity').update({ stage: 'closed_won' }, { where: { id: opp.id } }); + // `win_reason` is `requiredWhen` stage is closed_won (#593) — the engine + // rejects the close without it, exactly as it rejects the case close + // below without a `resolution`. Supplied here so this file keeps testing + // what it is about (flow-condition totality) rather than failing on a + // neighbouring object rule. + await api.object('crm_opportunity').update( + { stage: 'closed_won', win_reason: 'best_fit' }, + { where: { id: opp.id } }, + ); const kase = await api.object('crm_case').insert({ subject: 'Login broken', description: 'Cannot sign in', diff --git a/test/metadata-references.test.ts b/test/metadata-references.test.ts index 85ed4844..9c1ce387 100644 --- a/test/metadata-references.test.ts +++ b/test/metadata-references.test.ts @@ -1354,9 +1354,13 @@ describe('select fields are translated in every locale', () => { 'crm_lead.salutation': ['en', 'ja-JP', 'es-ES'], 'crm_lead.industry': ['en', 'ja-JP', 'es-ES'], 'crm_opportunity.competitors': UNTRANSLATED_EVERYWHERE, + // `crm_opportunity.win_reason` / `loss_reason` left the ledger in #593: + // both became REQUIRED at close and both now drive a Sales-dashboard + // widget, so an untranslated option is no longer a cosmetic gap — it is a + // raw stored value (`no_budget`, `quote_accepted`) in a picklist a rep is + // forced to choose from, and in a chart legend. `approval_status` stays: + // it is untouched by that change and belongs to #645's sweep. 'crm_opportunity.approval_status': ['ja-JP', 'es-ES'], - 'crm_opportunity.win_reason': ['ja-JP', 'es-ES'], - 'crm_opportunity.loss_reason': ['ja-JP', 'es-ES'], 'crm_product.category': ['en', 'ja-JP', 'es-ES'], 'crm_product.family': UNTRANSLATED_EVERYWHERE, 'crm_product.billing_type': UNTRANSLATED_EVERYWHERE, diff --git a/test/win-loss-capture.test.ts b/test/win-loss-capture.test.ts new file mode 100644 index 00000000..161b335d --- /dev/null +++ b/test/win-loss-capture.test.ts @@ -0,0 +1,851 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { ObjectQL, applySystemFields } from '@objectstack/objectql'; +import { InMemoryDriver } from '@objectstack/driver-memory'; +import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; +import { AnalyticsService } from '@objectstack/service-analytics'; +import stack from '../objectstack.config'; +import { OpportunityDataset } from '../src/datasets/opportunity.dataset'; + +/** + * Win/loss reasons are CAPTURED, and win rate is MEASURED (#593). + * + * Two halves, and neither is provable by reading metadata: + * + * ═══ 1. "declared" is not "enforced" ═══════════════════════════════════════ + * + * `crm_opportunity.loss_reason` carried the comment *"required when stage + * moves to closed_*"* for its whole life while nothing required anything. This + * repo has now measured FIVE separate metadata surfaces that accept a rule and + * then do not apply it, each failing differently: + * + * | surface | what actually happens | + * | ---------------------------------- | ------------------------------------ | + * | object script validation | unevaluable predicate → SKIPPED, WARN | + * | sharing rule condition | never interpreted — compiled to SQL | + * | flow record-change condition | run marked failed, THE WRITE LANDS | + * | `decision` node `config.condition` | key never read — inert metadata | + * | `flow.variables` declarations | declaring binds nothing at runtime | + * + * So the bar for this issue was never "the rule is declared". It is: drive a + * REAL write through a REAL driver and watch it be REJECTED. That is what the + * first two describe-blocks below do — on `InMemoryDriver` (whose stored rows + * omit unwritten columns, the shape that makes a non-total predicate abort) + * and again on a real SQLite database (column-complete rows, the shape a + * deployed install has). A rejection is asserted together with "and the record + * did not move", because a rule that reports a problem while the write lands + * anyway is the flow-condition failure mode above, not enforcement. + * + * `crm_case.resolution_required_for_closed` — the pattern the issue points at + * — was re-measured the same way rather than assumed, and it does block today; + * it is pinned below so that if it ever stops, this file says so instead of + * `crm_opportunity` quietly inheriting a broken idiom. + * + * ═══ 2. a ratio is the most dangerous kind of widget ═══════════════════════ + * + * `#614` shipped a dashboard that printed a 1,500,000 quarterly quota as + * 7,940,000; the root cause was a filter missing one of its two halves. It + * raised no error — a wrong denominator never does, it just returns a + * plausible number. A win rate, `won / (won + lost)`, fails identically. + * + * The last two describe-blocks therefore do not check that the widget exists. + * They run the SHIPPED widget bindings through the real dataset compiler and + * the real executor — once over a controlled fixture and once over the REAL + * seeded opportunities — and then PERTURB ONE DEAL AT A TIME: flip a won deal + * to lost, and the rate must fall; flip a lost deal to won, and it must rise; + * add an OPEN deal, and it must not move at all. A number that survives all + * three is a number whose numerator and denominator are both wired to + * something real. + */ + +type AnyRec = Record; + +const objects: AnyRec[] = (stack as any).objects ?? []; +const dashboards: AnyRec[] = (stack as any).dashboards ?? []; +const datasets: AnyRec[] = (stack as any).datasets ?? []; + +const opportunity = objects.find((o) => o.name === 'crm_opportunity') as AnyRec; +const kase = objects.find((o) => o.name === 'crm_case') as AnyRec; +const salesDashboard = dashboards.find((d) => d.name === 'sales_dashboard') as AnyRec; +const widget = (id: string): AnyRec => + (salesDashboard?.widgets ?? []).find((w: AnyRec) => w.id === id); + +/** `P` compiles to `{ dialect: 'cel', source }`. */ +const celSource = (v: unknown): string => + typeof v === 'string' ? v : String((v as AnyRec)?.source ?? ''); + +// ─────────────────────────────────── the shape the contract is declared in ── + +describe('the reason fields declare a conditional write contract', () => { + it('requires loss_reason exactly on closed_lost and win_reason on closed_won', () => { + expect(celSource(opportunity.fields.loss_reason.requiredWhen)).toBe( + 'has(record.stage) && record.stage == "closed_lost"', + ); + expect(celSource(opportunity.fields.win_reason.requiredWhen)).toBe( + 'has(record.stage) && record.stage == "closed_won"', + ); + }); + + it('guards the stage read with has(...) — the difference between enforced and inert', () => { + // Without `has()`, strict CEL aborts on any merged record that omits the + // column and the engine SKIPS the predicate ("requiredWhen for + // 'loss_reason' failed to evaluate — skipped"), leaving a rule that reads + // as enforced and requires nothing. `test/object-validation-predicates.ts` + // sweeps this property across the whole stack; it is restated here because + // it is the specific hazard #593 had to clear. + for (const field of ['win_reason', 'loss_reason']) { + expect(celSource(opportunity.fields[field].requiredWhen)).toContain('has(record.stage)'); + } + }); + + it('states the requirement in the field description a rep actually reads', () => { + expect(opportunity.fields.win_reason.description).toMatch(/required/i); + expect(opportunity.fields.loss_reason.description).toMatch(/required/i); + }); + + it('names the automated close path in the win_reason picklist', () => { + // `quote_on_accepted` wins the deal with no human in the write, so it has + // no rep answer to record. It writes `quote_accepted` rather than a + // fabricated "Better Product" — see the note on the field. + const values = (opportunity.fields.win_reason.options as AnyRec[]).map((o) => o.value); + expect(values).toContain('quote_accepted'); + }); +}); + +// ─────────────────────────── enforcement, on a driver with SPARSE records ── + +const OPEN_DEAL = { + name: 'Enforcement Probe', + crm_account: 'acc_stub', + amount: 120000, + stage: 'negotiation', + close_date: '2099-01-01', +}; + +describe('the write is REJECTED, not warned about (in-memory driver)', () => { + let ql: AnyRec; + + beforeAll(async () => { + ql = (await ObjectQL.create({ + datasources: { default: new InMemoryDriver({ persistence: false }) }, + objects: { crm_opportunity: opportunity } as never, + })) as never; + }); + afterAll(async () => { + await ql?.close(); + }); + + const newDeal = async () => { + const api = ql.createContext({ isSystem: true }); + const row = await api.object('crm_opportunity').insert({ ...OPEN_DEAL }); + return { api, row }; + }; + + it('stores no key for a column it was never given — the precondition', async () => { + const { api, row } = await newDeal(); + const stored = await api.object('crm_opportunity').findOne({ where: { id: row.id } }); + // Not `toBeNull()`: the key is ABSENT. That is the record shape that makes + // an unguarded predicate abort, and the reason this driver is the one the + // enforcement tests run on. If a platform upgrade makes it + // column-complete, this fails — which is the intended signal, because the + // tests below would stop proving the hard case. + expect('loss_reason' in (stored ?? {})).toBe(false); + }); + + it('refuses to close a deal as LOST with no loss_reason', async () => { + const { api, row } = await newDeal(); + await expect( + api.object('crm_opportunity').update({ stage: 'closed_lost' }, { where: { id: row.id } }), + ).rejects.toThrow(/Loss Reason is required/i); + + // The other half of the assertion, and the one that separates enforcement + // from the flow-condition failure mode (#633), where the rule complains + // and the triggering write lands anyway. + const after = await api.object('crm_opportunity').findOne({ where: { id: row.id } }); + expect(after?.stage).toBe('negotiation'); + }); + + it('refuses to close a deal as WON with no win_reason', async () => { + const { api, row } = await newDeal(); + await expect( + api.object('crm_opportunity').update({ stage: 'closed_won' }, { where: { id: row.id } }), + ).rejects.toThrow(/Win Reason is required/i); + const after = await api.object('crm_opportunity').findOne({ where: { id: row.id } }); + expect(after?.stage).toBe('negotiation'); + }); + + it('refuses an INSERT that lands directly in a closed stage', async () => { + // The path a data import, a seed file or an AI-authored write takes. On + // insert the engine fills absent fields with null rather than leaving them + // out, so this exercises a different branch of the same rule. + const api = ql.createContext({ isSystem: true }); + await expect( + api.object('crm_opportunity').insert({ ...OPEN_DEAL, name: 'Born Lost', stage: 'closed_lost' }), + ).rejects.toThrow(/Loss Reason is required/i); + await expect( + api.object('crm_opportunity').insert({ ...OPEN_DEAL, name: 'Born Won', stage: 'closed_won' }), + ).rejects.toThrow(/Win Reason is required/i); + }); + + it('accepts the close when the reason comes with it', async () => { + const { api, row } = await newDeal(); + await api + .object('crm_opportunity') + .update({ stage: 'closed_lost', loss_reason: 'price' }, { where: { id: row.id } }); + const after = await api.object('crm_opportunity').findOne({ where: { id: row.id } }); + expect(after?.stage).toBe('closed_lost'); + expect(after?.loss_reason).toBe('price'); + }); + + it('refuses to blank the reason out of a deal that is already closed', async () => { + // Otherwise the contract would hold for exactly one write: close with a + // reason, then clear it. + const api = ql.createContext({ isSystem: true }); + const row = await api + .object('crm_opportunity') + .insert({ ...OPEN_DEAL, name: 'Reason Eraser', stage: 'closed_won', win_reason: 'best_fit' }); + await expect( + api.object('crm_opportunity').update({ win_reason: null }, { where: { id: row.id } }), + ).rejects.toThrow(/Win Reason is required/i); + }); + + it('leaves writes that do not close anything alone', async () => { + // A rule that fires on unrelated edits is a rule someone disables. + const { api, row } = await newDeal(); + await api.object('crm_opportunity').update({ amount: 130000 }, { where: { id: row.id } }); + const after = await api.object('crm_opportunity').findOne({ where: { id: row.id } }); + expect(after?.amount).toBe(130000); + }); + + it('lets the OPPOSITE reason stay empty — the rule is per outcome', async () => { + const { api, row } = await newDeal(); + await api + .object('crm_opportunity') + .update({ stage: 'closed_won', win_reason: 'better_price' }, { where: { id: row.id } }); + const after = await api.object('crm_opportunity').findOne({ where: { id: row.id } }); + expect(after?.stage).toBe('closed_won'); + expect(after?.loss_reason ?? null).toBeNull(); + }); +}); + +// ────────────────────── the same contract on a real SQL database ────────── + +describe('the write is REJECTED on a real SQLite database too', () => { + // The in-memory driver hands back sparse records; a SQL driver hands back a + // full row with NULLs. Those are different inputs to the same predicate, and + // "which driver is underneath" is not something a marketplace app chooses — + // so the contract is measured on both rather than generalised from one. + let ql: AnyRec; + + beforeAll(async () => { + const driver = new SqliteWasmDriver({ filename: ':memory:' }); + await driver.connect(); + // The exact call the runtime makes at boot — `ObjectQL.create` wires the + // datasource but does not emit DDL, so without this the table does not + // exist and every write fails for the wrong reason. + const materialized = applySystemFields(opportunity as never, { multiTenant: false }) as AnyRec; + await driver.initObjects([ + { + name: 'crm_opportunity', + fields: materialized.fields as Record, + indexes: materialized.indexes, + } as never, + ]); + ql = (await ObjectQL.create({ + datasources: { default: driver }, + objects: { crm_opportunity: opportunity } as never, + })) as never; + }, 60_000); + afterAll(async () => { + await ql?.close(); + }); + + it('rejects the close and leaves the row where it was', async () => { + const api = ql.createContext({ isSystem: true }); + const row = await api.object('crm_opportunity').insert({ ...OPEN_DEAL, name: 'SQL Probe' }); + + const stored = await api.object('crm_opportunity').findOne({ where: { id: row.id } }); + // The opposite precondition to the in-memory suite: here the key IS + // present and null. Both shapes must reach the same verdict. + expect('loss_reason' in (stored ?? {})).toBe(true); + expect(stored?.loss_reason ?? null).toBeNull(); + + await expect( + api.object('crm_opportunity').update({ stage: 'closed_lost' }, { where: { id: row.id } }), + ).rejects.toThrow(/Loss Reason is required/i); + + const after = await api.object('crm_opportunity').findOne({ where: { id: row.id } }); + expect(after?.stage).toBe('negotiation'); + }); + + it('accepts the close with a reason', async () => { + const api = ql.createContext({ isSystem: true }); + const row = await api.object('crm_opportunity').insert({ ...OPEN_DEAL, name: 'SQL Probe 2' }); + await api + .object('crm_opportunity') + .update({ stage: 'closed_won', win_reason: 'relationship' }, { where: { id: row.id } }); + const after = await api.object('crm_opportunity').findOne({ where: { id: row.id } }); + expect(after?.stage).toBe('closed_won'); + expect(after?.win_reason).toBe('relationship'); + }); +}); + +// ───────────────────────── the pattern the issue told us to copy, re-measured ── + +describe('crm_case.resolution_required_for_closed is still live', () => { + // The issue named this as "exactly the pattern needed". It could have been + // one of the inert cases, so it was measured before being trusted. If this + // ever goes green-but-silent, the failure is here and not hidden inside + // crm_opportunity's copy of the idea. + let ql: AnyRec; + + beforeAll(async () => { + ql = (await ObjectQL.create({ + datasources: { default: new InMemoryDriver({ persistence: false }) }, + objects: { crm_case: kase } as never, + })) as never; + }); + afterAll(async () => { + await ql?.close(); + }); + + it('rejects closing a case with no resolution', async () => { + const api = ql.createContext({ isSystem: true }); + const row = await api.object('crm_case').insert({ + subject: 'Reference probe', + description: 'Measuring whether the named pattern still blocks writes.', + status: 'new', + priority: 'medium', + crm_account: 'acc_stub', + }); + await expect( + api.object('crm_case').update({ status: 'closed' }, { where: { id: row.id } }), + ).rejects.toThrow(/Resolution is required/i); + const after = await api.object('crm_case').findOne({ where: { id: row.id } }); + expect(after?.status).toBe('new'); + }); +}); + +// ─────────────────────────────────── the seeds cannot regress the contract ── + +/** `objectstack.config` registers seed buckets under `data`. */ +const seedRecords: AnyRec[] = (((stack as any).data ?? []) as AnyRec[]).find( + (s) => s.object === 'crm_opportunity', +)?.records ?? []; + +describe('every settled seed carries its reason', () => { + it('finds the opportunity seed at all', () => { + expect(seedRecords.length, 'no crm_opportunity seed records found').toBeGreaterThan(10); + }); + + it('gives every closed_won a win_reason and every closed_lost a loss_reason', () => { + // These rows go through the same `evaluateValidationRules` as a user write, + // so a missing reason does not degrade the demo — it stops the seed. The + // assertion exists so the failure names the record instead of surfacing as + // a boot-time ValidationError. + const bad = seedRecords + .filter((r) => r.stage === 'closed_won' || r.stage === 'closed_lost') + .filter((r) => (r.stage === 'closed_won' ? !r.win_reason : !r.loss_reason)) + .map((r) => `${r.name} (${r.stage})`); + expect(bad, `settled seeds with no reason:\n ${bad.join('\n ')}`).toEqual([]); + }); + + it('seeds enough distinct loss reasons for the breakdown to be a breakdown', () => { + const reasons = new Set( + seedRecords.filter((r) => r.stage === 'closed_lost').map((r) => r.loss_reason), + ); + expect(reasons.size).toBeGreaterThanOrEqual(4); + }); + + it('seeds at least one lead source that both wins and loses', () => { + // Otherwise `win_rate_by_lead_source` is a column of 100%s and blanks, and + // the denominator could be wrong without anybody noticing. + const won = new Set( + seedRecords.filter((r) => r.stage === 'closed_won').map((r) => r.lead_source), + ); + const mixed = seedRecords + .filter((r) => r.stage === 'closed_lost' && won.has(r.lead_source)) + .map((r) => r.lead_source); + expect([...new Set(mixed)].length).toBeGreaterThanOrEqual(2); + }); + + it('every seeded opportunity survives the engine that now enforces the rule', async () => { + // The rule applies to seed writes too — they go through the same + // `evaluateValidationRules` as a rep's form. So the cost of enforcing it is + // that a settled seed with no reason no longer degrades the demo, it stops + // the boot. This replays the real records through the REAL crm_opportunity + // metadata and asserts every one lands, which is the check that would have + // caught a missed row before `pnpm demo:reset` did. + // + // Only the CEL-valued columns are substituted: `close_date: cel`daysAgo(15)`` + // is an expression the seeder resolves at write time, and this assertion is + // about the validation verdict, not about date arithmetic. + const ql = (await ObjectQL.create({ + datasources: { default: new InMemoryDriver({ persistence: false }) }, + objects: { crm_opportunity: opportunity } as never, + })) as AnyRec; + try { + const api = ql.createContext({ isSystem: true }); + const isCel = (v: unknown) => !!v && typeof v === 'object' && 'source' in (v as AnyRec); + for (const rec of seedRecords) { + const row: AnyRec = {}; + for (const [k, v] of Object.entries(rec)) row[k] = isCel(v) ? '2026-05-01' : v; + await api.object('crm_opportunity').insert(row); + } + expect(await api.object('crm_opportunity').count({})).toBe(seedRecords.length); + } finally { + await ql?.close(); + } + }); + + it('uses only reason values the picklists declare', () => { + const allowed = (field: string) => + new Set((opportunity.fields[field].options as AnyRec[]).map((o) => String(o.value))); + const winValues = allowed('win_reason'); + const lossValues = allowed('loss_reason'); + const bad = seedRecords + .filter((r) => r.win_reason || r.loss_reason) + .filter( + (r) => + (r.win_reason && !winValues.has(String(r.win_reason))) + || (r.loss_reason && !lossValues.has(String(r.loss_reason))), + ) + .map((r) => `${r.name}: ${r.win_reason ?? r.loss_reason}`); + expect(bad, `seeded reason values off the picklist:\n ${bad.join('\n ')}`).toEqual([]); + }); +}); + +// ───────────────────────────────── the ratio, and both of its halves ── + +/** + * The three win/loss deals used by the runtime block, replayed per rep so the + * `by owner` grouping has something to group. Reasons are supplied because the + * engine under test is the same one that enforces them — which is itself a + * proof that the two halves of this issue agree with each other. + */ +const REPS = ['rep_alice', 'rep_bob'] as const; + +describe('win rate is measured, and both halves are load-bearing', () => { + let ql: AnyRec; + let analytics: AnalyticsService; + + const OPP_COLUMNS = ['name', 'stage', 'amount', 'owner', 'lead_source', 'win_reason', 'loss_reason', 'close_date']; + + beforeAll(async () => { + ql = (await ObjectQL.create({ + datasources: { default: new InMemoryDriver({ persistence: false }) }, + objects: { + // A shape-only stand-in: this block tests which rows the measures + // select and what they divide, not the object's own rules (those are + // measured above, against the real metadata). + crm_opportunity: { + name: 'crm_opportunity', + fields: { + id: { type: 'text' }, + name: { type: 'text' }, + stage: { type: 'text' }, + amount: { type: 'number' }, + owner: { type: 'text' }, + lead_source: { type: 'text' }, + win_reason: { type: 'text' }, + loss_reason: { type: 'text' }, + close_date: { type: 'date' }, + }, + }, + } as never, + })) as never; + + analytics = new AnalyticsService({ + // The same bridge `AnalyticsServicePlugin` wires at boot. + executeAggregate: async (objectName: string, opts: AnyRec) => + (ql as AnyRec).aggregate(objectName, { + where: opts.filter, + groupBy: opts.groupBy, + aggregations: opts.aggregations?.map((a: AnyRec) => ({ + function: a.method, field: a.field, alias: a.alias, + })), + timezone: opts.timezone, + context: opts.context, + }), + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + }); + }); + + afterAll(async () => { + await ql?.close(); + }); + + const api = () => ql.createContext({ isSystem: true }); + + /** Insert one deal; only the analytics columns are carried. */ + const add = async (rec: AnyRec) => { + const row: AnyRec = {}; + for (const c of OPP_COLUMNS) if (rec[c] !== undefined) row[c] = rec[c]; + return api().object('crm_opportunity').insert(row); + }; + + /** Run a SHIPPED widget binding through the real dataset executor. */ + const run = async (w: AnyRec, overrideFilter?: AnyRec) => { + const res = await analytics.queryDataset( + OpportunityDataset as never, + { + ...(w.dimensions ? { dimensions: w.dimensions } : {}), + measures: w.values, + ...(overrideFilter ?? w.filter ? { runtimeFilter: (overrideFilter ?? w.filter) as never } : {}), + } as never, + { isSystem: true } as never, + ); + return (res.rows ?? []) as AnyRec[]; + }; + + /** The overall win rate the KPI tile shows, with its date window dropped. */ + const overallWinRate = async () => { + const rows = await run(widget('win_rate_12m'), {}); + return rows[0]?.win_rate ?? null; + }; + + beforeAll(async () => { + // 4 won / 2 lost per rep → 8 won, 4 lost overall, win rate 2/3. + for (const rep of REPS) { + for (const [i, src] of ['web', 'web', 'referral', 'content'].entries()) { + await add({ + name: `${rep} win ${i}`, stage: 'closed_won', amount: 10000 * (i + 1), + owner: rep, lead_source: src, win_reason: 'best_fit', close_date: '2026-05-01', + }); + } + for (const [i, src] of ['web', 'cold_call'].entries()) { + await add({ + name: `${rep} loss ${i}`, stage: 'closed_lost', amount: 5000, + owner: rep, lead_source: src, loss_reason: i === 0 ? 'price' : 'no_budget', + close_date: '2026-05-01', + }); + } + // Open pipeline: must not touch either half of the ratio. + await add({ + name: `${rep} open`, stage: 'negotiation', amount: 999999, + owner: rep, lead_source: 'web', close_date: '2026-12-01', + }); + } + }); + + it('the shipped widgets exist and bind to the win/loss measures', () => { + // Guards the guard: every assertion below reads its query out of metadata, + // so a renamed widget would make them pass over nothing. + expect(widget('win_rate_12m')?.values).toEqual(['win_rate']); + expect(widget('win_rate_by_owner')?.dimensions).toEqual(['owner']); + expect(widget('win_rate_by_owner')?.values).toContain('won_count'); + expect(widget('win_rate_by_owner')?.values).toContain('lost_count'); + expect(widget('win_rate_by_lead_source')?.dimensions).toEqual(['lead_source']); + expect(widget('loss_reason_breakdown')?.dimensions).toEqual(['loss_reason']); + }); + + it('the win-rate widget carries NO stage filter of its own', () => { + // The #614 shape. A widget-level `stage` filter narrows numerator and + // denominator together, which turns the ratio into 1 (or into a division + // by itself) without any error. Both halves must come from the measures. + const f = JSON.stringify(widget('win_rate_12m')?.filter ?? {}); + expect(f).not.toContain('stage'); + const dataset = datasets.find((d) => d.name === 'opportunity_metrics') as AnyRec; + const measure = (n: string) => (dataset.measures as AnyRec[]).find((m) => m.name === n); + expect(measure('won_count')?.filter).toEqual({ stage: 'closed_won' }); + expect(measure('decided_count')?.filter).toEqual({ + stage: { $in: ['closed_won', 'closed_lost'] }, + }); + expect(measure('win_rate')?.derived).toEqual({ + op: 'ratio', + of: ['won_count', 'decided_count'], + }); + }); + + it('computes the overall win rate from the settled deals only', async () => { + // 8 won, 4 lost, 2 open. If the open deals leaked into the denominator the + // answer would be 8/14; if the numerator lost its filter it would be 1. + expect(await overallWinRate()).toBeCloseTo(8 / 12, 10); + }); + + it('the by-owner TABLE shows both halves next to the ratio', async () => { + const rows = await run(widget('win_rate_by_owner'), {}); + expect(rows).toHaveLength(REPS.length); + for (const rep of REPS) { + const row = rows.find((r) => r.owner === rep)!; + expect(row.won_count, `${rep} won`).toBe(4); + expect(row.lost_count, `${rep} lost`).toBe(2); + expect(row.decided_count, `${rep} settled`).toBe(6); + expect(row.win_rate, `${rep} rate`).toBeCloseTo(4 / 6, 10); + // The ratio is the two columns beside it, not an independent number. + expect(row.win_rate).toBeCloseTo(row.won_count / row.decided_count, 10); + // Revenue is won-only: 10k+20k+30k+40k. + expect(row.won_amount, `${rep} won revenue`).toBe(100000); + } + }); + + it('the by-lead-source table splits the same deals a different way', async () => { + const rows = await run(widget('win_rate_by_lead_source'), {}); + const bySource = new Map(rows.map((r) => [r.lead_source, r])); + // web: 2 wins + 1 loss per rep → 4 won, 2 lost. + expect(bySource.get('web')!.won_count).toBe(4); + expect(bySource.get('web')!.lost_count).toBe(2); + expect(bySource.get('web')!.win_rate).toBeCloseTo(4 / 6, 10); + // referral / content: wins only → 100%, NOT a blank. + expect(bySource.get('referral')!.win_rate).toBe(1); + expect(bySource.get('content')!.win_rate).toBe(1); + }); + + it('a source that only ever lost reports no wins — a measured platform gap', async () => { + // 17.0.0-rc.1: a filtered measure contributes no row for a group its + // filter selects nothing in, so `won_count` is ABSENT (not 0) for + // `cold_call`, and a derived ratio over an absent input is null. The rate + // reads blank where 0% would be right. + // + // Pinned rather than papered over: a consumer-side `?? 0` would be exactly + // the lenient-consumer habit this repo keeps paying for, and the fix + // belongs in the executor. The shipped table puts `lost_count` beside + // `win_rate` so the blank reads as "0 of 2", not as "no data". + const rows = await run(widget('win_rate_by_lead_source'), {}); + const coldCall = rows.find((r) => r.lead_source === 'cold_call')!; + expect(coldCall.lost_count).toBe(2); + expect(coldCall.decided_count).toBe(2); + expect(coldCall.won_count ?? null).toBeNull(); + expect(coldCall.win_rate ?? null).toBeNull(); + }); + + it('the loss-reason breakdown counts only lost deals, grouped by reason', async () => { + const rows = await run(widget('loss_reason_breakdown'), { stage: 'closed_lost' }); + const byReason = new Map(rows.map((r) => [r.loss_reason, r.opp_count])); + expect(byReason.get('price')).toBe(2); + expect(byReason.get('no_budget')).toBe(2); + // Won deals carry a win_reason and no loss_reason; none of them may appear. + expect(rows.every((r) => r.loss_reason != null)).toBe(true); + expect(rows.reduce((s, r) => s + Number(r.opp_count ?? 0), 0)).toBe(4); + }); + + // ─── the perturbation table: change one half, the number must move ─── + // + // | perturbation | won | settled | win rate | + // | ------------------------------- | --- | ------- | -------- | + // | (baseline) | 8 | 12 | 0.667 | + // | one WON deal flipped to lost | 7 | 12 | 0.583 | + // | one LOST deal flipped to won | 9 | 12 | 0.750 | + // | one OPEN deal added | 8 | 12 | 0.667 | + // + // Rows 2 and 3 are what make the numerator and the denominator load-bearing: + // row 2 moves the numerator only, row 3 moves it the other way, and row 4 + // proves the denominator is NOT "every opportunity". #614's quota table + // would have failed row 4's equivalent. + + it('falls when a won deal becomes a loss (numerator loses one)', async () => { + const baseline = await overallWinRate(); + expect(baseline).toBeCloseTo(8 / 12, 10); + + const target = await api() + .object('crm_opportunity') + .findOne({ where: { name: 'rep_alice win 0' } }); + await api().object('crm_opportunity').update( + { stage: 'closed_lost', win_reason: null, loss_reason: 'competitor' }, + { where: { id: target.id } }, + ); + expect(await overallWinRate()).toBeCloseTo(7 / 12, 10); + + // Restore, and confirm the restore is itself observable. + await api().object('crm_opportunity').update( + { stage: 'closed_won', loss_reason: null, win_reason: 'best_fit' }, + { where: { id: target.id } }, + ); + expect(await overallWinRate()).toBeCloseTo(8 / 12, 10); + }); + + it('rises when a lost deal becomes a win (numerator gains one)', async () => { + const target = await api() + .object('crm_opportunity') + .findOne({ where: { name: 'rep_bob loss 0' } }); + await api().object('crm_opportunity').update( + { stage: 'closed_won', loss_reason: null, win_reason: 'better_price' }, + { where: { id: target.id } }, + ); + expect(await overallWinRate()).toBeCloseTo(9 / 12, 10); + + await api().object('crm_opportunity').update( + { stage: 'closed_lost', win_reason: null, loss_reason: 'price' }, + { where: { id: target.id } }, + ); + expect(await overallWinRate()).toBeCloseTo(8 / 12, 10); + }); + + it('does NOT move when open pipeline is added — the denominator is settled deals', async () => { + const before = await overallWinRate(); + for (let i = 0; i < 5; i++) { + await add({ + name: `noise ${i}`, stage: 'proposal', amount: 500000, + owner: 'rep_alice', lead_source: 'web', close_date: '2026-11-01', + }); + } + expect(await overallWinRate()).toBeCloseTo(before as number, 10); + }); + + it('does not divide by zero when nothing has settled', async () => { + const rows = await run(widget('win_rate_12m'), { lead_source: 'webinar' }); + // No settled webinar deal exists: the honest answer is "no rate", not 0 + // and not a thrown error. + expect(rows[0]?.win_rate ?? null).toBeNull(); + }); +}); + +// ───────────────────── the acceptance criterion, over the shipped seeds ── + +/** + * "Sales dashboard shows win rate and a loss-reason breakdown with seeded + * data" — executed rather than eyeballed. + * + * The seed rows are replayed into a real engine and the SHIPPED widget + * bindings are run against them, so the numbers below are the numbers the + * dashboard prints on a freshly reset demo database. Only the columns the + * dataset reads are carried: `close_date` is a CEL expression in the seed + * (`daysAgo(15)`) that the seeder resolves at write time, and this block is + * about which rows the measures select, not about date arithmetic — the + * 12-month window is therefore applied as a fixed date here, and the widgets' + * own `{12_months_ago}` macro is covered by `analytics-integrity.test.ts`. + */ +describe('the shipped seeds produce a real win rate and a real loss breakdown', () => { + let ql: AnyRec; + let analytics: AnalyticsService; + + const settled = seedRecords.filter( + (r) => r.stage === 'closed_won' || r.stage === 'closed_lost', + ); + const wonSeeds = settled.filter((r) => r.stage === 'closed_won'); + const lostSeeds = settled.filter((r) => r.stage === 'closed_lost'); + + beforeAll(async () => { + ql = (await ObjectQL.create({ + datasources: { default: new InMemoryDriver({ persistence: false }) }, + objects: { + crm_opportunity: { + name: 'crm_opportunity', + fields: { + id: { type: 'text' }, + name: { type: 'text' }, + stage: { type: 'text' }, + amount: { type: 'number' }, + owner: { type: 'text' }, + lead_source: { type: 'text' }, + win_reason: { type: 'text' }, + loss_reason: { type: 'text' }, + close_date: { type: 'date' }, + }, + }, + } as never, + })) as never; + + const api = ql.createContext({ isSystem: true }); + for (const rec of seedRecords) { + await api.object('crm_opportunity').insert({ + name: rec.name, + stage: rec.stage, + amount: Number(rec.amount ?? 0), + lead_source: rec.lead_source ?? null, + win_reason: rec.win_reason ?? null, + loss_reason: rec.loss_reason ?? null, + close_date: '2026-05-01', + }); + } + + analytics = new AnalyticsService({ + executeAggregate: async (objectName: string, opts: AnyRec) => + (ql as AnyRec).aggregate(objectName, { + where: opts.filter, + groupBy: opts.groupBy, + aggregations: opts.aggregations?.map((a: AnyRec) => ({ + function: a.method, field: a.field, alias: a.alias, + })), + timezone: opts.timezone, + context: opts.context, + }), + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + }); + }); + + afterAll(async () => { + await ql?.close(); + }); + + const run = async (w: AnyRec, runtimeFilter?: AnyRec) => + ((await analytics.queryDataset( + OpportunityDataset as never, + { + ...(w.dimensions ? { dimensions: w.dimensions } : {}), + measures: w.values, + ...(runtimeFilter ? { runtimeFilter: runtimeFilter as never } : {}), + } as never, + { isSystem: true } as never, + )).rows ?? []) as AnyRec[]; + + it('ships both outcomes — the premise of a win RATE', () => { + // A seed set that is all wins makes every assertion below pass at 100% + // without measuring anything. + expect(wonSeeds.length).toBeGreaterThan(0); + expect(lostSeeds.length).toBeGreaterThan(0); + expect(seedRecords.length).toBeGreaterThan(settled.length); // open deals exist too + }); + + it('the KPI tile shows won / (won + lost) over the seeded deals', async () => { + const rows = await run(widget('win_rate_12m')); + const expected = wonSeeds.length / settled.length; + expect(rows[0]?.won_count).toBe(wonSeeds.length); + expect(rows[0]?.decided_count).toBe(settled.length); + expect(rows[0]?.win_rate).toBeCloseTo(expected, 10); + // And it is NOT "won over every opportunity" — the number the denominator + // would produce if the measure filter went missing. + expect(rows[0]?.win_rate).not.toBeCloseTo(wonSeeds.length / seedRecords.length, 6); + }); + + it('the loss-reason breakdown covers every seeded loss, one slice per reason', async () => { + const rows = await run(widget('loss_reason_breakdown'), { stage: 'closed_lost' }); + const total = rows.reduce((s, r) => s + Number(r.opp_count ?? 0), 0); + expect(total).toBe(lostSeeds.length); + expect(rows.length).toBe(new Set(lostSeeds.map((r) => r.loss_reason)).size); + expect(rows.every((r) => r.loss_reason != null)).toBe(true); + }); + + it('the by-source table has rows where BOTH halves are non-zero', async () => { + // The seeded lead sources are chosen so this holds (see sales.seed.ts). A + // table where every row is "n won, 0 lost" cannot detect a broken + // denominator, which is precisely how #614 shipped. + const rows = await run(widget('win_rate_by_lead_source')); + const mixed = rows.filter((r) => Number(r.won_count ?? 0) > 0 && Number(r.lost_count ?? 0) > 0); + expect(mixed.length).toBeGreaterThanOrEqual(2); + for (const row of mixed) { + expect(row.decided_count).toBe(Number(row.won_count) + Number(row.lost_count)); + expect(row.win_rate).toBeCloseTo(Number(row.won_count) / Number(row.decided_count), 10); + expect(row.win_rate).toBeGreaterThan(0); + expect(row.win_rate).toBeLessThan(1); + } + }); + + it('losing one seeded win moves the rate, and only by one deal', async () => { + const before = (await run(widget('win_rate_12m')))[0]; + const api = ql.createContext({ isSystem: true }); + const target = await api + .object('crm_opportunity') + .findOne({ where: { name: wonSeeds[0].name } }); + + await api.object('crm_opportunity').update( + { stage: 'closed_lost', win_reason: null, loss_reason: 'competitor' }, + { where: { id: target.id } }, + ); + const after = (await run(widget('win_rate_12m')))[0]; + + expect(after.won_count).toBe(Number(before.won_count) - 1); + expect(after.decided_count).toBe(before.decided_count); // denominator holds + expect(after.win_rate).toBeLessThan(Number(before.win_rate)); + + await api.object('crm_opportunity').update( + { stage: 'closed_won', loss_reason: null, win_reason: wonSeeds[0].win_reason }, + { where: { id: target.id } }, + ); + expect((await run(widget('win_rate_12m')))[0].win_rate).toBeCloseTo( + Number(before.win_rate), + 10, + ); + }); +}); From f847adff49ee2b1bc80e5515121ebf52edd96251 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 19:56:19 +0000 Subject: [PATCH 2/2] test(e2e): assert the win/loss capture rule on the real HTTP path (#593) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three `opportunity-lifecycle` fixtures that close a deal date from before closing required a reason: they PATCH an opportunity to closed_won/closed_lost with no reason, and the kernel now answers `400 {"code":"VALIDATION_FAILED","fields":[{"field":"win_reason",...}]}`. That is the rule working — and it is stronger evidence than any unit test in this PR, because it lands on the real REST write path against the real database the demo boots with, not in a harness. But it only existed as collateral damage to neighbouring tests, which disappears the moment the fixtures are fixed. So: - the three fixtures supply a reason (`best_fit` / `no_budget` / `better_price`, never `quote_accepted` — that value marks the CPQ automation path and a hand-closed fixture using it would blur exactly the distinction it exists to draw), keeping them about the lifecycle hook; - a new `opportunity-win-loss-capture.spec.ts` makes the rejection a STANDING assertion: PATCH to each closed stage without a reason is 400 AND the deal stays at `negotiation`, POST straight into a closed stage is 400 too, the same close succeeds once a reason is supplied (so the suite cannot pass by the API refusing stage changes outright), and every settled record the server actually loaded carries its reason — read back from the booted database, which is the only way to see the result of seeding under the new rule. Also names #656 in the two comments that previously said "filed upstream". Verified locally: `pnpm test:e2e` → 16 passed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019SS7C5SXpniKeCApxgARyf --- e2e/opportunity-lifecycle.spec.ts | 18 ++- e2e/opportunity-win-loss-capture.spec.ts | 160 +++++++++++++++++++++++ src/datasets/opportunity.dataset.ts | 2 +- test/win-loss-capture.test.ts | 13 +- 4 files changed, 185 insertions(+), 8 deletions(-) create mode 100644 e2e/opportunity-win-loss-capture.spec.ts diff --git a/e2e/opportunity-lifecycle.spec.ts b/e2e/opportunity-lifecycle.spec.ts index e6d01b72..87de154e 100644 --- a/e2e/opportunity-lifecycle.spec.ts +++ b/e2e/opportunity-lifecycle.spec.ts @@ -89,9 +89,17 @@ test.describe('opportunity_lifecycle hook (through the real kernel)', () => { const id = rec.id as string; created.push(id); - const updated = await patchOpportunity(api, id, { stage: 'closed_won' }); + // `win_reason` is `requiredWhen` stage is closed_won (#593) — the server + // rejects the close without it with a 400 VALIDATION_FAILED, which is what + // `opportunity-win-loss-capture.spec.ts` asserts on purpose. Here it is + // supplied so this file keeps testing the hook it is about. + const updated = await patchOpportunity(api, id, { + stage: 'closed_won', + win_reason: 'best_fit', + }); expect(updated.stage).toBe('closed_won'); + expect(updated.win_reason).toBe('best_fit'); expect(updated.probability).toBe(100); expect(updated.expected_revenue).toBe(25_000); expect(updated.forecast_category).toBe('closed'); @@ -124,9 +132,13 @@ test.describe('opportunity_lifecycle hook (through the real kernel)', () => { const id = rec.id as string; created.push(id); - const updated = await patchOpportunity(api, id, { stage: 'closed_lost' }); + const updated = await patchOpportunity(api, id, { + stage: 'closed_lost', + loss_reason: 'no_budget', // requiredWhen closed_lost (#593) + }); expect(updated.stage).toBe('closed_lost'); + expect(updated.loss_reason).toBe('no_budget'); expect(updated.probability).toBe(0); expect(updated.expected_revenue).toBe(0); expect(updated.forecast_category).toBe('omitted'); @@ -159,7 +171,7 @@ test.describe('opportunity_lifecycle hook (through the real kernel)', () => { }); const id = rec.id as string; created.push(id); - await patchOpportunity(api, id, { stage: 'closed_won' }); + await patchOpportunity(api, id, { stage: 'closed_won', win_reason: 'better_price' }); // Business field — the freeze guard must reject it. const rejected = await api.patch(`${BASE}/${id}`, { data: { amount: 999 } }); diff --git a/e2e/opportunity-win-loss-capture.spec.ts b/e2e/opportunity-win-loss-capture.spec.ts new file mode 100644 index 00000000..63e6cc32 --- /dev/null +++ b/e2e/opportunity-win-loss-capture.spec.ts @@ -0,0 +1,160 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { test, expect, recordOf, recordsOf, seededAccountId } from './fixtures'; +import type { APIRequestContext } from '@playwright/test'; + +/** + * Win/loss reason capture, asserted on the REAL HTTP write path (#593). + * + * `test/win-loss-capture.test.ts` proves the same contract through ObjectQL on + * two drivers. This file proves it one layer further out — through the REST + * API, over the real kernel, against the real database the demo boots with — + * because "the engine rejects it in a test harness" and "a client gets a 400" + * are not the same claim, and this issue's acceptance criterion is the second + * one. + * + * It exists as a STANDING assertion rather than as a side effect. When the rule + * first landed, its only evidence on this path was three neighbouring lifecycle + * fixtures blowing up because they closed deals without a reason — real + * evidence, but the kind that disappears the moment someone fixes the + * fixtures. A rule whose only proof is another test's collateral damage is a + * rule that can be silently removed. + */ + +const BASE = '/api/v1/data/crm_opportunity'; + +/** An open deal to close, and its id registered for cleanup. */ +async function openDeal( + api: APIRequestContext, + name: string, + accountId: string, +): Promise> { + const res = await api.post(BASE, { + data: { + name, + stage: 'negotiation', + amount: 30_000, + close_date: '2099-12-31', + crm_account: accountId, + }, + }); + expect(res.ok(), `create failed: ${res.status()} ${await res.text()}`).toBeTruthy(); + return recordOf(await res.json()); +} + +test.describe('closing an opportunity requires a reason (real HTTP path)', () => { + let accountId: string | undefined; + const created: string[] = []; + + test.beforeEach(async ({ api }) => { + accountId ??= await seededAccountId(api); + }); + + test.afterEach(async ({ api }) => { + while (created.length) { + const id = created.pop()!; + await api.delete(`${BASE}/${id}`).catch(() => undefined); + } + }); + + test('PATCH to closed_lost with no loss_reason is rejected, and the deal does not move', async ({ + api, + }) => { + const rec = await openDeal(api, 'E2E Lost Without Reason', accountId!); + const id = rec.id as string; + created.push(id); + + const res = await api.patch(`${BASE}/${id}`, { data: { stage: 'closed_lost' } }); + + expect(res.ok(), 'the server accepted a close with no loss_reason').toBeFalsy(); + expect(res.status()).toBe(400); + const body = (await res.json()) as Record; + expect(body.code).toBe('VALIDATION_FAILED'); + // Reported against the FIELD — that is why the rule is a `requiredWhen` and + // not a record-level script validation: the form can mark the empty + // picklist instead of showing a banner. + expect(JSON.stringify(body.fields)).toContain('loss_reason'); + + // The half that separates enforcement from a warning: the write did not + // land. A rule that complains while the record moves anyway is the + // flow-condition failure mode (#633), not enforcement. + const after = recordOf(await (await api.get(`${BASE}/${id}`)).json()); + expect(after.stage).toBe('negotiation'); + }); + + test('PATCH to closed_won with no win_reason is rejected, and the deal does not move', async ({ + api, + }) => { + const rec = await openDeal(api, 'E2E Won Without Reason', accountId!); + const id = rec.id as string; + created.push(id); + + const res = await api.patch(`${BASE}/${id}`, { data: { stage: 'closed_won' } }); + + expect(res.ok(), 'the server accepted a close with no win_reason').toBeFalsy(); + expect(res.status()).toBe(400); + expect(JSON.stringify(await res.json())).toContain('win_reason'); + + const after = recordOf(await (await api.get(`${BASE}/${id}`)).json()); + expect(after.stage).toBe('negotiation'); + }); + + test('POST that lands directly in a closed stage is rejected too', async ({ api }) => { + // The path an import or an API integration takes. Insert is a different + // branch of the rule — the engine fills absent fields with null there + // rather than leaving them out — so it is asserted separately. + const res = await api.post(BASE, { + data: { + name: 'E2E Born Lost', + stage: 'closed_lost', + amount: 30_000, + close_date: '2020-01-01', + crm_account: accountId, + }, + }); + expect(res.ok(), 'the server accepted an insert straight into closed_lost').toBeFalsy(); + expect(res.status()).toBe(400); + expect(JSON.stringify(await res.json())).toContain('loss_reason'); + }); + + test('the same close SUCCEEDS once the reason is supplied', async ({ api }) => { + // Without this, every assertion above would still pass if the API had + // simply stopped accepting stage changes at all. + const rec = await openDeal(api, 'E2E Lost With Reason', accountId!); + const id = rec.id as string; + created.push(id); + + const res = await api.patch(`${BASE}/${id}`, { + data: { stage: 'closed_lost', loss_reason: 'competitor' }, + }); + expect(res.ok(), `close with a reason failed: ${res.status()} ${await res.text()}`).toBeTruthy(); + + const updated = recordOf(await res.json()); + expect(updated.stage).toBe('closed_lost'); + expect(updated.loss_reason).toBe('competitor'); + }); + + test('every seeded settled deal carries its reason — the demo boots with the rule on', async ({ + api, + }) => { + // The rule applies to seed writes too, so a settled seed with no reason + // does not degrade the demo, it stops the boot. This reads the database the + // server actually loaded rather than the seed source, which is the only + // way to see the result of that boot. + const res = await api.get(`${BASE}?limit=200`); + expect(res.ok(), `could not list opportunities: ${res.status()}`).toBeTruthy(); + const rows = recordsOf(await res.json()); + + const won = rows.filter((r) => r.stage === 'closed_won'); + const lost = rows.filter((r) => r.stage === 'closed_lost'); + // Guards the guard: an empty database would make the sweep below vacuous. + expect(won.length, 'no closed_won opportunities loaded').toBeGreaterThan(0); + expect(lost.length, 'no closed_lost opportunities loaded').toBeGreaterThan(0); + + const missing = [ + ...won.filter((r) => !r.win_reason).map((r) => `${r.name}: no win_reason`), + ...lost.filter((r) => !r.loss_reason).map((r) => `${r.name}: no loss_reason`), + ]; + expect(missing, `settled deals with no reason:\n ${missing.join('\n ')}`).toEqual([]); + }); +}); diff --git a/src/datasets/opportunity.dataset.ts b/src/datasets/opportunity.dataset.ts index f801e91f..8f2b3d70 100644 --- a/src/datasets/opportunity.dataset.ts +++ b/src/datasets/opportunity.dataset.ts @@ -75,7 +75,7 @@ export const OpportunityDataset = defineDataset({ // all. The remaining asymmetry — a group with losses but NO wins reports a // null win rate rather than 0% — is a platform behaviour, not something a // widget can paper over; it is pinned by `test/win-loss-capture.test.ts` - // and filed upstream. The tables show `lost_count` next to `win_rate` + // and filed as #656. The tables show `lost_count` next to `win_rate` // precisely so that a blank rate reads as "0 wins out of 3", not as // "no data". { name: 'won_count', label: 'Won Deals', aggregate: 'count', filter: { stage: 'closed_won' } }, diff --git a/test/win-loss-capture.test.ts b/test/win-loss-capture.test.ts index 161b335d..c5144e4d 100644 --- a/test/win-loss-capture.test.ts +++ b/test/win-loss-capture.test.ts @@ -598,10 +598,15 @@ describe('win rate is measured, and both halves are load-bearing', () => { // `cold_call`, and a derived ratio over an absent input is null. The rate // reads blank where 0% would be right. // - // Pinned rather than papered over: a consumer-side `?? 0` would be exactly - // the lenient-consumer habit this repo keeps paying for, and the fix - // belongs in the executor. The shipped table puts `lost_count` beside - // `win_rate` so the blank reads as "0 of 2", not as "no data". + // Pinned rather than papered over (#656): a consumer-side `?? 0` would be + // exactly the lenient-consumer habit this repo keeps paying for, and the + // fix belongs in the executor's merge step, which is the only place that + // knows a `count` over no rows is 0 while an `avg` over no rows is not. + // The shipped table puts `lost_count` beside `win_rate` so the blank reads + // as "0 of 2", not as "no data". + // + // This assertion is deliberately written to FAIL when the platform fixes + // it — that is the notification, not a regression. const rows = await run(widget('win_rate_by_lead_source'), {}); const coldCall = rows.find((r) => r.lead_source === 'cold_call')!; expect(coldCall.lost_count).toBe(2);