Skip to content

fix: no labels visible for randomized selects with translations#764

Merged
lindsay-stevens merged 1 commit intoXLSForm:masterfrom
lindsay-stevens:fix-translate-randomize
May 6, 2025
Merged

fix: no labels visible for randomized selects with translations#764
lindsay-stevens merged 1 commit intoXLSForm:masterfrom
lindsay-stevens:fix-translate-randomize

Conversation

@lindsay-stevens
Copy link
Copy Markdown
Contributor

@lindsay-stevens lindsay-stevens commented Apr 30, 2025

Per report in https://forum.getodk.org/t/54492

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

As mentioned above I wasn't totally sure that these combinations are supported, considering enketo/enketo#322 but it seems like it works in Collect (per https://forum.getodk.org/t/42506/4) and presumably will in Webforms too. Also I wasn't able to locally reproduce the ODK Validate warnings mentioned in the reporting forum thread but maybe the Validate version on xlsform-online is out of sync?

What are the regression risks?

This should be a fix since the existing tests pass and the new code satisfies tests that worked on earlier code (as described above), and the code committed in between didn't intend to change randomize or choice_filter behaviour for users.

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:

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

- regression seems to be from pyxform/XLSForm#746 where the choices may be
  referenced by the survey only and the select question does not have a
  reference. This happens if the question is randomized, or external
  select, or search appearance.
  - this would suggest that maybe the choices should be in-line for
    randomized choices - however the existing tests for randomize expect
    an itemset in the body. Also, these new tests pass on tag 3.0.1
    (prior to XLSForm#746), and fail on master (e0b350b).
Copy link
Copy Markdown
Contributor

@lognaturel lognaturel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for spotting this on the forum and for the speedy fix!

@lognaturel
Copy link
Copy Markdown
Contributor

lognaturel commented May 1, 2025

maybe the choices should be in-line for randomized choices

Do you mean directly as children of the question? That wouldn't work with the way randomization currently works -- it relies on an itemset declaration that references a secondary instance being able to include a function call. It's possible to randomize nodes from external secondary instances and even repeats! I think Enketo supports those, what doesn't work is if there's a predicate in the expression (e.g. [age > 17]).

master is deployed to staging

Yes, exactly. It's to catch issues like this one as early as possible. Should we add this to the readme, maybe? The only place I can think of where it's documented is here (ODK internal)

when uploaded to a project on https://staging.getodk.cloud/

Ahhh, we have various staging servers. 🙈 That one reflects the latest Central but doesn't also update pyxform. The user was using the pyxform staging server.

@lindsay-stevens lindsay-stevens merged commit 04d7e5d into XLSForm:master May 6, 2025
10 checks passed
@lindsay-stevens lindsay-stevens deleted the fix-translate-randomize branch May 6, 2025 06:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants