lindsay-stevens
commented
1 year ago LGTM 👍
The worst case is probably that if my "in repeat" check is wrong we could be rejecting forms incorrectly.
Seems unlikely - there's a test to show that the check identifies a repeat successfully, the validation func checks that there's an entity binding before throwing any errors, and all other tests are passing. I can't currently think of any plausible way the new check would do the wrong thing - maybe if there was a group inside the repeat? Or the bound item wasn't the first in the repeat?
lognaturel
Closes #633 Addresses part of #632
Why is this the best possible solution? Were any other approaches considered?
I decided to clean up entities tests here as suggested by @lindsay-stevens to make it easier to identify what test coverage there currently is.
It took me a little bit to decide how to check whether a row is in a repeat. There was no such functionality at that level (e.g. survey_element has it) and it's only needed in one place so I did it with an inline expression. I think it's easy enough to read and to test from the outside.
What are the regression risks?
This doesn't touch any existing logic. The worst case is probably that if my "in repeat" check is wrong we could be rejecting forms incorrectly.
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:
testsnosetestsand verified all tests passblack pyxform teststo format code