Show error when there are save_tos in a repeat, simplify entities tests
#636
Conversation
This was confusing because pyxform is often wrapped in other tools and users don't know what it is.
lognaturel
force-pushed
the
entities-in-repeat
branch
from
48d35d7
to
cefd5a4
Compare
|
LGTM 👍
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? |
Good cases to consider. I tried those as an extra sanity check and confirmed they're ok but don't think they're worth adding to the test suite. |
|
Thank you so much, @lindsay-stevens 🎉 |
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