Options contains ${node} references and choice filter and no translation should generate itext fields.

[gushil]

Closes #515

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

The simplest one that works.

What are the regression risks?

None. All tests are passed.

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:

• [x] included test cases for core behavior and edge cases in tests_v1
• [x] run nosetests and verified all tests pass
• [x] run black pyxform to format code
• [x] verified that any code or assets from external sources are properly credited in comments

[lognaturel]

The general approach here is to iterate through all the labels for a list that has a choice filter and if any of them has a reference, generate itext for that list. This adds yet more special cases and I think will have a noticeable performance impact on forms with really long choice lists that don't use any references in labels. It's not unusual to have thousands of choices for filtered lists and the typical case is not to have references.

Here are a couple more approaches to consider:

• Only check the first item in a list with a choice filter and generate itext only if that first choice has a label with a ref. That doesn't slow down the common case and I have a hard time imagining a case where someone would want a mix of labels with references and labels without. We still end up needing some special cases but with comments we can mitigate some of that.
• Always generate itext for lists with filters. The big advantage is that it's simpler. I don't love that it will often unecessarily increase the size of the XML and I know Enketo has trouble with forms with itext so this is probably not a good option but maybe I'm missing something.

@MartijnR what do you think?

[pbowen-oc]

@lognaturel - I do not have a specific real-world example for your first bullet point, but I could definitely see codes like the following: name label filter 0 "Event not related to any medication" 0 1 "Event related to ${medication1}" 1 2 "Event related to ${medication2}" 2 3 "Event related to ${medication3}" 3

If that approach were taken, I suppose a workaround in this case would be to add an item reference to a null calculated item to the first choice. I think that could work from a form design perspective, but would not be ideal.

[MartijnR]

Thanks @gushil and @lognaturel.

Good point about the potential performance issue with thousands of items (and generally not using outputs, especially not in those long lists).

Of your suggestions, my preference would be to use a trick such as you proposed in your first approach and keep the XML output clean for the common use cases without outputs. @pbowen-oc's concern that the first option may be the odd one that doesn't use an output, is a good point. So how about we check the first 2 to be 99.9% sure (or the first 10 to be 99.999999% sure)?

I'm guessing a non-global regex test of the unparsed choices per list is not an option, right?

[lognaturel]

we check the first 2 to be 99.9% sure

I'm happy with that. I'd also be happy documenting that for a choice list labels to be treated as dynamic, the first item's label must be dynamic. I think @pbowen-oc's example is realistic but it seems the non-dynamic label could also go last.

non-global regex test of the unparsed choices per list

I'm not sure I know what that means! Do you mean doing a regex on the text of all the choice labels together? I don't think there's a performant way to do that.

[MartijnR]

I'm not sure I know what that means!

I think maybe it makes more sense in Dutch? 😂 But you actually parsed that correctly. Let's forget about that idea.

[lognaturel]

I think maybe it makes more sense in Dutch? 😂

😄😄 I thought "non-global regex" might be some technical concept I didn't know about. I think it'd work if we had all choices as text but coming out of a spreadsheet they're in a collection.

[gushil]

Hi @lognaturel and @MartijnR

Thanks for the very useful comments. I've made some changes to accommodate that. Hoping to get some review again.

Thanks!

[lognaturel]

Let's close this in favor of #521 and apply what we've learned there.