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

Remove all user data on logging out. #545

Merged
merged 1 commit into from
Feb 19, 2021

Conversation

seanwu1105
Copy link
Contributor

@seanwu1105 seanwu1105 commented Feb 18, 2021

See #369. This PR has several changes regarding data persistence.

  • Add clear method on Database.
  • Add clear method on Table interface.
  • Add clear method on PreferenceManager.
  • Adjust clear method on Preferences interface.
    • Each preference will be undefined after being cleared.
    • Each preference Observable will NOT emit new value after being cleared (with isNonNullable operator).
    • Each preference Promise will emit the default value after being cleared.
  • Adjust function types on Preferences implementations.
  • Add clear method on ImageStore.
  • After DiaBackendAuthService.logout$()
    • Clear all user data: ImageStore, Database and PreferenceManager.
    • Reload the app to re-initialize app.

This PR has been tested on Brave browser and Exodus 1.

@seanwu1105 seanwu1105 added this to the beta12 milestone Feb 18, 2021
@seanwu1105 seanwu1105 requested a review from shc261392 February 18, 2021 06:14
@seanwu1105 seanwu1105 self-assigned this Feb 18, 2021
@seanwu1105 seanwu1105 added the enhancement New feature or request label Feb 18, 2021
@seanwu1105 seanwu1105 marked this pull request as ready for review February 18, 2021 06:53
@seanwu1105 seanwu1105 changed the base branch from develop to release-remove-data-on-logout February 18, 2021 06:54
@@ -231,6 +232,12 @@ export class ImageStore {
});
});
}

async drop() {
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems the drop() function isn't used anywhere except for tests, and same for the capacitor filesystem table drop implementation. If it's not used then it should be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The drop method helps the tests to clean up the persistence storage without exposing the internal implementation. Thus, IMHO, the method still need to exist.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like it's a good reason and is the cleanest way to do it so I'm fine with that.

@@ -231,6 +232,12 @@ export class ImageStore {
});
});
}

async drop() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like it's a good reason and is the cleanest way to do it so I'm fine with that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants