Better handle loading notifications from a Gap

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 }) => {

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<{
         <LoadGap
           key={item.loadUrl}
           disabled={isLoading}
-          maxId={item.loadUrl}
+          param={item}
           onClick={handleLoadGap}
         />
       ) : (

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<NotificationGroupsState>(
   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;
-    });
   },
 );