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-6-ownerless-lead-assignment.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'hotcrm': patch
---

**A new lead with no owner no longer fails New Lead Routing & SLA.** A lead that lands without an owner — a Web-to-Lead submission while nobody can be assigned from the Sales Rep pool, or a lead written by an import or an integration without one — still got its Next Follow-up Date, but the routing run then failed while sending its alert, because there was no owner to send it to. The failure showed up only in the flow's run history and the server log.

Such a lead now gets its Next Follow-up Date (tomorrow at four stars or better, otherwise in three days) and no alert, and the run ends successfully. A lead with an owner is routed exactly as before: its owner is alerted.
44 changes: 39 additions & 5 deletions src/sales/flows/lead-assignment.flow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@ type Flow = Automation.Flow;
* Fires on lead *insert*, sets a rating-based `next_followup_date` SLA, and
* alerts the lead's OWNER — the accountable party — to follow up.
*
* A lead with NO owner gets its SLA stamp and no alert, and the run ends
* there successfully (#2060 item 6). Web-to-lead lands ownerless whenever
* `lead_auto_assign` stands down (an empty `sales_rep` pool, or a pool the
* writer cannot read), and so can a seed or integration write. There is
* nobody to alert: the business declares no recipient for ownerless intake
* (no queue, no position — `content/docs/administration/setup.mdx` §8), and
* `notify` REFUSES an audience that evaluates to nothing, so before the owner
* gates below the run FAILED at the notify node, after the stamp had landed.
*
* ⛔ Never address a CRM position (`sales_manager` and friends) as a recipient.
* Positions are not a messaging audience: the messaging service resolves
* `user:` / `role:` (sys_member) / `team:` / `owner_of:`, and a bare position
Expand Down Expand Up @@ -64,15 +73,21 @@ export const LeadAssignmentFlow: Flow = {
id: 'sla_hot', type: 'update_record', label: 'Set 1-Day SLA',
config: { objectName: 'crm_lead', filter: { id: cel`record.id` }, fields: { next_followup_date: cel`isoDate(daysFromNow(1))` } },
},
{
// Gateway only — the predicate lives on its out-edges `e8` / `e9`.
id: 'owner_hot', type: 'decision', label: 'Has an Owner?',
},
{
id: 'notify_hot', type: 'notify', label: 'Alert — Hot Lead',
config: {
// Route to the lead's OWNER (a real user id). A CRM position like
// 'sales_manager' is NOT a messaging audience — the messaging service
// resolves user:/role:(sys_member)/team:/owner_of:, and a bare
// 'sales_manager' was stored verbatim as sys_inbox_message.user_id,
// matching no real user, so these alerts reached nobody. owner is set
// on insert (defaultValue os.user.id) and is the accountable party.
// matching no real user, so these alerts reached nobody. A write that
// carries a user is stamped with that user as owner, and
// `lead_auto_assign` fills an ownerless one from the rep pool when it
// can; a lead still ownerless never reaches this node (`owner_hot`).
recipients: [cel`record.owner_id`],
channels: ['inbox', 'email'],
severity: 'warning',
Expand All @@ -91,11 +106,16 @@ export const LeadAssignmentFlow: Flow = {
id: 'sla_std', type: 'update_record', label: 'Set 3-Day SLA',
config: { objectName: 'crm_lead', filter: { id: cel`record.id` }, fields: { next_followup_date: cel`isoDate(daysFromNow(3))` } },
},
{
// Gateway only — the predicate lives on its out-edges `e10` / `e11`.
id: 'owner_std', type: 'decision', label: 'Has an Owner?',
},
{
id: 'notify_std', type: 'notify', label: 'Alert — New Lead',
config: {
// Route to the lead's owner — see notify_hot for why a bare
// 'sales_manager' position never reached anyone.
// 'sales_manager' position never reached anyone, and `owner_std` for
// the lead that has no owner.
recipients: [cel`record.owner_id`],
channels: ['inbox'],
topic: 'lead_routing',
Expand Down Expand Up @@ -129,9 +149,23 @@ export const LeadAssignmentFlow: Flow = {
// dyn<null> >= int` — so both guards are required, in this order.
{ id: 'e2', source: 'check_hot', target: 'sla_hot', type: 'conditional', condition: P`has(record.rating) && record.rating != null && record.rating >= 4`, label: 'Hot' },
{ id: 'e3', source: 'check_hot', target: 'sla_std', type: 'conditional', condition: P`!has(record.rating) || record.rating == null || record.rating < 4`, label: 'Standard' },
{ id: 'e4', source: 'sla_hot', target: 'notify_hot', type: 'default' },
{ id: 'e4', source: 'sla_hot', target: 'owner_hot', type: 'default' },
{ id: 'e5', source: 'notify_hot', target: 'end', type: 'default' },
{ id: 'e6', source: 'sla_std', target: 'notify_std', type: 'default' },
{ id: 'e6', source: 'sla_std', target: 'owner_std', type: 'default' },
{ id: 'e7', source: 'notify_std', target: 'end', type: 'default' },
// OWNER GATES, one per branch so the hot/standard split stays the single
// rating cut on `e2` / `e3`. The SLA stamp has already landed either way;
// only the alert needs someone to go to. "Has an owner" is the one
// predicate, and the ownerless route is the `isDefault` fallback rather
// than a hand-written complement, so the pair cannot drift apart: a
// `decision` with no `config.conditions` takes its first conditioned
// out-edge that holds, and its default edge only when none does.
// `isBlank` answers true for exactly what `notify` would refuse as no
// recipient — null, a blank string, an empty list — and `has()` covers the
// key a sparse driver leaves absent.
{ id: 'e8', source: 'owner_hot', target: 'notify_hot', type: 'conditional', condition: P`has(record.owner_id) && !isBlank(record.owner_id)`, label: 'Owned' },
{ id: 'e9', source: 'owner_hot', target: 'end', type: 'default', isDefault: true, label: 'No owner' },
{ id: 'e10', source: 'owner_std', target: 'notify_std', type: 'conditional', condition: P`has(record.owner_id) && !isBlank(record.owner_id)`, label: 'Owned' },
{ id: 'e11', source: 'owner_std', target: 'end', type: 'default', isDefault: true, label: 'No owner' },
],
};
74 changes: 73 additions & 1 deletion test/flow-record-change.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,19 @@ const as = (who: Person) => ({ as: who.token });
const notified = async (userId: string, topic: string, url: string) =>
(await notificationsTo(verify, userId, topic)).filter((n) => n.payload?.actionUrl === url);

/**
* The runs of `flow` for the record `id`, once the first history row has
* landed. The engine writes a finished run's row after the run returns (its
* `recordTerminal` write is not awaited), so a read straight after the write
* that fired the flow can come back empty — measured on a guest web-to-lead
* insert (#2060 item 6). For a case that asserts on the run itself.
*/
const landedRuns = (flow: string, id: string) => vi.waitFor(async () => {
const rows = await flowRuns(verify, flow, id);
expect(rows.length, `no ${flow} run recorded for ${id}`).toBeGreaterThan(0);
return rows;
}, { timeout: 10_000, interval: 50 });

describe('opportunity_won_alert — start condition', () => {
/**
* The rep's open deal. A deal of $100K or more is seeded with its large-deal
Expand Down Expand Up @@ -350,6 +363,62 @@ describe('lead_assignment — hot-lead SLA routing', () => {
});
});

/**
* A lead with NO owner (#2060 item 6).
*
* Web-to-lead lands ownerless whenever `lead_auto_assign` stands down (an
* empty `sales_rep` pool, or one the anonymous form grant cannot read), and a
* seed or integration write carries whatever owner it was given. The SLA stamp
* is the routing decision about the lead and lands either way; the alert has
* nobody to go to. `notify` refuses an audience that evaluates to nothing, so
* without the owner gates the run FAILED at the notify node after the stamp
* (measured on 18.0.0-next.0: "notify: at least one recipient is required …
* `record.owner_id`").
*
* The lead is written through the system's seed door (no owner stamp, no flow
* fired) and handed to the flow the way the trigger hands it a write, so the
* case does not depend on how intake left it ownerless. Both shapes the
* trigger can hand over are run: `owner_id: null` (a SQL driver, the public
* form door) and the key ABSENT (driver-memory / driver-mongodb store only the
* columns a row was written with). An OWNED lead still alerting its owner, on
* both branches, is the describe above and the integration-written case below.
*/
describe('lead_assignment — a lead with no owner (#2060)', () => {
const CASES = [
{ rating: 5, notify: 'notify_hot', owner: 'null' },
{ rating: 5, notify: 'notify_hot', owner: 'absent' },
{ rating: 1, notify: 'notify_std', owner: 'null' },
{ rating: 1, notify: 'notify_std', owner: 'absent' },
] as const;

it.each(CASES)('rating $rating, owner_id $owner: the SLA lands, $notify is passed over, the run completes', async ({ rating, notify, owner }) => {
const [seeded] = await verify.seed('crm_lead', [{
first_name: 'Ada', last_name: `Ownerless ${++k}`, company: `Ownerless Co ${k}`,
email: `ownerless${k}@flow-record-change.test`, rating,
}]);
const id = String(seeded!.id);
const [held] = await verify.rows('crm_lead', { id });
const { owner_id: _owner, ...unowned } = held!;
const record = owner === 'null' ? { ...unowned, owner_id: null } : unowned;

const result = await runRecordFlow(verify, 'lead_assignment', 'crm_lead', record);
expect(result.success, `the run failed on a lead with no owner: ${String(result.error ?? '')}`).toBe(true);

const [stored] = await verify.rows('crm_lead', { id });
expect(stored!.next_followup_date, 'the SLA stamp did not land').toBeTruthy();

const [run, ...more] = await landedRuns('lead_assignment', id);
expect(more).toHaveLength(0);
expect(run?.status).toBe('completed');
const step = (run?.summary?.nodes ?? []).find((n: Rec) => n.nodeId === notify);
expect(step?.status, `${notify} was not passed over`).toBe('skipped');

const sent = (await verify.rows('sys_notification_delivery', { topic: 'lead_routing' }))
.filter((n) => n.payload?.actionUrl === `/crm_lead/${id}`);
expect(sent, 'an alert went out for a lead with no owner').toHaveLength(0);
});
});

describe('opportunity_approval — start condition', () => {
const startCondition = (OpportunityApprovalFlow.nodes as Rec[])
.find((n) => n.id === 'start')?.config?.condition;
Expand Down Expand Up @@ -611,8 +680,11 @@ describe('record-change flows under a user-less trigger (#684)', () => {
first_name: 'Ada', last_name: `Web ${++k}`, company: `Web Co ${k}`,
email: `web${k}@flow-record-change.test`, rating: 5,
});
const [run] = await flowRuns(verify, 'lead_assignment', lead.id);
const [run] = await landedRuns('lead_assignment', String(lead.id));
expect(String(run?.error ?? '')).not.toContain('[runAs]');
// Owned or not, the run ends successfully (#2060 item 6): an ownerless
// lead used to fail at its notify node after the SLA stamp below.
expect(run?.status, String(run?.error ?? '')).toBe('completed');
const [stored] = await verify.rows('crm_lead', { id: lead.id });
expect(stored!.next_followup_date, 'hot lead got no SLA date').toBeTruthy();
// The anonymous form grant cannot read the rep pool, so `lead_auto_assign`
Expand Down
Loading