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

Change Import from webview to file picker #282

Closed
wants to merge 10 commits into from
Closed

Conversation

rparet
Copy link
Contributor

@rparet rparet commented Mar 29, 2020

Changes the Import screen and instructions from loading a web view to instructing the customer to get their takeout data from Google and then load it into PrivateKit using a file picker.

Tested on iOS and Android via Google Drive / iCloud Drive.

@popofibo
Copy link
Contributor

This effectively Resolves #77 & #114

@kenpugsley kenpugsley added the High Priority High importance / High value label Apr 4, 2020
@kenpugsley
Copy link
Collaborator

Once we have the dust fully settled from the rename to covid safe paths, we need to consider pulling this in, possibly for v1.0

@sergesemashko
Copy link
Contributor

sergesemashko commented Apr 7, 2020

@rparet, I started digging into issues #73 and #77 and I figured that the code from this PR would need to be merged first before addressing them.

FYI, Import feature is currently not really working properly in develop branch (need to parse takeout data for last 2 months, exceptions are thrown if files do not exist, racing conditions as async code is written in a sync way).

Also, I'd like to offer some help to make this PR merged faster as I sort of got your idea and have an understanding how to address mentioned import issues.

@tremblerz
Copy link
Contributor

@sergesemashko Can you branch off from the following one -
https://github.com/PrivateKit/private-kit/tree/import_picker
Once the issues are resolved we can test it and bring it inside the Safepaths

@sergesemashko
Copy link
Contributor

sergesemashko commented Apr 7, 2020

@tremblerz , I've checked import_picker and I can see an exception on an attempt to open file for month that does not exist in takeout report.

I can take a look into it and resolve conflicts from develop branch

ezgif com-video-to-gif (3)

@rparet
Copy link
Contributor Author

rparet commented Apr 7, 2020

@sergesemashko that condition also exists on the current importer. It only looks for this month's date file, which is problematic in 2 ways: 1. at the start of the month (like now), a customer will only be able to import a short amount of date. 2. the import fails silently for people who don't have this month's data, which can be a lot of people if they turned location history off at any time in the last 12-24 months.
I put this up as a PR because using the picker is a better solution for most customers, but if you want to make those enhancements I'd recommend surfacing to the customer when ENOENT is raised, as well as naively importing this month and last month's files.

@sergesemashko
Copy link
Contributor

@rparet, yeah, picker solution seems to be friendlier. I can take a look at ENOENT and parsing latest 2 months of data and will open a PR soon.

@tstirrat
Copy link
Contributor

tstirrat commented Apr 9, 2020

closing in favor of rebased #410

@tstirrat tstirrat closed this Apr 9, 2020
@tstirrat tstirrat deleted the import_picker branch April 9, 2020 02:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
High Priority High importance / High value
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants