Issue 812 GUI fails to load after CLI used

ggalmazor commented 5 years ago

Closes #812

In this PR

[ ] Pending to reproduce and fix the original issue reported in #812
[x] Avoid losing prefs by runnnig a CLI op

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

Testing of this PR involves this steps:

Run the -c CLI op to clear all preferences
Ensure the GUI environment is set and configured by running the GUI once and setting an sd
Run a pull from Central CLI op
Run the GUI again

In master, you are asked to set an sd for a second time, which shouldn't happen.

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

This is a super narrow fix. No other approaches were considered.

Somehow, setting the sd in the pull CLI ops was losing prefs for the GUI, and users were asked to set it again in the next GUI launch.

Also, setting the sd in CLI ops is not required (downstream code gets the briefcaseDir directly), so it's a safe change to do.

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?

I tried to track uses of BriefcasePreferences.getStorageDir() in downstream code that would require the call to BriefcasePreferences.setStorageDir() I've removed and I found none. I could have missed something, though.

Does this change require updates to documentation? If so, please file an issue at https://github.com/opendatakit/docs/issues/new and include the link below.

No.

ggalmazor commented 4 years ago

I think it doesn't make sense to block the release for more time. If the feedback comes and we can repro the original issue, we will release v1.17.1 with the fix.

chrissyhroberts commented 4 years ago

Hi @ggalmazor Sorry have been super busy and not managed to keep up with you on this. I guess as this flagged up an issue with preferences, it is possible that the fix you've made will change the behaviour I was seeing, so suggest we wait until next release and check to see if this fixes it. Thanks so much.

ggalmazor commented 4 years ago

No problem @chrissyhroberts! Thank you for understanding :)

getodk / briefcase