-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Register provider change receiver after loading form #3214
Conversation
@grzesiek2010 just let me know that he's out sick. Feel better! Let's go straight to QA since this doesn't add new code but rather just moves existing one. |
@lognaturel I have a problem with building the apk. This is the content of my output:
|
Also wait to unregister it onDestroy because we want it to receive changes when the user is changing settings.
Sorry about that, @kkrawczyk123! The JavaRosa snapshot updated but this branch was still tracking the old implementation. I've fixed it and pushed. |
Tested with success! @opendatakit-bot unlabel "needs testing" |
Closes #3210
What has been done to verify that this works as intended?
Started filling out AuditTest.xml.txt, switched to settings, turned off location tracking, kept filling out the form and then turned location tracking back on. Verified that both changes to location settings were written to the audit log.
Why is this the best possible solution? Were any other approaches considered?
There were two problems: registering the receiver in
onStart
only meant that it wouldn't actually be registered after form load becauseonStart
generally runs before the form controller is set. Because it was unregistered inonStop
, it wouldn't be running when the user was looking at the Settings app.Given those issues, I believe this is the simplest approach.
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?
This should be a narrow fix that only fixes the bug at #3210. The only real risk is that it's not a complete fix.
Do we need any specific form for testing your changes? If so, please attach one.
AuditTest.xml.txt
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 checkAll
and confirmed all checks still pass OR confirm CircleCI build passes and run./gradlew connectedDebugAndroidTest
locally.