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

Register provider change receiver after loading form #3214

Merged
merged 1 commit into from
Jul 24, 2019

Conversation

lognaturel
Copy link
Member

@lognaturel lognaturel commented Jul 17, 2019

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 because onStart generally runs before the form controller is set. Because it was unregistered in onStop, 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:

  • run ./gradlew checkAll and confirmed all checks still pass OR confirm CircleCI build passes and run ./gradlew connectedDebugAndroidTest locally.
  • verified that any code or assets from external sources are properly credited in comments and/or in the about file.
  • verified that any new UI elements use theme colors. UI Components Style guidelines

@lognaturel lognaturel added this to the v1.23 milestone Jul 17, 2019
@lognaturel lognaturel requested a review from grzesiek2010 July 17, 2019 05:04
@lognaturel
Copy link
Member Author

@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.

@kkrawczyk123
Copy link
Contributor

@lognaturel I have a problem with building the apk. This is the content of my output:

Starting a Gradle Daemon, 1 incompatible and 1 stopped Daemons could not be reused, use --status for details

> Configure project :collect_app
Could not find google-services.json while looking in [src/nullnull/odkCollectRelease, src/odkCollectRelease/nullnull, src/nullnull, src/odkCollectRelease, src/nullnullOdkCollectRelease]
registerResGeneratingTask is deprecated, use registerGeneratedResFolders(FileCollection)
registerResGeneratingTask is deprecated, use registerGeneratedResFolders(FileCollection)
registerResGeneratingTask is deprecated, use registerGeneratedResFolders(FileCollection)

> Task :collect_app:processDebugGoogleServices
Parsing json file: /home/kasia/projekty/collect2/collect/collect_app/src/debug/google-services.json

> Task :collect_app:compileDebugJavaWithJavac
Gradle may disable incremental compilation as the following annotation processors are not incremental: dagger-compiler-2.16.jar (com.google.dagger:dagger-compiler:2.16), dagger-android-processor-2.16.jar (com.google.dagger:dagger-android-processor:2.16), butterknife-compiler-10.0.0.jar (com.jakewharton:butterknife-compiler:10.0.0).
Consider setting the experimental feature flag android.enableSeparateAnnotationProcessing=true in the gradle.properties file to run annotation processing in a separate task and make compilation incremental.
/home/kasia/projekty/collect2/collect/collect_app/src/main/java/org/odk/collect/android/logic/FormController.java:29: error: package org.javarosa.core.model.actions.setlocation does not exist
import org.javarosa.core.model.actions.setlocation.SetLocationActionHandler;
                                                  ^
/home/kasia/projekty/collect2/collect/collect_app/src/main/java/org/odk/collect/android/logic/actions/setlocation/CollectSetLocationAction.java:23: error: package org.javarosa.core.model.actions.setlocation does not exist
import org.javarosa.core.model.actions.setlocation.SetLocationAction;
                                                  ^
/home/kasia/projekty/collect2/collect/collect_app/src/main/java/org/odk/collect/android/logic/actions/setlocation/CollectSetLocationAction.java:52: error: cannot find symbol
public class CollectSetLocationAction extends SetLocationAction implements LocationListener {
                                              ^
  symbol: class SetLocationAction
/home/kasia/projekty/collect2/collect/collect_app/src/main/java/org/odk/collect/android/logic/actions/setlocation/CollectSetLocationActionHandler.java:19: error: package org.javarosa.core.model.actions.setlocation does not exist
import org.javarosa.core.model.actions.setlocation.SetLocationAction;
                                                  ^
/home/kasia/projekty/collect2/collect/collect_app/src/main/java/org/odk/collect/android/logic/actions/setlocation/CollectSetLocationActionHandler.java:20: error: package org.javarosa.core.model.actions.setlocation does not exist
import org.javarosa.core.model.actions.setlocation.SetLocationActionHandler;
                                                  ^
/home/kasia/projekty/collect2/collect/collect_app/src/main/java/org/odk/collect/android/logic/actions/setlocation/CollectSetLocationActionHandler.java:25: error: cannot find symbol
public class CollectSetLocationActionHandler extends SetLocationActionHandler {
                                                     ^
  symbol: class SetLocationActionHandler
/home/kasia/projekty/collect2/collect/collect_app/src/main/java/org/odk/collect/android/logic/actions/setlocation/CollectSetLocationActionHandler.java:26: error: cannot find symbol
    public SetLocationAction getSetLocationAction() {
           ^
  symbol:   class SetLocationAction
  location: class CollectSetLocationActionHandler
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
7 errors

> Task :collect_app:compileDebugJavaWithJavac FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':collect_app:compileDebugJavaWithJavac'.
> Compilation failed; see the compiler error output for details.

Also wait to unregister it onDestroy because we want it to receive changes when the user is changing settings.
@lognaturel
Copy link
Member Author

Sorry about that, @kkrawczyk123! The JavaRosa snapshot updated but this branch was still tracking the old implementation. I've fixed it and pushed.

@kkrawczyk123
Copy link
Contributor

Tested with success!
Verified on Androids 4.2, 4.4, 5.1 6.0 an 8.1.
I have checked different cases of turning gps on/off when filling form. Location providers disabled/enabled is tracked in the audit file.

@opendatakit-bot unlabel "needs testing"
@opendatakit-bot label "behavior verified"

@lognaturel lognaturel merged commit 2558c12 into getodk:master Jul 24, 2019
@lognaturel lognaturel deleted the issue-3210 branch July 24, 2019 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provider change during form filling is not logged to audit
4 participants