-
Notifications
You must be signed in to change notification settings - Fork 557
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
feat: Implement interface persistence #2856
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2856 +/- ##
==========================================
- Coverage 94.47% 94.44% -0.04%
==========================================
Files 486 486
Lines 10368 10398 +30
Branches 1582 1591 +9
==========================================
+ Hits 9795 9820 +25
- Misses 573 578 +5 ☔ View full report in Codecov by Sentry. |
packages/snaps-controllers/src/interface/SnapInterfaceController.ts
Outdated
Show resolved
Hide resolved
packages/snaps-controllers/src/interface/SnapInterfaceController.ts
Outdated
Show resolved
Hide resolved
@@ -132,7 +140,22 @@ export class SnapInterfaceController extends BaseController< | |||
super({ | |||
messenger, | |||
metadata: { | |||
interfaces: { persist: false, anonymous: false }, | |||
interfaces: { | |||
persist: (interfaces: Record<string, StoredInterface>) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wondering if we should have any limits on this? How long do we persist Snap notifications for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this is mainly to cover the edge case where the interface is lost for the detailed view notification across sessions. I realize it would persist an unread notification's interface indefinitely if the user never reads it, but that's an extreme edge case. I've started conversation with the notification team about exposing some settings to the user to control the lifecycle of notifications.
Implementing interface persistence based on a new property stored in the interface object called
contentType
, it is used to indicate whether theSnapInterfaceController
should persist the interface. The property is added at various points in our system where an interface is created.