Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert ManagePayees components to Typescript #3867

Merged
merged 5 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions packages/desktop-client/src/components/payees/ManagePayees.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { useTranslation } from 'react-i18next';
import memoizeOne from 'memoize-one';

import { getNormalisedString } from 'loot-core/src/shared/normalisation';
import { groupById } from 'loot-core/src/shared/util';
import { type Diff, groupById } from 'loot-core/src/shared/util';
import { type PayeeEntity } from 'loot-core/types/models';

import {
Expand Down Expand Up @@ -70,10 +70,7 @@ type ManagePayeesProps = {
ruleCounts: ComponentProps<typeof PayeeTable>['ruleCounts'];
orphanedPayees: PayeeEntity[];
initialSelectedIds: string[];
onBatchChange: (arg: {
deleted?: Array<{ id: string }>;
updated?: Array<{ id: string; name?: string; favorite?: 0 | 1 }>;
}) => void;
onBatchChange: (diff: Diff<PayeeEntity>) => void;
onViewRules: ComponentProps<typeof PayeeTable>['onViewRules'];
onCreateRule: ComponentProps<typeof PayeeTable>['onCreateRule'];
onMerge: (ids: string[]) => Promise<void>;
Expand Down Expand Up @@ -126,7 +123,11 @@ export const ManagePayees = ({
) => {
const payee = payees.find(p => p.id === id);
if (payee && payee[name] !== value) {
onBatchChange({ updated: [{ id, [name]: value }] });
onBatchChange({
updated: [{ id, [name]: value }],
added: [],
deleted: [],
});
}
},
[payees, onBatchChange],
Expand All @@ -141,6 +142,8 @@ export const ManagePayees = ({
function onDelete(ids?: { id: string }[]) {
onBatchChange({
deleted: ids ?? [...selected.items].map(id => ({ id })),
updated: [],
added: [],
});
if (!ids) selected.dispatch({ type: 'select-none' });
}
Expand All @@ -152,10 +155,14 @@ export const ManagePayees = ({
if (allFavorited) {
onBatchChange({
updated: [...selected.items].map(id => ({ id, favorite: 0 })),
added: [],
deleted: [],
});
} else {
onBatchChange({
updated: [...selected.items].map(id => ({ id, favorite: 1 })),
added: [],
deleted: [],
});
}
selected.dispatch({ type: 'select-none' });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,23 @@ import React from 'react';
import { useTranslation } from 'react-i18next';
import { useLocation } from 'react-router-dom';

import { type PayeeEntity } from 'loot-core/types/models';

import { Page } from '../Page';

import { ManagePayeesWithData } from './ManagePayeesWithData';

export function ManagePayeesPage() {
const { t } = useTranslation();
const location = useLocation();
const locationState = location.state;
const initialSelectedIds =
locationState && 'selectedPayee' in locationState
? [locationState.selectedPayee as PayeeEntity['id']]
: [];
return (
<Page header={t('Payees')}>
<ManagePayeesWithData
initialSelectedIds={
location.state && location.state.selectedPayee
? [location.state.selectedPayee]
: null
}
/>
<ManagePayeesWithData initialSelectedIds={initialSelectedIds} />
</Page>
);
}
Original file line number Diff line number Diff line change
@@ -1,100 +1,101 @@
import React, { useState, useEffect } from 'react';
import { useSelector } from 'react-redux';

import React, { useState, useEffect, useCallback } from 'react';
import { useDispatch, useSelector } from 'react-redux';

import {
getPayees,
initiallyLoadPayees,
pushModal,
setLastUndoState,
} from 'loot-core/client/actions';
import { type UndoState } from 'loot-core/server/undo';
import { send, listen } from 'loot-core/src/platform/client/fetch';
import { applyChanges } from 'loot-core/src/shared/util';
import { applyChanges, type Diff } from 'loot-core/src/shared/util';
import { type NewRuleEntity, type PayeeEntity } from 'loot-core/types/models';

import { useActions } from '../../hooks/useActions';
import { useCategories } from '../../hooks/useCategories';
import { usePayees } from '../../hooks/usePayees';

import { ManagePayees } from './ManagePayees';

export function ManagePayeesWithData({ initialSelectedIds }) {
const initialPayees = usePayees();
const lastUndoState = useSelector(state => state.app.lastUndoState);
const { grouped: categoryGroups } = useCategories();
type ManagePayeesWithDataProps = {
initialSelectedIds: string[];
};

const { initiallyLoadPayees, getPayees, setLastUndoState, pushModal } =
useActions();
export function ManagePayeesWithData({
initialSelectedIds,
}: ManagePayeesWithDataProps) {
const payees = usePayees();
const lastUndoState = useSelector(state => state.app.lastUndoState);
const dispatch = useDispatch();

const [payees, setPayees] = useState(initialPayees);
const [ruleCounts, setRuleCounts] = useState({ value: new Map() });
const [orphans, setOrphans] = useState({ value: new Map() });
const [orphans, setOrphans] = useState<PayeeEntity[]>([]);

async function refetchOrphanedPayees() {
const refetchOrphanedPayees = useCallback(async () => {
const orphs = await send('payees-get-orphaned');
setOrphans(orphs);
}
}, []);

async function refetchRuleCounts() {
const refetchRuleCounts = useCallback(async () => {
let counts = await send('payees-get-rule-counts');
counts = new Map(Object.entries(counts));
setRuleCounts({ value: counts });
}
}, []);

useEffect(() => {
async function loadData() {
const result = await initiallyLoadPayees();

// Wait a bit before setting the data. This lets the modal
// settle and makes for a smoother experience.
await new Promise(resolve => setTimeout(resolve, 100));

if (result) {
setPayees(result);
}

refetchRuleCounts();
refetchOrphanedPayees();
await dispatch(initiallyLoadPayees());
await refetchRuleCounts();
await refetchOrphanedPayees();
}
loadData();

const unlisten = listen('sync-event', async ({ type, tables }) => {
if (type === 'applied') {
if (tables.includes('rules')) {
refetchRuleCounts();
await refetchRuleCounts();
}
}
});

return () => {
unlisten();
};
}, []);
}, [dispatch, refetchRuleCounts, refetchOrphanedPayees]);

async function onUndo({ tables, messages, meta }) {
if (!tables.includes('payees') && !tables.includes('payee_mapping')) {
return;
}
useEffect(() => {
async function onUndo({ tables, messages, meta }: UndoState) {
if (!tables.includes('payees') && !tables.includes('payee_mapping')) {
return;
}

setPayees(await getPayees());
refetchOrphanedPayees();
await dispatch(getPayees());
await refetchOrphanedPayees();

if (
(meta && meta.targetId) ||
messages.find(msg => msg.dataset === 'rules')
) {
refetchRuleCounts();
}
const targetId =
meta && typeof meta === 'object' && 'targetId' in meta
? meta.targetId
: null;

setLastUndoState(null);
}
if (targetId || messages.find(msg => msg.dataset === 'rules')) {
await refetchRuleCounts();
}

await dispatch(setLastUndoState(null));
}

useEffect(() => {
if (lastUndoState.current) {
onUndo(lastUndoState.current);
}

return listen('undo-event', onUndo);
}, []);
}, [lastUndoState, refetchRuleCounts, refetchOrphanedPayees]);

Check warning on line 91 in packages/desktop-client/src/components/payees/ManagePayeesWithData.tsx

View workflow job for this annotation

GitHub Actions / lint

React Hook useEffect has a missing dependency: 'dispatch'. Either include it or remove the dependency array
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add 'dispatch' to the dependency array in useEffect

The dispatch function is used inside the useEffect hook but is not included in its dependency array. This might lead to unexpected behavior if dispatch changes. To ensure the effect runs correctly, include dispatch in the dependency array.

Apply this diff to fix the issue:

-  }, [lastUndoState, refetchRuleCounts, refetchOrphanedPayees]);
+  }, [dispatch, lastUndoState, refetchRuleCounts, refetchOrphanedPayees]);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
}, [lastUndoState, refetchRuleCounts, refetchOrphanedPayees]);
}, [dispatch, lastUndoState, refetchRuleCounts, refetchOrphanedPayees]);
🧰 Tools
🪛 GitHub Check: lint

[warning] 91-91:
React Hook useEffect has a missing dependency: 'dispatch'. Either include it or remove the dependency array


function onViewRules(id) {
pushModal('manage-rules', { payeeId: id });
function onViewRules(id: PayeeEntity['id']) {
dispatch(pushModal('manage-rules', { payeeId: id }));
}

function onCreateRule(id) {
const rule = {
function onCreateRule(id: PayeeEntity['id']) {
const rule: NewRuleEntity = {
stage: null,
conditionsOp: 'and',
conditions: [
Expand All @@ -114,20 +115,18 @@
},
],
};
pushModal('edit-rule', { rule });
dispatch(pushModal('edit-rule', { rule }));
}

return (
<ManagePayees
payees={payees}
ruleCounts={ruleCounts.value}
orphanedPayees={orphans}
categoryGroups={categoryGroups}
initialSelectedIds={initialSelectedIds}
lastUndoState={lastUndoState}
onBatchChange={changes => {
send('payees-batch-change', changes);
setPayees(applyChanges(changes, payees));
onBatchChange={async (changes: Diff<PayeeEntity>) => {
await send('payees-batch-change', changes);
await dispatch(getPayees());
setOrphans(applyChanges(changes, orphans));
}}
onMerge={async ([targetId, ...mergeIds]) => {
Expand All @@ -145,7 +144,6 @@
}
filtedOrphans = filtedOrphans.filter(o => !mergeIds.includes(o.id));

const result = payees.filter(p => !mergeIds.includes(p.id));
mergeIds.forEach(id => {
const count = ruleCounts.value.get(id) || 0;
ruleCounts.value.set(
Expand All @@ -154,7 +152,7 @@
);
});

setPayees(result);
await dispatch(getPayees());
setOrphans(filtedOrphans);
setRuleCounts({ value: ruleCounts.value });
}}
Expand Down
1 change: 1 addition & 0 deletions packages/loot-core/src/types/server-handlers.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ export interface ServerHandlers {
}) => Promise<unknown>;

'payees-check-orphaned': (arg: { ids }) => Promise<unknown>;
'payees-get-orphaned': () => Promise<PayeeEntity[]>;

'payees-get-rules': (arg: { id: string }) => Promise<RuleEntity[]>;

Expand Down
6 changes: 6 additions & 0 deletions upcoming-release-notes/3867.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
category: Maintenance
authors: [joel-jeremy]
---

Convert ManagePayees page components to Typescript
Loading