Skip to content

Combine notification features for smss file update#208

Open
samarthKharote wants to merge 18 commits intodevfrom
combine-notification-feature-for-smss-file-update
Open

Combine notification features for smss file update#208
samarthKharote wants to merge 18 commits intodevfrom
combine-notification-feature-for-smss-file-update

Conversation

@samarthKharote
Copy link
Contributor

Description

This PR contains implementation which enables the "in-app notification" as well as "email notification" when user updates SMSS file.

Changes Made

Called notification utility methods from EngineRouteResource Class

How to Test

For In-app Notification:

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

For Email Notification:

  • To test the functionality locally, I used MailPit and added credentials in social.Properties file.
smssFileUpdate

@samarthKharote samarthKharote self-assigned this Oct 15, 2025
@samarthKharote samarthKharote requested a review from a team as a code owner October 15, 2025 09:11
@github-actions
Copy link

@CodiumAI-Agent /describe

@QodoAI-Agent
Copy link

Title

Combine notification features for smss file update


User description

Description

This PR contains implementation which enables the "in-app notification" as well as "email notification" when user updates SMSS file.

Changes Made

Called notification utility methods from EngineRouteResource Class

How to Test

For In-app Notification:

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

For Email Notification:

  • To test the functionality locally, I used MailPit and added credentials in social.Properties file.
smssFileUpdate

PR Type

Enhancement


Description

  • Add in-app notification on SMSS update

  • Send email notification for SMSS update

  • Import utilities for notifications and email


Diagram Walkthrough

flowchart LR
  updateSmss["updateSmssFile endpoint"] --> pushCloud["Push SMSS to cloud"]
  pushCloud --> notify["Add in-app notification (SecurityNotificationUtils)"]
  notify --> email["Send email (EmailUtility)"]
  updateSmss --> successResp["Return success response"]
Loading

File Walkthrough

Relevant files
Enhancement
EngineRouteResource.java
Add SMSS update notifications and email alerts                     

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

  • Import SecurityNotificationUtils and EmailUtility.
  • After SMSS push, add in-app notification.
  • Send email notification for SMSS update.
  • Minor variable initialization for engine type.
+9/-0     

@github-actions
Copy link

@CodiumAI-Agent /review

@samarthKharote samarthKharote linked an issue Oct 15, 2025 that may be closed by this pull request
1 task
@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

Error Handling

Newly added notification and email calls are not wrapped in try/catch. Failures in notification/email could cause the whole SMSS update API to return an error instead of proceeding after the core update succeeds. Consider isolating and logging failures so the main operation remains successful.

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

//Adding email notification
EmailUtility.sendEmailSmssFileUpdateNotification(user, "smssFileUpdate.html", engineId, "SEMOSS - SMSS File Updated");
Null Safety

The user object and engineType derivation are passed directly into notification/email utilities without null checks. Validate that user and engineType are non-null and meaningful, or guard accordingly to avoid NPEs or malformed notifications.

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

//Adding email notification
EmailUtility.sendEmailSmssFileUpdateNotification(user, "smssFileUpdate.html", engineId, "SEMOSS - SMSS File Updated");
Performance/Latency

Synchronous email sending within the request path can add latency. Consider deferring to an async job/queue or fire-and-forget mechanism after the SMSS update succeeds.

//Adding email notification
EmailUtility.sendEmailSmssFileUpdateNotification(user, "smssFileUpdate.html", engineId, "SEMOSS - SMSS File Updated");

@github-actions
Copy link

@CodiumAI-Agent /improve

@samarthKharote samarthKharote linked an issue Oct 15, 2025 that may be closed by this pull request
1 task
@QodoAI-Agent
Copy link

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Impact
Possible issue
Isolate notification failures

Wrap the notification and email calls in a try-catch so failures there don't mask a
successful SMSS update. Log exceptions and continue to return success, or degrade
gracefully, to avoid breaking the main flow.

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

 // Adding notification
-String engineType = String.valueOf(SecurityEngineUtils.getEngineType(engineId)).toLowerCase();
-SecurityNotificationUtils.addNotification(user, null, engineId, "SMSS_UPDATE", engineType, "MEDIUM", null, null);
+try {
+	String engineType = String.valueOf(SecurityEngineUtils.getEngineType(engineId)).toLowerCase();
+	SecurityNotificationUtils.addNotification(user, null, engineId, "SMSS_UPDATE", engineType, "MEDIUM", null, null);
+} catch (Exception notifyEx) {
+	logger.warn("Failed to add SMSS update notification for engineId={}", engineId, notifyEx);
+}
 
-//Adding email notification
-EmailUtility.sendEmailSmssFileUpdateNotification(user, "smssFileUpdate.html", engineId, "SEMOSS - SMSS File Updated");
+// Adding email notification
+try {
+	EmailUtility.sendEmailSmssFileUpdateNotification(user, "smssFileUpdate.html", engineId, "SEMOSS - SMSS File Updated");
+} catch (Exception emailEx) {
+	logger.warn("Failed to send SMSS update email for engineId={}", engineId, emailEx);
+}
Suggestion importance[1-10]: 7

__

Why: Wrapping notification and email sends in try-catch prevents auxiliary failures from breaking the main update flow; it's contextually accurate and improves resilience, though not critical to core functionality.

Medium
Null-safe engine type handling

Guard against null engine type to prevent a "null" string or NPE if getEngineType
returns null. Default to a safe value or skip notification when type is unavailable.

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

-String engineType = String.valueOf(SecurityEngineUtils.getEngineType(engineId)).toLowerCase();
+String engineType = null;
+Object typeObj = SecurityEngineUtils.getEngineType(engineId);
+if (typeObj != null) {
+	engineType = String.valueOf(typeObj).toLowerCase();
+} else {
+	engineType = "unknown";
+}
Suggestion importance[1-10]: 6

__

Why: Adding a null check around SecurityEngineUtils.getEngineType(engineId) is reasonable defensive coding; impact is moderate and the improved code aligns with the existing snippet.

Low
General
Check recipient email presence

Validate that user has a deliverable email before attempting to send to avoid
runtime errors or bounces. If missing, skip sending and log a warning.

src/prerna/semoss/web/services/local/EngineRouteResource.java [189-190]

-EmailUtility.sendEmailSmssFileUpdateNotification(user, "smssFileUpdate.html", engineId, "SEMOSS - SMSS File Updated");
+if (user != null && user.getEmail() != null && !user.getEmail().trim().isEmpty()) {
+	EmailUtility.sendEmailSmssFileUpdateNotification(user, "smssFileUpdate.html", engineId, "SEMOSS - SMSS File Updated");
+} else {
+	logger.warn("Skipping SMSS update email: missing user or email for engineId={}", engineId);
+}
Suggestion importance[1-10]: 6

__

Why: Verifying user and email before sending reduces errors and noisy bounces; it's a sensible guard with accurate placement, offering moderate robustness gains.

Low

@shubhammahure shubhammahure self-assigned this Nov 10, 2025
@shubhammahure shubhammahure linked an issue Nov 10, 2025 that may be closed by this pull request
1 task
@shubhammahure shubhammahure marked this pull request as draft November 13, 2025 13:46
@shubhammahure shubhammahure marked this pull request as ready for review November 26, 2025 11:16
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.

Implement DB Notification and Refactor Notification Logic Email Notifications System POC: Notifications system

3 participants