From 79a63b28e5742cd884d41664bf9f0667ffcbb40d Mon Sep 17 00:00:00 2001 From: Teale Fristoe Date: Thu, 14 Nov 2024 20:40:17 -0800 Subject: [PATCH] Better match v2 lookupBoundary behavior and expand jest tests. --- .../functions/lookup-functions.test.ts | 13 +++++++- .../formula/functions/lookup-functions.ts | 30 +++++++++++-------- 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/v3/src/models/formula/functions/lookup-functions.test.ts b/v3/src/models/formula/functions/lookup-functions.test.ts index 84818eb1d..2b32be8f1 100644 --- a/v3/src/models/formula/functions/lookup-functions.test.ts +++ b/v3/src/models/formula/functions/lookup-functions.test.ts @@ -9,10 +9,21 @@ describe("lookupBoundary", () => { // First argument must be a legal symbol expect(() => evaluate(`lookupBoundary("US_state_boundaries", "Alaska")`)).toThrow() expect(() => evaluate(`lookupBoundary(Mammal, "Alaska")`)).toThrow() + + // Second argument can't be a non-existant symbol + expect(() => evaluate(`lookupBoundary(US_state_boundaries, Alaska)`)).toThrow() + + // TODO The second argument cannot refer to a child collection + }) + + it("returns empty string in some situations", () => { + expect(evaluate(`lookupBoundary(US_state_boundaries, "nonstate")`, 1)).toBe("") + expect(evaluate(`lookupBoundary(US_state_boundaries, Mammal)`, 1)).toBe("") + expect(evaluate(`lookupBoundary(US_state_boundaries, v1)`, 1)).toBe("") }) it("returns boundary data", () => { - expect(() => JSON.parse(evaluate(`lookupBoundary(US_state_boundaries, "Alaska")`)).geometry).toBeDefined() + expect(JSON.parse(evaluate(`lookupBoundary(US_state_boundaries, "Alaska")`, 1)).geometry).toBeDefined() // TODO Test a symbol for the second argument. There is no state attribute in the test-doc, making this difficult. }) }) diff --git a/v3/src/models/formula/functions/lookup-functions.ts b/v3/src/models/formula/functions/lookup-functions.ts index 9a61f6b18..5febebff2 100644 --- a/v3/src/models/formula/functions/lookup-functions.ts +++ b/v3/src/models/formula/functions/lookup-functions.ts @@ -19,9 +19,7 @@ export const lookupFunctions = { } // Find the boundary set - if (!isSymbolNode(args[0])) { - throw new Error(t("DG.Formula.TypeError.message", { vars: [ "boundary_set" ] })) - } + if (!isSymbolNode(args[0])) throw new Error(t("DG.Formula.TypeError.message", { vars: [ "boundary_set" ] })) const boundarySetArg = args[0] const boundarySet = boundarySetArg?.name ?? "" if (!isBoundarySet(boundarySet)) { @@ -34,28 +32,34 @@ export const lookupFunctions = { boundaryKey = args[1].value } else if (isSymbolNode(args[1])) { const symbol = basicCanonicalNameToDependency(args[1].name) - if (symbol?.type === "localAttribute") { + if (!symbol) throw new Error(t("DG.Formula.VarReferenceError.message", { vars: [ args[1].name ] })) + + if (symbol.type === "localAttribute") { const attributeId = symbol.attrId const scope = getRootScope(currentScope) const dataset = scope.getLocalDataSet() + dataset.validateCases() const attribute = dataset.getAttribute(attributeId) - if (!attribute) { - throw new Error(t("DG.Formula.VarReferenceError.message", { vars: [ attributeId ] })) + if (!attribute) throw new Error(t("DG.Formula.VarReferenceError.message", { vars: [ attributeId ] })) + + // The referenced attribute must be in the same or a parent collection--child collections are not allowed + const attributeCollectionIndex = + dataset.getCollectionIndex(dataset.getCollectionForAttribute(attribute.id)?.id) + const thisCollectionIndex = + Math.max(dataset.getCollectionIndex(dataset.getCollectionForCase(scope.caseId)?.id), 0) + if (attributeCollectionIndex > thisCollectionIndex) { + throw new Error(t("DG.Formula.HierReferenceError.message", { vars: [ attribute.title] })) } + const aCase = dataset.caseInfoMap.get(scope.caseId) const caseIndex = dataset.getItemIndex(aCase?.childItemIds[0] ?? "") - boundaryKey = attribute?.strValues[caseIndex ?? -1] + boundaryKey = attribute?.strValues[caseIndex ?? -1] ?? "" } - } else { - throw new Error(t("DG.Formula.TypeError.message", { vars: [ "boundary_key" ] })) } // Find the boundary const boundary = lookupBoundary(boundarySet, boundaryKey) - if (!boundary) { - throw new Error(t("DG.Formula.VarReferenceError.message", { vars: [ boundaryKey ] })) - } - return boundary + return boundary ?? "" } },