e-mission / e-mission-docs

Repository for docs and issues. If you need help, please file an issue here. Public conversations are better for open source projects than private email.
https://e-mission.readthedocs.io/en/latest
BSD 3-Clause "New" or "Revised" License
15 stars 34 forks source link

Setting the config on start can lead to prompting for tracking permission before consent #267

Open shankari opened 7 years ago

shankari commented 7 years ago

When we switch to a different channel using the custom URL (e.g. https://e-mission.eecs.berkeley.edu/#/client_setup?new_client=cci&clear_usercache=true&clear_local_storage=true), we clear the usercache, which clears to configs.

In order to work around this, on several branches (e.g. cci-berkeley) we set the config on startup (see https://github.com/e-mission/e-mission-phone/pull/277). But that breaks startup directly from that branch - setting the config resets the finite state machine, which currently does not check consent. So we prompt for permission to turn on tracking in the background, although the user has not yet consented.

See screenshot and logs. This didn't happen when I tested switching the UI from the URL, so may be OK to just let it die as the need for it dies. Or we could fix it. Waiting to hear back from testers on reproducibility with switching UI from the URL.

Screenshot logs
simulator screen shot sep 16 2017 10 18 41 pm prompt_before_consent_logs.gz
shankari commented 5 years ago

Ran into this again while testing https://github.com/e-mission/e-mission-docs/issues/325 Essentially, the flow was:

One option to deal with this is to check the consent from the start tracking transition as well The other is to move to the start state, and then re-generate an initialize, which will check for consent This was the original implementation and we switched away from it in https://github.com/e-mission/e-mission-data-collection/commit/e2c9e20ea8a6e3212e262d88f621bac134a4ee8a#diff-cc871f92104d8b532fce9fd6471bf70bL500 because we want to avoid generating broadcasts from the service and dealing with re-entrant code.

But now with the tracking error, the code is re-entrant anyway. So we may as well re-introduce this for now. Note that as there are more options for geofence creation to fail, this makes more sense, because then we will go to the start state and not stay in the tracking_stopped state, which will allow us to resume tracking when the geofence creation blocker (e.g. location services) is turned on.