From 03f4f03244a47e8dc1f7ad6fc44a6fbbb75827d9 Mon Sep 17 00:00:00 2001 From: Renaud Chaput Date: Fri, 14 Jun 2024 15:30:26 +0200 Subject: [PATCH] Better handle loading notifications from a Gap --- .../mastodon/actions/notification_groups.ts | 78 ++++++++++++------- .../features/notifications_v2/index.tsx | 13 ++-- .../mastodon/reducers/notifications_groups.ts | 67 ++++++++++++---- 3 files changed, 110 insertions(+), 48 deletions(-) diff --git a/app/javascript/mastodon/actions/notification_groups.ts b/app/javascript/mastodon/actions/notification_groups.ts index 81699f63f9..01c0c7ef2f 100644 --- a/app/javascript/mastodon/actions/notification_groups.ts +++ b/app/javascript/mastodon/actions/notification_groups.ts @@ -8,6 +8,7 @@ import { selectSettingsNotificationsExcludedTypes, selectSettingsNotificationsQuickFilterActive, } from 'mastodon/selectors/settings'; +import type { AppDispatch } from 'mastodon/store'; import { createAppAsyncThunk, createDataLoadingThunk, @@ -21,11 +22,41 @@ function excludeAllTypesExcept(filter: string) { return allNotificationTypes.filter((item) => item !== filter); } +function dispatchAssociatedRecords( + dispatch: AppDispatch, + notifications: NotificationGroupJSON[], +) { + const fetchedAccounts: ApiAccountJSON[] = []; + const fetchedStatuses: ApiStatusJSON[] = []; + + notifications.forEach((notification) => { + if ('sample_accounts' in notification) { + fetchedAccounts.push(...notification.sample_accounts); + } + + if (notification.type === 'admin.report') { + fetchedAccounts.push(notification.report.target_account); + } + + if (notification.type === 'moderation_warning') { + fetchedAccounts.push(notification.moderation_warning.target_account); + } + + if ('status' in notification) { + fetchedStatuses.push(notification.status); + } + }); + + if (fetchedAccounts.length > 0) + dispatch(importFetchedAccounts(fetchedAccounts)); + + if (fetchedStatuses.length > 0) + dispatch(importFetchedStatuses(fetchedStatuses)); +} + export const fetchNotifications = createDataLoadingThunk( 'notificationGroups/fetch', - async (params: { url?: string } | undefined, { getState }) => { - if (params?.url) return apiFetchNotifications({}, params.url); - + async (_params, { getState }) => { const activeFilter = selectSettingsNotificationsQuickFilterActive(getState()); @@ -37,37 +68,12 @@ export const fetchNotifications = createDataLoadingThunk( }); }, ({ notifications, links }, { dispatch }) => { - const fetchedAccounts: ApiAccountJSON[] = []; - const fetchedStatuses: ApiStatusJSON[] = []; + dispatchAssociatedRecords(dispatch, notifications); // We ignore the previous link, as it will always be here but we know there are no more // recent notifications when doing the initial load const nextLink = links.refs.find((link) => link.rel === 'next'); - notifications.forEach((notification) => { - if ('sample_accounts' in notification) { - fetchedAccounts.push(...notification.sample_accounts); - } - - if (notification.type === 'admin.report') { - fetchedAccounts.push(notification.report.target_account); - } - - if (notification.type === 'moderation_warning') { - fetchedAccounts.push(notification.moderation_warning.target_account); - } - - if ('status' in notification) { - fetchedStatuses.push(notification.status); - } - }); - - if (fetchedAccounts.length > 0) - dispatch(importFetchedAccounts(fetchedAccounts)); - - if (fetchedStatuses.length > 0) - dispatch(importFetchedStatuses(fetchedStatuses)); - const payload: (NotificationGroupJSON | NotificationGap)[] = notifications; if (nextLink) payload.push({ type: 'gap', loadUrl: nextLink.uri }); @@ -77,6 +83,20 @@ export const fetchNotifications = createDataLoadingThunk( }, ); +export const fetchNotificationsGap = createDataLoadingThunk( + 'notificationGroups/fetchGat', + async (params: { gap: NotificationGap }) => + apiFetchNotifications({}, params.gap.loadUrl), + + ({ notifications, links }, { dispatch }) => { + dispatchAssociatedRecords(dispatch, notifications); + + const nextLink = links.refs.find((link) => link.rel === 'next'); + + return { notifications, nextLink }; + }, +); + export const setNotificationsFilter = createAppAsyncThunk( 'notifications/filter/set', ({ filterType }: { filterType: string }, { dispatch }) => { diff --git a/app/javascript/mastodon/features/notifications_v2/index.tsx b/app/javascript/mastodon/features/notifications_v2/index.tsx index e701d71bc1..7feefe6358 100644 --- a/app/javascript/mastodon/features/notifications_v2/index.tsx +++ b/app/javascript/mastodon/features/notifications_v2/index.tsx @@ -10,11 +10,15 @@ import { useDebouncedCallback } from 'use-debounce'; import DoneAllIcon from '@/material-icons/400-24px/done_all.svg?react'; import NotificationsIcon from '@/material-icons/400-24px/notifications-fill.svg?react'; -import { fetchNotifications } from 'mastodon/actions/notification_groups'; +import { + fetchNotifications, + fetchNotificationsGap, +} from 'mastodon/actions/notification_groups'; import { compareId } from 'mastodon/compare_id'; import { Icon } from 'mastodon/components/icon'; import { NotSignedInIndicator } from 'mastodon/components/not_signed_in_indicator'; import { useIdentity } from 'mastodon/identity_context'; +import type { NotificationGap } from 'mastodon/reducers/notifications_groups'; import { selectNeedsNotificationPermission, selectSettingsNotificationsExcludedTypes, @@ -165,9 +169,8 @@ export const Notifications: React.FC<{ }, [dispatch]); const handleLoadGap = useCallback( - (loadUrl: string) => { - // TODO: this should not be fetch (as this overrides the existing notifications), but expand? - void dispatch(fetchNotifications({ url: loadUrl })); + (gap: NotificationGap) => { + void dispatch(fetchNotificationsGap({ gap })); }, [dispatch], ); @@ -269,7 +272,7 @@ export const Notifications: React.FC<{ ) : ( diff --git a/app/javascript/mastodon/reducers/notifications_groups.ts b/app/javascript/mastodon/reducers/notifications_groups.ts index 1d6cf54420..0e1908c84b 100644 --- a/app/javascript/mastodon/reducers/notifications_groups.ts +++ b/app/javascript/mastodon/reducers/notifications_groups.ts @@ -1,6 +1,9 @@ -import { createReducer } from '@reduxjs/toolkit'; +import { createReducer, isAnyOf } from '@reduxjs/toolkit'; -import { fetchNotifications } from 'mastodon/actions/notification_groups'; +import { + fetchNotifications, + fetchNotificationsGap, +} from 'mastodon/actions/notification_groups'; import { createNotificationGroupFromJSON } from 'mastodon/models/notification_group'; import type { NotificationGroup } from 'mastodon/models/notification_group'; @@ -28,19 +31,55 @@ const initialState: NotificationGroupsState = { export const notificationGroupsReducer = createReducer( initialState, (builder) => { - builder.addCase(fetchNotifications.pending, (state) => { - state.isLoading = true; - }); + builder + .addCase(fetchNotifications.fulfilled, (state, action) => { + state.groups = action.payload.map((json) => + json.type === 'gap' ? json : createNotificationGroupFromJSON(json), + ); + state.isLoading = false; + }) + .addCase(fetchNotificationsGap.fulfilled, (state, action) => { + const { notifications, nextLink } = action.payload; - builder.addCase(fetchNotifications.fulfilled, (state, action) => { - state.groups = action.payload.map((json) => - json.type === 'gap' ? json : createNotificationGroupFromJSON(json), + // find the gap in the existing notifications + const gapIndex = state.groups.findIndex( + (groupOrGap) => + groupOrGap.type === 'gap' && groupOrGap.loadUrl === nextLink?.uri, + ); + + if (!gapIndex) + // We do not know where to insert, let's return + return; + + // replace the gap with the notifications + a new gap + + const toInsert: NotificationGroupsState['groups'] = notifications.map( + (json) => createNotificationGroupFromJSON(json), + ); + + if (nextLink?.uri && notifications.length > 0) { + // If we get an empty page, it means we reached the bottom, so we do not need to insert a new gap + toInsert.push({ + type: 'gap', + loadUrl: nextLink.uri, + } as NotificationGap); + } + + state.groups.splice(gapIndex, 1, ...toInsert); + + state.isLoading = false; + }) + .addMatcher( + isAnyOf(fetchNotifications.pending, fetchNotificationsGap.pending), + (state) => { + state.isLoading = true; + }, + ) + .addMatcher( + isAnyOf(fetchNotifications.rejected, fetchNotificationsGap.rejected), + (state) => { + state.isLoading = false; + }, ); - state.isLoading = false; - }); - - builder.addCase(fetchNotifications.rejected, (state) => { - state.isLoading = false; - }); }, );