Skip to content

Commit 0ebac91

Browse files
fix(graph): chart projection, cache refresh, validation and ADR compliance
- Use parent field for compound types (address, money, etc.) in projection to avoid clearProjectionPathCollision stripping sibling subfields (fixes address.district not appearing on graph X axis) - Honor Cache-Control: no-cache in withBlobCache for widget refresh - Validate graph config fields exist before stream; return 400 for GRAPH_FIELD_NOT_FOUND - Strip legacy yAxis when series present (graphStream, graphMetadata) - GRAPH_FIELD_NOT_FOUND message empathetic, non-technical (ADR-0024) - validateGraphFieldsExist: use .find() instead of for...of (ADR-0012) Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent c5dc095 commit 0ebac91

File tree

4 files changed

+131
-73
lines changed

4 files changed

+131
-73
lines changed

src/imports/data/api/graphMetadata.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,14 +149,18 @@ export function enrichGraphConfig(document: string, graphConfig: GraphConfig, la
149149
};
150150
}
151151

152-
// Enriquecer yAxis com label traduzido (legado)
153-
if (graphConfig.yAxis?.field) {
152+
// Enriquecer yAxis com label traduzido (legado — skip when series are present)
153+
const hasSeries = Array.isArray(graphConfig.series) && graphConfig.series.length > 0;
154+
if (graphConfig.yAxis?.field && !hasSeries) {
154155
const fieldMeta = resolveFieldMeta(document, graphConfig.yAxis.field, lang);
155156
const shouldUseMetaLabel = isUntranslatedLabel(graphConfig.yAxis.label, graphConfig.yAxis.field);
156157
enrichedConfig.yAxis = {
157158
...graphConfig.yAxis,
158159
label: shouldUseMetaLabel ? fieldMeta.label : graphConfig.yAxis.label,
159160
};
161+
} else if (hasSeries && graphConfig.yAxis?.field) {
162+
// Strip legacy yAxis when series are present to avoid downstream confusion
163+
enrichedConfig.yAxis = undefined;
160164
}
161165

162166
// Enriquecer series com labels traduzidos

src/imports/data/api/graphStream.ts

Lines changed: 58 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -66,33 +66,22 @@ function extractFieldsFromGraphConfig(document: string, graphConfig: GraphConfig
6666
if (field.type === 'lookup' || field.type === 'inheritLookup') {
6767
// For lookups, we only need _id - we'll populate the rest later
6868
fields.add(`${baseFieldName}._id`);
69-
} else if (field.type === 'address') {
70-
const addressFields = ['city', 'state', 'country', 'district', 'place', 'number', 'postalCode', 'complement', 'placeType'];
71-
addressFields.forEach(af => {
72-
fields.add(`${fieldPath}.${af}`);
73-
});
74-
} else if (field.type === 'money') {
75-
fields.add(`${fieldPath}.value`);
76-
fields.add(`${fieldPath}.currency`);
77-
} else if (field.type === 'personName') {
78-
fields.add(`${fieldPath}.full`);
79-
fields.add(`${fieldPath}.first`);
80-
fields.add(`${fieldPath}.last`);
81-
} else if (field.type === 'phone') {
82-
fields.add(`${fieldPath}.phoneNumber`);
83-
fields.add(`${fieldPath}.countryCode`);
84-
} else if (field.type === 'email') {
85-
fields.add(`${fieldPath}.address`);
69+
} else if (['address', 'money', 'personName', 'phone', 'email'].includes(field.type)) {
70+
// Use the parent field name to avoid clearProjectionPathCollision stripping sibling subfields
71+
fields.add(baseFieldName);
8672
} else {
8773
fields.add(fieldPath);
8874
}
8975
};
9076

9177
// Extract fields from xAxis, yAxis, categoryField, and series
78+
const hasSeries = Array.isArray(graphConfig.series) && graphConfig.series.length > 0;
79+
9280
if (graphConfig.xAxis?.field) {
9381
expandField(graphConfig.xAxis.field);
9482
}
95-
if (graphConfig.yAxis?.field) {
83+
// yAxis is legacy — skip when series are present (series take precedence)
84+
if (graphConfig.yAxis?.field && !hasSeries) {
9685
expandField(graphConfig.yAxis.field);
9786
}
9887
if (graphConfig.categoryField) {
@@ -101,9 +90,8 @@ function extractFieldsFromGraphConfig(document: string, graphConfig: GraphConfig
10190
// System fields starting with _ are already handled by expandField, but ensure it's explicitly added
10291
fields.add(graphConfig.categoryField);
10392
}
104-
// Extract fields from series
105-
if (graphConfig.series) {
106-
graphConfig.series.forEach(serie => {
93+
if (hasSeries) {
94+
graphConfig.series!.forEach(serie => {
10795
if (serie.field) {
10896
expandField(serie.field);
10997
}
@@ -114,7 +102,8 @@ function extractFieldsFromGraphConfig(document: string, graphConfig: GraphConfig
114102
// This ensures _createdAt, _updatedAt, etc. are available even if projection filters them
115103
// NOTE: System fields that are lookups are handled by expandField above (they get _id suffix)
116104
// Here we only add non-lookup system fields (like _createdAt, _updatedAt, _id)
117-
const systemFieldsToCheck = [graphConfig.categoryField, graphConfig.xAxis?.field, graphConfig.yAxis?.field];
105+
const systemFieldsToCheck: (string | undefined)[] = [graphConfig.categoryField, graphConfig.xAxis?.field];
106+
if (!hasSeries && graphConfig.yAxis?.field) systemFieldsToCheck.push(graphConfig.yAxis.field);
118107
if (graphConfig.series) {
119108
graphConfig.series.forEach(serie => {
120109
if (serie.field) systemFieldsToCheck.push(serie.field);
@@ -139,6 +128,38 @@ function extractFieldsFromGraphConfig(document: string, graphConfig: GraphConfig
139128
return Array.from(fields);
140129
}
141130

131+
/**
132+
* Validate that every field path used in the graph config exists on the document meta.
133+
* Prevents RPC "Field not found" from Python when the document has no such field.
134+
*/
135+
function validateGraphFieldsExist(
136+
document: string,
137+
graphConfig: GraphConfig,
138+
): { valid: true } | { valid: false; field: string } {
139+
const meta = MetaObject.Meta[document];
140+
if (meta == null) return { valid: true };
141+
142+
const fieldPaths: string[] = [];
143+
const hasSeries = Array.isArray(graphConfig.series) && graphConfig.series.length > 0;
144+
if (graphConfig.xAxis?.field) fieldPaths.push(graphConfig.xAxis.field);
145+
// yAxis is legacy — skip validation when series are present (series take precedence)
146+
if (graphConfig.yAxis?.field && !hasSeries) fieldPaths.push(graphConfig.yAxis.field);
147+
if (graphConfig.categoryField) fieldPaths.push(graphConfig.categoryField);
148+
if (hasSeries) {
149+
graphConfig.series!.forEach(serie => {
150+
if (serie.field) fieldPaths.push(serie.field);
151+
});
152+
}
153+
154+
const invalidField = fieldPaths.find(path => {
155+
if (!path) return false;
156+
const baseFieldName = path.split('.')[0];
157+
if (baseFieldName.startsWith('_')) return false;
158+
return meta.fields[baseFieldName] == null;
159+
});
160+
return invalidField != null ? { valid: false, field: invalidField } : { valid: true };
161+
}
162+
142163
/**
143164
* Identify lookup fields in the graph config
144165
*/
@@ -177,19 +198,19 @@ function getLookupFieldsInfo(
177198
}
178199
};
179200

180-
// Process xAxis, yAxis, categoryField, and series
201+
const hasSeries = Array.isArray(graphConfig.series) && graphConfig.series.length > 0;
202+
181203
if (graphConfig.xAxis?.field) {
182204
processField(graphConfig.xAxis.field);
183205
}
184-
if (graphConfig.yAxis?.field) {
206+
if (graphConfig.yAxis?.field && !hasSeries) {
185207
processField(graphConfig.yAxis.field);
186208
}
187209
if (graphConfig.categoryField) {
188210
processField(graphConfig.categoryField);
189211
}
190-
// Process series fields
191-
if (graphConfig.series) {
192-
graphConfig.series.forEach(serie => {
212+
if (hasSeries) {
213+
graphConfig.series!.forEach(serie => {
193214
if (serie.field) {
194215
processField(serie.field);
195216
}
@@ -377,6 +398,16 @@ export default async function graphStream({
377398
const enrichedConfig = enrichGraphConfig(findParams.document, graphConfig, findParams.lang || 'pt_BR');
378399
logger.debug({ lang: findParams.lang || 'pt_BR' }, 'Enriched graph config');
379400

401+
const fieldValidation = validateGraphFieldsExist(findParams.document, enrichedConfig);
402+
if (!fieldValidation.valid) {
403+
tracingSpan?.end();
404+
const errorMsg = getGraphErrorMessage('GRAPH_FIELD_NOT_FOUND', {
405+
field: fieldValidation.field,
406+
document: findParams.document,
407+
});
408+
return errorReturn([{ message: errorMsg.message, code: errorMsg.code, details: errorMsg.details } as KonectyError]);
409+
}
410+
380411
// 1.1 Extract fields from graph config for proper projection
381412
const graphFields = extractFieldsFromGraphConfig(findParams.document, enrichedConfig);
382413
tracingSpan?.addEvent('Extracted graph fields', { fields: graphFields.join(',') });

src/imports/utils/graphErrors.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ const ERROR_MESSAGES: Record<string, string> = {
5757
GRAPH_CONFIG_AXIS_Y_MISSING: "Y axis not configured. Please configure the Y axis for {{type}} charts.",
5858
GRAPH_CONFIG_CATEGORY_MISSING: "Category field not configured. Please configure the category field for pie charts.",
5959
GRAPH_FILTER_INVALID: "Applied filters are invalid. Please check the filters and try again.",
60+
GRAPH_FIELD_NOT_FOUND:
61+
"The selected field for this chart is no longer available. Please edit the chart and choose a different field.",
6062
GRAPH_PROCESSING_ERROR: "An error occurred while processing the graph. Please try again or contact support.",
6163
GRAPH_TIMEOUT: "Graph generation took too long. Please try again with a smaller dataset or more filters.",
6264
GRAPH_DATA_ERROR: "Could not load data for the graph. Please check your connection and try again.",

src/server/routes/rest/data/dataApi.ts

Lines changed: 65 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -803,39 +803,55 @@ export const dataApi: FastifyPluginCallback = (fastify, _, done) => {
803803
}
804804
const userId = userResult.data._id;
805805

806-
const cacheResult = await withBlobCache({
807-
req: req as unknown as { headers: Record<string, string | string[] | undefined> },
808-
reply,
809-
userId,
810-
document: req.params.document,
811-
operation: 'graph',
812-
configHash: hashConfig(graphConfig),
813-
filter: parsedFilter,
814-
cacheTTL,
815-
compute: async () => {
816-
const result = await graphStream({
817-
authTokenId,
818-
document: req.params.document,
819-
displayName: req.query.displayName,
820-
displayType: req.query.displayType,
821-
fields: req.query.fields,
822-
filter: parsedFilter,
823-
sort: req.query.sort,
824-
limit: String(limit),
825-
start: req.query.start,
826-
withDetailFields: req.query.withDetailFields,
827-
graphConfig,
828-
lang,
829-
tracingSpan,
830-
});
806+
let cacheResult: { blob: string; fromCache: boolean; notModified: boolean };
807+
try {
808+
cacheResult = await withBlobCache({
809+
req: req as unknown as { headers: Record<string, string | string[] | undefined> },
810+
reply,
811+
userId,
812+
document: req.params.document,
813+
operation: 'graph',
814+
configHash: hashConfig(graphConfig),
815+
filter: parsedFilter,
816+
cacheTTL,
817+
compute: async () => {
818+
const result = await graphStream({
819+
authTokenId,
820+
document: req.params.document,
821+
displayName: req.query.displayName,
822+
displayType: req.query.displayType,
823+
fields: req.query.fields,
824+
filter: parsedFilter,
825+
sort: req.query.sort,
826+
limit: String(limit),
827+
start: req.query.start,
828+
withDetailFields: req.query.withDetailFields,
829+
graphConfig,
830+
lang,
831+
tracingSpan,
832+
});
831833

832-
if (result.success === false) {
833-
throw new Error(JSON.stringify(result));
834-
}
834+
if (result.success === false) {
835+
throw new Error(JSON.stringify(result));
836+
}
835837

836-
return result.svg;
837-
},
838-
});
838+
return result.svg;
839+
},
840+
});
841+
} catch (err) {
842+
tracingSpan.end();
843+
const errMessage = err instanceof Error ? err.message : String(err);
844+
try {
845+
const parsed = JSON.parse(errMessage) as { success?: false; errors?: Array<{ code?: string }> };
846+
if (parsed.success === false && Array.isArray(parsed.errors)) {
847+
const status = parsed.errors[0]?.code === 'GRAPH_FIELD_NOT_FOUND' ? 400 : 500;
848+
return reply.status(status).type('application/json').send(parsed);
849+
}
850+
} catch {
851+
// not our JSON
852+
}
853+
return reply.status(500).type('application/json').send({ success: false, errors: [{ message: errMessage }] });
854+
}
839855

840856
tracingSpan.end();
841857

@@ -857,7 +873,7 @@ export const dataApi: FastifyPluginCallback = (fastify, _, done) => {
857873
/**
858874
* ADR-0049 / DRY: Shared cache helper for blob-based endpoints (graph SVG, pivot JSON).
859875
* Encapsulates: check cache → ETag/304 → cache miss → compute() → store → HTTP headers.
860-
* Keeps the cache pattern in a single place instead of duplicating across endpoints.
876+
* When client sends Cache-Control: no-cache (e.g. widget refresh), bypasses cache so compute() runs.
861877
*/
862878
const withBlobCache = async (opts: {
863879
req: { headers: Record<string, string | string[] | undefined> };
@@ -873,25 +889,30 @@ export const dataApi: FastifyPluginCallback = (fastify, _, done) => {
873889
const { req, reply, userId, document, operation, configHash, filter, cacheTTL, compute } = opts;
874890

875891
const cacheKey = buildCacheKey(userId, document, operation, configHash, filter);
892+
const clientCacheControl = req.headers['cache-control'];
893+
const cacheControlStr = Array.isArray(clientCacheControl) ? clientCacheControl[0] : clientCacheControl;
894+
const forceRefresh = typeof cacheControlStr === 'string' && cacheControlStr.toLowerCase().includes('no-cache');
895+
896+
// When client requests force refresh (e.g. widget refresh button), skip cache lookup
897+
if (!forceRefresh) {
898+
const cached = await getCachedBlob(cacheKey);
899+
if (cached != null) {
900+
const clientEtag = req.headers['if-none-match'];
901+
if (clientEtag != null && clientEtag === cached.etag) {
902+
reply.header('ETag', cached.etag);
903+
reply.header('Cache-Control', `private, max-age=${cacheTTL}, stale-while-revalidate=${STALE_WHILE_REVALIDATE_SECONDS}`);
904+
reply.header('Vary', 'Authorization, Cookie');
905+
return { blob: '', fromCache: true, notModified: true };
906+
}
876907

877-
// Check cache
878-
const cached = await getCachedBlob(cacheKey);
879-
if (cached != null) {
880-
const clientEtag = req.headers['if-none-match'];
881-
if (clientEtag != null && clientEtag === cached.etag) {
882908
reply.header('ETag', cached.etag);
883909
reply.header('Cache-Control', `private, max-age=${cacheTTL}, stale-while-revalidate=${STALE_WHILE_REVALIDATE_SECONDS}`);
884910
reply.header('Vary', 'Authorization, Cookie');
885-
return { blob: '', fromCache: true, notModified: true };
911+
return { blob: cached.blob, fromCache: true, notModified: false };
886912
}
887-
888-
reply.header('ETag', cached.etag);
889-
reply.header('Cache-Control', `private, max-age=${cacheTTL}, stale-while-revalidate=${STALE_WHILE_REVALIDATE_SECONDS}`);
890-
reply.header('Vary', 'Authorization, Cookie');
891-
return { blob: cached.blob, fromCache: true, notModified: false };
892913
}
893914

894-
// Cache miss — execute computation
915+
// Cache miss or force refresh — execute computation
895916
const blob = await compute();
896917

897918
// Store in cache (skip on force refresh to not pollute cache with forced results)

0 commit comments

Comments
 (0)