Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ class NotificationPanel : BorderLayoutPanel() {
}

private fun removeNotificationPanel(notificationId: String) = runInEdt {
ProcessNotificationsBase.showBannerNotification.remove(notificationId) // TODO: add id to dismissed notification list
ProcessNotificationsBase.showBannerNotification.remove(notificationId)
NotificationDismissalState.getInstance().dismissNotification(notificationId)
wrapper.removeAll()
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// 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 com.intellij.openapi.application.ApplicationManager
import com.intellij.openapi.components.PersistentStateComponent
import com.intellij.openapi.components.State
import com.intellij.openapi.components.Storage

@State(name = "notificationDismissals", storages = [Storage("aws.xml")])
Copy link
Contributor

Choose a reason for hiding this comment

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

consider using BaseState

class NotificationDismissalState : PersistentStateComponent<NotificationDismissalConfiguration> {

Check warning on line 12 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationPersistantState.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Extension class should be final and non-public

Service implementation should not be public. If a service is supposed to be used outside its module, extract an interface from it and specify it as serviceInterface in plugin.xml.
private var state = NotificationDismissalConfiguration()
Copy link
Contributor

Choose a reason for hiding this comment

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

val


override fun getState(): NotificationDismissalConfiguration = state

override fun loadState(state: NotificationDismissalConfiguration) {
this.state = state
}

fun isDismissed(notificationId: String): Boolean =
state.dismissedNotificationIds.contains(notificationId)

fun dismissNotification(notificationId: String) {
state.dismissedNotificationIds.add(notificationId)
}

companion object {
fun getInstance(): NotificationDismissalState =
ApplicationManager.getApplication().getService(NotificationDismissalState::class.java)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
fun getInstance(): NotificationDismissalState =
ApplicationManager.getApplication().getService(NotificationDismissalState::class.java)
fun getInstance(): NotificationDismissalState =
ApplicationManager.getApplication().getService<NotificationDismissalState>()

}
}

data class NotificationDismissalConfiguration(
var dismissedNotificationIds: MutableSet<String> = mutableSetOf(),
)

@State(name = "notificationEtag", storages = [Storage("aws.xml")])
class NotificationEtagState : PersistentStateComponent<NotificationEtagConfiguration> {

Check warning on line 39 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/NotificationPersistantState.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Extension class should be final and non-public

Service implementation should not be public. If a service is supposed to be used outside its module, extract an interface from it and specify it as serviceInterface in plugin.xml.
private var state = NotificationEtagConfiguration()
Copy link
Contributor

Choose a reason for hiding this comment

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

val


override fun getState(): NotificationEtagConfiguration = state

override fun loadState(state: NotificationEtagConfiguration) {
this.state = state
}

var etag: String?
get() = state.etag
set(value) {
state.etag = value
}

companion object {
fun getInstance(): NotificationEtagState =
ApplicationManager.getApplication().getService(NotificationEtagState::class.java)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
fun getInstance(): NotificationEtagState =
ApplicationManager.getApplication().getService(NotificationEtagState::class.java)
fun getInstance(): NotificationEtagState =
ApplicationManager.getApplication().getService<NotificationEtagState>()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm getting a syntax error when I do this. I think this is for newer versions of Intellij SDK. I don't see any other usages of the bottom code in our repository, but I do see the ::class.java version.

Copy link
Contributor

Choose a reason for hiding this comment

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

can you try using getInstance(): NotificationETagState = service()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that works!

}
}

data class NotificationEtagConfiguration(
var etag: String? = null,
)
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,7 @@ package software.aws.toolkits.jetbrains.core.notifications
import com.fasterxml.jackson.module.kotlin.readValue
import com.intellij.openapi.Disposable
import com.intellij.openapi.application.ApplicationManager
import com.intellij.openapi.components.PersistentStateComponent
import com.intellij.openapi.components.Service
import com.intellij.openapi.components.State
import com.intellij.openapi.components.Storage
import com.intellij.util.Alarm
import com.intellij.util.AlarmFactory
import com.intellij.util.io.HttpRequests
Expand Down Expand Up @@ -63,32 +60,6 @@ object NotificationEndpoint {
private const val DEFAULT_ENDPOINT = "" // TODO: Replace with actual endpoint
}

@State(name = "notificationEtag", storages = [Storage("aws.xml")])
class NotificationEtagState : PersistentStateComponent<NotificationEtagConfiguration> {
private var state = NotificationEtagConfiguration()

override fun getState(): NotificationEtagConfiguration = state

override fun loadState(state: NotificationEtagConfiguration) {
this.state = state
}

var etag: String?
get() = state.etag
set(value) {
state.etag = value
}

companion object {
fun getInstance(): NotificationEtagState =
ApplicationManager.getApplication().getService(NotificationEtagState::class.java)
}
}

data class NotificationEtagConfiguration(
var etag: String? = null,
)

@Service(Service.Level.APP)
internal final class NotificationPollingService : Disposable {
private val isFirstPoll = AtomicBoolean(true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ internal class NotificationServiceInitializer : ProjectActivity {
override suspend fun execute(project: Project) {
if (initialized.compareAndSet(false, true)) {
val service = NotificationPollingService.getInstance()
ProcessNotificationsBase()
ProcessNotificationsBase(project)
service.startPolling()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,21 @@
import software.aws.toolkits.core.utils.inputStream
import software.aws.toolkits.jetbrains.utils.notifyStickyWithData
import java.nio.file.Paths
import java.util.concurrent.atomic.AtomicBoolean

object NotificationMapperUtil {
val mapper = jacksonObjectMapper().configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false)
}

@Service(Service.Level.PROJECT)
class ProcessNotificationsBase {
class ProcessNotificationsBase(
private val project: Project,
) {
private val notifListener = mutableListOf<NotifListener>()
private var isStartup: AtomicBoolean = AtomicBoolean(true)
Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't you want startup notifications to propagate consistently across all projects on startup?

init {
NotificationPollingService.getInstance().addObserver { isStartup ->
retrieveStartupAndEmergencyNotifications(isStartup)
NotificationPollingService.getInstance().addObserver {
retrieveStartupAndEmergencyNotifications()
}
}

Expand All @@ -38,12 +42,31 @@
return NotificationMapperUtil.mapper.readValue(content)
}

fun retrieveStartupAndEmergencyNotifications(isStartup: Boolean) {
// TODO: separates notifications into startup and emergency
// iterates through the 2 lists and processes each notification(if it isn't dismissed)
fun retrieveStartupAndEmergencyNotifications() {
val notifications = getNotificationsFromFile()

notifications?.let { notificationsList ->
val (startupNotifications, emergencyNotifications) = notificationsList.notifications
?.partition { notification ->
notification.schedule.type.equals("StartUp", ignoreCase = true)
Copy link
Contributor

Choose a reason for hiding this comment

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

should be modeled in the deserializer

}
?: Pair(emptyList(), emptyList())

val dismissalState = NotificationDismissalState.getInstance()
val startupList = if (isStartup.compareAndSet(true, false)) {
startupNotifications
} else {
emptyList()
}

val combinedNotifications = startupList.plus(emergencyNotifications)
val activeNotifications = combinedNotifications.filter { !dismissalState.isDismissed(it.id) }

activeNotifications.forEach { processNotification(project, it) }
}
}

fun processNotification(project: Project, notificationData: NotificationData) {

Check notice on line 69 in plugins/core/jetbrains-community/src/software/aws/toolkits/jetbrains/core/notifications/ProcessNotificationsBase.kt

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Class member can have 'private' visibility

Function 'processNotification' could be private
val shouldShow = RulesEngine.displayNotification(project, notificationData)
if (shouldShow) {
val notificationContent = notificationData.content.locale
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import com.intellij.ui.ScrollPaneFactory
import org.slf4j.LoggerFactory
import software.aws.toolkits.core.utils.warn
import software.aws.toolkits.jetbrains.core.help.HelpIds
import software.aws.toolkits.jetbrains.core.notifications.NotificationDismissalState
import software.aws.toolkits.jetbrains.core.notifications.ProcessNotificationsBase
import software.aws.toolkits.resources.AwsCoreBundle
import javax.swing.JLabel
Expand Down Expand Up @@ -67,7 +68,7 @@ fun notifyStickyWithData(
object : AnAction("Dismiss") {
override fun actionPerformed(e: AnActionEvent) {
ProcessNotificationsBase.showBannerNotification.remove(id)
Copy link
Contributor

Choose a reason for hiding this comment

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

state should not be stored in companion objects

// TODO: add id to dismissed notification list
NotificationDismissalState.getInstance().dismissNotification(id)
}
}
)
Expand Down
1 change: 1 addition & 0 deletions plugins/core/src/main/resources/META-INF/plugin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
<extensions defaultExtensionNs="com.intellij">
<!-- each plugin needs its own instance of these -->
<applicationService serviceImplementation="software.aws.toolkits.jetbrains.core.notifications.NotificationEtagState"/>
<applicationService serviceImplementation="software.aws.toolkits.jetbrains.core.notifications.NotificationDismissalState"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

these can be light services

<applicationService serviceImplementation="migration.software.aws.toolkits.jetbrains.core.coroutines.PluginCoroutineScopeTracker"/>
<projectService serviceImplementation="migration.software.aws.toolkits.jetbrains.core.coroutines.PluginCoroutineScopeTracker"/>
<postStartupActivity implementation = "software.aws.toolkits.jetbrains.core.notifications.NotificationServiceInitializer"/>
Expand Down
Loading