fix: make CRM detail and stale counts reliable
This commit is contained in:
+97
-36
@@ -29,7 +29,6 @@ import {
|
|||||||
refFromRow,
|
refFromRow,
|
||||||
refIds,
|
refIds,
|
||||||
refNames,
|
refNames,
|
||||||
relationIds,
|
|
||||||
relationRefs,
|
relationRefs,
|
||||||
relationRows,
|
relationRows,
|
||||||
responseTotal,
|
responseTotal,
|
||||||
@@ -324,18 +323,21 @@ export class CrmService {
|
|||||||
projectId: unknown,
|
projectId: unknown,
|
||||||
options: { includeContacts?: boolean; includeFollowups?: boolean; includeUpdates?: boolean } = {},
|
options: { includeContacts?: boolean; includeFollowups?: boolean; includeUpdates?: boolean } = {},
|
||||||
): Promise<Record<string, unknown>> {
|
): Promise<Record<string, unknown>> {
|
||||||
|
const includeContacts = options.includeContacts ?? true;
|
||||||
const row = singleRow(
|
const row = singleRow(
|
||||||
await this.client.getRecord(this.config.project.resource, projectId, { fields: this.projectFields() }),
|
await this.client.getRecord(this.config.project.resource, projectId, {
|
||||||
|
fields: this.projectFields(includeContacts),
|
||||||
|
}),
|
||||||
);
|
);
|
||||||
if (!row)
|
if (!row)
|
||||||
return {
|
return {
|
||||||
error: { code: "project_not_found", message: `No project matched id ${JSON.stringify(projectId)}` },
|
error: { code: "project_not_found", message: `No project matched id ${JSON.stringify(projectId)}` },
|
||||||
};
|
};
|
||||||
const result: Record<string, unknown> = {
|
const result: Record<string, unknown> = {
|
||||||
project: this.compactProject(row, false),
|
project: this.compactProject(row, false, false),
|
||||||
timezone: this.config.timezone,
|
timezone: this.config.timezone,
|
||||||
};
|
};
|
||||||
if (options.includeContacts ?? true) {
|
if (includeContacts) {
|
||||||
result.contacts = relationRefs(row, [this.config.project.contactField], this.contactSpec());
|
result.contacts = relationRefs(row, [this.config.project.contactField], this.contactSpec());
|
||||||
}
|
}
|
||||||
if (options.includeFollowups ?? true) {
|
if (options.includeFollowups ?? true) {
|
||||||
@@ -710,6 +712,12 @@ export class CrmService {
|
|||||||
if (!/^\d{4}-\d{2}-\d{2}$/.test(asOf) || Number.isNaN(new Date(`${asOf}T00:00:00Z`).getTime())) {
|
if (!/^\d{4}-\d{2}-\d{2}$/.test(asOf) || Number.isNaN(new Date(`${asOf}T00:00:00Z`).getTime())) {
|
||||||
return { error: { code: "invalid_date", message: "as_of must use YYYY-MM-DD" } };
|
return { error: { code: "invalid_date", message: "as_of must use YYYY-MM-DD" } };
|
||||||
}
|
}
|
||||||
|
const followupFilter = buildAndFilter([
|
||||||
|
buildRelationIdCondition(this.config.followup.ownerField, ownerId),
|
||||||
|
buildRelationIdInCondition(this.config.followup.ownerField, teamMemberIds),
|
||||||
|
{ [this.config.followup.dateField]: { $dateNotAfter: asOf } },
|
||||||
|
{ [this.config.followup.statusField]: { $ne: INVALID_FOLLOWUP_STATUS } },
|
||||||
|
]);
|
||||||
const [customers, followups] = await Promise.all([
|
const [customers, followups] = await Promise.all([
|
||||||
this.scanRecords(this.config.customer.resource, {
|
this.scanRecords(this.config.customer.resource, {
|
||||||
filter: buildAndFilter([
|
filter: buildAndFilter([
|
||||||
@@ -719,38 +727,19 @@ export class CrmService {
|
|||||||
fields: this.customerCompactFields(),
|
fields: this.customerCompactFields(),
|
||||||
sort: this.config.customer.sort,
|
sort: this.config.customer.sort,
|
||||||
}),
|
}),
|
||||||
this.scanRecords(this.config.followup.resource, {
|
this.latestFollowupsByCustomer(followupFilter),
|
||||||
filter: buildAndFilter([
|
|
||||||
buildRelationIdCondition(this.config.followup.ownerField, ownerId),
|
|
||||||
buildRelationIdInCondition(this.config.followup.ownerField, teamMemberIds),
|
|
||||||
{ [this.config.followup.dateField]: { $dateNotAfter: asOf } },
|
|
||||||
{ [this.config.followup.statusField]: { $ne: INVALID_FOLLOWUP_STATUS } },
|
|
||||||
]),
|
|
||||||
fields: [
|
|
||||||
this.config.followup.idField,
|
|
||||||
`${this.config.followup.customerField}.${this.config.customer.idField}`,
|
|
||||||
this.config.followup.dateField,
|
|
||||||
this.config.followup.statusField,
|
|
||||||
],
|
|
||||||
sort: this.config.followup.sort,
|
|
||||||
}),
|
|
||||||
]);
|
]);
|
||||||
const lastFollowup = new Map<unknown, Date>();
|
const lastFollowup = new Map<string, Date>();
|
||||||
for (const followup of followups.rows) {
|
for (const followup of followups.rows) {
|
||||||
const date = parseDate(firstText(followup, [this.config.followup.dateField, ...FOLLOWUP_DATE_KEYS]));
|
const customerId = followup.customer_id;
|
||||||
if (!date) continue;
|
const date = parseDate(firstText(followup, ["last_followup_at"]));
|
||||||
for (const customerId of relationIds(
|
if (customerId === undefined || customerId === null || !date) continue;
|
||||||
followup[this.config.followup.customerField],
|
lastFollowup.set(String(customerId), date);
|
||||||
this.config.customer.idField,
|
|
||||||
)) {
|
|
||||||
const previous = lastFollowup.get(customerId);
|
|
||||||
if (!previous || date > previous) lastFollowup.set(customerId, date);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
const asOfDate = new Date(`${asOf}T00:00:00Z`);
|
const asOfDate = new Date(`${asOf}T00:00:00Z`);
|
||||||
const stale = customers.rows
|
const stale = customers.rows
|
||||||
.map((customer) => {
|
.map((customer) => {
|
||||||
const last = lastFollowup.get(customer[this.config.customer.idField]);
|
const last = lastFollowup.get(String(customer[this.config.customer.idField]));
|
||||||
const days = last
|
const days = last
|
||||||
? Math.floor((asOfDate.getTime() - dateOnlyUtc(last, this.config.timezone).getTime()) / 86_400_000)
|
? Math.floor((asOfDate.getTime() - dateOnlyUtc(last, this.config.timezone).getTime()) / 86_400_000)
|
||||||
: null;
|
: null;
|
||||||
@@ -773,7 +762,21 @@ export class CrmService {
|
|||||||
);
|
);
|
||||||
});
|
});
|
||||||
const result = paginatedRows(stale.slice(offset, offset + limit), stale.length, limit, offset);
|
const result = paginatedRows(stale.slice(offset, offset + limit), stale.length, limit, offset);
|
||||||
Object.assign(result, { as_of: asOf, inactive_days: inactiveDays, timezone: this.config.timezone });
|
const totalIsExact = !customers.truncation && !followups.truncation;
|
||||||
|
Object.assign(result, {
|
||||||
|
as_of: asOf,
|
||||||
|
inactive_days: inactiveDays,
|
||||||
|
timezone: this.config.timezone,
|
||||||
|
total_is_exact: totalIsExact,
|
||||||
|
});
|
||||||
|
if (!totalIsExact) {
|
||||||
|
const scannedStaleCount = result.total;
|
||||||
|
delete result.total;
|
||||||
|
Object.assign(result, {
|
||||||
|
scanned_stale_count: scannedStaleCount,
|
||||||
|
pagination_scope: "scanned_records_only",
|
||||||
|
});
|
||||||
|
}
|
||||||
const warnings: string[] = [];
|
const warnings: string[] = [];
|
||||||
const truncation: Record<string, unknown> = {};
|
const truncation: Record<string, unknown> = {};
|
||||||
if (customers.truncation) {
|
if (customers.truncation) {
|
||||||
@@ -781,7 +784,7 @@ export class CrmService {
|
|||||||
truncation.customers = customers.truncation;
|
truncation.customers = customers.truncation;
|
||||||
}
|
}
|
||||||
if (followups.truncation) {
|
if (followups.truncation) {
|
||||||
warnings.push("follow-up scan was truncated");
|
warnings.push("follow-up customer aggregation was truncated");
|
||||||
truncation.followups = followups.truncation;
|
truncation.followups = followups.truncation;
|
||||||
}
|
}
|
||||||
if (warnings.length > 0) Object.assign(result, { data_quality_warnings: warnings, truncation });
|
if (warnings.length > 0) Object.assign(result, { data_quality_warnings: warnings, truncation });
|
||||||
@@ -864,6 +867,60 @@ export class CrmService {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private async latestFollowupsByCustomer(filter: Filter): Promise<ScanResult> {
|
||||||
|
const followup = this.config.followup;
|
||||||
|
const rows: RecordRow[] = [];
|
||||||
|
let offset = 0;
|
||||||
|
const query = async (limit: number, queryOffset: number): Promise<RecordRow[]> =>
|
||||||
|
dataRows(
|
||||||
|
await this.client.queryRecords(followup.resource, {
|
||||||
|
dimensions: [
|
||||||
|
{
|
||||||
|
field: [followup.customerField, this.config.customer.idField],
|
||||||
|
alias: "customer_id",
|
||||||
|
},
|
||||||
|
],
|
||||||
|
measures: [
|
||||||
|
{
|
||||||
|
field: [followup.dateField],
|
||||||
|
aggregation: "max",
|
||||||
|
alias: "last_followup_at",
|
||||||
|
},
|
||||||
|
],
|
||||||
|
orders: [
|
||||||
|
{
|
||||||
|
field: [followup.customerField, this.config.customer.idField],
|
||||||
|
alias: "customer_id",
|
||||||
|
order: "asc",
|
||||||
|
},
|
||||||
|
],
|
||||||
|
filter,
|
||||||
|
limit,
|
||||||
|
offset: queryOffset,
|
||||||
|
timezone: this.config.timezone,
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
|
||||||
|
while (rows.length < MAX_SCAN_RECORDS) {
|
||||||
|
const batchLimit = Math.min(MAX_PAGE_SIZE, MAX_SCAN_RECORDS - rows.length);
|
||||||
|
const batch = await query(batchLimit, offset);
|
||||||
|
rows.push(...batch);
|
||||||
|
if (batch.length < batchLimit) return { rows };
|
||||||
|
offset += batch.length;
|
||||||
|
}
|
||||||
|
const overflow = await query(1, offset);
|
||||||
|
if (overflow.length === 0) return { rows };
|
||||||
|
return {
|
||||||
|
rows,
|
||||||
|
truncation: {
|
||||||
|
minimum_total: rows.length + 1,
|
||||||
|
returned: rows.length,
|
||||||
|
limit: MAX_SCAN_RECORDS,
|
||||||
|
next_offset: offset,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
private decorateStatusCounts(rows: RecordRow[]): RecordRow[] {
|
private decorateStatusCounts(rows: RecordRow[]): RecordRow[] {
|
||||||
return decorateDimensionCounts(
|
return decorateDimensionCounts(
|
||||||
rows,
|
rows,
|
||||||
@@ -1228,7 +1285,7 @@ export class CrmService {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private projectFields(): string[] {
|
private projectFields(includeContacts = true): string[] {
|
||||||
const project = this.config.project;
|
const project = this.config.project;
|
||||||
return fieldsWithProjections(
|
return fieldsWithProjections(
|
||||||
[
|
[
|
||||||
@@ -1247,12 +1304,16 @@ export class CrmService {
|
|||||||
],
|
],
|
||||||
[
|
[
|
||||||
...this.projection(project.customerRelationField, this.customerSpec()),
|
...this.projection(project.customerRelationField, this.customerSpec()),
|
||||||
...this.projection(project.contactField, this.contactSpec()),
|
...(includeContacts ? this.projection(project.contactField, this.contactSpec()) : []),
|
||||||
],
|
],
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
private compactProject(row: RecordRow, includeRaw = false): Record<string, unknown> {
|
private compactProject(
|
||||||
|
row: RecordRow,
|
||||||
|
includeRaw = false,
|
||||||
|
includeContacts = true,
|
||||||
|
): Record<string, unknown> {
|
||||||
const project = this.config.project;
|
const project = this.config.project;
|
||||||
const progress = row[project.progressField];
|
const progress = row[project.progressField];
|
||||||
const progressLabels = Object.fromEntries(
|
const progressLabels = Object.fromEntries(
|
||||||
@@ -1272,7 +1333,7 @@ export class CrmService {
|
|||||||
part_numbers: row[project.partNumbersField],
|
part_numbers: row[project.partNumbersField],
|
||||||
period_usage: row[project.periodUsageField],
|
period_usage: row[project.periodUsageField],
|
||||||
details: row[project.detailsField],
|
details: row[project.detailsField],
|
||||||
contacts: relationRefs(row, [project.contactField], this.contactSpec()),
|
...(includeContacts ? { contacts: relationRefs(row, [project.contactField], this.contactSpec()) } : {}),
|
||||||
createdAt: toZonedIso(row.createdAt, this.config.timezone),
|
createdAt: toZonedIso(row.createdAt, this.config.timezone),
|
||||||
updatedAt: toZonedIso(row.updatedAt, this.config.timezone),
|
updatedAt: toZonedIso(row.updatedAt, this.config.timezone),
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -143,6 +143,154 @@ test("health never returns the Authorize Key", async () => {
|
|||||||
assert.doesNotMatch(JSON.stringify(result), /token|secret|authorizeKey/i);
|
assert.doesNotMatch(JSON.stringify(result), /token|secret|authorizeKey/i);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("getProject returns contacts only at the top level when requested", async () => {
|
||||||
|
const config = structuredClone(DEFAULT_CONFIG);
|
||||||
|
const requests: Array<Record<string, unknown>> = [];
|
||||||
|
const fake = {
|
||||||
|
getRecord: async (_resource: string, _projectId: unknown, options: Record<string, unknown>) => {
|
||||||
|
requests.push(options);
|
||||||
|
return {
|
||||||
|
data: {
|
||||||
|
[config.project.idField]: 30,
|
||||||
|
[config.project.nameField]: "Project",
|
||||||
|
[config.project.contactField]: [{ [config.contact.idField]: 2, name: "Contact" }],
|
||||||
|
},
|
||||||
|
};
|
||||||
|
},
|
||||||
|
} as unknown as NocoBaseClient;
|
||||||
|
const service = new CrmService(config, fake);
|
||||||
|
|
||||||
|
const withoutContacts = await service.getProject(30, {
|
||||||
|
includeContacts: false,
|
||||||
|
includeFollowups: false,
|
||||||
|
includeUpdates: false,
|
||||||
|
});
|
||||||
|
assert.equal("contacts" in withoutContacts, false);
|
||||||
|
assert.equal("contacts" in (withoutContacts.project as Record<string, unknown>), false);
|
||||||
|
assert.equal(
|
||||||
|
((requests[0]?.fields ?? []) as string[]).some(
|
||||||
|
(field) => field === config.project.contactField || field.startsWith(`${config.project.contactField}.`),
|
||||||
|
),
|
||||||
|
false,
|
||||||
|
);
|
||||||
|
|
||||||
|
const withContacts = await service.getProject(30, {
|
||||||
|
includeContacts: true,
|
||||||
|
includeFollowups: false,
|
||||||
|
includeUpdates: false,
|
||||||
|
});
|
||||||
|
assert.deepEqual(withContacts.contacts, [{ id: 2, name: "Contact" }]);
|
||||||
|
assert.equal("contacts" in (withContacts.project as Record<string, unknown>), false);
|
||||||
|
assert.equal(
|
||||||
|
((requests[1]?.fields ?? []) as string[]).some((field) =>
|
||||||
|
field.startsWith(`${config.project.contactField}.`),
|
||||||
|
),
|
||||||
|
true,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("listStaleCustomers uses paginated max aggregation and returns an exact total", async () => {
|
||||||
|
const config = structuredClone(DEFAULT_CONFIG);
|
||||||
|
const customers = Array.from({ length: 101 }, (_, index) => ({
|
||||||
|
[config.customer.idField]: index + 1,
|
||||||
|
name: `Customer ${index + 1}`,
|
||||||
|
}));
|
||||||
|
const latest = customers.map((customer) => ({
|
||||||
|
customer_id: customer[config.customer.idField],
|
||||||
|
last_followup_at: "2026-01-01T00:00:00Z",
|
||||||
|
}));
|
||||||
|
const queryRequests: Array<Record<string, unknown>> = [];
|
||||||
|
const fake = {
|
||||||
|
listRecords: async (resource: string, options: Record<string, unknown>) => {
|
||||||
|
assert.equal(resource, config.customer.resource);
|
||||||
|
const pageSize = options.pageSize as number;
|
||||||
|
const start = ((options.page as number) - 1) * pageSize;
|
||||||
|
return { data: customers.slice(start, start + pageSize), meta: { count: customers.length } };
|
||||||
|
},
|
||||||
|
queryRecords: async (resource: string, options: Record<string, unknown>) => {
|
||||||
|
assert.equal(resource, config.followup.resource);
|
||||||
|
queryRequests.push(options);
|
||||||
|
const offset = options.offset as number;
|
||||||
|
const limit = options.limit as number;
|
||||||
|
return { data: latest.slice(offset, offset + limit) };
|
||||||
|
},
|
||||||
|
} as unknown as NocoBaseClient;
|
||||||
|
|
||||||
|
const result = await new CrmService(config, fake).listStaleCustomers({
|
||||||
|
asOf: "2026-08-20",
|
||||||
|
inactiveDays: 30,
|
||||||
|
limit: 20,
|
||||||
|
});
|
||||||
|
assert.equal(result.total, 101);
|
||||||
|
assert.equal(result.total_is_exact, true);
|
||||||
|
assert.deepEqual(
|
||||||
|
queryRequests.map((request) => request.offset),
|
||||||
|
[0, 100],
|
||||||
|
);
|
||||||
|
assert.deepEqual(queryRequests[0]?.dimensions, [
|
||||||
|
{
|
||||||
|
field: [config.followup.customerField, config.customer.idField],
|
||||||
|
alias: "customer_id",
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
assert.deepEqual(queryRequests[0]?.measures, [
|
||||||
|
{ field: [config.followup.dateField], aggregation: "max", alias: "last_followup_at" },
|
||||||
|
]);
|
||||||
|
assert.equal(queryRequests[0]?.timezone, config.timezone);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("listStaleCustomers never presents a truncated scan count as an exact total", async () => {
|
||||||
|
const config = structuredClone(DEFAULT_CONFIG);
|
||||||
|
const fake = {
|
||||||
|
listRecords: async () => ({
|
||||||
|
data: [{ [config.customer.idField]: 1, name: "Customer" }],
|
||||||
|
meta: { count: 2 },
|
||||||
|
}),
|
||||||
|
queryRecords: async () => ({ data: [] }),
|
||||||
|
} as unknown as NocoBaseClient;
|
||||||
|
|
||||||
|
const result = await new CrmService(config, fake).listStaleCustomers({
|
||||||
|
asOf: "2026-08-20",
|
||||||
|
inactiveDays: 30,
|
||||||
|
});
|
||||||
|
assert.equal("total" in result, false);
|
||||||
|
assert.equal(result.total_is_exact, false);
|
||||||
|
assert.equal(result.scanned_stale_count, 1);
|
||||||
|
assert.equal(result.pagination_scope, "scanned_records_only");
|
||||||
|
assert.deepEqual(result.data_quality_warnings, ["customer scan was truncated"]);
|
||||||
|
});
|
||||||
|
|
||||||
|
test("listStaleCustomers marks an overflowing follow-up aggregation as inexact", async () => {
|
||||||
|
const config = structuredClone(DEFAULT_CONFIG);
|
||||||
|
const latest = Array.from({ length: 5_001 }, (_, index) => ({
|
||||||
|
customer_id: index + 1,
|
||||||
|
last_followup_at: "2026-01-01T00:00:00Z",
|
||||||
|
}));
|
||||||
|
const fake = {
|
||||||
|
listRecords: async () => ({
|
||||||
|
data: [{ [config.customer.idField]: 1, name: "Customer" }],
|
||||||
|
meta: { count: 1 },
|
||||||
|
}),
|
||||||
|
queryRecords: async (_resource: string, options: Record<string, unknown>) => {
|
||||||
|
const offset = options.offset as number;
|
||||||
|
const limit = options.limit as number;
|
||||||
|
return { data: latest.slice(offset, offset + limit) };
|
||||||
|
},
|
||||||
|
} as unknown as NocoBaseClient;
|
||||||
|
|
||||||
|
const result = await new CrmService(config, fake).listStaleCustomers({
|
||||||
|
asOf: "2026-08-20",
|
||||||
|
inactiveDays: 30,
|
||||||
|
});
|
||||||
|
assert.equal("total" in result, false);
|
||||||
|
assert.equal(result.total_is_exact, false);
|
||||||
|
assert.equal(result.scanned_stale_count, 1);
|
||||||
|
assert.deepEqual(result.data_quality_warnings, ["follow-up customer aggregation was truncated"]);
|
||||||
|
assert.deepEqual(result.truncation, {
|
||||||
|
followups: { minimum_total: 5_001, returned: 5_000, limit: 5_000, next_offset: 5_000 },
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
test("all CRM service surfaces execute against the direct HTTP adapter contract", async () => {
|
test("all CRM service surfaces execute against the direct HTTP adapter contract", async () => {
|
||||||
const config = structuredClone(DEFAULT_CONFIG);
|
const config = structuredClone(DEFAULT_CONFIG);
|
||||||
const rows: Record<string, Record<string, unknown>> = {
|
const rows: Record<string, Record<string, unknown>> = {
|
||||||
|
|||||||
Reference in New Issue
Block a user