Send and finalize forms in oldest first order #6356
Conversation
grzesiek2010
force-pushed
the
COLLECT-6128
branch
from
265c860
to
f63d716
Compare
..._app/src/androidTest/java/org/odk/collect/android/feature/instancemanagement/AutoSendTest.kt
Outdated
Show resolved
Hide resolved
...androidTest/java/org/odk/collect/android/feature/instancemanagement/SendFinalizedFormTest.kt
Show resolved
Hide resolved
collect_app/src/main/java/org/odk/collect/android/tasks/InstanceUploaderTask.java
Show resolved
Hide resolved
.../src/androidTest/java/org/odk/collect/android/feature/formmanagement/BulkFinalizationTest.kt
Outdated
Show resolved
Hide resolved
|
|
||
| private fun parseXml(file: File): Document { | ||
| return StringReader(String(file.readBytes())).use { reader -> | ||
| val parser = KXmlParser() |
There was a problem hiding this comment.
Ah nice! I'd just meant doing a contains, but parsing the XML is definitely more exact. How about combining the common bits here with the parsing done in StubOpenRosaServer and pulling it out to some kind of helper function/object? I think we could actually use some of this in real form download code soon as well to avoid needing to use JavaRosa's static parser.
grzesiek2010
force-pushed
the
COLLECT-6128
branch
from
9a3fedd
to
08d7c37
Compare
grzesiek2010
force-pushed
the
COLLECT-6128
branch
2 times, most recently
from
ed2bba9
to
36be8e0
Compare
| @JvmStatic | ||
| @Throws(XmlPullParserException::class) | ||
| fun parseXml(formInputStream: InputStream): Document { | ||
| return XFormParser.getXMLDocument(InputStreamReader(formInputStream)) |
There was a problem hiding this comment.
I'd not noticed getXMLDocument! Maybe for the moment we can just that directly everywhere and remove this file? I'm a little worried about the extension functions as they could be confusing when working with a Document that's not an XForm.
grzesiek2010
force-pushed
the
COLLECT-6128
branch
from
36be8e0
to
64029a0
Compare
|
Tested with Success! Verified on a device Android 10 Verified Cases:
|
|
Tested with Success Verified on device with Android 14 |
Closes #6128
Why is this the best possible solution? Were any other approaches considered?
Some of the changes could have been skipped (such as sorting instances before sending/finalizing) because everything appeared to work as expected, and the tests passed (including the new ones). However, this functionality relied on implicit behavior - such as the order of rows in the database rather than on correctly maintaining the intended order. To avoid depending on such implicit factors, I decided to add explicit sorting to ensure consistent behavior in all cases.
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?
As described in the issue this requires testing:
Everything should happen in the oldest first order. The order of sending forms could be tested with Central. When it comes to bulk finalizing it might require looking into the database of instances and its
lastStatusChangeDateto check the dates that reflect the order of finalizing forms.Do we need any specific form for testing your changes? If so, please attach one.
No.
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:
./gradlew connectedAndroidTest(or./gradlew testLab) and confirmed all checks still passDateFormatsTest