From e745a4073bd0738f0c31ed8b56ac51492eb2affc Mon Sep 17 00:00:00 2001
From: Neil <55785687+carkom@users.noreply.github.com>
Date: Tue, 16 Jan 2024 20:14:38 +0000
Subject: [PATCH] Custom Reports: fix table rendering (#2192)

* reorg

* notes

* Add render

* privacy Filter additions

* merge fixes

* notes

* merge fixes

* Apply patches for strict mode

* review fixes

---------

Co-authored-by: Matiss Janis Aboltins <matiss@mja.lv>
---
 .../src/components/common/Block.tsx           |   3 +
 .../src/components/reports/ChooseGraph.tsx    |  23 +-
 .../src/components/reports/ReportOptions.ts   |   6 +-
 .../src/components/reports/entities.d.ts      |   8 +-
 .../reports/graphs/tableGraph/ReportTable.tsx |  78 ++++-
 .../graphs/tableGraph/ReportTableHeader.tsx   |   6 +-
 .../graphs/tableGraph/ReportTableList.tsx     | 298 +++++-------------
 .../graphs/tableGraph/ReportTableRow.tsx      | 135 ++++++++
 .../graphs/tableGraph/ReportTableTotals.tsx   |   2 +
 upcoming-release-notes/2192.md                |   6 +
 10 files changed, 301 insertions(+), 264 deletions(-)
 create mode 100644 packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableRow.tsx
 create mode 100644 upcoming-release-notes/2192.md

diff --git a/packages/desktop-client/src/components/common/Block.tsx b/packages/desktop-client/src/components/common/Block.tsx
index a1603aef0..7be0dd359 100644
--- a/packages/desktop-client/src/components/common/Block.tsx
+++ b/packages/desktop-client/src/components/common/Block.tsx
@@ -2,8 +2,11 @@ import { type HTMLProps, type Ref } from 'react';
 
 import { css } from 'glamor';
 
+import { type CSSProperties } from '../../style';
+
 type BlockProps = HTMLProps<HTMLDivElement> & {
   innerRef?: Ref<HTMLDivElement>;
+  style?: CSSProperties;
 };
 
 export function Block(props: BlockProps) {
diff --git a/packages/desktop-client/src/components/reports/ChooseGraph.tsx b/packages/desktop-client/src/components/reports/ChooseGraph.tsx
index c02c5d889..2019433b4 100644
--- a/packages/desktop-client/src/components/reports/ChooseGraph.tsx
+++ b/packages/desktop-client/src/components/reports/ChooseGraph.tsx
@@ -12,7 +12,6 @@ import { LineGraph } from './graphs/LineGraph';
 import { StackedBarGraph } from './graphs/StackedBarGraph';
 import { ReportTable } from './graphs/tableGraph/ReportTable';
 import { ReportTableHeader } from './graphs/tableGraph/ReportTableHeader';
-import { ReportTableList } from './graphs/tableGraph/ReportTableList';
 import { ReportTableTotals } from './graphs/tableGraph/ReportTableTotals';
 import { ReportOptions } from './ReportOptions';
 
@@ -42,6 +41,12 @@ export function ChooseGraph({
   viewLabels,
 }: ChooseGraphProps) {
   const balanceTypeOp = ReportOptions.balanceTypeMap.get(balanceType);
+  const groupByData =
+    groupBy === 'Category'
+      ? 'groupedData'
+      : ['Month', 'Year'].includes(groupBy)
+      ? 'monthData'
+      : 'data';
 
   const saveScrollWidth = value => {
     setScrollWidth(!value ? 0 : value);
@@ -128,16 +133,12 @@ export function ChooseGraph({
           saveScrollWidth={saveScrollWidth}
           listScrollRef={listScrollRef}
           handleScroll={handleScroll}
-        >
-          <ReportTableList
-            data={data}
-            empty={showEmpty}
-            monthsCount={months.length}
-            balanceTypeOp={balanceTypeOp}
-            mode={mode}
-            groupBy={groupBy}
-          />
-        </ReportTable>
+          balanceTypeOp={balanceTypeOp}
+          groupBy={groupBy}
+          data={data[groupByData]}
+          mode={mode}
+          monthsCount={months.length}
+        />
         <ReportTableTotals
           totalScrollRef={totalScrollRef}
           handleScroll={handleScroll}
diff --git a/packages/desktop-client/src/components/reports/ReportOptions.ts b/packages/desktop-client/src/components/reports/ReportOptions.ts
index 0d4c6f6a4..f32dbe5e1 100644
--- a/packages/desktop-client/src/components/reports/ReportOptions.ts
+++ b/packages/desktop-client/src/components/reports/ReportOptions.ts
@@ -7,9 +7,9 @@ import {
 } from 'loot-core/src/types/models';
 
 const balanceTypeOptions = [
-  { description: 'Payment', format: 'totalDebts' },
-  { description: 'Deposit', format: 'totalAssets' },
-  { description: 'Net', format: 'totalTotals' },
+  { description: 'Payment', format: 'totalDebts' as const },
+  { description: 'Deposit', format: 'totalAssets' as const },
+  { description: 'Net', format: 'totalTotals' as const },
 ];
 
 const groupByOptions = [
diff --git a/packages/desktop-client/src/components/reports/entities.d.ts b/packages/desktop-client/src/components/reports/entities.d.ts
index db251812e..753ec938a 100644
--- a/packages/desktop-client/src/components/reports/entities.d.ts
+++ b/packages/desktop-client/src/components/reports/entities.d.ts
@@ -1,7 +1,7 @@
 export type DataEntity = {
-  data: Array<ItemEntity>;
-  monthData: Array<MonthData>;
-  groupedData: Array<GroupedEntity>;
+  data: GroupedEntity[];
+  monthData: GroupedEntity[];
+  groupedData: GroupedEntity[];
   legend: LegendEntity[];
   startDate: string;
   endDate: string;
@@ -31,7 +31,7 @@ export type MonthData = {
   totalTotals: number;
 };
 
-type GroupedEntity = {
+export type GroupedEntity = {
   id: string;
   name: string;
   date?: string;
diff --git a/packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTable.tsx b/packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTable.tsx
index a022eee4a..d087bea38 100644
--- a/packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTable.tsx
+++ b/packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTable.tsx
@@ -1,29 +1,42 @@
 // @ts-strict-ignore
 import React, {
-  type UIEventHandler,
+  useCallback,
   useLayoutEffect,
   useRef,
-  type ReactNode,
+  type UIEventHandler,
 } from 'react';
 import { type RefProp } from 'react-spring';
 
 import { type CSSProperties } from '../../../../style';
+import { Block } from '../../../common/Block';
 import { View } from '../../../common/View';
+import { type GroupedEntity } from '../../entities';
+
+import { ReportTableList } from './ReportTableList';
+import { ReportTableRow } from './ReportTableRow';
 
 type ReportTableProps = {
-  saveScrollWidth?: (value: number) => void;
-  listScrollRef?: RefProp<HTMLDivElement>;
+  saveScrollWidth: (value: number) => void;
+  listScrollRef: RefProp<HTMLDivElement>;
+  handleScroll: UIEventHandler<HTMLDivElement>;
   style?: CSSProperties;
-  children?: ReactNode;
-  handleScroll?: UIEventHandler<HTMLDivElement>;
+  groupBy: string;
+  balanceTypeOp: 'totalDebts' | 'totalTotals' | 'totalAssets';
+  data: GroupedEntity[];
+  mode: string;
+  monthsCount: number;
 };
 
 export function ReportTable({
   saveScrollWidth,
   listScrollRef,
-  style,
-  children,
   handleScroll,
+  style,
+  groupBy,
+  balanceTypeOp,
+  data,
+  mode,
+  monthsCount,
 }: ReportTableProps) {
   const contentRef = useRef<HTMLDivElement>(null);
 
@@ -33,25 +46,56 @@ export function ReportTable({
     }
   });
 
+  const renderItem = useCallback(
+    ({ item, groupByItem, mode, style, key, monthsCount }) => {
+      return (
+        <ReportTableRow
+          key={key}
+          item={item}
+          balanceTypeOp={balanceTypeOp}
+          groupByItem={groupByItem}
+          mode={mode}
+          style={style}
+          monthsCount={monthsCount}
+        />
+      );
+    },
+    [],
+  );
+
   return (
     <View
-      innerRef={listScrollRef}
-      onScroll={handleScroll}
-      id="list"
       style={{
-        overflowY: 'auto',
-        scrollbarWidth: 'none',
-        '::-webkit-scrollbar': { display: 'none' },
         flex: 1,
+        flexDirection: 'row',
         outline: 'none',
         '& .animated .animated-row': { transition: '.25s transform' },
         ...style,
       }}
       tabIndex={1}
     >
-      <View>
-        <div ref={contentRef}>{children}</div>
-      </View>
+      <Block
+        innerRef={listScrollRef}
+        onScroll={handleScroll}
+        id="list"
+        style={{
+          overflowY: 'auto',
+          scrollbarWidth: 'none',
+          '::-webkit-scrollbar': { display: 'none' },
+          flex: 1,
+          outline: 'none',
+          '& .animated .animated-row': { transition: '.25s transform' },
+          ...style,
+        }}
+      >
+        <ReportTableList
+          data={data}
+          monthsCount={monthsCount}
+          mode={mode}
+          groupBy={groupBy}
+          renderItem={renderItem}
+        />
+      </Block>
     </View>
   );
 }
diff --git a/packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableHeader.tsx b/packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableHeader.tsx
index 7138c4ba9..5a16c916b 100644
--- a/packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableHeader.tsx
+++ b/packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableHeader.tsx
@@ -5,15 +5,15 @@ import { type RefProp } from 'react-spring';
 import { styles, theme } from '../../../../style';
 import { View } from '../../../common/View';
 import { Row, Cell } from '../../../table';
-import { type MonthData } from '../../entities';
+import { type GroupedEntity } from '../../entities';
 
 type ReportTableHeaderProps = {
   scrollWidth?: number;
   groupBy: string;
-  interval?: MonthData[];
+  interval?: GroupedEntity[];
   balanceType: string;
   headerScrollRef: RefProp<HTMLDivElement>;
-  handleScroll?: UIEventHandler<HTMLDivElement>;
+  handleScroll: UIEventHandler<HTMLDivElement>;
 };
 
 export function ReportTableHeader({
diff --git a/packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableList.tsx b/packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableList.tsx
index 3034fb293..8d27ccfeb 100644
--- a/packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableList.tsx
+++ b/packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableList.tsx
@@ -1,244 +1,90 @@
 // @ts-strict-ignore
-import React, { memo } from 'react';
+import React from 'react';
 
-import {
-  amountToCurrency,
-  amountToInteger,
-  integerToCurrency,
-} from 'loot-core/src/shared/util';
-
-import { type CSSProperties, styles, theme } from '../../../../style';
+import { type CSSProperties, theme } from '../../../../style';
 import { View } from '../../../common/View';
-import { Row, Cell } from '../../../table';
+import { Cell, Row } from '../../../table';
+import { type GroupedEntity } from '../../entities';
 
-type TableRowProps = {
-  item: {
-    date: string;
-    name: string;
-    monthData: [];
-    totalAssets: number;
-    totalDebts: number;
-  };
-  balanceTypeOp?: string;
-  groupByItem: string;
-  mode: string;
-  monthsCount: number;
-  style?: CSSProperties;
+type ReportTableListProps = {
+  data: GroupedEntity[];
+  mode?: string;
+  monthsCount?: number;
+  groupBy: string;
+  renderItem;
 };
 
-const TableRow = memo(
-  ({
-    item,
-    balanceTypeOp,
-    groupByItem,
-    mode,
-    monthsCount,
-    style,
-  }: TableRowProps) => {
-    const average = amountToInteger(item[balanceTypeOp]) / monthsCount;
-    return (
-      <Row
-        key={item[groupByItem]}
-        collapsed={true}
-        style={{
-          color: theme.tableText,
-          backgroundColor: theme.tableBackground,
-          ...style,
-        }}
-      >
-        <Cell
-          value={item[groupByItem]}
-          title={item[groupByItem].length > 12 && item[groupByItem]}
-          style={{
-            width: 120,
-            flexShrink: 0,
-            ...styles.tnum,
-          }}
-        />
-        {item.monthData && mode === 'time'
-          ? item.monthData.map(month => {
-              return (
-                <Cell
-                  style={{
-                    minWidth: 85,
-                    ...styles.tnum,
-                  }}
-                  key={amountToCurrency(month[balanceTypeOp])}
-                  value={amountToCurrency(month[balanceTypeOp])}
-                  title={
-                    Math.abs(month[balanceTypeOp]) > 100000 &&
-                    amountToCurrency(month[balanceTypeOp])
-                  }
-                  width="flex"
-                  privacyFilter
-                />
-              );
-            })
-          : balanceTypeOp === 'totalTotals' && (
-              <>
-                <Cell
-                  value={amountToCurrency(item.totalAssets)}
-                  title={
-                    Math.abs(item.totalAssets) > 100000 &&
-                    amountToCurrency(item.totalAssets)
-                  }
-                  width="flex"
-                  style={{
-                    minWidth: 85,
-                    ...styles.tnum,
-                  }}
-                />
-                <Cell
-                  value={amountToCurrency(item.totalDebts)}
-                  title={
-                    Math.abs(item.totalDebts) > 100000 &&
-                    amountToCurrency(item.totalDebts)
-                  }
-                  width="flex"
-                  style={{
-                    minWidth: 85,
-                    ...styles.tnum,
-                  }}
-                />
-              </>
-            )}
-        <Cell
-          value={amountToCurrency(item[balanceTypeOp])}
-          title={
-            Math.abs(item[balanceTypeOp]) > 100000 &&
-            amountToCurrency(item[balanceTypeOp])
-          }
-          style={{
-            fontWeight: 600,
-            minWidth: 85,
-            ...styles.tnum,
-          }}
-          width="flex"
-          privacyFilter
-        />
-        <Cell
-          value={integerToCurrency(Math.round(average))}
-          title={
-            Math.abs(Math.round(average / 100)) > 100000 &&
-            integerToCurrency(Math.round(average))
-          }
-          style={{
-            fontWeight: 600,
-            minWidth: 85,
-            ...styles.tnum,
-          }}
-          width="flex"
-          privacyFilter
-        />
-      </Row>
-    );
-  },
-);
-
-function GroupedTableRow({
-  item,
-  balanceTypeOp,
-  groupByItem,
-  mode,
-  monthsCount,
-  empty,
-}) {
-  return (
-    <>
-      <TableRow
-        key={item.id}
-        item={item}
-        balanceTypeOp={balanceTypeOp}
-        groupByItem={groupByItem}
-        mode={mode}
-        monthsCount={monthsCount}
-        style={{
-          color: theme.tableRowHeaderText,
-          backgroundColor: theme.tableRowHeaderBackground,
-          fontWeight: 600,
-        }}
-      />
-      <View>
-        {item.categories
-          .filter(i =>
-            !empty
-              ? balanceTypeOp === 'totalTotals'
-                ? i.totalAssets !== 0 ||
-                  i.totalDebts !== 0 ||
-                  i.totalTotals !== 0
-                : i[balanceTypeOp] !== 0
-              : true,
-          )
-          .map(cat => {
-            return (
-              <TableRow
-                key={cat.id}
-                item={cat}
-                balanceTypeOp={balanceTypeOp}
-                groupByItem={groupByItem}
-                mode={mode}
-                monthsCount={monthsCount}
-              />
-            );
-          })}
-      </View>
-      <Row height={20} />
-    </>
-  );
-}
-
 export function ReportTableList({
   data,
-  empty,
   monthsCount,
-  balanceTypeOp,
   mode,
   groupBy,
-}) {
+  renderItem,
+}: ReportTableListProps) {
   const groupByItem = ['Month', 'Year'].includes(groupBy) ? 'date' : 'name';
-  const groupByData =
-    groupBy === 'Category'
-      ? 'groupedData'
-      : ['Month', 'Year'].includes(groupBy)
-      ? 'monthData'
-      : 'data';
+
+  type RenderRowProps = {
+    key: string;
+    index: number;
+    parent_index?: number;
+    style?: CSSProperties;
+  };
+  function RenderRow({ index, parent_index, style, key }: RenderRowProps) {
+    const item = parent_index
+      ? data[parent_index].categories[index]
+      : data[index];
+
+    return renderItem({
+      item,
+      groupByItem,
+      mode,
+      style,
+      key,
+      monthsCount,
+    });
+  }
 
   return (
     <View>
-      {data[groupByData]
-        .filter(i =>
-          !empty
-            ? balanceTypeOp === 'totalTotals'
-              ? i.totalAssets !== 0 || i.totalDebts !== 0 || i.totalTotals !== 0
-              : i[balanceTypeOp] !== 0
-            : true,
-        )
-        .map(item => {
-          if (groupBy === 'Category') {
-            return (
-              <GroupedTableRow
-                key={item.id}
-                item={item}
-                balanceTypeOp={balanceTypeOp}
-                groupByItem={groupByItem}
-                mode={mode}
-                monthsCount={monthsCount}
-                empty={empty}
-              />
-            );
-          } else {
-            return (
-              <TableRow
-                key={item.id}
-                item={item}
-                balanceTypeOp={balanceTypeOp}
-                groupByItem={groupByItem}
-                mode={mode}
-                monthsCount={monthsCount}
-              />
-            );
-          }
-        })}
+      {data.map((item, index) => {
+        return (
+          <View key={item.id}>
+            {data ? (
+              <>
+                <RenderRow
+                  key={item.id}
+                  index={index}
+                  style={
+                    item.categories && {
+                      color: theme.tableRowHeaderText,
+                      backgroundColor: theme.tableRowHeaderBackground,
+                      fontWeight: 600,
+                    }
+                  }
+                />
+                {item.categories && (
+                  <>
+                    <View>
+                      {item.categories.map((category, i) => {
+                        return (
+                          <RenderRow
+                            key={category.id}
+                            index={i}
+                            parent_index={index}
+                          />
+                        );
+                      })}
+                    </View>
+                    <Row height={20} />
+                  </>
+                )}
+              </>
+            ) : (
+              <Cell width="flex" />
+            )}
+          </View>
+        );
+      })}
     </View>
   );
 }
diff --git a/packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableRow.tsx b/packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableRow.tsx
new file mode 100644
index 000000000..63c275a6e
--- /dev/null
+++ b/packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableRow.tsx
@@ -0,0 +1,135 @@
+import React, { memo } from 'react';
+
+import {
+  amountToCurrency,
+  amountToInteger,
+  integerToCurrency,
+} from 'loot-core/src/shared/util';
+
+import { type CSSProperties, styles, theme } from '../../../../style';
+import { Row, Cell } from '../../../table';
+import { type GroupedEntity } from '../../entities';
+
+type ReportTableRowProps = {
+  item: GroupedEntity;
+  balanceTypeOp: 'totalAssets' | 'totalDebts' | 'totalTotals';
+  groupByItem: 'id' | 'name';
+  mode: string;
+  style?: CSSProperties;
+  monthsCount: number;
+};
+
+export const ReportTableRow = memo(
+  ({
+    item,
+    balanceTypeOp,
+    groupByItem,
+    mode,
+    style,
+    monthsCount,
+  }: ReportTableRowProps) => {
+    const average = amountToInteger(item[balanceTypeOp]) / monthsCount;
+    return (
+      <Row
+        collapsed={true}
+        style={{
+          color: theme.tableText,
+          backgroundColor: theme.tableBackground,
+          ...style,
+        }}
+      >
+        <Cell
+          value={item[groupByItem]}
+          title={item[groupByItem].length > 12 ? item[groupByItem] : undefined}
+          style={{
+            width: 120,
+            flexShrink: 0,
+            ...styles.tnum,
+          }}
+        />
+        {item.monthData && mode === 'time'
+          ? item.monthData.map(month => {
+              return (
+                <Cell
+                  key={amountToCurrency(month[balanceTypeOp])}
+                  style={{
+                    minWidth: 85,
+                    ...styles.tnum,
+                  }}
+                  value={amountToCurrency(month[balanceTypeOp])}
+                  title={
+                    Math.abs(month[balanceTypeOp]) > 100000
+                      ? amountToCurrency(month[balanceTypeOp])
+                      : undefined
+                  }
+                  width="flex"
+                  privacyFilter
+                />
+              );
+            })
+          : balanceTypeOp === 'totalTotals' && (
+              <>
+                <Cell
+                  value={amountToCurrency(item.totalAssets)}
+                  title={
+                    Math.abs(item.totalAssets) > 100000
+                      ? amountToCurrency(item.totalAssets)
+                      : undefined
+                  }
+                  width="flex"
+                  privacyFilter
+                  style={{
+                    minWidth: 85,
+                    ...styles.tnum,
+                  }}
+                />
+                <Cell
+                  value={amountToCurrency(item.totalDebts)}
+                  title={
+                    Math.abs(item.totalDebts) > 100000
+                      ? amountToCurrency(item.totalDebts)
+                      : undefined
+                  }
+                  width="flex"
+                  privacyFilter
+                  style={{
+                    minWidth: 85,
+                    ...styles.tnum,
+                  }}
+                />
+              </>
+            )}
+        <Cell
+          value={amountToCurrency(item[balanceTypeOp])}
+          title={
+            Math.abs(item[balanceTypeOp]) > 100000
+              ? amountToCurrency(item[balanceTypeOp])
+              : undefined
+          }
+          style={{
+            fontWeight: 600,
+            minWidth: 85,
+            ...styles.tnum,
+          }}
+          width="flex"
+          privacyFilter
+        />
+        <Cell
+          value={integerToCurrency(Math.round(average))}
+          title={
+            Math.abs(Math.round(average / 100)) > 100000
+              ? integerToCurrency(Math.round(average))
+              : undefined
+          }
+          style={{
+            fontWeight: 600,
+            minWidth: 85,
+            ...styles.tnum,
+          }}
+          width="flex"
+          privacyFilter
+        />
+      </Row>
+    );
+  },
+);
diff --git a/packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableTotals.tsx b/packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableTotals.tsx
index 4b708a985..5b3962025 100644
--- a/packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableTotals.tsx
+++ b/packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableTotals.tsx
@@ -110,6 +110,7 @@ export function ReportTableTotals({
                     amountToCurrency(data.totalAssets)
                   }
                   width="flex"
+                  privacyFilter
                 />
                 <Cell
                   style={{
@@ -122,6 +123,7 @@ export function ReportTableTotals({
                     amountToCurrency(data.totalDebts)
                   }
                   width="flex"
+                  privacyFilter
                 />
               </>
             )}
diff --git a/upcoming-release-notes/2192.md b/upcoming-release-notes/2192.md
new file mode 100644
index 000000000..ff341b26d
--- /dev/null
+++ b/upcoming-release-notes/2192.md
@@ -0,0 +1,6 @@
+---
+category: Maintenance
+authors: [carkom]
+---
+
+Fix table graph rendering issue for custom reports.
-- 
GitLab