Skip to content

Conversation

@capnfabs
Copy link
Member

Previously, when submitting an Xform, we parsed the XML in the request body to add a set of
"temporary observations" to the database. These had the correct values, but didn't have UUIDs from
the server.

This mechanism was prone to the issues described in #125, so instead of using temporary
observations, we retrieve the result of the Xform submission from the server, and insert real,
server-provided observation information into the local data store instead.

Fixes #125.

Note: this change also includes some formatting fixes, I got sick of fighting the Android Studio auto-formatter.

@capnfabs capnfabs added this to the v1.0 (pilot) milestone Jan 28, 2016
Previously, when submitting an Xform, we parsed the XML in the request body to add a set of
"temporary observations" to the database. These had the correct values, but didn't have UUIDs from
the server.

This mechanism was prone to the issues described in #125, so instead of using temporary
observations, we retrieve the result of the Xform submission from the server, and insert real,
server-provided observation information into the local data store instead.

Fixes #125.
Copy link
Contributor

Choose a reason for hiding this comment

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

@capnfabs It's difficult to use the Generic error listener (the one with the toasts)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, don't need to do it right now though.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it would be difficult, it was just like this when I got here :)

@llvasconcellos
Copy link
Contributor

@capnfabs Looks good.

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome refactoring, @capnfabs. It helps fixing #125 in a such cleaner and clever way. Congrats.

@viniciusboson
Copy link
Contributor

Fantastic work @capnfabs.
These changes may fix the same error found in #172, right ?

@capnfabs
Copy link
Member Author

Hey @viniciusboson! Yeah, I think they'll fix the same problems that you're dealing with, so long as all the forms have definitely finished submitting before sync starts. There might still be a "flicker" where an observation exists, and then disappears, and then comes back again if a sync occurs before the form is uploaded, however. So it's important to make sure that all forms are submitted before sync proceeds :)

I realised overnight that your code still depends on a lot of the temporary observations logic that I deleted in this change, so I'll add that logic back in now, so it's not deleted when I merge this PR.

Vinicius is using this for offline form submission, so we need to keep it for when his changes get merged.
capnfabs added a commit that referenced this pull request Jan 29, 2016
Update observations from Xform submission response.
@capnfabs capnfabs merged commit c96983f into projectbuendia:dev Jan 29, 2016
@viniciusboson
Copy link
Contributor

Hey @capnfabs,
As far as i understood, these changes that you reverted could work fine with my PR too.
I would just need to merge and check if it still works ...
I really liked what you did ..

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.

PatientChartActivity drops temporary observations prematurely.

3 participants