Replace repetitive code in SettingsObjectWrapper.cpp by functions?

[Dirk Hohndel]

> On Nov 19, 2017, at 4:07 AM, Berthold Stoeger <bstoeger at mail.tuwien.ac.at> wrote:
>>> 1) In getCloudURL() in core/qthelper.cpp, the cloud_storate_email_encoded
>>> member is set, but it seems not to be written to disk.
>> 
>> I looked at that code recently and am struggling with the overall logic of
>> it. Right now I feel like I need to take a step back and really look at the
>> overall logic of how we manage user name and password - because as I read
>> the code as it is it confuses me (and I think I wrote a good part of this).
> 
> This one is not a bug, just a duplication of code issue. The reason is that 
> cloud_storage_email_encoded is one of those fields that are not written to disk 
> in SettingsObjectWrapper.
> 
> I guess the more consistent variant would be calling the method in 
> SettingsObjectWrapper and not manipulating the pref struct directly. But I'm 
> not going to meddle with this. 

I'm perfectly happy to have you work on this code and remove the code
duplications. My request would be that you try to devise tests for this so we
can make sure we understand what the functions are supposed to do and
can verify that they actually do what they are supposed to do.

Writing tests isn't a requirement for you to continue down this path - as I said,
it's a request as I think it would be helpful.

>>> 2) In uploadFinished() in core/cloudstorage.cpp, the
>>> cloud_storage_password
>>> member is set, but it likewise seems not to be written to disk.
>> 
>> The password we should only store if that checkbox is ticked.
> 
> This one I have no idea - I would have to test it. And I'm not going to touch 
> it unless I have a reliable method of testing it.

So today we don't store the password unless you tick that box - that was a
request from some of the testers back when we implemented cloud storage.
Funnily enough, every time I install Subsurface on a new test machine or
test VM I stumble across this. I enter credentials, forget to tick the box, and
then get annoyed the next time I start and the password is missing...

So the only question here is "is there a situation where we SHOULD store
the password (assuming the box is ticked) and today we don't?"

> What I got so far:
> - If the password changed, uploadFinished() sets the new password in prefs and 
> emits the passordChangeSuccesful signal.
> - This signal is connected to the passwordUpdateSuccesfull(sic) slot of 
> PreferencesNetwork.

Feel free to rename. Tomaz has done a lot of this code and as a Brazilian
his English grammar is sometimes entertaining... (I love you, Tomaz).

> - This slot sets the cloud_storage_password widget according to the prefs 
> field.
> - Back in uploadFinished context(!), the finishedAuthenticate signal is raised.
> - This signal is connected to the updateCloudAuthenticationState slot of 
> PreferencesNetwork.
> - This slot emits the settingsChanged() signal.

Yikes.

> - Maybe(!) this signal then actually writes the password out to disk, but I 
> don't see how.

Hmm. So we clearly successfully write the password. What I usually do with
code like this is that I run it under the debugger, put a break point where the
actual thing I'm tracking down (here, writing the password to the settings)
happens, trigger the event and then look through the backtrace to figure out
how we got there. Usually followed by an "OH!!! this is how that was supposed
to work" - often combined with quietly cursing Qt...

> Trying to understand this code I had a distinct association:
> https://en.wikipedia.org/wiki/COMEFROM

Yes. See above.

/D