Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/2060-item-2b-nonpositive-amount.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"hotcrm": patch
---

**Converting a lead with Create Opportunity ticked and an amount of zero or less no longer leaves it half-converted.** Every opportunity needs an amount greater than zero, but the conversion dialog accepted 0 or a negative figure. Submitting it created the account and the contact first, then failed on the opportunity with "Amount must be greater than zero". The lead was left with a new account and contact, no opportunity, and still not converted.

Such a conversion now stops right after the dialog, before anything is created, and says why: the opportunity amount must be greater than zero. Convert the lead again with the amount the deal is worth, or leave Create Opportunity unticked to convert it without one. A positive amount converts exactly as before.
53 changes: 48 additions & 5 deletions src/sales/flows/lead-conversion.flow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,12 @@ export const LeadConversionFlow: Flow = {
// pinned in `test/flow-variable-conditions.test.ts`.
{ name: 'createOpportunity', type: 'boolean', isInput: true, isOutput: false, defaultValue: false },
{ name: 'opportunityName', type: 'text', isInput: true, isOutput: false },
{ name: 'opportunityAmount', type: 'text', isInput: true, isOutput: false },
// Bound to `null` up front because `decision_amount`'s edges read it: a
// runner may post nothing for an unticked box, and an unbound read fails
// the run (`test/flow-variable-conditions.test.ts`, CLASS 2). The screen
// answer overwrites it, and a ticked box with no amount is still refused
// by the screen's `required` before any edge is read.
{ name: 'opportunityAmount', type: 'text', isInput: true, isOutput: false, defaultValue: null },
// The conversion's close date, collected on the screen below
// instead of stamped inside `create_opportunity`. Declared with NO
// `defaultValue`, and that is measured rather than chosen: the engine
Expand Down Expand Up @@ -138,7 +143,10 @@ export const LeadConversionFlow: Flow = {
// `notNull`, and without this a ticked box with no amount reached
// `create_opportunity` after the account and contact were written,
// leaving the lead half-converted (#2060 item 2). Visible-only, so
// an unticked box still converts with no amount.
// an unticked box still converts with no amount. The column's other
// rule, `amount_positive` (greater than zero), cannot be said here:
// a screen field's `min` is inclusive. `decision_amount` refuses it
// instead, before any write (#2060 item 2b).
{ name: 'opportunityName', label: 'Opportunity Name', type: 'text', required: true, defaultValue: cel`leadRecord.company`, visibleWhen: 'createOpportunity == true' },
{ name: 'opportunityAmount', label: 'Opportunity Amount', type: 'currency', required: true, defaultValue: cel`has(leadRecord.estimated_amount) ? leadRecord.estimated_amount : null`, visibleWhen: 'createOpportunity == true' },
// The close date, DEFAULTED AND VISIBLE. `close_date` is what files
Expand Down Expand Up @@ -362,6 +370,34 @@ export const LeadConversionFlow: Flow = {
waitForInput: true,
},
},
{
// Branching is on edges `e44` / `e45` — see `decision_account`. The
// only gate AFTER `screen_1`, because it judges an answer the rep typed
// there, and it dominates every write (`find_account` below only reads).
id: 'decision_amount', type: 'decision', label: 'Opportunity Amount Above Zero?',
},
{
// The NON-POSITIVE AMOUNT refusal (#2060 item 2b). `crm_opportunity`'s
// `amount_positive` refuses an amount of zero or less, and the platform
// does not undo a failed run, so letting it reach `create_opportunity`
// left the account and contact written and the lead unconverted. The
// edges restate that rule's predicate exactly (`<= 0`, null excluded:
// an empty amount is the screen's `required`), so the two cannot
// disagree about which amounts convert.
//
// A first-class refusal (`EndConfigSchema` `outcome: 'refused'`), as in
// `quote_generation`, and ⛔ not the message-only screen the three
// refusals above use: the run ends `refused` rather than `completed`,
// and the console shows the message with Close only. A screen here was
// measured to toast "Saved — next step" over a refusal, and its Submit
// toasts `convert_lead`'s "Lead converted successfully!". Translated as
// `refusals.refuse_nonpositive_amount.message` in the packs.
id: 'refuse_nonpositive_amount', type: 'end', label: 'Conversion Refused',
config: {
outcome: 'refused',
message: 'Conversion refused: an opportunity amount must be greater than zero. Nothing was created. Convert the lead again with the amount the deal is worth, or leave Create Opportunity unticked to convert it without one.',
},
},
{
// Account dedupe: match on the NORMALIZED company name, never on the raw
// one — "Acme Corp" and "ACME Corp" are the same account.
Expand Down Expand Up @@ -651,15 +687,15 @@ export const LeadConversionFlow: Flow = {

edges: [
// ⛔ A retired edge's id stays VACANT and every surviving edge keeps the id
// it has always had — `e1`, `e3`, `e7` and `e15` are gaps on purpose.
// it has always had — `e1`, `e2`, `e3`, `e7` and `e15` are gaps on purpose.
// Renumbering to close a gap rewrites ids that other notes, tests and run
// logs refer to.
{ id: 'e0', source: 'start', target: 'get_lead', type: 'default' },
// ⛔ When you add an edge, take the next id AFTER THE HIGHEST IN USE — do
// not reuse a vacant one, and do not assume an unused-looking id is free.
// A duplicate id is INERT (traversal filters out-edges by `source`, never
// by `id`), so nothing fails and the collision survives to trap the next
// editor picking out of the sequence. `e1`, `e3`, `e7` and `e15` are
// editor picking out of the sequence. `e1`, `e2`, `e3`, `e7` and `e15` are
// vacant retired ids and stay that way; `e27` is above the highest live
// id, not a gap-fill.
{ id: 'e27', source: 'get_lead', target: 'decision_approval', type: 'default' },
Expand Down Expand Up @@ -736,7 +772,14 @@ export const LeadConversionFlow: Flow = {
// every create/update sits behind `screen_1`, which this path never
// reaches.
{ id: 'e26', source: 'refuse_confirmed_duplicate', target: 'end', type: 'default' },
{ id: 'e2', source: 'screen_1', target: 'find_account', type: 'default' },
// `e2` (screen_1 → find_account) is RETIRED and stays vacant (#2060 item
// 2b): the amount gate sits between them now, on `e43` – `e45`. The pair
// is TOTAL and PARTITIONING (`e45` is `e44` negated by De Morgan), pinned
// in `test/flow-conversion.test.ts`; an unticked box never reads the amount.
// `e45` ends the run at its target, a refusing `end`: no edge leaves it.
{ id: 'e43', source: 'screen_1', target: 'decision_amount', type: 'default' },
{ id: 'e44', source: 'decision_amount', target: 'find_account', type: 'default', condition: P`vars.createOpportunity != true || vars.opportunityAmount == null || vars.opportunityAmount > 0`, label: 'Amount accepted' },
{ id: 'e45', source: 'decision_amount', target: 'refuse_nonpositive_amount', type: 'default', condition: P`vars.createOpportunity == true && vars.opportunityAmount != null && vars.opportunityAmount <= 0`, label: 'Zero or less' },
{ id: 'e4', source: 'find_account', target: 'decision_account', type: 'default' },
// Existing account → reuse; no account → create. Both converge on create_contact.
// `find_account` is reached only with a match key (`e31`), so a match
Expand Down
6 changes: 6 additions & 0 deletions src/sales/translations/es-ES/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -686,6 +686,12 @@ export const appSurface: Omit<TranslationData, 'objects'> = {
'La empresa de este prospecto no tiene clave de coincidencia, así que la conversión no puede saber a qué cuenta pertenece y no lo va a adivinar: una suposición podría archivar el contacto y la oportunidad bajo otro cliente. Guardar la Empresa del prospecto reconstruye la clave, así que guárdela de nuevo y luego conviértalo. No se ha creado nada.',
},
},
refusals: {
refuse_nonpositive_amount: {
message:
'Conversión rechazada: el monto de una oportunidad debe ser mayor que cero. No se creó nada. Vuelva a convertir el prospecto con el monto que vale el negocio, o deje sin marcar ¿Crear oportunidad? para convertirlo sin una.',
},
},
},
schedule_followup: {
label: 'Programar Seguimiento',
Expand Down
6 changes: 6 additions & 0 deletions src/sales/translations/ja-JP/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,12 @@ export const appSurface: Omit<TranslationData, 'objects'> = {
'このリードの会社名には照合キーがないため、変換ではどの取引先に属するかを判断できず、推測もしません。推測すると、取引先責任者と商談が別の顧客の下に登録されるおそれがあります。リードの「会社名」を保存するとキーが再作成されるので、もう一度保存してから変換してください。何も作成されていません。',
},
},
refusals: {
refuse_nonpositive_amount: {
message:
'変換は拒否されました。商談金額は 0 より大きくなければなりません。何も作成されていません。取引の金額を入力してもう一度リードを変換するか、「商談を作成しますか?」のチェックを外して商談なしで変換してください。',
},
},
},
schedule_followup: {
label: 'フォローアップを設定',
Expand Down
6 changes: 6 additions & 0 deletions src/sales/translations/zh-CN/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,12 @@ export const appSurface: Omit<TranslationData, 'objects'> = {
'该线索的公司没有匹配键,因此转化无法判断它属于哪个客户,也不会去猜测:猜测可能会把联系人和商机归到另一个客户名下。保存线索的「公司」会重新生成匹配键,请重新保存后再转化。尚未创建任何记录。',
},
},
refusals: {
refuse_nonpositive_amount: {
message:
'转化被拒绝:商机金额必须大于零。尚未创建任何内容。请填写这笔交易的金额后重新转化该线索,或不勾选「是否创建商机?」,以不创建商机的方式转化。',
},
},
},
schedule_followup: {
label: '安排跟进',
Expand Down
73 changes: 73 additions & 0 deletions test/flow-conversion.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,79 @@ describe('lead_conversion — a ticked Create Opportunity needs an amount before
});
});

/**
* A ticked Create Opportunity with an amount of zero or less is refused
* BEFORE anything is written (#2060 item 2b).
*
* `crm_opportunity`'s `amount_positive` rule refuses an amount of zero or
* less, and a screen field's `min` is inclusive, so the dialog cannot state
* "greater than zero". On 18.0.0-next.1 such a submission reached
* `create_opportunity` after `create_account` and `create_contact` had run:
* `FLOW_FAILED`, one account, one contact, no opportunity, an unconverted
* lead. `decision_amount` now turns it away right after the dialog, at a
* refusing `end`: the resume SUCCEEDS and the run ends `refused` (the only
* refusal in this flow that ends that way), and the claim is that outcome
* and an empty store, compared in one object.
*/
describe('lead_conversion — a ticked Create Opportunity with an amount of zero or less is refused before anything is written (#2060 item 2b)', () => {
const state = async (who: { company: string; email: string }, leadId: string) => {
const out = await after(who, leadId);
return {
accounts: out.accounts.length, contacts: out.contacts.length, opportunities: out.opportunities.length,
isConverted: Boolean(out.lead.is_converted), leadStatus: out.lead.status,
};
};
const ticked = (amount: number) => ({ createOpportunity: true, opportunityName: 'Deal', opportunityAmount: amount });

it.each([['zero', 0], ['negative', -5]] as const)('amount %s: refused with nothing written, and a new run with an amount converts', async (_label, amount) => {
const who = identity();
const lead = await qualifiedLead(who);
const refused = (await runConversion(lead.id, ticked(amount)).catch((e: unknown) => e)) as Rec;
expect({
code: refused.code, success: refused.success, status: refused.status, runId: refused.runId,
...(await state(who, lead.id)),
}).toEqual({
code: undefined, success: true, status: 'refused', runId: undefined,
accounts: 0, contacts: 0, opportunities: 0, isConverted: false, leadStatus: 'qualified',
});
expect(String(refused.refusalMessage ?? ''), 'the rep is not told why').not.toBe('');

expect((await runConversion(lead.id, ticked(25_000))).error ?? null).toBeNull();
expect(await state(who, lead.id)).toEqual({
accounts: 1, contacts: 1, opportunities: 1, isConverted: true, leadStatus: 'converted',
});
expect((await after(who, lead.id)).opportunities[0]!.amount).toBe(25_000);
});

it('control: an unticked box converts without an opportunity whatever amount is posted', async () => {
const who = identity();
const lead = await qualifiedLead(who);
expect((await runConversion(lead.id, { createOpportunity: false, opportunityAmount: 0 })).error ?? null).toBeNull();
expect(await state(who, lead.id)).toEqual({
accounts: 1, contacts: 1, opportunities: 0, isConverted: true, leadStatus: 'converted',
});
});

it('decision_amount answers with exactly one live edge on every shape', () => {
const out = (LeadConversionFlow.edges as Rec[]).filter((e) => e.source === 'decision_amount');
expect(out, 'decision_amount must branch two ways').toHaveLength(2);
for (const [shape, vars, refuse] of [
['unticked, no amount', { createOpportunity: false, opportunityAmount: null }, false],
['unticked, zero', { createOpportunity: false, opportunityAmount: 0 }, false],
['ticked, zero', { createOpportunity: true, opportunityAmount: 0 }, true],
['ticked, negative', { createOpportunity: true, opportunityAmount: -5 }, true],
['ticked, a cent', { createOpportunity: true, opportunityAmount: 0.01 }, false],
['ticked, positive', { createOpportunity: true, opportunityAmount: 25_000 }, false],
// Unreachable at runtime (the screen's `required` refuses it first); the edges stay total on it.
['ticked, no amount', { createOpportunity: true, opportunityAmount: null }, false],
] as Array<[string, Rec, boolean]>) {
const live = out.filter((e) => conditionHolds(verify, e.condition, vars));
expect(live, shape).toHaveLength(1);
expect(live[0]!.target === 'refuse_nonpositive_amount', shape).toBe(refuse);
}
});
});

/**
* A sales REP converts a lead into a NEW account (#2016).
*
Expand Down
Loading