Skip to content

Commit

Permalink
Fixed expression with $context evaluation on node delete (#253)
Browse files Browse the repository at this point in the history
* Fixed expression with $context evaluation on node delete

* update Mergify configuration

---------

Co-authored-by: Stefano Ricci <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored Nov 25, 2024
1 parent ad2106d commit 2d9bf18
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 19 deletions.
8 changes: 4 additions & 4 deletions .mergify.yml
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
queue_rules:
- name: default
conditions:
queue_conditions:
- check-success=build
- check-success=DeepScan

merge_method: squash

pull_request_rules:
- name: Automatic merge on approval
conditions:
- "#approved-reviews-by>=1"
- '#approved-reviews-by>=1'
actions:
queue:
method: squash
name: default
- name: automatic branch update
conditions:
Expand Down
22 changes: 11 additions & 11 deletions src/nodeDefExpressionEvaluator/node/identifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,13 @@ const findActualContextNode = (params: {
export class NodeDefIdentifierEvaluator extends IdentifierEvaluator<NodeDefExpressionContext> {
evaluate(expressionNode: IdentifierExpression): any {
const { context } = this
const {
nodeDefContext,
nodeDefCurrent,
selfReferenceAllowed,
object: objectContext,
referencedNodeDefUuids,
itemsFilter,
} = context
const { nodeDefContext, nodeDefCurrent, selfReferenceAllowed, object: objectContext, itemsFilter } = context
const { name: exprName } = expressionNode

if (exprName === ExpressionVariable.CONTEXT) {
if (nodeDefContext) {
this.addReferencedNodeDefUuid(nodeDefContext.uuid)
}
return nodeDefContext
}

Expand Down Expand Up @@ -76,12 +72,10 @@ export class NodeDefIdentifierEvaluator extends IdentifierEvaluator<NodeDefExpre

const referencedNodeDef = this.findIdentifierAmongReachableNodeDefs(expressionNode)
if (referencedNodeDef) {
context.referencedNodeDefUuids = (referencedNodeDefUuids ?? new Set()).add(referencedNodeDef.uuid)

if (!selfReferenceAllowed && referencedNodeDef.uuid === nodeDefCurrent?.uuid) {
throw new SystemError(ValidatorErrorKeys.expressions.cannotUseCurrentNode, { name: exprName })
}

this.addReferencedNodeDefUuid(referencedNodeDef.uuid)
return referencedNodeDef
}
throw new SystemError('expression.identifierNotFound', {
Expand All @@ -90,6 +84,12 @@ export class NodeDefIdentifierEvaluator extends IdentifierEvaluator<NodeDefExpre
})
}

private addReferencedNodeDefUuid(uuid: string) {
const { context } = this
const { referencedNodeDefUuids = new Set() } = context
context.referencedNodeDefUuids = referencedNodeDefUuids.add(uuid)
}

/**
* Tries to find the specified identifier among the node defs that can be "reached" from the context node def.
*/
Expand Down
14 changes: 12 additions & 2 deletions src/survey/surveys/dependencies.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,12 @@ describe('Survey Dependencies', () => {
entityDef(
'plot',
integerDef('plot_id').key(),
integerDef('plot_id_double').readOnly().defaultValue('plot_id * 2')
integerDef('plot_id_double').readOnly().defaultValue('plot_id * 2'),
integerDef('plot_index').readOnly().defaultValue('index($context)').defaultValueEvaluatedOnlyOneTime()
)
.multiple()
.applyIf('accessible')
.applyIf('accessible'),
integerDef('plot_count').readOnly().defaultValue('plot.length')
)
).build()
}, 10000)
Expand Down Expand Up @@ -68,6 +70,14 @@ describe('Survey Dependencies', () => {
})
})

test('Default values dependency ($context)', () => {
expectDependents({
sourceName: 'plot',
dependencyType: SurveyDependencyType.defaultValues,
expectedDependentNames: ['plot_index', 'plot_count'],
})
})

test('Apply if dependency', () => {
expectDependents({
sourceName: 'accessible',
Expand Down
4 changes: 2 additions & 2 deletions src/survey/surveys/dependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ import { NodeDefExpressionFactory } from '../../nodeDef/nodeDef'

const isContextParentByDependencyType = {
[SurveyDependencyType.applicable]: true,
[SurveyDependencyType.defaultValues]: false,
[SurveyDependencyType.defaultValues]: true,
[SurveyDependencyType.fileName]: true,
[SurveyDependencyType.formula]: false,
[SurveyDependencyType.validations]: false,
[SurveyDependencyType.validations]: true,
}

const selfReferenceAllowedByDependencyType = {
Expand Down
5 changes: 5 additions & 0 deletions src/tests/builder/surveyBuilder/nodeDefAttributeBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ export class NodeDefAttributeBuilder extends NodeDefBuilder {
return this
}

defaultValueEvaluatedOnlyOneTime(): this {
this.propsAdvanced.defaultValueEvaluatedOneTime = true
return this
}

validationExpressions(...expressions: (string | NodeDefExpression)[]): this {
if (!this.propsAdvanced.validations) this.propsAdvanced.validations = {}
this.propsAdvanced.validations.expressions = expressions.map((expression) =>
Expand Down

0 comments on commit 2d9bf18

Please sign in to comment.