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

fix parsing of (multiple) pulldata calls in a (multi-line) expression #547

Merged
merged 1 commit into from
Aug 31, 2021

Conversation

lindsay-stevens
Copy link
Contributor

  • previous code would find a pulldata call only if the expression had no line breaks and one pulldata call.
  • new code finds all pulldata calls in single or multi-line expressions. A fix and a feature 🎉
  • multiple pulldata calls and the resulting multiple external instances pass ODK Validate checks.

Closes #509

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

Users may wish to format their large / complex expressions with newlines and still utilise the pulldata() function. It seems most XLSForm documentation and examples show pulldata used on its own in an expression. However the existing test test_external_instance_pulldata_constraint_in_expression shows that pulldata is valid as part of compound expressions. It may be a bit unusual but there seems to be no technical reason why an expression could not use multiple pulldata calls. These changes result in support for multi-line formatted expressions, and multiple pulldata calls in an expression.

Alternatives may be to raise an error if a) multi-line formatting or b) multiple pulldata calls are detected in an expression. However there seems no spec or technical reason to forbid these usages.

What are the regression risks?

It may be that some XForm collectors / tools do not support expressions with line breaks, or multiple pulldata calls. In which case these applications could check for these and reject the form design. The current changes are accepted by ODK Validate so presumably they are OK.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

No, they seem to be consistent with the documentation. The section "How to pull data from CSV" refers to pulldata as a function, so it's reasonable to assume from that that it could be used more than once in an expression.

Before submitting this PR, please make sure you have:

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

- previous code would find a pulldata call if the expression had no
  line breaks and there was only one pulldata call in the expression
- new code finds all pulldata calls in single or multi-line calls
@lindsay-stevens
Copy link
Contributor Author

Tested a form with multiple pulldata and line-break on ODK Collect v2021.2.3 on Android 10 and it works.

Using the following files, the expected if and pulldata functionality applies:

  • QA = 1 and QB = 1 then the note QD shows "d: 'a'"
  • QA = 1 and QB = 2 then the note QD shows "d: 'b'"
  • QA = 2 and QC = 1 then the note QD shows "d: 'g'"
  • QA = 2 and QC = 2 then the note QD shows "d: 'f'"
/sdcard/Android/data/org.odk.collect.android/files/projects/DEMO/forms/test.xml
<?xml version="1.0"?>
<h:html xmlns="http://www.w3.org/2002/xforms" xmlns:ev="http://www.w3.org/2001/xml-events" xmlns:h="http://www.w3.org/1999/xhtml" xmlns:jr="http://openrosa.org/javarosa" xmlns:odk="http://www.opendatakit.org/xforms" xmlns:orx="http://openrosa.org/xforms" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
  <h:head>
    <h:title>pyxform_autotesttitle</h:title>
    <model odk:xforms-version="1.0.0">
      <instance>
        <pyxform_autotestname id="pyxform_autotest_id_string">
          <a>1</a>
          <b>1</b>
          <c>1</c>
          <d/>
          <f/>
          <meta>
            <instanceID/>
          </meta>
        </pyxform_autotestname>
      </instance>
      <instance id="my_data_b" src="jr://file-csv/my_data_b.csv"/>
      <instance id="my_data_c" src="jr://file-csv/my_data_c.csv"/>
      <bind nodeset="/pyxform_autotestname/a" type="int"/>
      <bind nodeset="/pyxform_autotestname/b" type="int"/>
      <bind nodeset="/pyxform_autotestname/c" type="int"/>
      <bind calculate="if( /pyxform_autotestname/a = 1, 
pulldata('my_data_b', 'metainstanceID', 'aref',  /pyxform_autotestname/b ),
pulldata('my_data_c', 'metainstanceID', 'aref',  /pyxform_autotestname/c ))" nodeset="/pyxform_autotestname/d" type="string"/>
      <bind nodeset="/pyxform_autotestname/f" readonly="true()" type="string"/>
      <bind jr:preload="uid" nodeset="/pyxform_autotestname/meta/instanceID" readonly="true()" type="string"/>
    </model>
  </h:head>
  <h:body>
    <input ref="/pyxform_autotestname/a">
      <label>QA</label>
    </input>
    <input ref="/pyxform_autotestname/b">
      <label>QB</label>
    </input>
    <input ref="/pyxform_autotestname/c">
      <label>QC</label>
    </input>
    <input ref="/pyxform_autotestname/f">
      <label> calculated a: '<output value=" /pyxform_autotestname/a "/>', b: '<output value=" /pyxform_autotestname/b "/>', c: '<output value=" /pyxform_autotestname/c "/>', d: '<output value=" /pyxform_autotestname/d "/>' </label>
    </input>
  </h:body>
</h:html>

CSV files in /sdcard/Android/data/org.odk.collect.android/files/projects/DEMO/forms/test-media/

my_data_b.csv

aref,metainstanceID
1,a
2,b

my_data_c.csv

aref,metainstanceID
1,g
2,f

@lognaturel lognaturel merged commit 7b0ed81 into XLSForm:master Aug 31, 2021
@lindsay-stevens lindsay-stevens deleted the pyxform-509 branch September 25, 2021 07:45
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.

Incorrect parsing of pulldata() with a newline
2 participants