Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Evaluate triggerables for all repeat instances where needed #586

Merged
merged 24 commits into from
Jun 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
f410dd9
Add tests
lognaturel Jun 11, 2020
17d8171
Isolate and document getting all triggerables to trigger
lognaturel Jun 19, 2020
3f27d29
Group and rename methods for repeat instances
lognaturel Jun 19, 2020
f57b2ae
Reorder methods and add regions
lognaturel Jun 19, 2020
e5ea009
Remove unused repeat conditions per targets
lognaturel Jun 19, 2020
03f868b
Remove trust in previously-committed answer
lognaturel Jun 19, 2020
b2a4ea3
Make naming consistent, remove indirection
lognaturel Jun 19, 2020
bc6da8d
Explain context node contextualization
lognaturel Jun 19, 2020
4b7073f
Identify triggerables that affect all repeat instances
lognaturel Jun 20, 2020
fab613e
Rename and update test that now passes
lognaturel Jun 20, 2020
739e3a8
Re-establish test with position predicate
lognaturel Jun 20, 2020
ddc8138
Remove test that now passes
lognaturel Jun 20, 2020
31d96d4
Remove incorrect explanation and split instance ref tests
lognaturel Jun 20, 2020
3fce590
Update expected DAG events from deletion
lognaturel Jun 20, 2020
e26834e
Rename tests about adding or removing repeat instances
lognaturel Jun 20, 2020
2abe0d2
Move tests about adding or removing repeat instances
lognaturel Jun 20, 2020
a14b992
Test for deleting repeat instance without refs to repeat
lognaturel Jun 20, 2020
d7783a2
Add test for initialization with defaults
lognaturel Jun 20, 2020
2efde42
Remove unused method
lognaturel Jun 20, 2020
e95ef85
Describe use of expandReference call in Triggerable.apply
lognaturel Jun 20, 2020
5057f2c
Favor predicates over multiplicity when contextualizing
lognaturel Jun 20, 2020
21033fe
Test deletion with repeat sibling calculation
lognaturel Jun 20, 2020
be5d8b1
Explain why next-number case is different from prev-number
lognaturel Jun 20, 2020
c89c1a5
Add hypothesis about why predicate count is wrong
lognaturel Jun 20, 2020
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: 2 additions & 5 deletions src/main/java/org/javarosa/core/model/FormDef.java
Original file line number Diff line number Diff line change
Expand Up @@ -393,9 +393,6 @@ public void setValue(IAnswerData data, TreeReference ref, TreeElement node,
boolean valueChanged = !objectEquals(answerDataSerializer.serializeAnswerData(oldValue),
answerDataSerializer.serializeAnswerData(data));

if (midSurvey && dagImpl.shouldTrustPreviouslyCommittedAnswer() && !valueChanged) {
return;
}
setAnswer(data, node);

QuestionDef currentQuestion = findQuestionByRef(ref, this);
Expand Down Expand Up @@ -502,7 +499,7 @@ public FormIndex deleteRepeat(FormIndex index) {
}
}

dagImpl.deleteRepeatGroup(getMainInstance(), getEvaluationContext(), deleteRef, parentElement, deleteElement);
dagImpl.deleteRepeatInstance(getMainInstance(), getEvaluationContext(), deleteRef, deleteElement);

return newIndex;
}
Expand All @@ -523,7 +520,7 @@ public void createNewRepeat(FormIndex index) throws InvalidReferenceException {
// Trigger actions nested in the new repeat
getChild(index).getActionController().triggerActionsFromEvent(Action.EVENT_ODK_NEW_REPEAT, this, repeatContextRef, this);

dagImpl.createRepeatGroup(getMainInstance(), getEvaluationContext(), repeatContextRef, newNode);
dagImpl.createRepeatInstance(getMainInstance(), getEvaluationContext(), repeatContextRef, newNode);
}

@Override
Expand Down
4 changes: 0 additions & 4 deletions src/main/java/org/javarosa/core/model/QuickTriggerable.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,6 @@ boolean isRecalculate() {
return triggerable instanceof Recalculate;
}

TreeReference contextualizeContextRef(TreeReference anchorRef) {
return triggerable.contextualizeContextRef(anchorRef);
}

public List<EvaluationResult> apply(FormInstance mainInstance, EvaluationContext ec, TreeReference qualified) {
return triggerable.apply(mainInstance, ec, qualified);
}
Expand Down
567 changes: 274 additions & 293 deletions src/main/java/org/javarosa/core/model/TriggerableDag.java

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,6 @@ public Set<TreeReference> getTriggers() {
return expr.getTriggers(originalContextRef);
}

public TreeReference contextualizeContextRef(TreeReference anchorRef) {
// Contextualize the reference used by the triggerable against
// the anchor
return contextRef.contextualize(anchorRef);
}

/**
* Dispatches all of the evaluation
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -321,10 +321,14 @@ public TreeReference contextualize(TreeReference contextRef) {
}

if (contextRef.getName(i).equals(newRef.getName(i))) {
if (newRef.getPredicate(i) == null) {
if (newRef.getPredicate(i) == null && contextRef.getPredicate(i) != null) {
// Positive integer multiplicity is equivalent to a predicate with a position expression so a
// reference level shouldn't have both. If there's an explicit predicate on either reference, always
// favor that and clear out multiplicity on the contextualized node.
newRef.addPredicate(i, contextRef.getPredicate(i));
newRef.setMultiplicity(i, INDEX_UNBOUND);
}
if (i + refLevel <= newRef.size()) {
if (newRef.getPredicate(i) == null && i + refLevel <= newRef.size()) {
newRef.setMultiplicity(i, contextRef.getMultiplicity(i));
}
} else {
Expand Down
Loading