XLSForm / pyxform

A Python package to create XForms for ODK Collect.
BSD 2-Clause "Simplified" License
75 stars 133 forks source link

576: fix _count suffix name clash with repeats targeting another item #674

Closed lindsay-stevens closed 6 months ago

lindsay-stevens commented 6 months ago

Closes #576

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

Given a form with an item named "a_count" that holds the desired repeat count, and a repeat called "a", when pyxform injects an internal item suffixed with "_count" to hold the repeat count for "a", then pyxform emits a duplicate survey element error (2x "a_count"). This PR adds new condition to existing logic, where if the expression parses to a single pyxform reference, then the item injection is skipped.

As described in #435 (case 1), when the repeat count is just a reference to another item, then that item could be used directly, instead of injecting an item with a calculate item, thereby causing a name clash and thus the error. I tried also satisfying #435 case 3 by allowing single number expressions, but a few test failed with "something broke the parser!" which I think would be due to https://github.com/getodk/javarosa/issues/399

For future reference, the section of code in xls2json.py around repeats is a bit confusing but the dict structure of json["control"]["jr:count"] is a result of processing in dealias_and_group_headers (repeat_count -> control::jr:count -> dict).

What are the regression risks?

ODK Validate seems OK with this, but I have not tested it with Collect or Enketo. If they or any other XForms tool expects there to always be an injected instance item to hold the count, then I expect there would be an error.

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

No, it's an internal form structure issue, which addresses a user-facing bug.

Before submitting this PR, please make sure you have: