fix: survey json->xml round trip not working for some question types

lindsay-stevens commented 2 years ago

Closes #605

details of issue noted in new test under js2x_test_import_from_json.py
xls2json updated to make it easier to be confident that parameters will be a dict (vs. none or a string).
- moved off params processing to parse/validate step validators package to be consistent with other utils of this nature.
- updated error message to show all invalid params at once, instead of one at a time (which may have resulted in users to having convert a few times to get a valid params set).

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

PR #606 also fixes this but I wanted to be a bit more sure that xls2json is really passing in a dict in all cases. Also this PR has a separate test which isolates the issue to the scenario where a question that accepts parameters doesn't have parameters, and that survey is passed in as json and then converted to XML.

What are the regression risks?

Hopefully none! The error message for invalid params is updated, maybe some users expect the old message. But I think it is more user friendly to say what all the issues are at once, if possible.

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

ukanga commented 2 years ago

Looks good, you took a step further than I did. I prefer this approach as well. Let's tag a release, please.

lognaturel commented 2 years ago

@ukanga Forgot the release is not just tag and go yet! Doesn't look like I'll get to it today. Fine by me if you want to merge your own PR and tag (tagging will release to pypi). Or if you get the PR ready I can tag and release tomorrow.

I've gone back and forth on whether this should be v1.10.0 or v1.9.1. I could go either way. I was leaning towards v1.10.0 because the detection of dynamic defaults is a change in behavior, not just a fix.

XLSForm / pyxform