Skip to content
This repository has been archived by the owner on Jun 16, 2022. It is now read-only.

Ensure sqlite folder is removed after clear cache / hard reset #1585

Merged
merged 3 commits into from
Oct 17, 2018

Conversation

meriadec
Copy link
Member

@meriadec meriadec commented Oct 16, 2018

closes #1554

Clean cache / hard reset has been reported (& reproduced) failing to remove the <user-data>/sqlite folder on Windows (high chance of folder being "hold" by some process, even if we kill libcore before..) and we get some EPERM and ENOTEMPTY (relatable: isaacs/rimraf#72).

Fallback here is to detect this behaviour, and provide a workaround: open the user data folder then quit the app, explaining the user he has to delete manually.

Additionally for support guys, here is a video of the process (@dasilvarosa feel free to discuss the wording):

DEMO:

kbwuldq

@meriadec meriadec added HODL for PRs: this is blocked, we can't merge yet and removed HODL for PRs: this is blocked, we can't merge yet labels Oct 16, 2018
@gre gre requested a review from Arnaud97234 October 16, 2018 14:17
src/components/SettingsPage/CleanButton.js Show resolved Hide resolved
src/components/SettingsPage/ResetButton.js Show resolved Hide resolved
title="Couldnt remove app files"
desc={
<div>
<p>{'Cache folder couldnt be deleted. You will have to delete it manually.'}</p>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to have all these texts put in wording.

src/helpers/reset.js Show resolved Hide resolved
@gre gre requested a review from dasilvarosa October 16, 2018 14:21
@gre
Copy link
Contributor

gre commented Oct 16, 2018

@dasilvarosa could you check wording? thx

gre
gre previously approved these changes Oct 16, 2018
@Arnaud97234
Copy link
Contributor

No regression on macos and Ubuntu, sqlite folder is removed without need for user to delete the folder manually (even when application is syncing).

On Windows if "Clear cache" or "Reset Ledger Live" action is made just after sync ends, sqlite folder is removed without need for user to delete it manually.
If sqlite folder can't be removed automatically, the user will have to remove it manually.
A popup is displayed, on click on "Open folder" button app folder opens and Ledger Live process is killed. After removing sqlite folder, it's necessary to manually restart Ledger Live application.

Arnaud97234
Arnaud97234 previously approved these changes Oct 16, 2018
@@ -406,6 +406,13 @@
"title": "Clear cache",
"desc": "Clearing the Ledger Live cache forces network resynchronization. Your settings and accounts are not affected. The private keys to access your crypto assets in the blockchain remain secure on your Ledger device and on your Recovery sheet."
},
"resetFallbackModal": {
"part1": "Cache folder couldnt be deleted. You will have to delete it manually.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"part1": "Could not delete cache folder. Please delete the folder manually:",

@@ -406,6 +406,13 @@
"title": "Clear cache",
"desc": "Clearing the Ledger Live cache forces network resynchronization. Your settings and accounts are not affected. The private keys to access your crypto assets in the blockchain remain secure on your Ledger device and on your Recovery sheet."
},
"resetFallbackModal": {
"part1": "Cache folder couldnt be deleted. You will have to delete it manually.",
"part2": "Click on \"Open folder\", then the ",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"part2": "Click the Open folder button, the ",

"part1": "Cache folder couldnt be deleted. You will have to delete it manually.",
"part2": "Click on \"Open folder\", then the ",
"part3": "app will close",
"part4": ", and you will have to delete the \"sqlite\" folder.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"part4": ", make sure to delete the "sqlite" folder.",

"part2": "Click on \"Open folder\", then the ",
"part3": "app will close",
"part4": ", and you will have to delete the \"sqlite\" folder.",
"part5": "After that, you can restart the app."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"part5": "Then you can restart the app normally."

@@ -24,18 +26,20 @@ class ResetFallbackModal extends PureComponent<Props> {
title="Couldnt remove app files"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

title="User action required"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@gre
Copy link
Contributor

gre commented Oct 16, 2018

Accidentally resolved one conversation but I think you can find it in history. Dumb GitHub big buttons on mobile.
Just wanted to say there are some remaining words not in translation files but only few 😀

@meriadec meriadec dismissed stale reviews from Arnaud97234 and gre via f97f592 October 17, 2018 08:09
@meriadec
Copy link
Member Author

meriadec commented Oct 17, 2018

Updated !

2018-10-17_517x305

Copy link
Contributor

@dasilvarosa dasilvarosa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final wording polish ;). I recommend taking away the underline on the app will close, because it distracts from the more important info of deleting the sqlite folder.

"part3": "app will close",
"part4": ", and you will have to delete the \"sqlite\" folder.",
"part5": "After that, you can restart the app."
"part4": ", make sure to delete the \"sqlite\" folder.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"part4": ", and manually delete the "sqlite" folder.",

@meriadec
Copy link
Member Author

Updated !

2018-10-17_515x300

@gre gre requested a review from dasilvarosa October 17, 2018 10:21
@gre gre merged commit 8926b50 into LedgerHQ:develop Oct 17, 2018
@meriadec meriadec deleted the fix-1554-reset branch October 29, 2018 21:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rimraf the sqlite folder doesn't work... on Windows
4 participants