From 0767e290f4d89a16f6d1f6fa501758bfaeaeb4d4 Mon Sep 17 00:00:00 2001 From: Dave Richer Date: Wed, 26 Feb 2025 13:11:49 -0500 Subject: [PATCH] feature/IO-3096-GlobalNotifications - Checkpoint - Fix user getting all bodyshop notifications (now by associationId), fix regression in 'Assigned To' scenario. --- .../components/header/header.component.jsx | 20 ++++--- .../notification-center.container.jsx | 54 ++++++++++++------- .../src/contexts/SocketIO/socketContext.jsx | 23 +++++--- client/src/graphql/notifications.queries.js | 11 ++-- server/graphql-client/queries.js | 14 +++++ server/notifications/queues/appQueue.js | 29 +++------- server/notifications/scenarioBuilders.js | 4 +- server/notifications/scenarioParser.js | 9 ++-- 8 files changed, 99 insertions(+), 65 deletions(-) diff --git a/client/src/components/header/header.component.jsx b/client/src/components/header/header.component.jsx index 3dccc1842..cce95a067 100644 --- a/client/src/components/header/header.component.jsx +++ b/client/src/components/header/header.component.jsx @@ -49,7 +49,7 @@ import { useState, useEffect } from "react"; import { debounce } from "lodash"; import { useQuery } from "@apollo/client"; import { GET_UNREAD_COUNT } from "../../graphql/notifications.queries.js"; -import { NotificationCenterContainer } from "../notification-center/notification-center.container.jsx"; +import NotificationCenterContainer from "../notification-center/notification-center.container.jsx"; import { useSocket } from "../../contexts/SocketIO/socketContext.jsx"; // Used to Determine if the Header is in Mobile Mode, and to toggle the multiple menus @@ -129,27 +129,33 @@ function Header({ const { isConnected } = useSocket(bodyshop); const [notificationVisible, setNotificationVisible] = useState(false); + const userAssociationId = bodyshop?.associations?.[0]?.id; + const { data: unreadData, refetch: refetchUnread, loading: unreadLoading } = useQuery(GET_UNREAD_COUNT, { + variables: { associationid: userAssociationId }, fetchPolicy: "network-only", - pollInterval: isConnected ? 0 : 30000 // Poll only if socket is down + pollInterval: isConnected ? 0 : 30000, // Poll only if socket is down + skip: !userAssociationId // Skip query if no userAssociationId }); const unreadCount = unreadData?.notifications_aggregate?.aggregate?.count ?? 0; // Initial fetch and socket status handling useEffect(() => { - refetchUnread(); - }, [refetchUnread]); + if (userAssociationId) { + refetchUnread().catch((e) => console.error(`Something went wrong fetching unread notifications: ${e?.message}`)); + } + }, [refetchUnread, userAssociationId]); useEffect(() => { - if (!isConnected && !unreadLoading) { - refetchUnread(); + if (!isConnected && !unreadLoading && userAssociationId) { + refetchUnread().catch((e) => console.error(`Something went wrong fetching unread notifications: ${e?.message}`)); } - }, [isConnected, unreadLoading, refetchUnread]); + }, [isConnected, unreadLoading, refetchUnread, userAssociationId]); const handleNotificationClick = (e) => { setNotificationVisible(!notificationVisible); diff --git a/client/src/components/notification-center/notification-center.container.jsx b/client/src/components/notification-center/notification-center.container.jsx index 9bdc42114..5bb5f1cf0 100644 --- a/client/src/components/notification-center/notification-center.container.jsx +++ b/client/src/components/notification-center/notification-center.container.jsx @@ -1,10 +1,11 @@ -// notification-center.container.jsx -import { useState, useEffect, useCallback } from "react"; -import { useQuery, useMutation } from "@apollo/client"; +import { useCallback, useEffect, useMemo, useState } from "react"; +import { useMutation, useQuery } from "@apollo/client"; import { connect } from "react-redux"; import NotificationCenterComponent from "./notification-center.component"; import { GET_NOTIFICATIONS, MARK_ALL_NOTIFICATIONS_READ } from "../../graphql/notifications.queries"; import { useSocket } from "../../contexts/SocketIO/socketContext.jsx"; +import { createStructuredSelector } from "reselect"; +import { selectBodyshop } from "../../redux/user/user.selectors.js"; export function NotificationCenterContainer({ visible, onClose, bodyshop }) { const [showUnreadOnly, setShowUnreadOnly] = useState(false); @@ -12,6 +13,16 @@ export function NotificationCenterContainer({ visible, onClose, bodyshop }) { const [error, setError] = useState(null); const { isConnected } = useSocket(); + const userAssociationId = bodyshop?.associations?.[0]?.id; + + const baseWhereClause = useMemo(() => { + return { associationid: { _eq: userAssociationId } }; + }, [userAssociationId]); + + const whereClause = useMemo(() => { + return showUnreadOnly ? { ...baseWhereClause, read: { _is_null: true } } : baseWhereClause; + }, [baseWhereClause, showUnreadOnly]); + const { data, fetchMore, @@ -22,12 +33,12 @@ export function NotificationCenterContainer({ visible, onClose, bodyshop }) { variables: { limit: 20, offset: 0, - where: showUnreadOnly ? { read: { _is_null: true } } : {} + where: whereClause }, fetchPolicy: "cache-and-network", notifyOnNetworkStatusChange: true, pollInterval: isConnected ? 0 : 30000, - skip: false, + skip: !userAssociationId, // Skip query if no userAssociationId onError: (err) => { setError(err.message); console.error("GET_NOTIFICATIONS error:", err); @@ -36,13 +47,14 @@ export function NotificationCenterContainer({ visible, onClose, bodyshop }) { }); const [markAllReadMutation, { error: mutationError }] = useMutation(MARK_ALL_NOTIFICATIONS_READ, { + variables: { associationid: userAssociationId }, update: (cache, { data: mutationData }) => { const timestamp = new Date().toISOString(); cache.modify({ fields: { notifications(existing = [], { readField }) { return existing.map((notif) => { - if (readField("read", notif) === null) { + if (readField("read", notif) === null && readField("associationid", notif) === userAssociationId) { return { ...notif, read: timestamp }; } return notif; @@ -60,7 +72,7 @@ export function NotificationCenterContainer({ visible, onClose, bodyshop }) { variables: { limit: 20, offset: 0, - where: showUnreadOnly ? { read: { _is_null: true } } : {} + where: whereClause } }); @@ -70,7 +82,7 @@ export function NotificationCenterContainer({ visible, onClose, bodyshop }) { variables: { limit: 20, offset: 0, - where: showUnreadOnly ? { read: { _is_null: true } } : {} + where: whereClause }, data: { notifications: cachedNotifications.notifications.map((notif) => @@ -102,21 +114,19 @@ export function NotificationCenterContainer({ visible, onClose, bodyshop }) { scenarioMeta = {}; } if (!Array.isArray(scenarioText)) scenarioText = [scenarioText]; - // Derive RO number from scenario_meta or assume it's available in notif - const roNumber = notif.job.ro_number || "RO Not Found"; // Adjust based on your data structure + const roNumber = notif.job.ro_number; if (!Array.isArray(scenarioMeta)) scenarioMeta = [scenarioMeta]; - const processed = { + return { id: notif.id, jobid: notif.jobid, associationid: notif.associationid, scenarioText, scenarioMeta, - roNumber, // Add RO number to notification object + roNumber, created_at: notif.created_at, read: notif.read, __typename: notif.__typename }; - return processed; }) .sort((a, b) => new Date(b.created_at) - new Date(a.created_at)); setNotifications(processedNotifications); @@ -133,7 +143,7 @@ export function NotificationCenterContainer({ visible, onClose, bodyshop }) { const loadMore = useCallback(() => { if (!loading && data?.notifications.length) { fetchMore({ - variables: { offset: data.notifications.length }, + variables: { offset: data.notifications.length, where: whereClause }, updateQuery: (prev, { fetchMoreResult }) => { if (!fetchMoreResult) return prev; return { @@ -145,7 +155,7 @@ export function NotificationCenterContainer({ visible, onClose, bodyshop }) { console.error("Fetch more error:", err); }); } - }, [data?.notifications?.length, fetchMore, loading]); + }, [data?.notifications?.length, fetchMore, loading, whereClause]); const handleToggleUnreadOnly = (value) => { setShowUnreadOnly(value); @@ -157,7 +167,12 @@ export function NotificationCenterContainer({ visible, onClose, bodyshop }) { const timestamp = new Date().toISOString(); setNotifications((prev) => { const updatedNotifications = prev.map((notif) => - notif.read === null ? { ...notif, read: timestamp } : notif + notif.read === null && notif.associationid === userAssociationId + ? { + ...notif, + read: timestamp + } + : notif ); return [...updatedNotifications]; }); @@ -165,7 +180,6 @@ export function NotificationCenterContainer({ visible, onClose, bodyshop }) { .catch((e) => console.error(`Error marking all notifications read: ${e?.message || ""}`)); }; - // TODO Tinker useEffect(() => { if (visible && !isConnected) { refetch(); @@ -187,4 +201,8 @@ export function NotificationCenterContainer({ visible, onClose, bodyshop }) { ); } -export default connect((state) => ({ bodyshop: state.user.bodyshop }), null)(NotificationCenterContainer); +const mapStateToProps = createStructuredSelector({ + bodyshop: selectBodyshop +}); + +export default connect(mapStateToProps, null)(NotificationCenterContainer); diff --git a/client/src/contexts/SocketIO/socketContext.jsx b/client/src/contexts/SocketIO/socketContext.jsx index 6733f2d06..d82557475 100644 --- a/client/src/contexts/SocketIO/socketContext.jsx +++ b/client/src/contexts/SocketIO/socketContext.jsx @@ -1,4 +1,3 @@ -// contexts/SocketIO/socketContext.jsx import React, { createContext, useContext, useEffect, useRef, useState } from "react"; import SocketIO from "socket.io-client"; import { auth } from "../../firebase/firebase.utils"; @@ -15,10 +14,11 @@ export const SocketProvider = ({ children, bodyshop }) => { const [clientId, setClientId] = useState(null); const [isConnected, setIsConnected] = useState(false); const notification = useNotification(); + const userAssociationId = bodyshop?.associations?.[0]?.id; useEffect(() => { const initializeSocket = async (token) => { - if (!bodyshop || !bodyshop.id || socketRef.current) return; // Prevent multiple instances + if (!bodyshop || !bodyshop.id || socketRef.current) return; const endpoint = import.meta.env.PROD ? import.meta.env.VITE_APP_AXIOS_BASE_API_URL : ""; const socketInstance = SocketIO(endpoint, { @@ -41,7 +41,6 @@ export const SocketProvider = ({ children, bodyshop }) => { default: break; } - if (!import.meta.env.DEV) return; }; const handleConnect = () => { @@ -87,6 +86,10 @@ export const SocketProvider = ({ children, bodyshop }) => { const handleNotification = (data) => { const { jobId, jobRoNumber, notificationId, associationId, notifications } = data; + // Filter out notifications not matching the user's associationId + // Technically not required. + if (associationId !== userAssociationId) return; + const newNotification = { __typename: "notifications", id: notificationId, @@ -102,7 +105,11 @@ export const SocketProvider = ({ children, bodyshop }) => { } }; - const baseVariables = { limit: 20, offset: 0, where: {} }; + const baseVariables = { + limit: 20, + offset: 0, + where: { associationid: { _eq: userAssociationId } } + }; try { const existingNotifications = @@ -126,8 +133,10 @@ export const SocketProvider = ({ children, bodyshop }) => { broadcast: true }); - // Handle showUnreadOnly case - const unreadVariables = { ...baseVariables, where: { read: { _is_null: true } } }; + const unreadVariables = { + ...baseVariables, + where: { ...baseVariables.where, read: { _is_null: true } } + }; const unreadNotifications = client.cache.readQuery({ query: GET_NOTIFICATIONS, @@ -228,7 +237,7 @@ export const SocketProvider = ({ children, bodyshop }) => { setIsConnected(false); } }; - }, [bodyshop, notification]); + }, [bodyshop, notification, userAssociationId]); return ( diff --git a/client/src/graphql/notifications.queries.js b/client/src/graphql/notifications.queries.js index e9a586e39..ef8527e77 100644 --- a/client/src/graphql/notifications.queries.js +++ b/client/src/graphql/notifications.queries.js @@ -19,8 +19,8 @@ export const GET_NOTIFICATIONS = gql` `; export const GET_UNREAD_COUNT = gql` - query GetUnreadCount { - notifications_aggregate(where: { read: { _is_null: true } }) { + query GetUnreadCount($associationid: uuid!) { + notifications_aggregate(where: { read: { _is_null: true }, associationid: { _eq: $associationid } }) { aggregate { count } @@ -29,8 +29,11 @@ export const GET_UNREAD_COUNT = gql` `; export const MARK_ALL_NOTIFICATIONS_READ = gql` - mutation MarkAllNotificationsRead { - update_notifications(where: { read: { _is_null: true } }, _set: { read: "now()" }) { + mutation MarkAllNotificationsRead($associationid: uuid!) { + update_notifications( + where: { read: { _is_null: true }, associationid: { _eq: $associationid } } + _set: { read: "now()" } + ) { affected_rows } } diff --git a/server/graphql-client/queries.js b/server/graphql-client/queries.js index 4108ec1fb..14e016089 100644 --- a/server/graphql-client/queries.js +++ b/server/graphql-client/queries.js @@ -2745,3 +2745,17 @@ query GET_NOTIFICATION_ASSOCIATIONS($emails: [String!]!, $shopid: uuid!) { } } `; + +exports.INSERT_NOTIFICATIONS_MUTATION = ` mutation INSERT_NOTIFICATIONS($objects: [notifications_insert_input!]!) { + insert_notifications(objects: $objects) { + affected_rows + returning { + id + jobid + associationid + scenario_text + fcm_text + scenario_meta + } + } + }`; diff --git a/server/notifications/queues/appQueue.js b/server/notifications/queues/appQueue.js index 5cb04439a..a2e4f609b 100644 --- a/server/notifications/queues/appQueue.js +++ b/server/notifications/queues/appQueue.js @@ -1,4 +1,5 @@ const { Queue, Worker } = require("bullmq"); +const { INSERT_NOTIFICATIONS_MUTATION } = require("../../graphql-client/queries"); const graphQLClient = require("../../graphql-client/graphql-client").client; // Base time-related constant in minutes, sourced from environment variable or defaulting to 1 @@ -20,23 +21,6 @@ const RATE_LIMITER_DURATION = APP_CONSOLIDATION_DELAY * 0.1; // 6 seconds (tenth let addQueue; let consolidateQueue; -// Updated GraphQL mutation to insert notifications with the new schema -const INSERT_NOTIFICATIONS_MUTATION = ` - mutation INSERT_NOTIFICATIONS($objects: [notifications_insert_input!]!) { - insert_notifications(objects: $objects) { - affected_rows - returning { - id - jobid - associationid - scenario_text - fcm_text - scenario_meta - } - } - } -`; - /** * Builds the scenario_text, fcm_text, and scenario_meta for a batch of notifications. * @@ -165,16 +149,16 @@ const loadAppQueue = async ({ pubClient, logger, redisHelpers, ioRedis }) => { for (const [user, bodyShopData] of Object.entries(allNotifications)) { const userRecipients = recipients.filter((r) => r.user === user); - const employeeId = userRecipients[0]?.employeeId; + const associationId = userRecipients[0]?.associationId; for (const [bodyShopId, notifications] of Object.entries(bodyShopData)) { const { scenario_text, fcm_text, scenario_meta } = buildNotificationContent(notifications); notificationInserts.push({ jobid: jobId, - associationid: employeeId || null, - scenario_text: JSON.stringify(scenario_text), // JSONB requires stringified input + associationid: associationId, + scenario_text: JSON.stringify(scenario_text), fcm_text: fcm_text, - scenario_meta: JSON.stringify(scenario_meta) // JSONB requires stringified input + scenario_meta: JSON.stringify(scenario_meta) }); notificationIdMap.set(`${user}:${bodyShopId}`, null); } @@ -200,9 +184,8 @@ const loadAppQueue = async ({ pubClient, logger, redisHelpers, ioRedis }) => { // Emit notifications to users via Socket.io with notification ID for (const [user, bodyShopData] of Object.entries(allNotifications)) { const userMapping = await redisHelpers.getUserSocketMapping(user); - // Get all recipients for the user and extract the associationId (employeeId) const userRecipients = recipients.filter((r) => r.user === user); - const associationId = userRecipients[0]?.employeeId; + const associationId = userRecipients[0]?.associationId; for (const [bodyShopId, notifications] of Object.entries(bodyShopData)) { const notificationId = notificationIdMap.get(`${user}:${bodyShopId}`); diff --git a/server/notifications/scenarioBuilders.js b/server/notifications/scenarioBuilders.js index 7901f4cac..447585918 100644 --- a/server/notifications/scenarioBuilders.js +++ b/server/notifications/scenarioBuilders.js @@ -8,8 +8,8 @@ const { getJobAssignmentType } = require("./stringHelpers"); */ const populateWatchers = (data, result) => { data.scenarioWatchers.forEach((recipients) => { - const { user, app, fcm, email, firstName, lastName, employeeId } = recipients; - if (app === true) result.app.recipients.push({ user, bodyShopId: data.bodyShopId, employeeId }); + const { user, app, fcm, email, firstName, lastName, employeeId, associationId } = recipients; + if (app === true) result.app.recipients.push({ user, bodyShopId: data.bodyShopId, employeeId, associationId }); if (fcm === true) result.fcm.recipients.push(user); if (email === true) result.email.recipients.push({ user, firstName, lastName }); }); diff --git a/server/notifications/scenarioParser.js b/server/notifications/scenarioParser.js index d897f07ea..5f40f910e 100644 --- a/server/notifications/scenarioParser.js +++ b/server/notifications/scenarioParser.js @@ -31,7 +31,7 @@ const scenarioParser = async (req, jobIdField) => { const { event, trigger, table } = req.body; const { logger } = req; - // Validate we know what user commited the action that fired the parser + // 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 @@ -140,9 +140,10 @@ const scenarioParser = async (req, jobIdField) => { email: settings.email, app: settings.app, fcm: settings.fcm, - firstName: matchingWatcher ? matchingWatcher.firstName : undefined, - lastName: matchingWatcher ? matchingWatcher.lastName : undefined, - employeeId: matchingWatcher ? assoc.id : undefined + firstName: matchingWatcher?.firstName, + lastName: matchingWatcher?.lastName, + employeeId: matchingWatcher?.employeeId, + associationId: assoc.id }; }) }));