feature/IO-3096-GlobalNotifications - Code Review Part 2

client/src/pages/jobs-detail/job-watcher-toggle.component.jsx

@@ -40,9 +40,9 @@ const JobWatcherToggle = ({ job, currentUser, bodyshop }) => {
 
   // Extract watchers list
   const jobWatchers = useMemo(() => watcherData?.job_watchers || [], [watcherData]);
-  const isWatching = useMemo(() => jobWatchers.some((w) => w.user_email === userEmail), [jobWatchers, userEmail]);
 
-  // Add watcher mutation
+  const isWatching = jobWatchers.some((w) => w.user_email === userEmail);
+
   const [addWatcher, { loading: adding }] = useMutation(ADD_JOB_WATCHER, {
     refetchQueries: [{ query: GET_JOB_WATCHERS, variables: { jobid } }]
   });
@@ -81,12 +81,11 @@ const JobWatcherToggle = ({ job, currentUser, bodyshop }) => {
       );
     }
 
-    // Clear selection
     setSelectedWatcher(null);
   };
 
   const handleTeamSelect = (team) => {
-    const selectedTeamMembers = JSON.parse(team); // Parse the array of emails
+    const selectedTeamMembers = JSON.parse(team);
 
     const newWatchers = selectedTeamMembers.filter(
       (email) => !jobWatchers.some((watcher) => watcher.user_email === email)
@@ -99,7 +98,6 @@ const JobWatcherToggle = ({ job, currentUser, bodyshop }) => {
       );
     });
 
-    // Clear selection
     setSelectedTeam(null);
   };
 
@@ -147,14 +145,12 @@ const JobWatcherToggle = ({ job, currentUser, bodyshop }) => {
         style={{ minWidth: "100%" }}
         options={bodyshop.employees.filter((e) => jobWatchers.every((w) => w.user_email !== e.user_email))}
         placeholder={t("notifications.labels.employee-search")}
-        value={selectedWatcher} // Controlled value
+        value={selectedWatcher}
         onChange={(value) => {
-          setSelectedWatcher(value); // Update selected state
+          setSelectedWatcher(value);
-          handleWatcherSelect(value); // Add watcher logic
+          handleWatcherSelect(value);
         }}
       />
-      {/* Divider for UI separation */}
-      {/* Only show team selection if there are available teams */}
       {Enhanced_Payroll && bodyshop?.employee_teams?.length > 0 && (
         <>
           <Divider />
@@ -172,11 +168,11 @@ const JobWatcherToggle = ({ job, currentUser, bodyshop }) => {
                   const employee = bodyshop.employees.find((e) => e.id === member.employeeid);
                   return employee ? employee.user_email : null;
                 })
-                .filter(Boolean); // Remove nulls
+                .filter(Boolean);
 
               return {
-                value: JSON.stringify(teamMembers), // Store array as string
+                value: JSON.stringify(teamMembers),
-                label: team.name // Use team name as label
+                label: team.name
               };
             })}
           />

server/notifications/eventParser.js

@@ -3,15 +3,14 @@
  *
  * This function analyzes the differences between previous (`oldData`) and current (`newData`)
  * data states to identify changed fields. It determines if the event is a new entry or an update
- * and optionally extracts a `jobId` based on a specified field. The result includes details
+ * and returns details about changed fields, the event type, and associated metadata.
- * about changed fields, the event type, and associated metadata.
  *
  * @param {Object} options - Configuration options for parsing the event.
  * @param {Object} [options.oldData] - The previous state of the data (undefined for new entries).
  * @param {Object} options.newData - The current state of the data.
  * @param {string} options.trigger - The type of event trigger (e.g., 'INSERT', 'UPDATE').
  * @param {string} options.table - The name of the table associated with the event.
- * @param {string} [options.jobIdField] - The field name used to extract the jobId (optional).
+ * @param {string} [options.jobId] - The job ID, if already extracted by the caller (optional).
  * @returns {Object} An object containing the parsed event details:
  *   - {Array<string>} changedFieldNames - List of field names that have changed.
  *   - {Object} changedFields - Map of changed fields with their old and new values.
@@ -19,9 +18,9 @@
  *   - {Object} data - The current data state (`newData`).
  *   - {string} trigger - The event trigger type.
  *   - {string} table - The table name.
- *   - {string|null} jobId - The extracted jobId or null if not applicable.
+ *   - {string|null} jobId - The provided jobId or null if not provided.
  */
-const eventParser = async ({ oldData, newData, trigger, table, jobIdField }) => {
+const eventParser = async ({ oldData, newData, trigger, table, jobId = null }) => {
   const isNew = !oldData; // True if no old data exists, indicating a new entry
   let changedFields = {};
   let changedFieldNames = [];
@@ -61,19 +60,6 @@ const eventParser = async ({ oldData, newData, trigger, table, jobIdField }) =>
     }
   }
 
-  // Extract jobId if jobIdField is provided
-  let jobId = null;
-  if (jobIdField) {
-    let keyName = jobIdField;
-    const prefix = "req.body.event.new.";
-    // Strip prefix if present to isolate the actual field name
-    if (keyName.startsWith(prefix)) {
-      keyName = keyName.slice(prefix.length);
-    }
-    // Look for jobId in newData first, then fallback to oldData if necessary
-    jobId = newData[keyName] || (oldData && oldData[keyName]) || null;
-  }
-
   return {
     changedFieldNames, // Array of fields that changed
     changedFields, // Object with old/new values for changed fields
@@ -81,7 +67,7 @@ const eventParser = async ({ oldData, newData, trigger, table, jobIdField }) =>
     data: newData, // Current data state
     trigger, // Event trigger (e.g., 'INSERT', 'UPDATE')
     table, // Associated table name
-    jobId // Extracted jobId or null
+    jobId // Provided jobId or null
   };
 };
 

server/notifications/scenarioParser.js

@@ -23,7 +23,7 @@ const FILTER_SELF_FROM_WATCHERS = process.env?.FILTER_SELF_FROM_WATCHERS === "tr
  * Queries job watchers and notification settings before triggering scenario builders.
  *
  * @param {Object} req - The request object containing event data, trigger, table, and logger.
- * @param {string} jobIdField - The field name used to extract the job ID from the event data.
+ * @param {string} jobIdField - The field path (e.g., "req.body.event.new.id") to extract the job ID.
  * @returns {Promise<void>} Resolves when the parsing and notification dispatching process is complete.
  * @throws {Error} If required request fields (event data, trigger, or table) or body shop data are missing.
  */
@@ -34,8 +34,9 @@ const scenarioParser = async (req, jobIdField) => {
   // Validate we know what user committed the action that fired the parser
   const hasuraUserId = event?.session_variables?.["x-hasura-user-id"];
 
-  // Bail if we don't know
+  // Bail if we don't know who started the scenario
   if (!hasuraUserId) {
+    logger.log("No Hasura user ID found, skipping notification parsing", "info", "notifications");
     return;
   }
 
@@ -44,18 +45,25 @@ const scenarioParser = async (req, jobIdField) => {
     throw new Error("Missing required request fields: event data, trigger, or table.");
   }
 
-  // Step 1: Parse the event data to extract details like job ID and changed fields
+  // Step 1a: Extract just the jobId using the provided jobIdField
-  const eventData = await eventParser({
+  let jobId = null;
-    newData: event.data.new,
+  if (jobIdField) {
-    oldData: event.data.old,
+    let keyName = jobIdField;
-    trigger,
+    const prefix = "req.body.event.new.";
-    table,
+    if (keyName.startsWith(prefix)) {
-    jobIdField
+      keyName = keyName.slice(prefix.length);
-  });
+    }
+    jobId = event.data.new[keyName] || (event.data.old && event.data.old[keyName]) || null;
+  }
+
+  if (!jobId) {
+    logger.log(`No jobId found using path "${jobIdField}", skipping notification parsing`, "info", "notifications");
+    return;
+  }
 
   // Step 2: Query job watchers associated with the job ID using GraphQL
   const watcherData = await gqlClient.request(queries.GET_JOB_WATCHERS, {
-    jobid: eventData.jobId
+    jobid: jobId
   });
 
   // Transform watcher data into a simplified format with email and employee details
@@ -73,9 +81,19 @@ const scenarioParser = async (req, jobIdField) => {
 
   // Exit early if no job watchers are found for this job
   if (isEmpty(jobWatchers)) {
+    logger.log(`No watchers found for jobId "${jobId}", skipping notification parsing`, "info", "notifications");
     return;
   }
 
+  // Step 1b: Perform the full event diff now that we know there are watchers
+  const eventData = await eventParser({
+    newData: event.data.new,
+    oldData: event.data.old,
+    trigger,
+    table,
+    jobId
+  });
+
   // Step 3: Extract body shop information from the job data
   const bodyShopId = watcherData?.job?.bodyshop?.id;
   const bodyShopName = watcherData?.job?.bodyshop?.shopname;
@@ -97,6 +115,11 @@ const scenarioParser = async (req, jobIdField) => {
 
   // Exit early if no matching scenarios are identified
   if (isEmpty(matchingScenarios)) {
+    logger.log(
+      `No matching scenarios found for jobId "${jobId}", skipping notification dispatch`,
+      "info",
+      "notifications"
+    );
     return;
   }
 
@@ -117,6 +140,11 @@ const scenarioParser = async (req, jobIdField) => {
 
   // Exit early if no notification associations are found
   if (isEmpty(associationsData?.associations)) {
+    logger.log(
+      `No notification associations found for jobId "${jobId}", skipping notification dispatch`,
+      "info",
+      "notifications"
+    );
     return;
   }
 
@@ -150,6 +178,11 @@ const scenarioParser = async (req, jobIdField) => {
 
   // Exit early if no scenarios have eligible watchers after filtering
   if (isEmpty(finalScenarioData?.matchingScenarios)) {
+    logger.log(
+      `No eligible watchers after filtering for jobId "${jobId}", skipping notification dispatch`,
+      "info",
+      "notifications"
+    );
     return;
   }
 
@@ -204,35 +237,29 @@ const scenarioParser = async (req, jobIdField) => {
     );
   }
 
-  // Exit early if no scenarios are ready to dispatch
   if (isEmpty(scenariosToDispatch)) {
+    logger.log(`No scenarios to dispatch for jobId "${jobId}" after building`, "info", "notifications");
     return;
   }
 
-  // Step 9: Dispatch email notifications to the email queue
+  // Step 8: Dispatch email notifications to the email queue
   const emailsToDispatch = scenariosToDispatch.map((scenario) => scenario?.email);
   if (!isEmpty(emailsToDispatch)) {
-    dispatchEmailsToQueue({
+    dispatchEmailsToQueue({ emailsToDispatch, logger }).catch((e) =>
-      emailsToDispatch,
-      logger
-    }).catch((e) =>
-      // Log any errors encountered during email dispatching
       logger.log("Something went wrong dispatching emails to the Email Notification Queue", "error", "queue", null, {
-        message: e?.message
+        message: e?.message,
+        stack: e?.stack
       })
     );
   }
 
-  // Step 10: Dispatch app notifications to the app queue
+  // Step 9: Dispatch app notifications to the app queue
   const appsToDispatch = scenariosToDispatch.map((scenario) => scenario?.app);
   if (!isEmpty(appsToDispatch)) {
-    dispatchAppsToQueue({
+    dispatchAppsToQueue({ appsToDispatch, logger }).catch((e) =>
-      appsToDispatch,
-      logger
-    }).catch((e) =>
-      // Log any errors encountered during app notification dispatching
       logger.log("Something went wrong dispatching apps to the App Notification Queue", "error", "queue", null, {
-        message: e?.message
+        message: e?.message,
+        stack: e?.stack
       })
     );
   }