Add navigable ordered task and step navigation rules

[rsanchezsaez]

New feature dealing with issues #86 and #138.

See the issue pages, the header documentation, and the researchkit-dev mailing list thread (subject: 'ResearchKit: navigation rules') for implementation details.

For now, this has one follow-on issue: Implement an NSPredicate JSON serializer (#152).

[rsanchezsaez]

Any ideas for how we could relax the limitation that the task results have step identifiers that don't overlap the step identifiers in this task? We've thought of task identifiers as namespacing step identifiers in the past, so limitation would be unfortunate.

I managed to rework how predicates are built, and they now must contain a task result identifier and a question result identifier. This supports question results with the same identifier as long as they belong to different task results. I like the code better now, because the new predicates match against the vanilla ORKTaskResult structures, with no flattening needed. You can cross match question result from different ORKTaskResults by doing compound predicates, which is nice.

See the corresponding commit for additional commentary.

---

With this I think I took care of all your feedback, with the exception of this:

Wonder if the task state should be separated from the navigable task. So, you ask the navigable task object for a TaskState object, which itself implements the protocol and owns a reference to the navigable task. And that is what you attach to the task view controller. It just seems odd having this navigable task object which is stateful, when none of our other objects are stateful in the same way.

As we discussed, ORKNavigableOrderedTask should gracefully reset its navigation stack if it's reused, so I don't think it's much of a problem having it like this. Also, do note that although the stack is currently being saved in key archiving and JSON serialization, it's not not being used in the isEqual: and hash methods.

But if you have better idea to try and keep things more stateless, I'm all ears. I don't yet see how a separate task state object or a task generator could make things cleaner.

[jwe-apple]

I've not been through your most recent commit in detail yet. One quick question that popped up on first viewing - do you think it would be nicer for the common case if task identifier could be nullable, and a nil task identifier could match the current task's result?

[rsanchezsaez]

I've not been through your most recent commit in detail yet. One quick question that popped up on first viewing - do you think it would be nicer for the common case if task identifier could be nullable, and a nil task identifier could match the current task's result?

Yeah, I initially thought so too. But the problem is that the task identifier needs to be embed into the predicate when you build it, and you don't know the current task identifier at this time (you can potentially build predicates anywhere, but you only know the current task result identifier on the stepAfterStep:withResult: method).

One potential solution to achieve task identifier nullability would be to turn ORKResultPredicate into an instantiable class with a taskIdentifier property, which could then be nil-checked by ORKPredicateStepNavigationRule and matched accordingly. But this would make compounding result predicates from different tasks much more cumbersome (or not possible at all).

[jwe-apple]

@rsanchezsaez I'd imagined using NSPredicate's -predicateWithSubstitutionVariables: on the predicates to substitute the task identifier. This works right down through predicates. So you'd use $CURRENT_TASK_IDENTIFIER right down through all the predicates when passed a nil predicate, and then pass @{@"CURRENT_TASK_IDENTIFIER" : task.identifier } . I've not tried it but the documentation indicates you can do this on a root compound predicate, and it gets applied through the entire expression hierarchy.

See Creating Predicates, search for predicateWithSubstitutionVariables:.

[rsanchezsaez]

Oh, that's a good idea. I'll look into it.

[rsanchezsaez]

I made taskIdentifier it nullable. I also added ORKResultPredicate convenience methods without the taskIdentifier argument, and added the corresponding Unit Tests. See the commit message for more details.

[jwe-apple]

/Users/Shared/Jenkins/Home/workspace/ResearchKit-GitHub-PullRequests/ResearchKit/ResearchKit/Common/ORKStepNavigationRule.m:182:45: error: use of undeclared identifier 'ORKTaskIdentifierResultPredicateVariableName'
                    substitutionVariables:@{ORKTaskIdentifierResultPredicateVariableName: taskResult.identifier}]) {
                                            ^
1 error generated.

[jwe-apple]

ok to test

[rsanchezsaez]

Sorry for the botched commit. Fixed.

[Erin-Mounts]

nitpick: two z's in Dizziness (seems to be used with one z throughout the file)

[rsanchezsaez]

Thanks! Fixed.

[Erin-Mounts]

ok I didn't spot anything new (other than the one nitpick) so r=me

[rsanchezsaez]

Replaced nil value by ORKNullStepIdentifier to signal that a step navigation rule should finish the ongoing task.

[jwe-apple]

r=me