Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

678: itextId node not added to instance items for choice list names with dashes in multilingual forms #680

Merged
merged 1 commit into from
Jan 9, 2024

Conversation

lindsay-stevens
Copy link
Contributor

Closes #678

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

In survey.py, _generate_static_instances has a check for multi_lang, which involves matching the list_name to the translations dict. That split the list_name on the itextId separator (a dash) but a dash can also appear in the list_name, so it was broken when a list_name contained a dash.

The fix is to still split the string, but chop off the last group (the number) to do the list_name match. Another way could be to not use a string internally (e.g. a tuple) to avoid parsing issues, then output a string when necessary.

What are the regression risks?

This fixes a regression.

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 nosetests and verified all tests pass
  • run black pyxform tests to format code
  • verified that any code or assets from external sources are properly credited in comments

- in survey.py, _generate_static_instances has a check for multi_lang,
  which involves matching the list_name to the translations dict. That
  split the list_name on the itextId separator (a dash) but a dash can
  also appear in the list_name, so it was broken when a list_name
  contained a dash.
Copy link
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.

Oh, of course! Too bad we didn't catch that but great that it's such a straightforward fix. 🎉

@lognaturel lognaturel merged commit 2797258 into XLSForm:master Jan 9, 2024
10 checks passed
yanokwa added a commit to getodk/pyxform-http that referenced this pull request Jan 9, 2024
@lindsay-stevens lindsay-stevens deleted the pyxform-678 branch January 10, 2024 01:56
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.

itextId node not added to instance items for choice list names with dashes in multilingual forms
2 participants