Skip to content

Notification feature for SMSS file updation #198

Open
samarthKharote wants to merge 2 commits intodevfrom
190-notification-feature-for-smss-file-updation
Open

Notification feature for SMSS file updation #198
samarthKharote wants to merge 2 commits intodevfrom
190-notification-feature-for-smss-file-updation

Conversation

@samarthKharote
Copy link
Contributor

Description

This PR contains implementation which allows user to get notify when there is any change in SMSS file for any engine by any other user.

Changes Made

Called SecurityNotificationUtils's addNotification() method to store notification in database in order to display it to the users.

How to Test

To test notification feature, we need front-end UI, here is the front-end branch - link

Notes

In order to work this functionality, Semoss should have Notification feature link

@samarthKharote samarthKharote self-assigned this Sep 26, 2025
@samarthKharote samarthKharote requested a review from a team as a code owner September 26, 2025 14:17
@samarthKharote samarthKharote linked an issue Sep 26, 2025 that may be closed by this pull request
1 task
@github-actions
Copy link

@CodiumAI-Agent /describe

@samarthKharote samarthKharote marked this pull request as draft September 26, 2025 14:17
@QodoAI-Agent
Copy link

Title

Notification feature for SMSS file updation


User description

Description

This PR contains implementation which allows user to get notify when there is any change in SMSS file for any engine by any other user.

Changes Made

Called SecurityNotificationUtils's addNotification() method to store notification in database in order to display it to the users.

How to Test

To test notification feature, we need front-end UI, here is the front-end branch - link

Notes

In order to work this functionality, Semoss should have Notification feature link


PR Type

Enhancement


Description

  • Add SMSS update notification on save

  • Import notification utility dependency

  • Derive and include engine type context


Diagram Walkthrough

flowchart LR
  UpdateSmss["updateSmssFile endpoint"] --> PushCloud["ClusterUtil.pushEngineSmss"]
  UpdateSmss --> EngineType["SecurityEngineUtils.getEngineType"]
  EngineType --> Notify["SecurityNotificationUtils.addNotification (SMSS_UPDATE)"]
  UpdateSmss --> SuccessResp["Return success response"]
Loading

File Walkthrough

Relevant files
Enhancement
EngineRouteResource.java
Add SMSS update notification after push                                   

src/prerna/semoss/web/services/local/EngineRouteResource.java

  • Import SecurityNotificationUtils.
  • Compute engineType via SecurityEngineUtils.getEngineType.
  • Call SecurityNotificationUtils.addNotification after SMSS push.
  • Preserve existing SMSS update flow and responses.
+5/-0     

@github-actions
Copy link

@CodiumAI-Agent /review

@QodoAI-Agent
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Null Handling

The call to get the engine type and adding a notification does not guard against nulls or unexpected values (e.g., null engineId, null user, null/unknown engine type), which could cause NPEs or invalid notification records. Consider validating inputs and handling failures from the notification utility.

String engineType = String.valueOf(SecurityEngineUtils.getEngineType(engineId)).toLowerCase();
SecurityNotificationUtils.addNotification(user, null, engineId, "SMSS_UPDATE", engineType, "MEDIUM", null, null);
Error Propagation

addNotification is called without try/catch. If it throws, the entire update flow may fail after successfully pushing to cloud, leaving inconsistent state. Consider wrapping the notification step and logging failures while still returning success for the core update.

// Adding Notification
String engineType = String.valueOf(SecurityEngineUtils.getEngineType(engineId)).toLowerCase();
SecurityNotificationUtils.addNotification(user, null, engineId, "SMSS_UPDATE", engineType, "MEDIUM", null, null);
Severity/Type Constants

The notification uses string literals for event type and severity ("SMSS_UPDATE", "MEDIUM"). Prefer using enums or shared constants from SecurityNotificationUtils (if available) to avoid typos and ensure consistency.

String engineType = String.valueOf(SecurityEngineUtils.getEngineType(engineId)).toLowerCase();
SecurityNotificationUtils.addNotification(user, null, engineId, "SMSS_UPDATE", engineType, "MEDIUM", null, null);

@github-actions
Copy link

@CodiumAI-Agent /improve

@QodoAI-Agent
Copy link

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Impact
Possible issue
Isolate notification failures

Guard the notification call to prevent failures from breaking the API response. Wrap
it in a try-catch and log failures so SMSS updates still succeed even if
notifications fail.

src/prerna/semoss/web/services/local/EngineRouteResource.java [184-186]

 String engineType = String.valueOf(SecurityEngineUtils.getEngineType(engineId)).toLowerCase();
-SecurityNotificationUtils.addNotification(user, null, engineId, "SMSS_UPDATE", engineType, "MEDIUM", null, null);
+try {
+	SecurityNotificationUtils.addNotification(user, null, engineId, "SMSS_UPDATE", engineType, "MEDIUM", null, null);
+} catch (Exception ex) {
+	logger.warn("Failed to add notification for SMSS update. engineId={}, engineType={}", engineId, engineType, ex);
+}
Suggestion importance[1-10]: 7

__

Why: Adding a try-catch around SecurityNotificationUtils.addNotification prevents a non-critical notification failure from breaking a successful SMSS update, improving robustness without altering core logic.

Medium
Null-safe engine type handling

Avoid potential NullPointerException by null-checking the engine type before calling
toLowerCase. Default to a safe value if it's null.

src/prerna/semoss/web/services/local/EngineRouteResource.java [184]

-String engineType = String.valueOf(SecurityEngineUtils.getEngineType(engineId)).toLowerCase();
+Object engineTypeObj = SecurityEngineUtils.getEngineType(engineId);
+String engineType = engineTypeObj != null ? engineTypeObj.toString().toLowerCase() : "unknown";
Suggestion importance[1-10]: 6

__

Why: Null-guarding before toLowerCase() avoids a potential NPE if getEngineType returns null; it's a reasonable defensive fix with moderate impact.

Low
General
Validate notification inputs

Validate inputs passed to the notification system to prevent malformed
notifications. Ensure user and engineId are non-null and engineType is non-empty
before calling.

src/prerna/semoss/web/services/local/EngineRouteResource.java [185]

-SecurityNotificationUtils.addNotification(user, null, engineId, "SMSS_UPDATE", engineType, "MEDIUM", null, null);
+if (user != null && engineId != null && engineType != null && !engineType.isEmpty()) {
+	SecurityNotificationUtils.addNotification(user, null, engineId, "SMSS_UPDATE", engineType, "MEDIUM", null, null);
+} else {
+	logger.warn("Skipping notification due to invalid data. user={}, engineId={}, engineType={}", user, engineId, engineType);
+}
Suggestion importance[1-10]: 6

__

Why: Checking user, engineId, and engineType before notifying can prevent malformed calls and noisy errors, improving reliability though the impact is moderate.

Low

@samarthKharote samarthKharote linked an issue Oct 1, 2025 that may be closed by this pull request
1 task
@samarthKharote samarthKharote marked this pull request as ready for review October 1, 2025 13:05
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.

Notification System for SMSS file updation

2 participants