791: fix support for a source referencing an ancestor repeat target

[lindsay-stevens]

Closes #791

Why is this the best possible solution? Were any other approaches considered?

See commit messages for details.

Considered keeping lowest_common_ancestor behaviour of identifying other node relationship types but the name of the function is about common ancestors and it's only used for that purpose.

What are the regression risks?

Fixes a regression.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

No

Before submitting this PR, please make sure you have:

• included test cases for core behavior and edge cases in tests
• run python -m unittest and verified all tests pass
• run ruff format pyxform tests and ruff check pyxform tests to lint code
• verified that any code or assets from external sources are properly credited in comments

[lindsay-stevens]

The CI test failure here is due to the issue described in this comment.

[lognaturel]

Thank you! Looks good to me but some of the tests would benefit from referencing real nodes, I think. Their assertions are technically correct but feel more confusing than necessary.

I also want to note that there is ambiguity in the targeted expressions. Someone could conceivably use a ${} from within a repeat and want a relative reference. But I think the behavior from this PR is more likely to match their expectations. I assume that if someone knows how to formulate this kind of more complex expression they can also write relative references themselves comfortably. Most importantly, we didn't intend to change the behavior from what it was before.

[lindsay-stevens]

Thanks I updated the test case paths to match the form structure in 5b0289a. Not sure if it's what you meant but the tests added in this PR under TestGetPathRelativeToLCAR show that if the target repeat (parent of source) is itself inside a repeat then the path will be relative (e.g. /y/r1o/at/s -> ../../at); while the tests under TestReferencesToAncestorRepeat show that if the target repeat (parent of source) is not inside a repeat then the path will be absolute (e.g. /y/at/s -> /y/at). Then within those sets of tests are combinations of one or two groups or repeats either above or below the target. The confusing case might be when the source is in a nested repeat but the target (which is also a repeat) is not in a repeat so the path is absolute (e.g. /y/at/r1s/s -> /y/at).

[lognaturel]

Thanks! It feels easier to understand when the paths exist. Of course the expressions with repeats still wouldn’t really make sense because they return nodesets but at least the shape of the paths look right.

There are a couple things in the other PR I’m not yet understanding. I’ll finish that review after some rest! I think it will be fine to merge first.