diff --git a/web/components/notifications-icon.tsx b/web/components/notifications-icon.tsx index 02fbea5b..e2618870 100644 --- a/web/components/notifications-icon.tsx +++ b/web/components/notifications-icon.tsx @@ -4,11 +4,13 @@ import { Row } from 'web/components/layout/row' import { useEffect, useState } from 'react' import { useUser } from 'web/hooks/use-user' import { useRouter } from 'next/router' -import { useNotifications } from 'web/hooks/use-notifications' +import { usePreferredGroupedNotifications } from 'web/hooks/use-notifications' export default function NotificationsIcon(props: { className?: string }) { const user = useUser() - const notifications = useNotifications(user?.id, { unseenOnly: true }) + const notifications = usePreferredGroupedNotifications(user?.id, { + unseenOnly: true, + }) const [seen, setSeen] = useState(false) const router = useRouter() @@ -21,7 +23,9 @@ export default function NotificationsIcon(props: { className?: string }) {
{!seen && notifications && notifications.length > 0 && ( -
+
+ {notifications.length} +
)}
diff --git a/web/hooks/use-notifications.ts b/web/hooks/use-notifications.ts index 0e303036..ece1a8fa 100644 --- a/web/hooks/use-notifications.ts +++ b/web/hooks/use-notifications.ts @@ -3,8 +3,68 @@ import { listenForPrivateUser } from 'web/lib/firebase/users' import { notification_subscribe_types, PrivateUser } from 'common/user' import { Notification } from 'common/notification' import { listenForNotifications } from 'web/lib/firebase/notifications' +import { groupBy, map } from 'lodash' -export function useNotifications( +export type NotificationGroup = { + notifications: Notification[] + sourceContractId: string + isSeen: boolean + timePeriod: string +} + +export function usePreferredGroupedNotifications( + userId: string | undefined, + options: { unseenOnly: boolean } +) { + const [notificationGroups, setNotificationGroups] = useState< + NotificationGroup[] + >([]) + + const notifications = usePreferredNotifications(userId, options) + useEffect(() => { + if (!notifications) return + + const groupedNotifications = groupNotifications(notifications) + setNotificationGroups(groupedNotifications) + }, [notifications]) + + return notificationGroups +} + +export function groupNotifications(notifications: Notification[]) { + let notificationGroups: NotificationGroup[] = [] + const notificationGroupsByDay = groupBy(notifications, (notification) => + new Date(notification.createdTime).toDateString() + ) + Object.keys(notificationGroupsByDay).forEach((day) => { + // Group notifications by contract: + const groupedNotificationsByContractId = groupBy( + notificationGroupsByDay[day], + (notification) => { + return notification.sourceContractId + } + ) + notificationGroups = notificationGroups.concat( + map(groupedNotificationsByContractId, (notifications, contractId) => { + // Create a notification group for each contract within each day + const notificationGroup: NotificationGroup = { + notifications: groupedNotificationsByContractId[contractId].sort( + (a, b) => { + return b.createdTime - a.createdTime + } + ), + sourceContractId: contractId, + isSeen: groupedNotificationsByContractId[contractId][0].isSeen, + timePeriod: day, + } + return notificationGroup + }) + ) + }) + return notificationGroups +} + +function usePreferredNotifications( userId: string | undefined, options: { unseenOnly: boolean } ) { @@ -25,7 +85,7 @@ export function useNotifications( setNotifications, unseenOnly ) - }, [privateUser]) + }, [privateUser, unseenOnly]) useEffect(() => { if (!privateUser) return diff --git a/web/pages/notifications.tsx b/web/pages/notifications.tsx index 6ceff130..98499bd2 100644 --- a/web/pages/notifications.tsx +++ b/web/pages/notifications.tsx @@ -5,6 +5,7 @@ import { Notification, notification_reason_types, notification_source_types, + notification_source_update_types, } from 'common/notification' import { Avatar } from 'web/components/avatar' import { Row } from 'web/components/layout/row' @@ -25,7 +26,6 @@ import { ChoicesToggleGroup } from 'web/components/choices-toggle-group' import { listenForPrivateUser, updatePrivateUser } from 'web/lib/firebase/users' import { LoadingIndicator } from 'web/components/loading-indicator' import clsx from 'clsx' -import { groupBy, map } from 'lodash' import { UsersIcon } from '@heroicons/react/solid' import { RelativeTimestamp } from 'web/components/relative-timestamp' import { Linkify } from 'web/components/linkify' @@ -33,70 +33,46 @@ import { FreeResponseOutcomeLabel, OutcomeLabel, } from 'web/components/outcome-label' -import { useNotifications } from 'web/hooks/use-notifications' +import { + groupNotifications, + NotificationGroup, + usePreferredGroupedNotifications, +} from 'web/hooks/use-notifications' import { getContractFromId } from 'web/lib/firebase/contracts' import { CheckIcon, XIcon } from '@heroicons/react/outline' import toast from 'react-hot-toast' -type NotificationGroup = { - notifications: Notification[] - sourceContractId: string - isSeen: boolean - timePeriod: string -} - export default function Notifications() { const user = useUser() - const [allNotificationGroups, setAllNotificationsGroups] = useState< - NotificationGroup[] - >([]) const [unseenNotificationGroups, setUnseenNotificationGroups] = useState< NotificationGroup[] >([]) - const notifications = useNotifications(user?.id, { unseenOnly: false }) + const allNotificationGroups = usePreferredGroupedNotifications(user?.id, { + unseenOnly: false, + }) useEffect(() => { - const notificationIdsToShow = notifications.map( - (notification) => notification.id - ) - // Hide notifications the user doesn't want to see. - const notificationIdsToHide = notifications - .filter( - (notification) => !notificationIdsToShow.includes(notification.id) - ) - .map((notification) => notification.id) - - // Because hidden notifications won't be rendered, set them to seen here - setNotificationsAsSeen( - notifications.filter((n) => notificationIdsToHide.includes(n.id)) - ) - - // Group notifications by contract and 24-hour time period. - const allGroupedNotifications = groupNotifications( - notifications, - notificationIdsToHide - ) - - // Don't add notifications that are already visible or have been seen. + // Don't re-add notifications that are visible right now or have been seen already. const currentlyVisibleUnseenNotificationIds = Object.values( unseenNotificationGroups ) .map((n) => n.notifications.map((n) => n.id)) .flat() const unseenGroupedNotifications = groupNotifications( - notifications.filter( - (notification) => - !notification.isSeen || - currentlyVisibleUnseenNotificationIds.includes(notification.id) - ), - notificationIdsToHide + allNotificationGroups + .map((notification: NotificationGroup) => notification.notifications) + .flat() + .filter( + (notification: Notification) => + !notification.isSeen || + currentlyVisibleUnseenNotificationIds.includes(notification.id) + ) ) - setAllNotificationsGroups(allGroupedNotifications) setUnseenNotificationGroups(unseenGroupedNotifications) // We don't want unseenNotificationsGroup to be in the dependencies as we update it here. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [notifications]) + }, [allNotificationGroups]) if (user === undefined) { return @@ -129,7 +105,10 @@ export default function Notifications() { ) : ( ) )} @@ -151,7 +130,10 @@ export default function Notifications() { ) : ( ) )} @@ -188,47 +170,6 @@ const setNotificationsAsSeen = (notifications: Notification[]) => { return notifications } -function groupNotifications( - notifications: Notification[], - hideNotificationIds: string[] -) { - // Then remove them from the list of notifications to show - notifications = notifications.filter( - (notification) => !hideNotificationIds.includes(notification.id) - ) - - let notificationGroups: NotificationGroup[] = [] - const notificationGroupsByDay = groupBy(notifications, (notification) => - new Date(notification.createdTime).toDateString() - ) - Object.keys(notificationGroupsByDay).forEach((day) => { - // Group notifications by contract: - const groupedNotificationsByContractId = groupBy( - notificationGroupsByDay[day], - (notification) => { - return notification.sourceContractId - } - ) - notificationGroups = notificationGroups.concat( - map(groupedNotificationsByContractId, (notifications, contractId) => { - // Create a notification group for each contract within each day - const notificationGroup: NotificationGroup = { - notifications: groupedNotificationsByContractId[contractId].sort( - (a, b) => { - return b.createdTime - a.createdTime - } - ), - sourceContractId: contractId, - isSeen: groupedNotificationsByContractId[contractId][0].isSeen, - timePeriod: day, - } - return notificationGroup - }) - ) - }) - return notificationGroups -} - function NotificationGroupItem(props: { notificationGroup: NotificationGroup className?: string @@ -280,16 +221,15 @@ function NotificationGroupItem(props: {
{!expanded ? ( <> - {notifications - .slice(0, numSummaryLines) - .map((notification, i) => { - return ( - - ) - })} + {notifications.slice(0, numSummaryLines).map((notification) => { + return ( + + ) + })}
{notifications.length - numSummaryLines > 0 ? 'And ' + @@ -300,7 +240,7 @@ function NotificationGroupItem(props: { ) : ( <> - {notifications.map((notification, i) => ( + {notifications.map((notification) => ( void +function isNotificationAboutContractResolution( + sourceType: notification_source_types | undefined, + sourceUpdateType: notification_source_update_types | undefined, + contract: Contract | null | undefined ) { - if (sourceType === 'answer') { - const answer = await getValue( - doc(db, `contracts/${sourceContractId}/answers/`, sourceId) - ) - setText(answer?.text ?? '') - } else { - const comment = await getValue( - doc(db, `contracts/${sourceContractId}/comments/`, sourceId) - ) - setText(comment?.text ?? '') - } + return ( + (sourceType === 'contract' && !sourceUpdateType && contract?.resolution) || + (sourceType === 'contract' && + sourceUpdateType === 'resolved' && + contract?.resolution) + ) } function NotificationItem(props: { @@ -547,6 +481,7 @@ function NotificationItem(props: { sourceId, sourceUserName, sourceUserAvatarUrl, + sourceUpdateType, reasonText, reason, sourceUserUsername, @@ -554,6 +489,7 @@ function NotificationItem(props: { } = notification const [notificationText, setNotificationText] = useState('') const [contract, setContract] = useState(null) + useEffect(() => { if (!sourceContractId) return getContractFromId(sourceContractId).then((contract) => { @@ -562,27 +498,32 @@ function NotificationItem(props: { }, [sourceContractId]) useEffect(() => { - if (!contract || !sourceContractId) return - if (sourceType === 'contract') { - // We don't handle anything other than contract updates & resolution yet. - if (contract.resolution) setNotificationText(contract.resolution) - else setNotificationText(contract.question) - return - } - if (!sourceId) return - - if (sourceType === 'answer' || sourceType === 'comment') { - getNotificationSummaryText( + if (!contract || !sourceContractId || !sourceId) return + if ( + sourceType === 'answer' || + sourceType === 'comment' || + sourceType === 'contract' + ) { + getNotificationText( sourceId, sourceContractId, sourceType, - setNotificationText + sourceUpdateType, + setNotificationText, + contract ) } else if (reasonText) { // Handle arbitrary notifications with reason text here. setNotificationText(reasonText) } - }, [contract, reasonText, sourceContractId, sourceId, sourceType]) + }, [ + contract, + reasonText, + sourceContractId, + sourceId, + sourceType, + sourceUpdateType, + ]) useEffect(() => { setNotificationsAsSeen([notification]) @@ -608,8 +549,39 @@ function NotificationItem(props: { } } - function isNotificationContractResolution() { - return sourceType === 'contract' && contract?.resolution + async function getNotificationText( + sourceId: string, + sourceContractId: string, + sourceType: 'answer' | 'comment' | 'contract', + sourceUpdateType: notification_source_update_types | undefined, + setText: (text: string) => void, + contract?: Contract + ) { + if (sourceType === 'contract' && !contract) + contract = await getContractFromId(sourceContractId) + + if (sourceType === 'contract' && contract) { + if ( + isNotificationAboutContractResolution( + sourceType, + sourceUpdateType, + contract + ) && + contract?.resolution + ) + setText(contract.resolution) + else setText(contract.question) + } else if (sourceType === 'answer') { + const answer = await getValue( + doc(db, `contracts/${sourceContractId}/answers/`, sourceId) + ) + setText(answer?.text ?? '') + } else { + const comment = await getValue( + doc(db, `contracts/${sourceContractId}/comments/`, sourceId) + ) + setText(comment?.text ?? '') + } } if (justSummary) { @@ -625,9 +597,10 @@ function NotificationItem(props: {
{sourceType && reason && - getReasonTextFromReason( + getReasonForShowingNotification( sourceType, reason, + sourceUpdateType, contract, true ).replace(' on', '')} @@ -635,8 +608,10 @@ function NotificationItem(props: { {contract ? ( ) : sourceType != 'follow' ? ( - - - - + + +
+
+ +
+ {sourceType && reason && ( +
+ {getReasonForShowingNotification( + sourceType, + reason, + sourceUpdateType, + contract + )} + + {contract?.question} + +
+ )} +
+
+ {contract && sourceId && ( + + )} +
+
- {isNotificationContractResolution() && ' Resolved:'}{' '} {contract ? ( ) : sourceType != 'follow' ? ( @@ -718,17 +698,22 @@ function NotificationItem(props: { function NotificationTextLabel(props: { contract: Contract - notificationText: string + defaultText: string + sourceType?: notification_source_types + sourceUpdateType?: notification_source_update_types className?: string }) { - const { contract, notificationText, className } = props - if (notificationText === contract.question) { - return ( -
- {notificationText} -
- ) - } else if (notificationText === contract.resolution) { + const { contract, className, sourceUpdateType, sourceType, defaultText } = + props + + if ( + isNotificationAboutContractResolution( + sourceType, + sourceUpdateType, + contract + ) && + contract.resolution + ) { if (contract.outcomeType === 'FREE_RESPONSE') { return ( ) + } else if (sourceType === 'contract') { + return ( +
+ {defaultText} +
+ ) } else { return (
- +
) } } -function getReasonTextFromReason( +function getReasonForShowingNotification( source: notification_source_types, reason: notification_reason_types, + sourceUpdateType: notification_source_update_types | undefined, contract: Contract | undefined | null, simple?: boolean ) { - let reasonText = '' + let reasonText: string switch (source) { case 'comment': if (reason === 'reply_to_users_answer') @@ -783,7 +775,14 @@ function getReasonTextFromReason( else reasonText = `commented on` break case 'contract': - if (contract?.resolution) reasonText = `resolved` + if ( + isNotificationAboutContractResolution( + source, + sourceUpdateType, + contract + ) + ) + reasonText = `resolved` else reasonText = `updated` break case 'answer':