Skip to content

Conversation

@manodnyab
Copy link
Contributor

@manodnyab manodnyab commented Nov 15, 2024

Description

  1. Implement a custom Deserializer for notifications received from the notifications file.
  2. Implementing Rule engine that compares the actual IDE, system , plugin values with the values that require the notification to be displayed.

License

I confirm that my contribution is made under the terms of the Apache 2.0 license.

@manodnyab manodnyab requested a review from a team as a code owner November 15, 2024 21:54
@manodnyab manodnyab marked this pull request as draft November 15, 2024 21:55
@github-actions
Copy link

github-actions bot commented Nov 15, 2024

Qodana Community for JVM

4 new problems were found

Inspection name Severity Problems
Unused symbol 🔶 Warning 2
Control flow with empty body 🔶 Warning 1
Class member can have 'private' visibility ◽️ Notice 1

💡 Qodana analysis was run in the pull request mode: only the changed files were checked
☁️ View the detailed Qodana report

Contact Qodana team

Contact us at [email protected]

return shouldShow
}

fun matchesAllRules(notificationConditions: NotificationDisplayCondition, project: Project): Boolean {

Check notice

Code scanning / QDJVMC

Class member can have 'private' visibility Note

Function 'matchesAllRules' could be private
@manodnyab manodnyab marked this pull request as ready for review November 20, 2024 07:53
Comment on lines +92 to +102
is NotificationExpression.NotCondition -> performNotOp(notificationExpression, value)
is NotificationExpression.OrCondition -> performOrOp(notificationExpression, value)
is NotificationExpression.AndCondition -> performAndOp(notificationExpression, value)
is NotificationExpression.ComparisonCondition -> notificationExpression.value == value
is NotificationExpression.NotEqualsCondition -> notificationExpression.value != value
is NotificationExpression.GreaterThanCondition -> value > notificationExpression.value
is NotificationExpression.LessThanCondition -> value < notificationExpression.value
is NotificationExpression.GreaterThanOrEqualsCondition -> value >= notificationExpression.value
is NotificationExpression.LessThanOrEqualsCondition -> value <= notificationExpression.value
is NotificationExpression.AnyOfCondition -> notificationExpression.value.contains(value)
is NotificationExpression.NoneOfCondition -> !notificationExpression.value.contains(value)
Copy link
Contributor

Choose a reason for hiding this comment

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

it would be cleaner to just have an evaluate() method on the expression class

Copy link
Contributor Author

@manodnyab manodnyab Nov 20, 2024

Choose a reason for hiding this comment

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

I actually wanted to keep the operations separate from deserialization!


package software.aws.toolkits.jetbrains.core.notifications

fun checkSeverity(notificationSeverity: String): NotificationSeverity = when (notificationSeverity) {

Check warning

Code scanning / QDJVMC

Unused symbol Warning

Function "checkSeverity" is never used
// TODO: calls the Rule engine and notifies listeners
fun processNotification(project: Project, notificationData: NotificationData) {
val shouldShow = RulesEngine.displayNotification(project, notificationData)
if(shouldShow) {

Check warning

Code scanning / QDJVMC

Control flow with empty body Warning

'if' has empty body

package software.aws.toolkits.jetbrains.core.notifications

object DisplayToastNotifications

Check warning

Code scanning / QDJVMC

Unused symbol Warning

Object "DisplayToastNotifications" is never used
@rli
Copy link
Contributor

rli commented Nov 21, 2024

please make sure all cases are exercised by test

@manodnyab manodnyab enabled auto-merge (squash) November 21, 2024 18:21
@manodnyab manodnyab merged commit a1a5728 into feature/ideNotifs Nov 21, 2024
15 checks passed
@manodnyab manodnyab deleted the manodnyb/checkRulesForNotifications branch November 21, 2024 18:48
samgst-amazon added a commit that referenced this pull request Nov 29, 2024
* Add framework for processing notifications (#5112)

* Add deserialization for notification messages retrieved from the notification file and criteria on whether it should be displayed (#5093)

* Display toast notifications with actions

* Condition matcher for displaying notifications

* Modified deserialization cases and added tests

* not required file change

* feedback 1

* modified the base class

* modified test instance lifecycle

* Show toasts for notifications and notification banner on critical notifications(#5097)

* Display toast notifications with actions

* Condition matcher for displaying notifications

* Show notification banner

* feedback 1

* Modified deserialization cases and added tests

* not required file change

* feedback 1

* feedback 1

* modified the base class

* merge conflicts resolved

* rearranged call site

* show notifications when panel is opened

* fixed tests

* detekt

* feedback

* convert panels into wrappers

* fixed test

* Adding update/restart action to notifications (#5136)

* Poll for new notifications (#5119)

* initial commit

* run on startup

* detekt

* move vals

* remote resource implementation

* comments

* detekt

* Validate file before saving

* cache path

* observer implementation

* deserialize notifs from file

* detekt

* remove unused interface

* internal class

* Fix observer

* etag singleton state component

* add telemetry

* atomicBoolean

* initialize once per IDE startup

* code scan

* Omit (Unit)

* specify etag storage location

* detekt

* fix detekt issues

* basic tests

* no star imports

* coroutine scope delay instead of thread.sleep

* feedback fixes

* test fix

* Application Exists for tests

* endpoint object

* detekt

* detekt fixes

* boolean flag

* boolean flag

* update tests

* move startup flag handling to processBase

* fix delay

* fix delay

* Notification dismissal state tracking  (#5129)

* split notifications into separated lists.

* add persistent notification dismissal state logic

* boolean changes

* group persistant states

* comments

* Service initialized automatically

* isStartup global

* Deserialized notification schedule type

* tests

* persistent state syntax

* convert to light services

* Remove state from companion object

* detekt

* endpoint as registryKey

* detekt

* fix startup issues

* Expiry issues

* Add logging info to IDE notification polling and processing (#5138)

* add logs for polling and processing notifs

* redundant

* finish log

* fix isFirstPoll not setting to false on first pass

---------

Co-authored-by: aws-toolkit-automation <[email protected]>
Co-authored-by: manodnyab <[email protected]>
Co-authored-by: Bryce Ito <[email protected]>
bryceitoc9 added a commit that referenced this pull request Nov 30, 2024
* Add framework for processing notifications (#5112)

* Add deserialization for notification messages retrieved from the notification file and criteria on whether it should be displayed (#5093)

* Display toast notifications with actions

* Condition matcher for displaying notifications

* Modified deserialization cases and added tests

* not required file change

* feedback 1

* modified the base class

* modified test instance lifecycle

* Show toasts for notifications and notification banner on critical notifications(#5097)

* Display toast notifications with actions

* Condition matcher for displaying notifications

* Show notification banner

* feedback 1

* Modified deserialization cases and added tests

* not required file change

* feedback 1

* feedback 1

* modified the base class

* merge conflicts resolved

* rearranged call site

* show notifications when panel is opened

* fixed tests

* detekt

* feedback

* convert panels into wrappers

* fixed test

* Adding update/restart action to notifications (#5136)

* Poll for new notifications (#5119)

* initial commit

* run on startup

* detekt

* move vals

* remote resource implementation

* comments

* detekt

* Validate file before saving

* cache path

* observer implementation

* deserialize notifs from file

* detekt

* remove unused interface

* internal class

* Fix observer

* etag singleton state component

* add telemetry

* atomicBoolean

* initialize once per IDE startup

* code scan

* Omit (Unit)

* specify etag storage location

* detekt

* fix detekt issues

* basic tests

* no star imports

* coroutine scope delay instead of thread.sleep

* feedback fixes

* test fix

* Application Exists for tests

* endpoint object

* detekt

* detekt fixes

* boolean flag

* boolean flag

* update tests

* move startup flag handling to processBase

* fix delay

* fix delay

* Notification dismissal state tracking  (#5129)

* split notifications into separated lists.

* add persistent notification dismissal state logic

* boolean changes

* group persistant states

* comments

* Service initialized automatically

* isStartup global

* Deserialized notification schedule type

* tests

* persistent state syntax

* convert to light services

* Remove state from companion object

* detekt

* endpoint as registryKey

* detekt

* fix startup issues

* Expiry issues

* Add logging info to IDE notification polling and processing (#5138)

* add logs for polling and processing notifs

* redundant

* finish log

* show message instead of yesNoDialogue

* initialize ProcessNotificationsBase listener before Polling service

* fix isFirstPoll not setting to false on first pass

---------

Co-authored-by: aws-toolkit-automation <[email protected]>
Co-authored-by: manodnyab <[email protected]>
Co-authored-by: Bryce Ito <[email protected]>
karanA-aws pushed a commit to karanA-aws/aws-toolkit-jetbrains that referenced this pull request Jan 17, 2025
* Add framework for processing notifications (aws#5112)

* Add deserialization for notification messages retrieved from the notification file and criteria on whether it should be displayed (aws#5093)

* Display toast notifications with actions

* Condition matcher for displaying notifications

* Modified deserialization cases and added tests

* not required file change

* feedback 1

* modified the base class

* modified test instance lifecycle

* Show toasts for notifications and notification banner on critical notifications(aws#5097)

* Display toast notifications with actions

* Condition matcher for displaying notifications

* Show notification banner

* feedback 1

* Modified deserialization cases and added tests

* not required file change

* feedback 1

* feedback 1

* modified the base class

* merge conflicts resolved

* rearranged call site

* show notifications when panel is opened

* fixed tests

* detekt

* feedback

* convert panels into wrappers

* fixed test

* Adding update/restart action to notifications (aws#5136)

* Poll for new notifications (aws#5119)

* initial commit

* run on startup

* detekt

* move vals

* remote resource implementation

* comments

* detekt

* Validate file before saving

* cache path

* observer implementation

* deserialize notifs from file

* detekt

* remove unused interface

* internal class

* Fix observer

* etag singleton state component

* add telemetry

* atomicBoolean

* initialize once per IDE startup

* code scan

* Omit (Unit)

* specify etag storage location

* detekt

* fix detekt issues

* basic tests

* no star imports

* coroutine scope delay instead of thread.sleep

* feedback fixes

* test fix

* Application Exists for tests

* endpoint object

* detekt

* detekt fixes

* boolean flag

* boolean flag

* update tests

* move startup flag handling to processBase

* fix delay

* fix delay

* Notification dismissal state tracking  (aws#5129)

* split notifications into separated lists.

* add persistent notification dismissal state logic

* boolean changes

* group persistant states

* comments

* Service initialized automatically

* isStartup global

* Deserialized notification schedule type

* tests

* persistent state syntax

* convert to light services

* Remove state from companion object

* detekt

* endpoint as registryKey

* detekt

* fix startup issues

* Expiry issues

* Add logging info to IDE notification polling and processing (aws#5138)

* add logs for polling and processing notifs

* redundant

* finish log

* fix isFirstPoll not setting to false on first pass

---------

Co-authored-by: aws-toolkit-automation <[email protected]>
Co-authored-by: manodnyab <[email protected]>
Co-authored-by: Bryce Ito <[email protected]>
karanA-aws pushed a commit to karanA-aws/aws-toolkit-jetbrains that referenced this pull request Jan 17, 2025
* Add framework for processing notifications (aws#5112)

* Add deserialization for notification messages retrieved from the notification file and criteria on whether it should be displayed (aws#5093)

* Display toast notifications with actions

* Condition matcher for displaying notifications

* Modified deserialization cases and added tests

* not required file change

* feedback 1

* modified the base class

* modified test instance lifecycle

* Show toasts for notifications and notification banner on critical notifications(aws#5097)

* Display toast notifications with actions

* Condition matcher for displaying notifications

* Show notification banner

* feedback 1

* Modified deserialization cases and added tests

* not required file change

* feedback 1

* feedback 1

* modified the base class

* merge conflicts resolved

* rearranged call site

* show notifications when panel is opened

* fixed tests

* detekt

* feedback

* convert panels into wrappers

* fixed test

* Adding update/restart action to notifications (aws#5136)

* Poll for new notifications (aws#5119)

* initial commit

* run on startup

* detekt

* move vals

* remote resource implementation

* comments

* detekt

* Validate file before saving

* cache path

* observer implementation

* deserialize notifs from file

* detekt

* remove unused interface

* internal class

* Fix observer

* etag singleton state component

* add telemetry

* atomicBoolean

* initialize once per IDE startup

* code scan

* Omit (Unit)

* specify etag storage location

* detekt

* fix detekt issues

* basic tests

* no star imports

* coroutine scope delay instead of thread.sleep

* feedback fixes

* test fix

* Application Exists for tests

* endpoint object

* detekt

* detekt fixes

* boolean flag

* boolean flag

* update tests

* move startup flag handling to processBase

* fix delay

* fix delay

* Notification dismissal state tracking  (aws#5129)

* split notifications into separated lists.

* add persistent notification dismissal state logic

* boolean changes

* group persistant states

* comments

* Service initialized automatically

* isStartup global

* Deserialized notification schedule type

* tests

* persistent state syntax

* convert to light services

* Remove state from companion object

* detekt

* endpoint as registryKey

* detekt

* fix startup issues

* Expiry issues

* Add logging info to IDE notification polling and processing (aws#5138)

* add logs for polling and processing notifs

* redundant

* finish log

* show message instead of yesNoDialogue

* initialize ProcessNotificationsBase listener before Polling service

* fix isFirstPoll not setting to false on first pass

---------

Co-authored-by: aws-toolkit-automation <[email protected]>
Co-authored-by: manodnyab <[email protected]>
Co-authored-by: Bryce Ito <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants