getodk / collect

ODK Collect is an Android app for filling out forms. It's been used to collect billions of data points in challenging environments around the world. Contribute and make the world a better place! ✨📋✨
https://docs.getodk.org/collect-intro
Other
718 stars 1.38k forks source link

Add support for pulldata with local entities #6451

Closed seadowg closed 1 month ago

seadowg commented 1 month ago

Closes #6443 Blocked by https://github.com/getodk/javarosa/pull/802

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

I'd initially wanted to implement this by just executing a dynamically built XPath expression, but this proved tricky as JavaRosa currently doesn't attach secondary instances that aren't referenced by an instance expression. This would mean that a form that contained only a pulldata referencing an entity list wouldn't work (as the external instance would never be parsed and attached to the main instance). We'll look to move to the originally intended implementation in the next release as part of https://github.com/getodk/collect/issues/6454.

Instead, I've ended up using the LocalEntityInstanceAdapter in a similar way to LocalEntityFilterStrategy with modifications so that it now supports querying any child of an entity item (like label for example). This new support doesn't add anything significant in the way of optimisation however: queries for children like label work by just loading every entity in the list from the DB and then performing an in-mem filter. This limits the amount of work we do to just support pulldata.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

The big risks here will be to all pulldata forms (including ones that use with entities obviously) and to entity filters in forms in general.

One thing to note: I've not extended the instance "normalising" that pulldata does to entity lists. For normal secondary instances, pulldata would let you use the wrong case for the name or add .csv to the end. We'll discuss if we want to maintain this (currently undocumented) behaviour separately.

Before submitting this PR, please make sure you have:

seadowg commented 1 month ago

@lognaturel @grzesiek2010 I'll add an issue to follow up.

WKobus commented 1 month ago

Tested with success

Verified on device with Android 15

Verified cases:

dbemke commented 1 month ago

Tested with success

Verified on device with Android 10