-
Notifications
You must be signed in to change notification settings - Fork 274
Set expiry for dismissed notifications #5186
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
Changes from 12 commits
ebdce9b
bcfbb27
a23cd78
4367447
824314d
dc61742
3d26f16
3dcac05
97acf1c
ef6ec85
ba2638f
9d3e162
ed92b9f
2a96250
35d1134
960f5f1
2475c07
5ae5b7f
1c244db
66d27e6
cb7d9f9
3afef56
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -9,37 +9,68 @@ | |||||
| import com.intellij.openapi.components.State | ||||||
| import com.intellij.openapi.components.Storage | ||||||
| import com.intellij.openapi.components.service | ||||||
| import com.intellij.util.xmlb.Converter | ||||||
| import com.intellij.util.xmlb.annotations.Attribute | ||||||
| import com.intellij.util.xmlb.annotations.Property | ||||||
| import software.aws.toolkits.core.utils.ETagProvider | ||||||
| import java.time.Duration | ||||||
| import java.time.Instant | ||||||
|
|
||||||
| class InstantConverter : Converter<Instant>() { | ||||||
| override fun toString(value: Instant): String = value.toEpochMilli().toString() | ||||||
|
Check warning on line 20 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationStateUtils.kt
|
||||||
|
|
||||||
| override fun fromString(value: String): Instant = Instant.ofEpochMilli(value.toLong()) | ||||||
|
Check warning on line 22 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationStateUtils.kt
|
||||||
| } | ||||||
|
|
||||||
| data class DismissedNotification( | ||||||
| @Attribute | ||||||
samgst-amazon marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| val id: String = "", | ||||||
| @Attribute(converter = InstantConverter::class) | ||||||
| val dismissedAt: Instant = Instant.now(), | ||||||
| ) | ||||||
|
|
||||||
| data class NotificationDismissalConfiguration( | ||||||
| @Property | ||||||
samgst-amazon marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| var dismissedNotifications: MutableSet<DismissedNotification> = mutableSetOf(), | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These properties need to be var for the serialization to work without the property/attribute tags |
||||||
| ) | ||||||
|
|
||||||
| @Service | ||||||
| @State(name = "notificationDismissals", storages = [Storage("aws.xml", roamingType = RoamingType.DISABLED)]) | ||||||
| class NotificationDismissalState : PersistentStateComponent<NotificationDismissalConfiguration> { | ||||||
| private val state = NotificationDismissalConfiguration() | ||||||
| private val retentionPeriod = Duration.ofDays(60) // 2 months | ||||||
|
|
||||||
| override fun getState(): NotificationDismissalConfiguration = state | ||||||
|
|
||||||
| override fun loadState(state: NotificationDismissalConfiguration) { | ||||||
| this.state.dismissedNotificationIds.clear() | ||||||
| this.state.dismissedNotificationIds.addAll(state.dismissedNotificationIds) | ||||||
| this.state.dismissedNotifications.clear() | ||||||
samgst-amazon marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| this.state.dismissedNotifications.addAll(state.dismissedNotifications) | ||||||
| cleanExpiredNotifications() | ||||||
| } | ||||||
|
|
||||||
| fun isDismissed(notificationId: String): Boolean = | ||||||
| state.dismissedNotificationIds.contains(notificationId) | ||||||
| state.dismissedNotifications.any { it.id == notificationId } | ||||||
|
|
||||||
| fun dismissNotification(notificationId: String) { | ||||||
| state.dismissedNotificationIds.add(notificationId) | ||||||
| state.dismissedNotifications.add( | ||||||
| DismissedNotification( | ||||||
| id = notificationId | ||||||
| ) | ||||||
| ) | ||||||
| } | ||||||
|
|
||||||
| private fun cleanExpiredNotifications() { | ||||||
| val now = Instant.now() | ||||||
| state.dismissedNotifications.removeAll { notification -> | ||||||
| Duration.between(notification.dismissedAt, now) > retentionPeriod | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| companion object { | ||||||
| fun getInstance(): NotificationDismissalState = | ||||||
| service() | ||||||
| fun getInstance(): NotificationDismissalState = service() | ||||||
|
Check warning on line 70 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationStateUtils.kt
|
||||||
| } | ||||||
| } | ||||||
|
|
||||||
| data class NotificationDismissalConfiguration( | ||||||
| var dismissedNotificationIds: MutableSet<String> = mutableSetOf(), | ||||||
| ) | ||||||
|
|
||||||
| @Service | ||||||
| @State(name = "notificationEtag", storages = [Storage("aws.xml", roamingType = RoamingType.DISABLED)]) | ||||||
| class NotificationEtagState : PersistentStateComponent<NotificationEtagConfiguration>, ETagProvider { | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| // Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package software.aws.toolkits.jetbrains.core.notifications | ||
|
|
||
| import org.junit.jupiter.api.Assertions.assertEquals | ||
| import org.junit.jupiter.api.Assertions.assertFalse | ||
| import org.junit.jupiter.api.Assertions.assertTrue | ||
| import org.junit.jupiter.api.BeforeEach | ||
| import org.junit.jupiter.api.Test | ||
| import java.time.Instant | ||
| import java.time.temporal.ChronoUnit | ||
|
|
||
| class NotificationDismissalStateTest { | ||
| private lateinit var state: NotificationDismissalState | ||
|
|
||
| @BeforeEach | ||
| fun setUp() { | ||
| state = NotificationDismissalState() | ||
| } | ||
|
|
||
| @Test | ||
| fun `notifications less than 2 months old are not removed`() { | ||
| val recentNotification = DismissedNotification( | ||
| id = "recent-notification", | ||
| dismissedAt = Instant.now().minus(30, ChronoUnit.DAYS) | ||
| ) | ||
|
|
||
| state.loadState(NotificationDismissalConfiguration(mutableSetOf(recentNotification))) | ||
|
|
||
| val persistedState = state.getState() | ||
|
|
||
| assertEquals(1, persistedState.dismissedNotifications.size) | ||
| assertTrue(persistedState.dismissedNotifications.any { it.id == "recent-notification" }) | ||
| assertTrue(state.isDismissed("recent-notification")) | ||
| } | ||
|
|
||
| @Test | ||
| fun `notifications older than 2 months are removed`() { | ||
| val oldNotification = DismissedNotification( | ||
| id = "old-notification", | ||
| dismissedAt = Instant.now().minus(61, ChronoUnit.DAYS) | ||
| ) | ||
|
|
||
| state.loadState(NotificationDismissalConfiguration(mutableSetOf(oldNotification))) | ||
|
|
||
| val persistedState = state.getState() | ||
|
|
||
| assertEquals(0, persistedState.dismissedNotifications.size) | ||
| assertFalse(state.isDismissed("old-notification")) | ||
| } | ||
|
|
||
| @Test | ||
| fun `mixed age notifications are handled correctly`() { | ||
| val recentNotification = DismissedNotification( | ||
| id = "recent-notification", | ||
| dismissedAt = Instant.now().minus(30, ChronoUnit.DAYS) | ||
| ) | ||
| val oldNotification = DismissedNotification( | ||
| id = "old-notification", | ||
| dismissedAt = Instant.now().minus(61, ChronoUnit.DAYS) | ||
| ) | ||
|
|
||
| state.loadState( | ||
| NotificationDismissalConfiguration( | ||
| mutableSetOf(recentNotification, oldNotification) | ||
| ) | ||
| ) | ||
|
|
||
| val persistedState = state.getState() | ||
|
|
||
| assertEquals(1, persistedState.dismissedNotifications.size) | ||
| assertTrue(state.isDismissed("recent-notification")) | ||
| assertFalse(state.isDismissed("old-notification")) | ||
| } | ||
|
|
||
| @Test | ||
| fun `dismissing new notification retains it`() { | ||
| state.dismissNotification("new-notification") | ||
|
|
||
| assertTrue(state.isDismissed("new-notification")) | ||
| } | ||
|
|
||
| @Test | ||
samgst-amazon marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| fun `clean up happens on load state`() { | ||
| val oldNotification = DismissedNotification( | ||
| id = "old-notification", | ||
| dismissedAt = Instant.now().minus(61, ChronoUnit.DAYS) | ||
| ) | ||
| val recentNotification = DismissedNotification( | ||
| id = "recent-notification", | ||
| dismissedAt = Instant.now() | ||
| ) | ||
|
|
||
| state.loadState( | ||
| NotificationDismissalConfiguration( | ||
| mutableSetOf(oldNotification, recentNotification) | ||
| ) | ||
| ) | ||
|
|
||
| val persistedState = state.getState() | ||
| assertEquals(1, persistedState.dismissedNotifications.size) | ||
| assertTrue(persistedState.dismissedNotifications.any { it.id == "recent-notification" }) | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.