[Mobile] Fix uncategorized banner + overspent banner showing previously active month's overspent categories (#4875)

* [Mobile] Fix overspent banner showing previously active month's overspent categories

* Update packages/desktop-client/src/hooks/useOverspentCategories.ts

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Fix typecheck error

* Enable uncategorized banner and overspent banner for tracking budgets

* Fix lint error

* Update VRT

* Dummy commit

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

packages/desktop-client/src/components/mobile/budget/BudgetTable.jsx

@@ -1,10 +1,4 @@
-import React, {
-  useCallback,
-  useContext,
-  useEffect,
-  useMemo,
-  useState,
-} from 'react';
+import React, { useCallback, useMemo } from 'react';
 import { GridList, GridListItem } from 'react-aria-components';
 import { Trans, useTranslation } from 'react-i18next';
 
@@ -30,13 +24,12 @@ import { View } from '@actual-app/components/view';
 import { AutoTextSize } from 'auto-text-size';
 import memoizeOne from 'memoize-one';
 
-import { collapseModals, pushModal } from 'loot-core/client/modals/modalsSlice';
+import { pushModal } from 'loot-core/client/modals/modalsSlice';
 import {
   envelopeBudget,
   trackingBudget,
   uncategorizedCount,
 } from 'loot-core/client/queries';
-import { useSpreadsheet } from 'loot-core/client/SpreadsheetProvider';
 import * as monthUtils from 'loot-core/shared/months';
 import { groupById } from 'loot-core/shared/util';
 
@@ -44,14 +37,13 @@ import { useCategories } from '../../../hooks/useCategories';
 import { useLocale } from '../../../hooks/useLocale';
 import { useLocalPref } from '../../../hooks/useLocalPref';
 import { useNavigate } from '../../../hooks/useNavigate';
-import { usePrevious } from '../../../hooks/usePrevious';
+import { useOverspentCategories } from '../../../hooks/useOverspentCategories';
 import { useSyncedPref } from '../../../hooks/useSyncedPref';
 import { useUndo } from '../../../hooks/useUndo';
 import { useDispatch } from '../../../redux';
 import { MobilePageHeader, Page } from '../../Page';
 import { PrivacyFilter } from '../../PrivacyFilter';
 import { CellValue } from '../../spreadsheet/CellValue';
-import { NamespaceContext } from '../../spreadsheet/NamespaceContext';
 import { useFormat } from '../../spreadsheet/useFormat';
 import { useSheetValue } from '../../spreadsheet/useSheetValue';
 import { MOBILE_NAV_HEIGHT } from '../MobileNavTabs';
@@ -574,100 +566,19 @@ function OverspendingBanner({ month, onBudgetAction, ...props }) {
   const { list: categories, grouped: categoryGroups } = useCategories();
   const categoriesById = groupById(categories);
 
-  const categoryBalanceBindings = useMemo(
-    () =>
-      categories.map(category => [
-        category.id,
-        envelopeBudget.catBalance(category.id),
-      ]),
-    [categories],
-  );
-
-  const categoryCarryoverBindings = useMemo(
-    () =>
-      categories.map(category => [
-        category.id,
-        envelopeBudget.catCarryover(category.id),
-      ]),
-    [categories],
-  );
-
-  const [overspentByCategory, setOverspentByCategory] = useState({});
-  const [carryoverFlagByCategory, setCarryoverFlagByCategory] = useState({});
-  const sheetName = useContext(NamespaceContext);
-  const spreadsheet = useSpreadsheet();
-
-  useEffect(() => {
-    const unbindList = [];
-    for (const [categoryId, carryoverBinding] of categoryCarryoverBindings) {
-      const unbind = spreadsheet.bind(sheetName, carryoverBinding, result => {
-        const isRolloverEnabled = Boolean(result.value);
-        if (isRolloverEnabled) {
-          setCarryoverFlagByCategory(prev => ({
-            ...prev,
-            [categoryId]: result.value,
-          }));
-        } else {
-          // Update to remove covered category.
-          setCarryoverFlagByCategory(prev => {
-            const { [categoryId]: _, ...rest } = prev;
-            return rest;
-          });
-        }
-      });
-      unbindList.push(unbind);
-    }
-
-    return () => {
-      unbindList.forEach(unbind => unbind());
-    };
-  }, [categoryCarryoverBindings, sheetName, spreadsheet]);
-
-  useEffect(() => {
-    const unbindList = [];
-    for (const [categoryId, balanceBinding] of categoryBalanceBindings) {
-      const unbind = spreadsheet.bind(sheetName, balanceBinding, result => {
-        if (result.value < 0) {
-          setOverspentByCategory(prev => ({
-            ...prev,
-            [categoryId]: result.value,
-          }));
-        } else if (result.value === 0) {
-          // Update to remove covered category.
-          setOverspentByCategory(prev => {
-            const { [categoryId]: _, ...rest } = prev;
-            return rest;
-          });
-        }
-      });
-      unbindList.push(unbind);
-    }
-
-    return () => {
-      unbindList.forEach(unbind => unbind());
-    };
-  }, [categoryBalanceBindings, sheetName, spreadsheet]);
-
   const dispatch = useDispatch();
 
-  // Ignore those that has rollover enabled.
-  const overspentCategoryIds = Object.keys(overspentByCategory).filter(
-    id => !carryoverFlagByCategory[id],
-  );
+  const overspentCategories = useOverspentCategories({ month });
 
   const categoryGroupsToShow = useMemo(
     () =>
       categoryGroups
-        .filter(g =>
-          g.categories?.some(c => overspentCategoryIds.includes(c.id)),
-        )
+        .filter(g => overspentCategories.some(c => c.group === g.id))
         .map(g => ({
           ...g,
-          categories:
-            g.categories?.filter(c => overspentCategoryIds.includes(c.id)) ||
-            [],
+          categories: overspentCategories.filter(c => c.group === g.id),
         })),
-    [categoryGroups, overspentCategoryIds],
+    [categoryGroups, overspentCategories],
   );
 
   const { showUndoNotification } = useUndo();
@@ -728,29 +639,7 @@ function OverspendingBanner({ month, onBudgetAction, ...props }) {
     );
   }, [categoryGroupsToShow, dispatch, month, onOpenCoverCategoryModal, t]);
 
-  const numberOfOverspentCategories = overspentCategoryIds.length;
-  const previousNumberOfOverspentCategories = usePrevious(
-    numberOfOverspentCategories,
-  );
-
-  useEffect(() => {
-    if (numberOfOverspentCategories < previousNumberOfOverspentCategories) {
-      // Re-render the modal when the overspent categories are covered.
-      dispatch(collapseModals({ rootModalName: 'category-autocomplete' }));
-      onOpenCategorySelectionModal();
-
-      // All overspent categories have been covered.
-      if (numberOfOverspentCategories === 0) {
-        dispatch(collapseModals({ rootModalName: 'category-autocomplete' }));
-      }
-    }
-  }, [
-    dispatch,
-    onOpenCategorySelectionModal,
-    numberOfOverspentCategories,
-    previousNumberOfOverspentCategories,
-  ]);
-
+  const numberOfOverspentCategories = overspentCategories.length;
   if (numberOfOverspentCategories === 0) {
     return null;
   }
@@ -792,11 +681,6 @@ function Banners({ month, onBudgetAction }) {
   const { t } = useTranslation();
   const [budgetType = 'rollover'] = useSyncedPref('budgetType');
 
-  // Limit to rollover for now.
-  if (budgetType !== 'rollover') {
-    return null;
-  }
-
   return (
     <GridList
       aria-label={t('Banners')}
@@ -804,7 +688,9 @@ function Banners({ month, onBudgetAction }) {
     >
       <UncategorizedTransactionsBanner />
       <OverspendingBanner month={month} onBudgetAction={onBudgetAction} />
-      <OverbudgetedBanner month={month} onBudgetAction={onBudgetAction} />
+      {budgetType === 'rollover' && (
+        <OverbudgetedBanner month={month} onBudgetAction={onBudgetAction} />
+      )}
     </GridList>
   );
 }

packages/desktop-client/src/hooks/useOverspentCategories.ts

@@ -0,0 +1,118 @@
+import { useEffect, useMemo, useState } from 'react';
+
+import { envelopeBudget, trackingBudget } from 'loot-core/client/queries';
+import { useSpreadsheet } from 'loot-core/client/SpreadsheetProvider';
+import * as monthUtils from 'loot-core/shared/months';
+
+import { useCategories } from './useCategories';
+import { useSyncedPref } from './useSyncedPref';
+
+type UseOverspentCategoriesProps = {
+  month: string;
+};
+
+export function useOverspentCategories({ month }: UseOverspentCategoriesProps) {
+  const spreadsheet = useSpreadsheet();
+  const [budgetType = 'rollover'] = useSyncedPref('budgetType');
+
+  const { list: categories } = useCategories();
+
+  const categoryBalanceBindings = useMemo(
+    () =>
+      categories.map(category => [
+        category.id,
+        budgetType === 'rollover'
+          ? envelopeBudget.catBalance(category.id)
+          : trackingBudget.catBalance(category.id),
+      ]),
+    [budgetType, categories],
+  );
+
+  const categoryCarryoverBindings = useMemo(
+    () =>
+      categories.map(category => [
+        category.id,
+        budgetType === 'rollover'
+          ? envelopeBudget.catCarryover(category.id)
+          : trackingBudget.catCarryover(category.id),
+      ]),
+    [budgetType, categories],
+  );
+
+  const [overspentByCategory, setOverspentByCategory] = useState<
+    Record<string, number>
+  >({});
+  const [carryoverFlagByCategory, setCarryoverFlagByCategory] = useState<
+    Record<string, boolean>
+  >({});
+
+  useEffect(() => {
+    setOverspentByCategory({});
+    setCarryoverFlagByCategory({});
+  }, [month]);
+
+  const sheetName = monthUtils.sheetForMonth(month);
+
+  useEffect(() => {
+    const unbindList: (() => void)[] = [];
+    for (const [categoryId, carryoverBinding] of categoryCarryoverBindings) {
+      const unbind = spreadsheet.bind(sheetName, carryoverBinding, result => {
+        const isRolloverEnabled = Boolean(result.value);
+        if (isRolloverEnabled) {
+          setCarryoverFlagByCategory(prev => ({
+            ...prev,
+            [categoryId]: isRolloverEnabled,
+          }));
+        } else {
+          // Update to remove covered category.
+          setCarryoverFlagByCategory(prev => {
+            const { [categoryId]: _, ...rest } = prev;
+            return rest;
+          });
+        }
+      });
+      unbindList.push(unbind);
+    }
+
+    return () => {
+      unbindList.forEach(unbind => unbind());
+    };
+  }, [categoryCarryoverBindings, sheetName, spreadsheet]);
+
+  useEffect(() => {
+    const unbindList: (() => void)[] = [];
+    for (const [categoryId, balanceBinding] of categoryBalanceBindings) {
+      const unbind = spreadsheet.bind(sheetName, balanceBinding, result => {
+        const balance = result.value as number;
+        if (balance < 0) {
+          setOverspentByCategory(prev => ({
+            ...prev,
+            [categoryId]: balance,
+          }));
+        } else if (balance >= 0) {
+          // Update to remove covered category.
+          setOverspentByCategory(prev => {
+            const { [categoryId]: _, ...rest } = prev;
+            return rest;
+          });
+        }
+      });
+      unbindList.push(unbind);
+    }
+
+    return () => {
+      unbindList.forEach(unbind => unbind());
+    };
+  }, [categoryBalanceBindings, sheetName, spreadsheet]);
+
+  // Ignore those that has rollover enabled.
+  const overspentCategoryIds = Object.keys(overspentByCategory).filter(
+    id => !carryoverFlagByCategory[id],
+  );
+
+  return useMemo(
+    () =>
+      categories.filter(category => overspentCategoryIds.includes(category.id)),
+    [categories, overspentCategoryIds],
+  );
+}

upcoming-release-notes/4875.md

@@ -0,0 +1,6 @@
+---
+category: Bugfix
+authors: [joel-jeremy]
+---
+
+[Mobile] Fix overspent banner showing previously active month's overspent categories.