Open SarahRemus opened 1 year ago
:exclamation: No coverage uploaded for pull request base (
sync@f47ad2f
). Click here to learn what that means. The diff coverage isn/a
.
@@ Coverage Diff @@
## sync #930 +/- ##
=======================================
Coverage ? 57.18%
=======================================
Files ? 28
Lines ? 2254
Branches ? 331
=======================================
Hits ? 1289
Misses ? 965
Partials ? 0
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
Thanks for this! I'm trying to find some time soon to review this.
General tip to rebase to main, as a lot of fixes were pushed in december, and to use git rebase -i to merge a few of the commits so the commit structure is more of a timeline of changes.
I'm also seeing a importDatabaseFromGoogleDrive
text in my terminal from which I start time to leave.
So after working on the small change requests, it's time to tackle the bigger problems. Were you at all able to import/ export anything to/from Google Drive?
Your problem looks like the credential file can't be found? Where did you place that file?
I tried to test the PR but got this error when trying to import without having exported:
Maybe something is missing from package.json?
So after working on the small change requests, it's time to tackle the bigger problems. Were you at all able to import/ export anything to/from Google Drive?
Your problem looks like the credential file can't be found? Where did you place that file?
I tried to test the PR but got this error when trying to import without having exported: Maybe something is missing from package.json?
Oh, I didn't create the credential to test it, totally misread the description. Let me discuss with the team how we'll create a Google account to generate the credentials.
@tupaschoal and @thamara FYI. Do we have a TTL email or have we been using thamara's?
Hi @SarahRemus, first of all, thanks a lot for this great feature, and sorry it took me so long to get to it. I have followed the steps described and it works as expected! But I'm a bit concerned with a few things:
Adding the credentials.json on the repo root file: It seems like something that should not be made public just like some APIs keys. Adding it as a secret in the repo doesn't seem like a possibility as we need the information from the credentials.json at runtime, so it would need to be hardcoded/shipped anyway. There should be a better way of doing this, otherwise other apps would have the same problem. We need to investigate this a bit more. This SO question mighe help.
Limitation of filename and location: As my tests showed, whenever we save the database to the drive, it creates a new file with the same name. This can get complicated to maintain as well as making the user's drive a bit dirty. When loading the database, it seems to choose the newer one, but I'm not sure if that's by lucky (we are relying on the position returned by service.files.list). Although we can resolve this one later, I would prefer to get it close to its final form before including this change on the release. Releasing as is, can cause problems in the future such as having to maintain two forms of exporting/importing. It looks like a good way is to use the concept of app-specific data. We need to better understand this.
Related issue
Towards #913
Context / Background
Sync the database from more than one device with TTL installed. For example, travel with a notebook and load up the data from your home desktop, punch your time, then arrive back home and have the new data synced back to your desktop.
This PR provides the functionality to sync data through Google Drive. It provides only the basic functionalities of importing and exporting to and from Google Drive.
The feature can only be used if authentication with Google is set up To use the functionality of syncing to Google Drive, we need to create a Google Cloud project for TTL and turn on the Google Drive API. After that, the authentication needs to be set up. The credential.json file should be placed in the highest level of the TTL folder. For development purposes I did this with my own Google Account, but for production we should use a TTL Google Account.
What change is being introduced by this PR?
This PR enables the user to upload his TTL database to his Google Drive and also import his database from Google Drive into TTL.
When the user first uses this functionality, he will be asked to authenticate himself. Once the app is registered with Google, the user will not see the middle screen. After signing in once, the token file of the user is saved, and he does not have to sign in again.
After the authentication is done, the user will see this window in his browser. This is only a temporary solution and should be changed in a future PR.
If the user is exporting data, he can now see the TTL JSON file in his Drive, named _time_to_leaveexport .
If the user is importing data, he can now see the imported data in TTL. The calendar is automatically reloaded. At the moment, the name of the file that is imported is hard-coded as _time_to_leaveexport . Only a file with this name can be imported. This should also be changed in a future PR.
Explaining implementation details In the already existing
import-export.js
file, I added the two functionsimportDatabaseFromBuffer(..)
andgetDatabaseAsJSON()
. Then I created two new files,import-export-online.js
andgoogle-drive.js
. The idea behind that is, that inimport-export-online.js
all functions to import/ export to/from different web services can be defined. I think this will be beneficial once we introduce more web services.google-drive.js
stores all Google Drive specific functions. This separation also makes it easier to mock the functions using API calls in testing.How will this be tested?
For the two new functions in import-export.js,
importDatabaseFromBuffer(..)
andgetDatabaseAsJSON()
, I added unit tests in the import-export.js test file. To testimportDatabaseFromGoogleDrive()
I mocked the API calls to Google Drive to not slow down the test process too much. And this way I could test my implementation independently of the API. I'm not sure how to testexportDatabaseToGoogleDrive(..)
though.Manually, the feature can be tested using the workflow described above (after setting up authentication).
Future PRs
As this PR only covers the very basic functionality of syncing data to Google Drive, these are functionalities that should/ could be implemented in the future:
I'm looking forward to your feedback @araujoarthur0 @tupaschoal @thamara!