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

Allow choosing a custom directory for local backups #302

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

victor-marino
Copy link

Pull request (PR) checklist

Please check if your pull request fulfills the following requirements:

  • I've read and understand the Code Contributions Guide.
  • I confirm that I've run the code locally and everything works as expected.

What's changed?

As discussed in #268, we'd like to make incremental improvements to the backup & export functionality, ultimately leading to the implementation of automated, scheduled backups (hopefully in the cloud as well as locally).

The first step towards this goal should be to offer the user the option to choose a custom directory when making a manual backup.

This doesn't close the above issue, it just contributes to it.

Couple of things to note:

Removal of getDownloadPath()

Since we're now allowing the user to choose where to save the exports, both .exportDatabaseFile() and .exportSpreadsheet() methods take the path as an argument and no longer need to figure it out by themselves. Therefore, I think we will no longer need the getDownloadPath() function and I've deleted it.

This has the added benefit of removing the need to find a valid cross-platform "default" path, which is not trivial and introduces a few problems (e.g.: only the /Downloads directory truly works across platforms, but for some reason Android's FilePicker() dialogue does NOT allow the user to select it... which can be confusing).

Of course let me know if you prefer to keep it.

New names

I have taken the liberty of renaming a couple of symbols in a way that I think will make more sense once we start implementing the rest of features:

  • downloadDatabaseFile() -> exportDatabaseFile()
  • downloadPath -> exportPath

Of course feel free to revert that if you disagree.

Does this PR close any GitHub issues? (do not delete)


@enrique-lozano
Copy link
Owner

Thanks, I will test it once the conflicts are resolved :)

@victor-marino
Copy link
Author

Thanks, I will test it once the conflicts are resolved :)

Ah sorry, had missed that!

Conflict solved ;-)

@victor-marino
Copy link
Author

victor-marino commented Jan 21, 2025

My apologies. Just realized I wasn't correctly handling the case where the user cancels the file picker.

Fixed in the last commit, and simplified the code a bit as a result.

@enrique-lozano
Copy link
Owner

Hi @victor-marino! I've taken the liberty of making some quick changes to your PR. Nothing too relevant to your code, just more things that needed to be improved that were already there.

I've tested your code and it works fine on Windows and Android, if you give me your final confirmation when you see these changes, I'll merge the PR.

@enrique-lozano enrique-lozano added type: enhancement New feature or request status: READY The task is done and will be released to the app soon labels Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: READY The task is done and will be released to the app soon type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants