Skip to content

Notification feature for Project and Engine#1360

Draft
samarthKharote wants to merge 4 commits intodevfrom
1219-notification-feature
Draft

Notification feature for Project and Engine#1360
samarthKharote wants to merge 4 commits intodevfrom
1219-notification-feature

Conversation

@samarthKharote
Copy link
Contributor

@samarthKharote samarthKharote commented Sep 26, 2025

Description

This PR has an implementation for "Notification System" feature which allows users to initiate/receive notification whenever project(application) or engine related any activity occurs.

  • As User story 1 & User story 2, 3 this implementation has covered five use cases for notification feature.

  • 1. When new user is added to the project

  • 2. When user requests access for the project

  • 3. When user’s role changes

  • 4. When project member approves new user request or permission change

  • 5. When project member declines new user request or permission change

  • 6. When new user is added to the engine

  • 7. When user requests access for the engine

  • 8. When user’s role changes

  • 9. When engine member approves new user request or permission change

  • 10. When engine member declines new user request or permission change

As per the use cases, we have called notification utility method addNotification() across different ProjectUtility as well as EngineUtility methods.

Changes Made

  1. Created new table NOTIFICATION table in Security DB to store all the notifications.

  2. Created a separate utility class for notifications -> SecurityNotificationUtils : It contains all the utility methods to perform operations related to notifications.

  3. Created PollingNotifications Reactor: This reactor returns number of new notifications for logged in user from Notification database table.

  4. Created GetNotifications Reactor: This reactor returns all the notifications for the logged in user from Notification database table. (New Notifications+ Read Notifications+ Unread Notifications)

  5. Created UpdateReadNotifications Reactor: This reactor is responsible to update “Read Notifications” in Notification database table. This reactor accepts notificationId & updates ‘Read’ status of the notification.

  6. Created DeleteNotifications Reactor: Using logged in user, this reactor deletes single or multiple notification from the Notification database table for that given user.

NOTIFICATION table structure :
image (14)

How to Test

  • To test notification feature, we need front-end UI, here is the front-end branch - link
  • Working video of notification feature
485259853-426eb3a0-26e4-4989-8576-fc8b84dc11cd.mp4

@samarthKharote samarthKharote self-assigned this Sep 26, 2025
@samarthKharote samarthKharote requested a review from a team as a code owner September 26, 2025 13:39
@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

@QodoAI-Agent
Copy link

Title

Notification feature for Project and Engine


User description

Description

This PR has an implementation for "Notification System" feature which allows users to initiate/receive notification whenever project(application) or engine related any activity occurs.

  • As User story 1 & User story 2, 3 this implementation has covered five use cases for notification feature.

  • 1. When new user is added to the project

  • 2. When user requests access for the project

  • 3. When user’s role changes

  • 4. When project member approves new user request or permission change

  • 5. When project member declines new user request or permission change

  • 6. When new user is added to the engine

  • 7. When user requests access for the engine

  • 8. When user’s role changes

  • 9. When engine member approves new user request or permission change

  • 10. When engine member declines new user request or permission change

As per the use cases, we have called notification utility method addNotification() across different ProjectUtility as well as EngineUtility methods.

Changes Made

  1. Created new table NOTIFICATION table in Security DB to store all the notifications.

  2. Created a separate utility class for notifications -> SecurityNotificationUtils : It contains all the utility methods to perform operations related to notifications.

  3. Created PollingNotifications Reactor: This reactor returns number of new notifications for logged in user from Notification database table.

  4. Created GetNotifications Reactor: This reactor returns all the notifications for the logged in user from Notification database table. (New Notifications+ Read Notifications+ Unread Notifications)

  5. Created UpdateReadNotifications Reactor: This reactor is responsible to update “Read Notifications” in Notification database table. This reactor accepts notificationId & updates ‘Read’ status of the notification.

  6. Created DeleteNotifications Reactor: Using logged in user, this reactor deletes single or multiple notification from the Notification database table for that given user.

NOTIFICATION table structure :
image (14)

How to Test

  • To test notification feature, we need front-end UI, here is the front-end branch - link
  • Working video of notification feature
485259853-426eb3a0-26e4-4989-8576-fc8b84dc11cd.mp4

PR Type

Enhancement


Description

  • Add notification system utilities and reactors

  • Create NOTIFICATION table and OWL concept

  • Emit notifications on project/engine actions

  • Expose polling, fetch, update, delete APIs


Diagram Walkthrough

flowchart LR
  DB["NOTIFICATION table"]
  Utils["SecurityNotificationUtils"]
  ProjUtils["SecurityProjectUtils"]
  EngUtils["SecurityEngineUtils"]
  ReqProj["RequestProjectReactor"]
  ReqEng["RequestEngineReactor"]
  Poll["PollingNotificationsReactor"]
  GetN["GetNotificationsReactor"]
  ReadN["UpdateReadNotificationsReactor"]
  DelN["DeleteNotificationsReactor"]
  OWL["SecurityOwlCreator updates"]

  ProjUtils -- "emit add/approve/deny/change" --> Utils
  EngUtils -- "emit add/approve/deny/change" --> Utils
  ReqProj -- "emit user request" --> Utils
  ReqEng -- "emit user request" --> Utils
  Utils -- "insert/select/update/delete" --> DB
  GetN -- "fetch all + mark seen" --> Utils
  Poll -- "count NEW" --> Utils
  ReadN -- "mark read" --> Utils
  DelN -- "delete" --> Utils
  OWL -- "define NOTIFICATION concept" --> DB
Loading

File Walkthrough

Relevant files
Enhancement
12 files
AbstractSecurityUtils.java
Initialize NOTIFICATION table and index                                   
+39/-0   
SecurityEngineUtils.java
Emit engine notifications on requests and permissions       
+42/-1   
SecurityNotificationUtils.java
Notification utility: CRUD, queries, helpers                         
+374/-0 
SecurityOwlCreator.java
Register NOTIFICATION concept and properties                         
+24/-2   
SecurityProjectUtils.java
Emit project notifications on requests and permissions     
+35/-0   
RequestEngineReactor.java
Add notification on engine access request                               
+5/-0     
DeleteNotificationsReactor.java
Reactor to delete notifications                                                   
+41/-0   
GetNotificationsReactor.java
Reactor to fetch user notifications                                           
+45/-0   
PollingNotificationsReactor.java
Reactor to poll new notification count                                     
+27/-0   
UpdateReadNotificationsReactor.java
Reactor to mark notification as read                                         
+40/-0   
RequestProjectReactor.java
Add notification on project access request                             
+5/-0     
ReactorKeysEnum.java
Add notificationId reactor key                                                     
+1/-0     

@github-actions
Copy link

@CodiumAI-Agent /review

@github-actions
Copy link

@CodiumAI-Agent /improve

@QodoAI-Agent
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

Possible Issue

In denyEngineUserAccessRequests, the code fetches deniedUserDetails per requestId but then indexes the list with i, which may not be 0 and can cause IndexOutOfBounds or mismatched user/permission. Use index 0 or iterate over the returned list safely.

for (int i = 0; i < requestIds.size(); i++) {
	String requestId = requestIds.get(i);
	List<Map<String, Object>> deniedUserDetails = SecurityNotificationUtils.getUserDetailsFromEngineAccessRequest(requestId);
	String permission = AccessPermissionEnum.getPermissionValueById((Integer) deniedUserDetails.get(i).get("permission"));
	SecurityNotificationUtils.addNotification(user, (String) deniedUserDetails.get(i).get("userId"),
						             engineId, "REQUEST_DENIAL", engineType, "MEDIUM", null, permission);
}
Data Model Mismatch

The NOTIFICATION table creation arrays define 18 columns, but defaultValues only contains 15 entries. Although not directly used in createTableIfNotExists, this mismatch suggests potential maintenance errors; ensure schema/defaults alignment.

Potential NPE/Null Handling

addNotification inserts null for MESSAGE and READAT and assumes member has a login; member.getLogins().get(0) may throw if empty. Guard against empty logins and consider non-null defaults for text fields.

public static void addNotification(User member, String userId, String catalogId, String notificationType,
		String notificationSource, String priority, String userExistingRole, String userNewRole) {

	List<Map<String, Object>> usersOfCatalog = getAuthors(catalogId, notificationSource, userId);
	for (Map<String, Object> recipient : usersOfCatalog) {
		String recipientId = (String) recipient.get("userId");
		String recipientType = (String) recipient.get("userType");
		String actionTarget = "In-app";
		String createdBy = member.getAccessToken(member.getLogins().get(0)).getId();
		Timestamp createdAt = Utility.getCurrentSqlTimestampUTC();

		String query = "INSERT INTO NOTIFICATION (NOTIFICATIONID,RECIPIENTID,RECIPIENTTYPE,NOTIFICATIONTITLE,MESSAGE,ACTIONTYPE,ACTIONTARGET,ISREAD,PRIORITY,NOTIFICATIONTYPE,CATALOGID,CREATEDBY,CREATEDAT,READAT,NOTIFICATIONSOURCE,USERID,USEREXISTINGROLE,USERNEWROLE) VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)";
		PreparedStatement ps = null;
		try {
			ps = securityDb.getPreparedStatement(query);
			int parameterIndex = 1;
			ps.setString(parameterIndex++, UUID.randomUUID().toString()); // notificationId
			ps.setString(parameterIndex++, recipientId);
			ps.setString(parameterIndex++, recipientType);
			ps.setString(parameterIndex++, "NOTIFICATION"); // notificationTitle
			ps.setString(parameterIndex++, null); // message
			ps.setString(parameterIndex++, "NEW"); // actionType
			ps.setString(parameterIndex++, actionTarget);
			ps.setBoolean(parameterIndex++, false); // isRead
			ps.setString(parameterIndex++, priority);
			ps.setString(parameterIndex++, notificationType);
			ps.setString(parameterIndex++, catalogId);
			ps.setString(parameterIndex++, createdBy);
			ps.setTimestamp(parameterIndex++, createdAt);
			ps.setTimestamp(parameterIndex++, null); // readAt
			ps.setString(parameterIndex++, notificationSource);
			ps.setString(parameterIndex++, userId);
			ps.setString(parameterIndex++, userExistingRole);
			ps.setString(parameterIndex++, userNewRole);

			ps.execute();
			if (!ps.getConnection().getAutoCommit()) {
				ps.getConnection().commit();
			}
		} catch (SQLException e) {
			classLogger.error(Constants.STACKTRACE, e);
		} finally {
			ConnectionUtils.closeAllConnectionsIfPooling(securityDb, ps);
		}
	}

@QodoAI-Agent
Copy link

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix mismatched default values

The defaultValues array length does not match colNames (18 vs 16), which can cause
SQL creation failures or misaligned defaults. Provide a default for each column or
pass the correct number of defaults to the table creation to avoid runtime errors.

src/prerna/auth/utils/AbstractSecurityUtils.java [1322-1330]

 colNames = new String[] { "NOTIFICATIONID", "RECIPIENTID", "RECIPIENTTYPE", "NOTIFICATIONTITLE", "MESSAGE",
 		"ACTIONTYPE", "ACTIONTARGET", "ISREAD", "PRIORITY", "NOTIFICATIONTYPE", "CATALOGID", "CREATEDBY",
 		"CREATEDAT", "READAT", "NOTIFICATIONSOURCE", "USERID", "USEREXISTINGROLE", "USERNEWROLE" };
 types = new String[] { "VARCHAR(255)", "VARCHAR(255)", "VARCHAR(255)", "VARCHAR(255)", CLOB_DATATYPE_NAME,
 		"VARCHAR(50)", "VARCHAR(255)", BOOLEAN_DATATYPE_NAME, "VARCHAR(20)", "VARCHAR(255)", "VARCHAR(255)",
 		"VARCHAR(255)", TIMESTAMP_DATATYPE_NAME, TIMESTAMP_DATATYPE_NAME, "VARCHAR(255)", "VARCHAR(255)",
 		"VARCHAR(255)", "VARCHAR(255)" };
 defaultValues = new Object[] { null, null, null, "NOTIFICATION", null, "NEW", null, false, "MEDIUM", null,
-		null, null, null, null, null };
+		null, null, null, null, null, null, null, null };
Suggestion importance[1-10]: 8

__

Why: Correctly identifies that defaultValues has fewer entries than colNames (16 vs 18), which can break table creation; the proposed fix aligns lengths and prevents runtime SQL issues.

Medium
Prevent out-of-bounds on detail list

Indexing deniedUserDetails by i assumes each query returns a list at least of size
i+1, which may cause IndexOutOfBounds. Use the first (only) row or iterate the
returned list independently to avoid runtime exceptions.

src/prerna/auth/utils/SecurityProjectUtils.java [3673-3679]

 for (int i = 0; i < requestIdList.size(); i++) {
 	String requestId = requestIdList.get(i);
 	List<Map<String, Object>> deniedUserDetails = SecurityNotificationUtils.getUserDetailsFromProjectAccessRequest(requestId);
-	String permission = AccessPermissionEnum.getPermissionValueById((Integer) deniedUserDetails.get(i).get("permission"));
-	SecurityNotificationUtils.addNotification(user, (String) deniedUserDetails.get(i).get("userId"),
-				                             projectId, "REQUEST_DENIAL", "app", "MEDIUM", null, permission);
+	if (deniedUserDetails != null && !deniedUserDetails.isEmpty()) {
+		Map<String, Object> row = deniedUserDetails.get(0);
+		String permission = AccessPermissionEnum.getPermissionValueById((Integer) row.get("permission"));
+		SecurityNotificationUtils.addNotification(user, (String) row.get("userId"),
+			projectId, "REQUEST_DENIAL", "app", "MEDIUM", null, permission);
+	}
 }
Suggestion importance[1-10]: 7

__

Why: Using deniedUserDetails.get(i) ties the inner list to the outer loop index and can cause IndexOutOfBounds; guarding and using the first row is safer and likely correct.

Medium
Guard against empty query results

Similar to the project path, indexing deniedUserDetails with i risks
IndexOutOfBounds and null pointer issues. Safely read the first returned row (or
iterate that list) and guard against empty results before sending notifications.

src/prerna/auth/utils/SecurityEngineUtils.java [581-587]

 for (int i = 0; i < requestIds.size(); i++) {
 	String requestId = requestIds.get(i);
 	List<Map<String, Object>> deniedUserDetails = SecurityNotificationUtils.getUserDetailsFromEngineAccessRequest(requestId);
-	String permission = AccessPermissionEnum.getPermissionValueById((Integer) deniedUserDetails.get(i).get("permission"));
-	SecurityNotificationUtils.addNotification(user, (String) deniedUserDetails.get(i).get("userId"),
-									             engineId, "REQUEST_DENIAL", engineType, "MEDIUM", null, permission);
+	if (deniedUserDetails != null && !deniedUserDetails.isEmpty()) {
+		Map<String, Object> row = deniedUserDetails.get(0);
+		String permission = AccessPermissionEnum.getPermissionValueById((Integer) row.get("permission"));
+		SecurityNotificationUtils.addNotification(user, (String) row.get("userId"),
+			engineId, "REQUEST_DENIAL", engineType, "MEDIUM", null, permission);
+	}
 }
Suggestion importance[1-10]: 7

__

Why: Mirrors the same indexing risk as the project path; adding null/empty checks and indexing the first element avoids exceptions and improves robustness.

Medium

@samarthKharote samarthKharote marked this pull request as draft October 29, 2025 06:56
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.

3 participants