Cgeo: New backup handling - todo list

[fm-sys]

Since #9096 is merged, there will probably be some discussion, UI feedback, bugs, etc. about the new backup/restore implementation (#8959). Let's collect them in this issue.

Important to-dos

• [x] always backup database and settings (to keep the UI clear and simple)

Next Steps for multiple backup support

• [x] don't move the entire folder but just the backup files
• [x] always store the backup in backup/yyyy-mm-dd hh-mm (independent of advanced settings)
• [x] multiple backups at one day
• [x] move existing backup into new folder structure

• [x] Advanced settings: I would make the explanation text always visible & integrate it into the "create backup" section

• [x] message, that the old backup has been moved into the new folder structure

And later on...

• [x] choose dialog for restore backup from backup history (use c:geo's SimpleDirChooser component)

• [x] Add warning to description that backup is only offline (see #9175)

• [x] Simplify the "backup successful" message to something like backup was created at <dir path>

• [x] add the backup state to system information (see #9049)
• [x] Change overwrite notice to something like This will purge your oldest existing *backup dir* as of dd MMM, hh:mm - continue?
• [x] "share option" after completing the backup (maybe as a button below the "backup successful" message) #4705

Discussed, but outdated or opposed...

• [outdated] we could add meaningful icons on the left side in "create backup" section
• [outdated] Better wording for "Start backup"
• [outdated] Restore settings do not remember currently if I want to restore my settings as well (is it needed?)
• [outdated] After a fresh install, if you try to create a backup, c:geo shows a dialog "database is empty, no backup needed" _(This message was introduced for secure reason, so that users do not overwrite their backup when actually trying to restore it...)_

---

Feel free to complement the list...

(checked points are done already, pull request will follow if everything works so far)

[geocachermgo]

See my comment at #9096

[geocachermgo]

I just had the names of the backup files extended like cgeo_2020-10-01.sqlite in the backup dir. Do we really need a separate directory old_backup with subdirectories for each date of the backup?
We could also put the date subdirs under the backup dir if we do not want to touch the filenames.

[MagpieFourtyTwo]

I would add the time in any case, because of three reasons:

1. Consistency
2. Information, as it may be good to know if the backup has been created in the morning or in the evening, e. g. before or after the "Grand Tour" ;)
3. When you just add it on consecutive backups only, you never really know without doubt which one is the newest.

[MagpieFourtyTwo]

Do we really need a separate directory old_backup with subdirectories for each date of the backup?

IMO, the additional subdir old_backup is not needed, as for me it would be sufficient to just rename backup to yymmdd hhmm backup - which BTW would even make more visible, how many old backups are stored besides the last one.

I just had the names of the backup files extended like cgeo_2020-10-01.sqlite in the backup dir.

I agree with @fm-sys to not rename the files but the complete directory, cause in order to restore, manually, you just have to rename one element and not two. Moreover, in case of GUI supported selection of an older backup, you just have to ask for one dir instead of having to ask for two files and can continue to work with the "old" file names. And apart from this, an own directory will additionally give the opportunity to store the matching apk in it (which in some cases may be essential, too).

[MagpieFourtyTwo]

When performing a consecutive backup, I missed a message, that the old backup has been renamed to "..."

[MagpieFourtyTwo]

BTW: Would it be possible to reduce the size of screenshots to make the thread more readable ...?
With images in FullHD you have to scroll a lot ... and s/th smaller like this

can even better be seen in it's context ...

[geocachermgo]

This is my suggestion for the UI, the buttons can be pressed:

Maybe we could include also the 5th option: include pictures (could lead to even higher disk usage!)

[fm-sys]

from https://github.com/cgeo/cgeo/pull/9096#issuecomment-701935830:

In the acknowledgement window the creation and copying of the old_backup is missing

Yes, you are right! We should add an information about this...

Thanks for the improvment, I like it. 3 remarks
I find the "Start backup" text a little unobtrusive and the heading would be more correctly "backup settings"

I totally agree that it's probably not the best wording. But why "backup settings"? This feature will not only backup the settings...

I propose action buttons as in "add waypoint" to start the backup.

This is AFAIK not possible or would require a complete style change in our settings...

Maybe on top "Create backup" "Restore from " and "Restore from old backup" followed by a subheading "settings"? The setting for the restore (checkboxes for db, setting) that are now in a pop up window could also be added in the list.

This will probably confuse our users. I would leave it extracted to the dialog cause you normally don't use the restore function as often as the backup. And you will have the additional overwrite warning in the restore dialog.

If we create old backups there should be a possibility to restore from old backups cf. suggestion of a third button that should result in a selection menu.

Would be a nice improvement but is not really needed IMHO. The targeted user group of old backups are primary nightly testers, who should know how to move the old backup to the backup folder.

I would omit the separate section "advance settings" and just add it as a 4th option.

Yes, would also be OK for me, even if it's not really related to the content of the backup

[MagpieFourtyTwo]

The targeted user group of old backups are primary nightly testers, who should know how to move the old backup to the backup folder.

Totally agreed. And this is in fact the reason, why I would not put too much effort into the handling and esp. restore of old backups.

Had this topic with @moving-bits a while ago, while he wanted to ease my day ;) but we decided to just leave it as it is, as my daily manual action (in fact just renaming the backup folder) is not that effortful, and esp. even way less effort (for the tester) than (for the dev) to implement an automatism, which additionally has to be - without wanting to appear disrespectful - fool proof. ;)

[MagpieFourtyTwo]

So, renaming the backup folder, thus implementing an automatism to backup history, would of course be nice to have, but having that, I would end the story.

[fm-sys]

Do we really need a separate directory old_backup with subdirectories for each date of the backup?

Let me explain, why I did this: In the current implementation the complete backup folder just gets moved to the /old_backups/<timestamp> path. But we can change this logic to just move the backup files inside we can also use another path. Would cgeo/backups/<timestamp> be ok? Because putting them directly into cgeo/ would probably ruin the complete clean and logical directory structure

---

BTW thanks at @MagpieFourtyTwo, I changed the images in the initial post as suggested

[geocachermgo]

..Would cgeo/backups/<timestamp> be ok?

For me, yes

[fm-sys]

I thought about it again and came to the conclusion that it would probably be best to hide the function totally by default. It is only a "quick and dirty" implementation and I don't want to put too much effort into it. Currently we don't want that users are using this function (am I right?) and testers like @MagpieFourtyTwo could still use it by changing the preferences key manually. So this function will be more like a backdoor than a regular settings option. @MagpieFourtyTwo OK for you?

[MagpieFourtyTwo]

hide the function totally by default.

Yep, would of course be ok for me,
Although I have to admit, that I would probably just continue to do it my manual way, as I made the experience that just renaming the folder from backup to yymmdd hhmm backup resp. vice versa is the easiest way - at least for me.

Would cgeo/backups/<timestamp> be ok?

Was this a typo? Cause this would end up in a second folder, as the current name is cgeo/backup ... without "s".

[fm-sys]

Was this a typo? ... without "s".

Yes of course ;-)

Although I have to admit, that I would probably just continue to do it my manual way, as I made the experience that just renaming the folder from backup to yymmdd hhmm backup resp. vice versa is the easiest way - at least for me.

@MagpieFourtyTwo As you will be the only person using this function, just tell me where you want to store your old backups. ;-)

Moving all old backups directly into /cgeo/<timestamp> instead would be work of maybe 5 minutes for me. Could you show me one of your real folder names as example?

I really want to help you with simplifying your daily tester life. I'm just trying to understand how your manual work flow currently is. Should the folder be renamed directly after creating the backup (no direct restore without renaming again possible) or only while creating a new one (two folder renames for old backup restore)?

Would this changes make the function helpful for you?

[MagpieFourtyTwo]

As you will be the only person using this function

Will I? In fact I guess (hope) there are more people intensively testing than just me, ;)
Moreover I know at least one, who is doing it the same way (cause it's proven to be bullet proof), so we're not alone. ;)

Ok, so my workflow is, as soon the New nightly message pops up:

1. check commits vs. deployment state for the changes in the new version
2. perform backup of DB and settings
3. moving the matching (i. e. last) downloaded cgeo.apk from \Download to \cgeo\backup
4. rename \cgeo\backup to \cgeo\yymmdd hhmm backup, where yymmdd hhmm is the exact time of the backup itself
5. delete some old backup folders (keep at least 4 to 5, resp. up to one or two which were working good for sure)
6. go back to c:geo, tap New nightly, download, install ... and test/work with it.

This I do nearly every day (depends on commits, too - if there were just translations or minor changes, or we're on tour, I may skip the one or the other). Main advantage of this procedure is, that I always have several most definitely consistent sets of files (apk, DB, settings) to always be able to go back to a definitely working state. So, if worst case happens, I will just lose one or two days of data changes - and this, to be honest, saved my ass several times, already. ;)

One or the other might call this exaggerated, but c:geo has always been - and most probably will remain to be - my main data source. As it's really nice and really helpful (esp. for others ;) ) to have all the stuff of solved Mysteries and Multis at hand.

If an automatism would be helpful? Of course it will, as it will save a few minutes and a couple of clicks every day, but OTOH I am already used to do this every day and it happens nearly automatic, while nipping the first three sips of coffee every morning ... ;)

[moving-bits]

Thanks @fm-sys for restructuring the backup and restore process.
Having used the new backup function for the first time I'd like to share some observations and ideas:

1) Advanced settings: I would make the explanation text always visible, independent from the checkbox. Makes the option easier to understand.
2) Advanced settings: Maybe they could be integrated into the "create backup" section to make them more prominent/visible.
3) Doing a backup with "advanced settings" activated moved ALL the files in the backup dir to the new location. This is pretty confusing, as I had other files stored in the backup dir, not related to the last backup of settings there (which I had to search for now...). Those unrelated files should be untouched, IMHO.
4) Having done a backup today (database + settings) I wanted to do another backup, this time database only. Now it asks me whether I want to overwrite my existing backup.

• As I have "advanced settings" activated I would expext to simply create another backup set in a separate directory. - But that would probably mean to use date + time as dir name (which would be a useful thing anyway).
• Also critical in the current solution: The current backup database could not be moved to the backup directory, as that one already existed. Admittedly, there is a warning toast - but that could be overseen easily.
  5) BTW: Why is the current backup stored to backup main directory at all? Why not store all backups to a dir named backup/yyyy-mm-dd hh-mm-ss? This could also simplify the whole process and avoid some pitfalls. - Could help with 3) and 6) as well.
  6) It would be nice if I could select a backup to restore. This would be as easy as selecting a certain directory, if all backups would be stored in subdir. (We have a SimpleDirChooser component which could be used for that.)
  7) Restore settings do not remember currently if I want to restore my settings as well.
  8) After a fresh install, if you try to create a backup and only check "program settings", c:geo shows a dialog "database is empty, no backup needed" (which is correct, but not necessary in this case, as I did not request a backup of the database), and does NOT create a backup of the settings at all. - If the database is non-empty, a backup of the settings will be created in this use case.

[fm-sys]

@moving-bits one question about 5): Would you suggest to always store the backup in backup/yyyy-mm-dd hh-mm-ss irrespective of whether or not advanced setting in activated?

It would make things way easier, otherwise a new created backup will not be able to be opened easily on older c:geo versions...

Why is the current backup stored backup main directory at all?

Same reason as in the sentence above...

[fm-sys]

we could add meanfull icons on the left side of the backup checkboxes

What do you think about following icons?

...and a settings icon we already use in other places AFAIK

[moving-bits]

@moving-bits one question about 5): Would you suggest to always store the backup in backup/yyyy-mm-dd hh-mm-ss irrespective of whether or not advanced setting in activated?

Yes.

It would make things way easier, otherwise a new created backup will not be able to be opened easily on older c:geo versions...

IMHO that's ok. We can add a "change" notice in the release notes describing that you need to move the files manually, if you want to return to a pre-2020-10 release.

[MagpieFourtyTwo]

@moving-bits

1. Doing a backup with "advanced settings" activated moved ALL the files in the backup dir to the new location. This is pretty confusing, as I had other files stored in the backup dir, not related to the last backup of settings there (which I had to search for now...). Those unrelated files should be untouched, IMHO.

IMHO, "unrelated" files should just not be there. In a folder, created by and for a backup, there should reside only files, which are related to this backup. Otherwise a dedicated folder would make no sense. Esp. when having several backup folders. For files, which are kind of "backup version spanning", one should use an own folder. IMHO. ;)

5): Would you suggest to always store the backup in backup/yyyy-mm-dd hh-mm-ss

Yes.

I would suppress the -ss, for clarity, as it's easier to distinguish a 4-digit time from a date. Moreover seconds do not really provide additional information, as it's pretty unlikely that s/one will perform more than one backup per minute anyway. ;)

What just came to my mind: If one usually just backups the DB (as settings did not change for him, anyway), but every backup gets hos own folder, how can be ensured that the last settings backup does not get lost while purging old backups ...?

[fm-sys]

If one usually just backups the DB (as settings did not change for him, anyway), but every backup gets hos own folder, how can be ensured that the last settings backup does not get lost while purging old backups ...?

Someone non-technical will expect that the old settings backup will be "overwritten" by the newly created backup. So if someone wants to backup his settings, he should select the "backup settings" checkbox.

[MagpieFourtyTwo]

Interesting thought. And comprehensive.
But ... why should then still exist a checkbox for settings backup?
So, why not just drop the settings backup checkbox
and just leave the "account data" choice?

And on restore he can really choose to restore db only or settings, or both ...

[moving-bits]

1. Doing a backup with "advanced settings" activated moved ALL the files in the backup dir to the new location. This is pretty confusing, as I had other files stored in the backup dir, not related to the last backup of settings there (which I had to search for now...). Those unrelated files should be untouched, IMHO.

IMHO, "unrelated" files should just not be there. In a folder, created by and for a backup, there should reside only files, which are related to _this_ backup. Otherwise a dedicated folder would make no sense. Esp. when having several backup folders. For files, which are kind of "backup version spanning", one should use an own folder. IMHO.

Those extra files are related to backups, but not from the most recent backup. Files are something like 'cgeo-settings-user1.xml' or '2020-01-01 cgeo.sqlite' etc.

5): Would you suggest to always store the backup in backup/yyyy-mm-dd hh-mm-ss

Yes.

I would suppress the -ss, for clarity, as it's easier to distinguish a 4-digit time from a date. Moreover seconds do not really provide additional information, as it's pretty unlikely that s/one will perform more than one backup per minute anyway. ;)

agreed

[moving-bits]

But ... why should then still exist a checkbox for settings backup?
So, why not just drop the settings backup checkbox
and just leave the "account data" choice?

I had the same thought, would make backup a bit easier.
Only thing that might contradict this would if someone wants to create only a settings backup - but that's probably an rare edge case.

And on restore he can _really_ choose to restore db only or settings, or both ...

yes

[MagpieFourtyTwo]

Those extra files are related to backups, but not from the most recent backup. Files are something like 'cgeo-settings-user1.xml' or '2020-01-01 cgeo.sqlite' etc.

This is in fact just another "implementation" of a backup history.
These can (could) of course happily live in the "normal" backup folder - but just as long as this folder is the only one.
But as soon as each backup gets his own folder, they should go to an own one, to not get lost.
Over here I e. g. have one folder called190217 prior to first nightly installation.
Cause once was said that you can't easily go back from nightly to release - so I kept it over time. Just to be safe. ;)

[fm-sys]

But ... why should then still exist a checkbox for settings backup?
So, why not just drop the settings backup checkbox
and just leave the "account data" choice?

I had the same thought, would make backup a bit easier.
Only thing that might contradict this would if someone wants to create _only_ a settings backup - but that's probably an rare edge case.

So a backup should always be database & settings - and the only checkbox which will remain on the UI is the "include account data". Did I get that right?

[MagpieFourtyTwo]

Would say so ... cause processing time and storage space should not be an issue (anymore).
Moreover it would help to avoid a lot of problems in implementation and misunderstandings on user's side.
While on restore you should of course still be asked if DB, settings, or both should be restored.

[fm-sys]

PR #9179 will fix the urgent issues of this thread and will make the backup history more useful and user-friendly. Even an optional cleanup feature for old backups is integrated.

There are still some minor points left, which could also be done later on. The mentioned PR should be OK for testing in the nightlys...

[geocachermgo]

There is an old issue #4705 that requests for a direct backup to the cloud.
And one requesting for a date #2547 (can be closed)?

[fm-sys]

@geocachermgo AFAIK you wrote before that "Start backup" might not be the best wording. Is it fine for you as it is today or do you have any ideas for a better name?

@moving-bits You mentioned before, that the restore settings dialog do not remember what is selected for restoring. It is quite difficult to implement, as the settings file may gets overwritten while restoring afterwards. Do you think, it is really needed or would it be just a nice to have extra feature?

@MagpieFourtyTwo #9179 now is at master with a completely reworked multiple backup handling. So maybe another testing round? ;-)

[geocachermgo]

"Start backup" is ok. I proposed an action button but nobody else supported it.
I also proposed that the text should make clear that its a local backup protecting mainly against a cgeo malfunction corrupting the database but not against loss of the phone or hareware error. That was in it but is not there any more (see also #4705)

[MagpieFourtyTwo]

Just played a bit with the "reworked backup" - and I really like it.
Well designed and thought out. Starting with the slider, going over the overwrite notice, up to the last minute toast. ;)
Great stuff! Good to work with. *thumbs up*

Only thing I would slightly change is the message when backup count limit is reached, as ...
Do you want to *overwrite* your oldest existing backup from dd MMM, hh:mm?
... is not completely in accordance with reality, as the backup will not be overwritten, but in fact the dir gets purged completely.
May sound a bit petty, but to be honest I was a little bit confused ... and did in fact not know if the backup dir possibly just gets renamed and the backup files overwritten for real, in order to possibly keep stuff other than cgeo.sqlite and cgeo-settings.xml ... but test showed, the entire directory will be deleted - which is perfectly fine. Nevertheless I would prefer s/th like
This will purge your oldest existing backup dir as of dd MMM, hh:mm - continue?
or s/th alike ...

BTW: The icing on the cake would be (at least for me and my work flow ;) ), if additionally the most recent cgeo-nightly.apk (resp. cgeo-nightly(n).apk) (if exists) would be copied from default download destination to the new backup dir ... ok, admitted, this may be really individual, but, hey, this is a wishlist over here, isn't it? :blush:

[Lineflyer]

I also like the new backup function. Easy to handle and pretty failsafe. Thanks!

The icing on the cake would be (at least for me and my work flow ;) ), if additionally the most recent cgeo-nightly.apk (resp. cgeo-nightly(n).apk) (if exists)

I think we should not include any nightly specific feature here as these are pure alpha developer versions.

[moving-bits]

@moving-bits You mentioned before, that the restore settings dialog do not remember what is selected for restoring. It is quite difficult to implement, as the settings file may gets overwritten while restoring afterwards. Do you think, it is really needed or would it be just a nice to have extra feature?

Let's keep it as is for now. The new backup/restore function looks well-rounded to me, but I need to play around a bit more with it. I'll open a separate issue if needed.

[Lineflyer]

"share option" after completing the backup (maybe as a button below the "backup successful" message) #4705

+1 for this from support mail, where a user asked for exactly this add-on (share option to allow him to directly send the files to Dropbox (in his case))

[MagpieFourtyTwo]

While "Share" should not offer any email app ... and should be limited to or at least ask if to send via WLAN on next connect ...
Cause e. g. my backup has about 320 MB, growing ... would be great to send files like this via email ... or using Edge ... :p

[moving-bits]

While "Share" should _not_ offer any email app ... and should be limited to or at least ask if to send via WLAN on next connect ...
Cause e. g. my backup has about 320 MB, growing ... would be great to send files like this via email ... or using Edge ... :p

Probably nothing we could influence, at least not if we refrain from programming yet another home-brew share-menu with a lot of guessing. Users still need to do what they are doing...

[alan666notb]

Could the "Start backup" text be made to look a bit more like a 'button'?
The first time I used the new release, it took me a while to spot how to actually make the backup happen.
Same for "Start restore", I guess.

[MagpieFourtyTwo]

Users still need to do what they are doing...

Reading this another good old software development principle, phrased by a really knowledgeable person, came to my mind:
"It's impossible to make anything foolproof, because fools are so ingenious." :zany_face:

[MagpieFourtyTwo]

There still was the suggestion of @alan666notb to make the Start backup text more prominent ...

While I would suggest the same for Start restore and Restore a different backup, too,
cause for a not knowing user it's not really obvious that theses texts are in fact buttons ...
(to be honest, I stumbled upon this, too, when I used the new dialog for the first trime,
although I already was really experienced in performing backups ... ;) ).