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

Expand the config to support "manual only" sync #39

Open
shankari opened this issue Mar 26, 2020 · 7 comments
Open

Expand the config to support "manual only" sync #39

shankari opened this issue Mar 26, 2020 · 7 comments

Comments

@shankari
Copy link
Contributor

If that is turned on, then the sync can only be launched manually

@shankari
Copy link
Contributor Author

@stephhuerre this is the issue for the auto-sync switch. Please LMK if you have any questions.

@shankari
Copy link
Contributor Author

shankari commented Apr 2, 2020

the related PR #41
is right for windows, but not for iOS

Let us review the differences between android and iOS.
For android, the sync is triggered by the periodic sync thread.
For iOS, the sync is triggered at the end of a trip.

So for android, we can simply choose not to trigger the periodic sync thread.
But for iOS, we need to handle the case in which we get a callback from the trip generation module as well.

So I'm going to make the following changes.

  • handle crash here. If the user is not logged in, and userToken is null, we crash. Instead, we should generate a warning that we are not logged in.
  • handle any other crashes related to the simple case in which the user is not logged in and the server is not configured

Then, take a break and work on other fixes before merging the config flag.
If the manual sync is set to true:

  • on android: don't schedule the periodic sync (similar to current PR)
  • on iOS, add a check to backgroundSync to bail out in the case of manual sync.

@shankari
Copy link
Contributor Author

shankari commented Apr 2, 2020

ok so one challenge with this approach is this prompt. We generate it if we don't have a stored token for the user. I haven't seen this for a really long time, but at one time it was a pretty big issue, and I don't want to remove it. After all, once we do start logging in, we may need an API key or sth to be passed in to the backend.

And you should ideally have to login even for manual sync, although in that case, the operation can happen in the foreground, and maybe it is OK for the auth to fail when the plugin is initialized.

We really need to figure out whether there are two separate configs for foreground login and manual sync or a single option that controls both but adds another dependency between modules.
@stephhuerre @PatGendre @jf87, any thoughts on what the long-term design should be?

In the meanwhile, while thinking through that decision, I realized a workaround. We can just set the token to something random (e.g. Not logged in) in the javascript. The default auth is "skip", so the auth module will accept it. It will create the sync message with it, but it will fail because there is no server.

Voila! No prompts, no crash, no server.

@shankari
Copy link
Contributor Author

shankari commented Apr 2, 2020

ok so one challenge with this approach is this prompt. We generate it if we don't have a stored token for the user. I haven't seen this for a really long time, but at one time it was a pretty big issue, and I don't want to remove it. After all, once we do start logging in, we may need an API key or sth to be passed in to the backend.

And you should ideally have to login even for manual sync, although in that case, the operation can happen in the foreground, and maybe it is OK for the auth to fail when the plugin is initialized.

We really need to figure out whether there are two separate configs for foreground login and manual sync or a single option that controls both but adds another dependency between modules.
@stephhuerre @PatGendre @jf87, any thoughts on what the long-term design should be?

In the meanwhile, while thinking through that decision, I realized a workaround. We can just set the token to something random (e.g. Not logged in) in the javascript. The default auth is "skip", so the auth module will accept it. It will create the sync message with it, but it will fail because there is no server.

Voila! No prompts, no crash, no sync. Implemented much more quickly than any alternative. And guaranteed to work.

@PatGendre
Copy link

Hi @shankari
so you solved the issue, at least for the short term:-)
Maybe if the app never connects to a server the sqlite db will grow forever and you will have to modify a little the app for that? (e.g. clear the logs periodically, not log the sync message errors and so on)

On the long the term I guess the user will continue to need an account anyway as the other features will require it.

@shankari
Copy link
Contributor Author

shankari commented Apr 7, 2020

We really need to figure out whether there are two separate configs for foreground login and
manual sync or a single option that controls both but adds another dependency between
modules.

I will be re-visiting this soon. I think that the current use case is that the users will authenticate, but the sync will not happen automatically. Instead, it will happen manually when the user presses a button. (the call may not actually be the e-mission sync, but that's a different story).

So it looks like we should have two different settings:

  • manual/auto sync
  • auth/no auth

The four options are then:

  • auth/manual: COVID-19 use case
  • no auth/manual: data logger with export
  • no auth/auto sync: ??? anonymized upload? But then how to avoid server farms?
  • auth/auto sync: e-mission use case

Thoughts? @jf87 @PatGendre @stephaniehuerre

@PatGendre
Copy link

@shankari yes it is a good idea to present the 4 options, it is much clearer.
In the no auth/auto sync, the user should have very clear consent information, because he/she has no control of the tracking. It seems similar to me as the Floating Car Data systems, for which I don't know how the user consents, nor exactly where the data are going and how they are processed. If we need to look deeper at this option, I would investigate more on FCD as this seems similar (replacing the car by the phone) and as FCD is widely deployed and seems to work already.

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

No branches or pull requests

2 participants