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

Fixed the order of events when location tracking is enabled #5744

Merged
merged 2 commits into from
Oct 5, 2023

Conversation

grzesiek2010
Copy link
Member

@grzesiek2010 grzesiek2010 commented Sep 18, 2023

Closes #5262

What has been done to verify that this works as intended?

I've tested the fix manually and added automated tests.

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

Enabling background recording takes place when we call activityDisplayed() when we navigate back from the hierarchy view we need to do that earlier than in onResume() because it's after refreshing the screen and logging the question event.

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?

Apart from verifying that the issue has been fixed, it would be good to play a little bit with background location recording to make sure there is no regression.

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:

  • 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

@grzesiek2010 grzesiek2010 marked this pull request as ready for review September 18, 2023 13:51
@dbemke
Copy link

dbemke commented Oct 3, 2023

We're testing the PR with the CircleCI apk and there is only OSM available. Do you think there might be any differences for other sources of maps in background location? Or testing OSM is enough?

@grzesiek2010
Copy link
Member Author

We're testing the PR with the CircleCI apk and there is only OSM available. Do you think there might be any differences for other sources of maps in background location? Or testing OSM is enough?

testing one provider will be enough.

@dbemke
Copy link

dbemke commented Oct 4, 2023

When "don't keep activities" setting is enabled after following the steps from the issue tracking location event appears after the first opened question after form resume.
If the user fills the form and at some point goes to the hierarchy view, taps "Go to end" tracking location events appears after end screen event - it occurs also on the master version but I'm not sure if it's ok for tracking location events to appear in that situation?
dontKeepActivities

@grzesiek2010
Copy link
Member Author

grzesiek2010 commented Oct 4, 2023

This should be fixed now.

@dbemke
Copy link

dbemke commented Oct 5, 2023

With don't keep activities on and the steps from the issue it works as expected.
Going to the hierarchy view (+don't keep activities) still adds location events - should I file this one separately?

dontkeepact

@grzesiek2010
Copy link
Member Author

should I file this one separately?

yes.

@srujner
Copy link

srujner commented Oct 5, 2023

Tested with Success!

Verified on device with Android 12,13

Verified cases:

@dbemke
Copy link

dbemke commented Oct 5, 2023

Tested with Success!

Verified on device with Android 10

@grzesiek2010 grzesiek2010 merged commit 8ebe286 into getodk:master Oct 5, 2023
6 checks passed
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.

The order of question and location tracking enabled events in audit log after form resume
4 participants