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

Remembering previously entered values (auto-fill) #406

Merged
merged 9 commits into from
Mar 12, 2019

Conversation

cooperka
Copy link
Contributor

Companion to getodk/collect#2882. See that PR for a full description.

This change basically just maps an instance's src value of "jr://instance/last-saved" to whatever is passed in as lastSavedSrc.

Note: A null value for lastSavedSrc is fine and means no instance src is provided, meaning the instance will be treated as internal instead of external. The internal <instance id="last-saved"/> has no children and thus will provide empty values for anything referring to it.

@codecov-io
Copy link

codecov-io commented Feb 15, 2019

Codecov Report

Merging #406 into master will increase coverage by 0.04%.
The diff coverage is 75%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #406      +/-   ##
============================================
+ Coverage     48.67%   48.72%   +0.04%     
- Complexity     2897     2905       +8     
============================================
  Files           241      241              
  Lines         13591    13603      +12     
  Branches       2631     2635       +4     
============================================
+ Hits           6616     6628      +12     
- Misses         6132     6133       +1     
+ Partials        843      842       -1
Impacted Files Coverage Δ Complexity Δ
src/org/javarosa/xform/util/XFormUtils.java 44.28% <100%> (+0.8%) 15 <1> (+1) ⬆️
src/org/javarosa/xform/parse/XFormParser.java 64.63% <71.42%> (+0.08%) 234 <5> (+5) ⬆️
...org/javarosa/core/model/condition/Triggerable.java 67.76% <0%> (ø) 25% <0%> (ø) ⬇️
...varosa/core/model/condition/EvaluationContext.java 71.05% <0%> (+0.65%) 39% <0%> (+1%) ⬆️
src/org/javarosa/xml/TreeElementParser.java 83.33% <0%> (+8.33%) 6% <0%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e958f3...bf57c08. Read the comment docs.

@yanokwa yanokwa requested a review from ggalmazor February 18, 2019 03:30
@yanokwa
Copy link
Member

yanokwa commented Feb 18, 2019

@cooperka This will likely require a PR to https://github.com/opendatakit/xforms-spec as well. Can you please file an issue there so we don't forget?

Copy link
Contributor

@ggalmazor ggalmazor left a comment

Choose a reason for hiding this comment

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

The changes are narrow and clean, which is super nice, thanks! I think that this PR not only solves the issue but also makes it easier to understand.

The only change I'd want to ask for is to add a new junit test that verifies the new parser method using an external file. I can give a hand if you're not sure how to go about it.

src/org/javarosa/xform/parse/XFormParser.java Show resolved Hide resolved
src/org/javarosa/xform/parse/XFormParser.java Show resolved Hide resolved
src/org/javarosa/xform/parse/XFormParser.java Outdated Show resolved Hide resolved
@cooperka
Copy link
Contributor Author

cooperka commented Feb 21, 2019

Thanks @ggalmazor, I added one basic test and I'd love help with the second one. It's probably a simple thing; I have a file I want to read from, but I get this error:

XFormParseException: Unable to parse external secondary instance: org.javarosa.core.reference.InvalidReferenceException:
The reference "jr://file/resources/org/javarosa/xform/parse/last-saved-filled.xml" was invalid and couldn't be understood. The javarosa jr:// reference root "file" is not available on this system and may have been mis-typed.

Any advice for how to specify the filename? See commit 9d8f637.

@yanokwa yanokwa added this to the v2.14.0 milestone Feb 26, 2019
@ggalmazor
Copy link
Contributor

ggalmazor commented Feb 27, 2019

Hi, @cooperka! Sorry for the delay :P

You could try this:

Path lastSavedSubmissionDirectory = r("last-saved-filled.xml").toAbsolutePath().getParent();
ReferenceManagerTestUtils.setUpSimpleReferenceManager("file", lastSavedSubmissionDirectory);
FormDef formDef = parse(formName, "jr://file/last-saved-filled.xml");

I've tried it in your branch and it passes the test. Here's how that works:

  • We configure JR to believe that the jr://file schema has to be resolved to the directory holding the last saved submission in your computer.
    That's the ReferenceManagerTestUtils.setUpSimpleReferenceManager() part
  • We tell JR to parse the form assuming the last saved submission is at the root of jr://file

Please, feel free to tinker in ReferenceManagerTestUtils. It's a tests utility class we've just started using for other stuff and it's very limited at this point. Maybe you need to add stuff there to support your tests.

@cooperka
Copy link
Contributor Author

All feedback addressed, thank you both!

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.

5 participants