Apply suggestions from code rabbit

actualbudget · Nov 12, 2024 · a6db37e · a6db37e
1 parent 56f601f
commit a6db37e
Show file tree

Hide file tree

Showing 4 changed files with 90 additions and 25 deletions.
diff --git a/packages/desktop-client/src/components/reports/SummaryNumber.tsx b/packages/desktop-client/src/components/reports/SummaryNumber.tsx
@@ -70,6 +70,8 @@ export function SummaryNumber({
       {/* Visible element */}
       <View
         ref={mergedRef as Ref<HTMLDivElement>}
+        role="text"
+        aria-label={`${value < 0 ? 'Negative' : 'Positive'} amount: ${amountToCurrency(Math.abs(value))}${suffix}`}
         style={{
           alignItems: 'center',
           height: '100%',
@@ -80,7 +82,7 @@ export function SummaryNumber({
           color: value < 0 ? chartTheme.colors.red : chartTheme.colors.blue,
         }}
       >
-        <span>
+        <span aria-hidden="true">
           {amountToCurrency(Math.abs(value))}
           {suffix}
         </span>

diff --git a/packages/desktop-client/src/components/reports/reports/Summary.tsx b/packages/desktop-client/src/components/reports/reports/Summary.tsx
@@ -17,7 +17,6 @@ import {
 
 import { useFilters } from '../../../hooks/useFilters';
 import { useNavigate } from '../../../hooks/useNavigate';
-import { useResponsive } from '../../../ResponsiveProvider';
 import { styles, theme } from '../../../style';
 import { Button } from '../../common/Button2';
 import { View } from '../../common/View';
@@ -28,6 +27,7 @@ import { Checkbox } from '../../forms';
 import { MobileBackButton } from '../../mobile/MobileBackButton';
 import { FieldSelect } from '../../modals/EditRuleModal';
 import { MobilePageHeader, Page, PageHeader } from '../../Page';
+import { useResponsive } from '../../responsive/ResponsiveProvider';
 import { Header } from '../Header';
 import { LoadingIndicator } from '../LoadingIndicator';
 import { calculateTimeRange } from '../reportRanges';
@@ -75,7 +75,10 @@ function SummaryInner({ widget }: SummaryInnerProps) {
     onDelete: onDeleteFilter,
     onUpdate: onUpdateFilter,
     onConditionsOpChange,
-  } = useFilters(widget?.meta?.conditions, widget?.meta?.conditionsOp);
+  } = useFilters(
+    widget?.meta?.conditions ?? [],
+    widget?.meta?.conditionsOp ?? 'and',
+  );
 
   const [content, setContent] = useState<SummaryContent>(
     widget?.meta?.content
@@ -84,10 +87,20 @@ function SummaryInner({ widget }: SummaryInnerProps) {
             return JSON.parse(widget.meta.content);
           } catch (error) {
             console.error('Failed to parse widget meta content:', error);
-            return { type: 'sum' };
+            return {
+              type: 'sum',
+              divisorIncludeDateRange: true,
+              divisorConditions: [],
+              divisorConditionsOp: 'and',
+            };
           }
         })()
-      : { type: 'sum' },
+      : {
+          type: 'sum',
+          divisorIncludeDateRange: true,
+          divisorConditions: [],
+          divisorConditionsOp: 'and',
+        },
   );
 
   const {
@@ -97,7 +110,12 @@ function SummaryInner({ widget }: SummaryInnerProps) {
     onDelete: divisorOnDeleteFilter,
     onUpdate: divisorOnUpdateFilter,
     onConditionsOpChange: divisorOnConditionsOpChange,
-  } = useFilters(content?.divisorConditions, content?.divisorConditionsOp);
+  } = useFilters(
+    content.type === 'percentage' ? (content?.divisorConditions ?? []) : [],
+    content.type === 'percentage'
+      ? (content?.divisorConditionsOp ?? 'and')
+      : 'and',
+  );
 
   const params = useMemo(
     () => summarySpreadsheet(start, end, conditions, conditionsOp, content),
@@ -153,10 +171,16 @@ function SummaryInner({ widget }: SummaryInnerProps) {
 
   const onSaveWidgetName = async (newName: string) => {
     if (!widget) {
-      throw new Error('No widget that could be saved.');
+      dispatch(
+        addNotification({
+          type: 'error',
+          message: t('Cannot save: No widget available.'),
+        }),
+      );
+      return;
     }
 
-    const name = newName || t('Net Worth');
+    const name = newName || t('Summary');
     await send('dashboard-update-widget', {
       id: widget.id,
       meta: {
@@ -175,7 +199,13 @@ function SummaryInner({ widget }: SummaryInnerProps) {
 
   async function onSaveWidget() {
     if (!widget) {
-      throw new Error('No widget that could be saved.');
+      dispatch(
+        addNotification({
+          type: 'error',
+          message: t('Cannot save: No widget available.'),
+        }),
+      );
+      return;
     }
 
     await send('dashboard-update-widget', {
@@ -311,10 +341,13 @@ function SummaryInner({ widget }: SummaryInnerProps) {
             onChange={(
               newValue: 'sum' | 'avgPerMonth' | 'avgPerTransact' | 'percentage',
             ) =>
-              setContent((prev: SummaryContent) => ({
-                ...prev,
-                type: newValue,
-              }))
+              setContent(
+                (prev: SummaryContent) =>
+                  ({
+                    ...prev,
+                    type: newValue,
+                  }) as SummaryContent,
+              )
             }
           />
         </View>
@@ -364,12 +397,14 @@ function SummaryInner({ widget }: SummaryInnerProps) {
                 <Checkbox
                   id="enabled-field"
                   checked={content.divisorIncludeDateRange ?? true}
-                  onChange={() =>
+                  onChange={() => {
+                    const currentValue =
+                      content.divisorIncludeDateRange ?? true;
                     setContent(prev => ({
                       ...prev,
-                      divisorIncludeDateRange: !content.divisorIncludeDateRange,
-                    }))
-                  }
+                      divisorIncludeDateRange: !currentValue,
+                    }));
+                  }}
                 />{' '}
                 Include summary date range
               </View>

diff --git a/packages/desktop-client/src/components/reports/spreadsheets/summary-spreadsheet.ts b/packages/desktop-client/src/components/reports/spreadsheets/summary-spreadsheet.ts
@@ -21,9 +21,15 @@ export function summarySpreadsheet(
     spreadsheet: ReturnType<typeof useSpreadsheet>,
     setData: (data: { total: number }) => void,
   ) => {
-    const { filters } = await send('make-filters-from-conditions', {
-      conditions: conditions.filter(cond => !cond.customName),
-    });
+    let filters = [];
+    try {
+      const response = await send('make-filters-from-conditions', {
+        conditions: conditions.filter(cond => !cond.customName),
+      });
+      filters = response.filters;
+    } catch (error) {
+      console.error('Error fetching filters:', error);
+    }
     const conditionsOpKey = conditionsOp === 'or' ? '$or' : '$and';
 
     const startDay = d.parse(
@@ -38,6 +44,10 @@ export function summarySpreadsheet(
       new Date(),
     );
 
+    if (d.isAfter(startDay, endDay)) {
+      throw new Error('Start date must be before or equal to end date.');
+    }
+
     const getOneDatePerMonth = (start: Date, end: Date) => {
       const months = [];
       let currentDate = d.startOfMonth(start);
@@ -82,7 +92,13 @@ export function summarySpreadsheet(
       query = query.groupBy(['date']);
     }
 
-    const data = await runQuery(query);
+    let data;
+    try {
+      data = await runQuery(query);
+    } catch (error) {
+      console.error('Error executing query:', error);
+      return;
+    }
 
     switch (summaryContent.type) {
       case 'sum':
@@ -157,6 +173,12 @@ async function calculatePercentage(
   startDay: Date,
   endDay: Date,
 ) {
+  if (summaryContent.type !== 'percentage') {
+    return {
+      total: 0,
+    };
+  }
+
   const conditionsOpKey =
     summaryContent.divisorConditionsOp === 'or' ? '$or' : '$and';
   const { filters } = await send('make-filters-from-conditions', {

diff --git a/packages/loot-core/src/types/models/dashboard.d.ts b/packages/loot-core/src/types/models/dashboard.d.ts
@@ -101,9 +101,15 @@ export type SummaryWidget = AbstractWidget<
   } | null
 >;
 
-export type SummaryContent = {
-  type: 'sum' | 'avgPerMonth' | 'avgPerTransact' | 'percentage';
-  divisorConditions?: RuleConditionEntity[];
-  divisorConditionsOp?: 'and' | 'or';
+export type BaseSummaryContent = {
+  type: 'sum' | 'avgPerMonth' | 'avgPerTransact';
+};
+
+export type PercentageSummaryContent = {
+  type: 'percentage';
+  divisorConditions: RuleConditionEntity[];
+  divisorConditionsOp: 'and' | 'or';
   divisorIncludeDateRange?: boolean;
 };
+
+export type SummaryContent = BaseSummaryContent | PercentageSummaryContent;