694: search and pulldata secondary instance conflict

lindsay-stevens commented 7 months ago

Closes #694

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

The previous solution for search + translations was pretty close, but created the possibility of conflicts in the secondary instances. This PR removes the secondary instance that would have been emitted. It also raises an error two search feature combinations that are not supported, and even if they were, would be broken by this PR's strategy (see tests for examples, where errored=True). These combinations are: 1) search appearance on a select_from_file, 2) search appearance choice list used in other selects.

An alternative detailed in #694 was to merge compatible secondary instances, but @lognaturel clarified offline that this wouldn't work because the src attribute is how Collect/Enketo identify external instances. So having choices in a secondary instances with a non-blank src wouldn't work.

What are the regression risks?

If users had forms which included the above 2 scenarios which now raise an error, they would need to fix up their form. If the error seems to harsh maybe a warning could be emitted, but ODK Validate raises an ugly error for these cases anyway (due to missing instance).

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

I tidied up and included some settings tests I wrote for recent forum issues. These settings could be included in the XLSForm reference template.

Before submitting this PR, please make sure you have:

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

lognaturel commented 7 months ago

1) search appearance on a select_from_file, 2) search appearance choice list used in other selects.

I don't think either of these has reasonable expected behavior and errors are an improvement. 👍

lognaturel commented 7 months ago

These settings could be included in the XLSForm reference template

Filed settings docs issue at https://github.com/XLSForm/xlsform.github.io/issues/266

lindsay-stevens commented 7 months ago

Thanks! search() vs search is a bit confusing. I've gone with search() for the errors and clarified it in the code + tests as well.

XLSForm / pyxform