Support non standard itemset value and label refs and external instances #68
Conversation
|
Ooh, I totally missed this before responding to #66. Looks great to me (tests!). My only question would be whether this slows down the validation process significantly? I guess regex are pretty fast and this is an offline process anyway so it doesn't particularly matter. My sense is that we'll still want to do something at that JavaRosa level to address various clients' needs as described in getodk/javarosa#444. This generalization doesn't change anything for pyxform users (as opposed to #66) because the itemset structure is always the same there. It does mean that users who write XML by hand could validate those forms. We don't know if any of those users exist. Certainly the cleanup alone is worthwhile. |
| import java.nio.file.Files; | ||
| import java.nio.file.Path; | ||
| import java.util.Set; | ||
| import org.javarosa.core.reference.Reference; | ||
|
|
||
| /** | ||
| * Provides the local URI of a simple XML document. This allows forms with external secondary instances to |
There was a problem hiding this comment.
This whole comment is now a lie and should be updated.
- Since the test directory is inside src, where the main sourceset is defined, we need to exclude the tests directory. Otherwise the project won't compile, because the test dependencies are only available for the test sourceset. - Also, we don't want that the test sources end up being included in the JAR file.
- Only supports itemset definitions like the ones generated by xslforms, which use <value ref="name"/> and <label ref="label"/>
|
Closing in favour of #69 |
Sorry for commenting after closing. I just wanted to respond to the quoted comment, not challenge any technical piece here. I caught this comment from @lognaturel when scanning the log and these users do exist. In fact, when changes to xlsform offline/online disabled pretty-printing it caused some important workflows to break for my clients. They were hand-editing large converted xlsform forms to make form maintenance from year to year easier while retaining certain advanced form capabilities. The one I recall immediately was to use dynamic itemsets populated from a repeat question. This is useful when building a list of items/individuals during an interview and later wanting to select from that list elsewhere in the form. I don't know if this is now possible in xlsform with recent changes, but if so, it is was not obvious from the docs last time I checked. |
Hopefully they were able to add an auto-indent step? The driver for that change was the big difference in file size.
Absolutely, I do this myself and hope XLSForm/pyxform#38 can be addressed sooner than later to expose that power via XLSForm (no, it's still not possible). Do those users tend to use Validate as part of their workflow? I generally go straight to Collect. In fact, I only ever use Validate as part of |
|
And thanks for chiming in. It's really helpful to get some of those assumptions pushed on. |
They had me to back them up. They just reported the issue to me and I pretty-printed it for them. They were savvy enough to edit the xml file per the instructions in the SOP for the survey, but not enough to figure out why the xml output from xlsform put everything on one line all of a sudden.
It depends on the user. The power users so far tend to eliminate all errors and warnings from validate before trying it in collect (based on experience that many of those tend to become necessary to understand later when the form doesn't work as expected in Collect). Some do go straight to Collect, but normally those users are less inclined to wrestle with the tooling at all - they usually just punt and tell me "it's not working". After the changes in xlsform caught us by surprise (I was busy and wasn't watching the changelogs), I just developed simple tool-centric web services for validation and conversion and use those to eliminate the conversion step entirely. If they need to edit, they can manually convert in which case it pretty-prints by default (a safe assumption, since it's only useful to manually convert in this paradigm when you need to edit it by hand). |
|
Thanks for that background, @batkinson, super helpful. I think that for this specific issue we're still ok to start with a narrow fix. Users leveraging external secondary instances are writing XML docs and so more likely to be savvy. We do intend on having a more general JavaRosa fix sooner than later. |
Closes #65
This PR goes one step further towards generic support for external instance by creating a synthetic external instance that would have all the refs required by the form, thus passing validation.
What has been done to verify that this works as intended?
Added unit tests that verify this claim.
Why is this the best possible solution? Were any other approaches considered?
This could be considered hacky because it involves using regular expressions to configure the StubReferenceManager.
I considered parsing the form with KXML2 and then searching for those ref names in the
<body>element. This would be less brittle than using regexps, but it felt like we would only get marginal benefits in exchange of making the search code much more complicated.Are there any risks to merging this code? If so, what are they?
Although the regexps have safety whitespace placeholders to account for weird formattings, it won't detect ref names correctly when the
valueandlabelelements use more than one line:Odds that this happens feel low, though.
The regexps could be missing other unidentified common use cases.