✨ New Rule delete transaction action
#4603
Conversation
actual-github-bot
bot
changed the title
delete transaction actiondelete transaction action
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Bundle Stats — desktop-clientHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller No assets were smaller Unchanged
|
Bundle Stats — loot-coreHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller No assets were smaller Unchanged No assets were unchanged |
lelemm
changed the title
delete transaction actiondelete transaction action
|
This is working pretty well already, so we maybe could add this basically as is. edit: I added the logic to show if something will get deleted upon import. |
| ' url(\'data:image/svg+xml; utf8,<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20"><path fill="white" d="M10 1l10 6-10 6L0 7l10-6zm6.67 10L20 13l-10 6-10-6 3.33-2L10 15l6.67-4z" /></svg>\') 9px 9px', | ||
| }, | ||
| '&': { | ||
| background: theme.buttonNormalDisabledBorder, |
There was a problem hiding this comment.
Please make this better if there are better ways to set this checkbox up
youngcw
changed the title
delete transaction actiondelete transaction action
WalkthroughThis pull request integrates a new operation, delete-transaction, across multiple components in both the front-end and back-end. In the desktop client, UI components such as rule management, account reconciliation, editing modals, and transaction import interfaces have been updated to include conditional logic specifically handling this operation. These changes modify the representation of actions, user interface elements, and tooltip content to account for transactions marked for deletion (tombstone). On the server side, rule validation, action execution, and transaction processing logic have been extended to recognize the new operation, including support for soft deletion via tombstone flags. Additionally, type definitions have been updated to introduce a new rule action entity specific to delete transactions. The overall functionality is expanded while preserving the control flow and error handling for existing operations. Possibly related PRs
Suggested labels
Suggested reviewers
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/loot-core/src/shared/rules.ts (1)
228-229: Add translation support for the 'delete transaction' string.The friendly representation for the new operation is not wrapped in a translation function (
t()) unlike most other operations in this file. This could lead to inconsistent localization.case 'delete-transaction': - return 'delete transaction'; + return t('delete transaction');packages/desktop-client/src/components/modals/ImportTransactionsModal/Transaction.tsx (1)
127-175: Consider refactoring complex checkbox styling logic.The checkbox styling logic has become quite complex with multiple nested conditions and inline SVG data URLs. While the implementation works correctly, this could be a candidate for refactoring into more maintainable styled components in the future.
You could consider extracting this into separate styled components or utility functions to make the component more readable and maintainable. For example:
// In a separate file like CheckboxStyles.tsx export function getCheckboxStyle(transaction) { if (transaction.tombstone) { return { /* tombstone styles */ }; } else if (transaction.selected_merge) { return { /* merge styles */ }; } else { return { /* normal styles */ }; } } // In Transaction.tsx <Checkbox checked={transaction.selected && !transaction.tombstone} onChange={() => onCheckTransaction(transaction.trx_id)} style={getCheckboxStyle(transaction)} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
upcoming-release-notes/4603.mdis excluded by!**/*.md
📒 Files selected for processing (12)
packages/desktop-client/src/components/ManageRules.tsx(1 hunks)packages/desktop-client/src/components/accounts/Account.tsx(1 hunks)packages/desktop-client/src/components/modals/EditRuleModal.jsx(4 hunks)packages/desktop-client/src/components/rules/ActionExpression.tsx(3 hunks)packages/loot-core/src/server/rules/app.ts(1 hunks)packages/loot-core/src/server/rules/index.ts(2 hunks)packages/loot-core/src/server/transactions/transaction-rules.ts(1 hunks)packages/loot-core/src/shared/rules.ts(1 hunks)packages/loot-core/src/types/models/rule.d.ts(2 hunks)packages/desktop-client/src/components/modals/ImportTransactionsModal/ImportTransactionsModal.jsx(3 hunks)packages/desktop-client/src/components/modals/ImportTransactionsModal/Transaction.tsx(2 hunks)packages/loot-core/src/server/accounts/sync.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
packages/desktop-client/src/components/accounts/Account.tsx (1)
Learnt from: joel-jeremy
PR: actualbudget/actual#3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-11-10T16:45:31.225Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
packages/desktop-client/src/components/modals/ImportTransactionsModal/Transaction.tsx (1)
Learnt from: MatissJanis
PR: actualbudget/actual#3570
File: packages/desktop-client/src/components/modals/ImportTransactionsModal/Transaction.tsx:83-90
Timestamp: 2024-11-10T16:45:25.627Z
Learning: In the `Transaction` component in `Transaction.tsx`, both `rawTransaction` and `transaction` should be included in the dependency arrays of `useMemo` hooks, even though `transaction` derives from `rawTransaction`.
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Functional
- GitHub Check: Visual regression
- GitHub Check: build (windows-latest)
- GitHub Check: build (ubuntu-latest)
🔇 Additional comments (24)
packages/desktop-client/src/components/ManageRules.tsx (1)
94-95: LGTM! Implementation of the new delete-transaction rule action.The added code correctly handles the string representation of the delete-transaction rule action, following the same pattern as other actions in the function. This allows the UI to properly display rules that use this new action type.
packages/desktop-client/src/components/modals/ImportTransactionsModal/ImportTransactionsModal.jsx (3)
297-297: LGTM! Adding tombstone property for handling delete-transaction rules.This change correctly adds support for the tombstone flag that will be set by delete-transaction rules. This is a necessary foundation for properly handling transactions marked for deletion.
1075-1079: LGTM! Consistent handling of tombstoned transactions in transaction count.This change ensures consistency with the button's disabled state by also excluding tombstoned transactions from the count displayed to the user. This provides clear feedback about which transactions will actually be imported.
1061-1065:❓ Verification inconclusive
Verify proper handling of tombstoned transactions.
The filter condition for disabling the import button now correctly excludes tombstoned transactions, ensuring users can't import transactions that would be immediately deleted by rules.
🏁 Script executed:
#!/bin/bash # Check if the tombstone property is properly handled elsewhere in the codebase # Look for other places where tombstone is checked in transaction filtering rg -A 2 -B 2 "tombstone.*(filter|exclude)" --type ts --type tsx --type jsxLength of output: 104
Action: Verify Consistent Tombstoned Transaction Handling
The filter condition in the import modal now explicitly excludes transactions marked as tombstoned (via the
!trans.tombstonecheck), which aligns with our intent to prevent importing transactions that would be immediately dropped by subsequent rules. However, the automated search for additional usages of thetombstoneproperty encountered file type issues (e.g., with TSX files). Please manually verify that similar tombstone-handling logic is applied consistently throughout the codebase.
- File to check:
packages/desktop-client/src/components/modals/ImportTransactionsModal/ImportTransactionsModal.jsx(Lines 1061–1065)- Key snippet:
trans => !trans.isMatchedTransaction && trans.selected && !trans.tombstone,- Next steps: Ensure there are no unintended omissions or discrepancies in how tombstoned transactions are filtered out elsewhere in the project.
packages/loot-core/src/server/transactions/transaction-rules.ts (1)
614-615: LGTM! Server-side implementation of delete-transaction action.The implementation correctly follows the pattern used for other special actions in this function, creating an Action object with the appropriate parameters. This server-side change completes the full implementation of the delete-transaction feature.
packages/loot-core/src/server/rules/index.ts (2)
638-638: Added new 'delete-transaction' operation to allowed rule operations.This adds the delete-transaction operation to the ACTION_OPS array, making it a valid operation for rules. Good addition that enables the new feature for deleting transactions via rules.
746-748: Implementation for delete-transaction operation.The implementation for the delete-transaction operation correctly sets the tombstone property to 1, effectively implementing a soft delete mechanism. This approach maintains consistency with how deletions are handled elsewhere in the codebase.
packages/loot-core/src/types/models/rule.d.ts (2)
144-145: Added DeleteTransactionRuleActionEntity to union type.The RuleActionEntity union type is properly extended to include the new DeleteTransactionRuleActionEntity. This ensures the new action type is recognized throughout the system's type checking.
182-185: New DeleteTransactionRuleActionEntity interface definition.Well-defined interface for the delete transaction rule action. The interface follows the same pattern as other rule action entities, with an operation identifier and a value property.
packages/desktop-client/src/components/accounts/Account.tsx (1)
1102-1104: Improved handling of transactions with tombstone flag.Great enhancement to the reconciliation transaction processing. The code now properly separates transactions based on their tombstone status, sending non-tombstone transactions as "added" and tombstone transactions as "deleted". This ensures the transaction list is correctly updated when rules mark transactions for deletion.
packages/loot-core/src/server/rules/app.ts (1)
42-50: Updated validator to handle new delete-transaction action.The validation logic has been appropriately extended to support the new delete-transaction action type. The code also benefits from consolidated handling of prepend-notes and append-notes operations, which improves readability and maintainability.
packages/loot-core/src/server/accounts/sync.ts (2)
479-479: Appropriate type definition for tombstone transactions.The addition of the optional
tombstoneproperty to theReconcileTransactionsResulttype is a good way to indicate transactions that will be deleted by rules.
569-576: Clear handling of tombstone transactions in reconciliation.This implementation correctly handles transactions marked for deletion during reconciliation preview. The approach ensures that tombstone transactions are properly represented in the UI while maintaining the expected behavior of the reconciliation process.
packages/desktop-client/src/components/rules/ActionExpression.tsx (3)
20-20: Good type import for the DeleteTransactionRuleActionEntity.The import is correctly added to support the new delete-transaction operation.
60-61: Well-integrated conditional for delete-transaction operation.This conditional rendering follows the pattern used for other operations, maintaining code consistency.
146-150: Appropriately simple implementation of DeleteTransactionActionExpression.Since delete transactions don't require additional parameters or configuration, the implementation is appropriately minimal, just displaying the friendly name of the operation.
packages/desktop-client/src/components/modals/EditRuleModal.jsx (5)
404-407: Good addition of delete-transaction to the operations list.The delete-transaction operation has been appropriately added to the operations list in the OpSelect component.
494-497: Consistent inclusion of delete-transaction in prepend/append notes operations.This maintains consistency with the first OpSelect component, ensuring delete-transaction is available in all relevant operation selectors.
510-516: Clean handling of the delete-transaction operation UI.The implementation for rendering the delete-transaction operation is appropriately simple since this operation doesn't require additional configuration.
518-531: Logical UI adjustment for delete-transaction operation.The change to only show editor buttons for non-delete-transaction operations makes sense, as delete operations don't need additional configuration. This simplifies the UI when users select the delete operation.
1196-1196: Complete integration with delete-transaction in all relevant operation lists.The operation has been correctly added to all necessary operation lists throughout the component.
packages/desktop-client/src/components/modals/ImportTransactionsModal/Transaction.tsx (3)
100-102: Appropriate visual treatment for tombstone transactions.Applying the inactive text color to tombstone transactions provides a clear visual indicator that these transactions will be deleted.
111-119: Clear tooltip content for tombstone transactions.The tooltip message "This transaction will be deleted by Rules" provides clear information to the user about what will happen to this transaction.
124-124: Correct checkbox behavior for tombstone transactions.Preventing selection of tombstone transactions is appropriate since they're marked for deletion and shouldn't be modified during import.
|
Can you explain me how are you testing the file import? |
I made a rule that will delete one of the transactions in a file, then try to import the file. The transaction that will get deleted will be marked differently in the import preview. The rule I was testing with was an Account is, and imported_payee is rule It should look like this |
|
I tried using const transaction = useMemo(
() =>
fieldMappings && !rawTransaction.isMatchedTransaction
? applyFieldMappings(rawTransaction, fieldMappings)
: rawTransaction,
[rawTransaction, fieldMappings],
);This part of Also, when I open the mapping fields, it shows tombstone: I tried to fix this problem but I just made it worse lol. |
Fixed
That might be an existing issue. Need to confirm though wait, it worked for me just now. The transaction shows as getting deleted based on a notes contains rule. I still need to look at your recording |
…et#4613) * ♻️ solve most peer dependency issues reported on install * Release notes * Patch unit tests
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/loot-core/package.json (1)
52-52: New Dependency for Node.js Type DefinitionsThe addition of
"@types/node": "^17.0.45"in thedevDependenciessection will improve type checking for Node.js APIs during development. Please verify that this version aligns with the Node.js version supported by your project. If your project uses a more recent Node.js version, consider updating the version constraint accordingly.packages/eslint-plugin-actual/package.json (1)
13-19: New ESLint Dependency Addition:
Adding"eslint": "^9.22.0"in thedevDependenciesensures that development environments rely on the updated ESLint version. This addition helps enforce more current linting rules and can be beneficial for verifying recent code changes (including features like thedelete transactionrule).Please verify that this ESLint version aligns well with your peer dependency constraint (
"eslint": ">=7") and does not force downstream consumers to upgrade unnecessarily.packages/sync-server/package.json (1)
44-48: Babel Dependency Additions:
The new dependencies@babel/coreand@types/babel__coreare now added in thedevDependenciessection, which will improve Babel transpilation and type-checking capabilities. Please verify that these versions align with the rest of the build tooling and do not introduce any compatibility issues with other packages in the monorepo.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
upcoming-release-notes/4609.mdis excluded by!**/*.mdupcoming-release-notes/4613.mdis excluded by!**/*.mdyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (61)
packages/component-library/package.json(2 hunks)packages/desktop-client/package.json(3 hunks)packages/desktop-client/src/components/admin/UserAccess/UserAccess.tsx(1 hunks)packages/desktop-client/src/components/admin/UserDirectory/UserDirectory.tsx(1 hunks)packages/desktop-client/src/components/admin/UserDirectory/UserDirectoryRow.tsx(1 hunks)packages/desktop-client/src/components/autocomplete/FilterAutocomplete.tsx(1 hunks)packages/desktop-client/src/components/autocomplete/ReportAutocomplete.tsx(1 hunks)packages/desktop-client/src/components/banksync/AccountRow.tsx(1 hunks)packages/desktop-client/src/components/banksync/AccountsList.tsx(1 hunks)packages/desktop-client/src/components/banksync/EditSyncAccount.tsx(1 hunks)packages/desktop-client/src/components/budget/envelope/budgetsummary/BudgetSummary.tsx(3 hunks)packages/desktop-client/src/components/budget/index.tsx(3 hunks)packages/desktop-client/src/components/budget/tracking/budgetsummary/BudgetSummary.tsx(1 hunks)packages/desktop-client/src/components/filters/FiltersStack.tsx(1 hunks)packages/desktop-client/src/components/filters/SavedFilterMenuButton.tsx(1 hunks)packages/desktop-client/src/components/manager/subscribe/Login.tsx(1 hunks)packages/desktop-client/src/components/manager/subscribe/OpenIdForm.tsx(1 hunks)packages/desktop-client/src/components/mobile/transactions/TransactionList.tsx(1 hunks)packages/desktop-client/src/components/mobile/transactions/TransactionListItem.tsx(1 hunks)packages/desktop-client/src/components/mobile/transactions/TransactionListWithBalances.tsx(1 hunks)packages/desktop-client/src/components/modals/EditUser.tsx(1 hunks)packages/desktop-client/src/components/modals/OpenIDEnableModal.tsx(1 hunks)packages/desktop-client/src/components/modals/PluggyAiInitialiseModal.tsx(1 hunks)packages/desktop-client/src/components/reports/ChooseGraph.tsx(1 hunks)packages/desktop-client/src/components/reports/ReportSidebar.tsx(1 hunks)packages/desktop-client/src/components/reports/ReportSummary.tsx(1 hunks)packages/desktop-client/src/components/reports/ReportTopbar.tsx(1 hunks)packages/desktop-client/src/components/reports/SaveReportName.tsx(1 hunks)packages/desktop-client/src/components/reports/graphs/AreaGraph.tsx(1 hunks)packages/desktop-client/src/components/reports/graphs/BarGraph.tsx(1 hunks)packages/desktop-client/src/components/reports/graphs/DonutGraph.tsx(1 hunks)packages/desktop-client/src/components/reports/graphs/LineGraph.tsx(1 hunks)packages/desktop-client/src/components/reports/graphs/SpendingGraph.tsx(1 hunks)packages/desktop-client/src/components/reports/graphs/StackedBarGraph.tsx(1 hunks)packages/desktop-client/src/components/reports/graphs/showActivity.ts(1 hunks)packages/desktop-client/src/components/reports/graphs/tableGraph/RenderTableRow.tsx(1 hunks)packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTable.tsx(1 hunks)packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableHeader.tsx(1 hunks)packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableList.tsx(1 hunks)packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableRow.tsx(1 hunks)packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableTotals.tsx(1 hunks)packages/desktop-client/src/components/reports/reports/CustomReport.tsx(1 hunks)packages/desktop-client/src/components/reports/reports/CustomReportListCards.tsx(1 hunks)packages/desktop-client/src/components/reports/reports/GetCardData.tsx(1 hunks)packages/desktop-client/src/components/reports/reports/Spending.tsx(1 hunks)packages/desktop-client/src/components/reports/setSessionReport.ts(1 hunks)packages/desktop-client/src/components/reports/spreadsheets/calculateLegend.ts(1 hunks)packages/desktop-client/src/components/reports/spreadsheets/custom-spreadsheet.ts(1 hunks)packages/desktop-client/src/components/reports/spreadsheets/filterEmptyRows.ts(1 hunks)packages/desktop-client/src/components/reports/spreadsheets/grouped-spreadsheet.ts(1 hunks)packages/desktop-client/src/components/reports/spreadsheets/recalculate.ts(1 hunks)packages/desktop-client/src/components/reports/spreadsheets/sortData.ts(1 hunks)packages/desktop-client/src/components/reports/spreadsheets/spending-spreadsheet.ts(1 hunks)packages/desktop-client/src/hooks/useFilters.ts(1 hunks)packages/desktop-client/src/hooks/usePluggyAiStatus.ts(1 hunks)packages/eslint-plugin-actual/package.json(1 hunks)packages/loot-core/package.json(1 hunks)packages/loot-core/src/shared/user.ts(1 hunks)packages/loot-core/src/types/models/index.d.ts(1 hunks)packages/loot-core/src/types/models/user.ts(0 hunks)packages/sync-server/package.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/loot-core/src/types/models/user.ts
✅ Files skipped from review due to trivial changes (44)
- packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableList.tsx
- packages/desktop-client/src/components/reports/spreadsheets/recalculate.ts
- packages/desktop-client/src/components/banksync/AccountsList.tsx
- packages/desktop-client/src/hooks/usePluggyAiStatus.ts
- packages/desktop-client/src/components/reports/reports/Spending.tsx
- packages/desktop-client/src/components/admin/UserDirectory/UserDirectory.tsx
- packages/desktop-client/src/components/filters/FiltersStack.tsx
- packages/desktop-client/src/components/modals/OpenIDEnableModal.tsx
- packages/desktop-client/src/components/reports/setSessionReport.ts
- packages/desktop-client/src/components/reports/graphs/LineGraph.tsx
- packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableTotals.tsx
- packages/desktop-client/src/components/admin/UserAccess/UserAccess.tsx
- packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableHeader.tsx
- packages/desktop-client/src/hooks/useFilters.ts
- packages/desktop-client/src/components/modals/PluggyAiInitialiseModal.tsx
- packages/desktop-client/src/components/manager/subscribe/Login.tsx
- packages/desktop-client/src/components/reports/graphs/SpendingGraph.tsx
- packages/desktop-client/src/components/reports/graphs/tableGraph/RenderTableRow.tsx
- packages/desktop-client/src/components/reports/graphs/DonutGraph.tsx
- packages/desktop-client/src/components/autocomplete/ReportAutocomplete.tsx
- packages/desktop-client/src/components/banksync/AccountRow.tsx
- packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTableRow.tsx
- packages/desktop-client/src/components/filters/SavedFilterMenuButton.tsx
- packages/desktop-client/src/components/reports/reports/CustomReportListCards.tsx
- packages/desktop-client/src/components/reports/graphs/StackedBarGraph.tsx
- packages/desktop-client/src/components/manager/subscribe/OpenIdForm.tsx
- packages/desktop-client/src/components/reports/spreadsheets/calculateLegend.ts
- packages/desktop-client/src/components/reports/graphs/AreaGraph.tsx
- packages/desktop-client/src/components/reports/ReportTopbar.tsx
- packages/desktop-client/src/components/banksync/EditSyncAccount.tsx
- packages/desktop-client/src/components/reports/SaveReportName.tsx
- packages/desktop-client/src/components/reports/ReportSummary.tsx
- packages/desktop-client/src/components/reports/graphs/tableGraph/ReportTable.tsx
- packages/desktop-client/src/components/reports/spreadsheets/spending-spreadsheet.ts
- packages/desktop-client/src/components/modals/EditUser.tsx
- packages/desktop-client/src/components/mobile/transactions/TransactionListItem.tsx
- packages/desktop-client/src/components/mobile/transactions/TransactionList.tsx
- packages/desktop-client/src/components/mobile/transactions/TransactionListWithBalances.tsx
- packages/desktop-client/src/components/reports/spreadsheets/sortData.ts
- packages/desktop-client/src/components/reports/ChooseGraph.tsx
- packages/desktop-client/src/components/reports/reports/CustomReport.tsx
- packages/desktop-client/src/components/reports/graphs/showActivity.ts
- packages/desktop-client/src/components/reports/graphs/BarGraph.tsx
- packages/desktop-client/src/components/reports/reports/GetCardData.tsx
🧰 Additional context used
🧠 Learnings (3)
packages/loot-core/src/types/models/index.d.ts (1)
Learnt from: matt-fidd
PR: actualbudget/actual#3581
File: packages/loot-core/src/types/models/bank-sync.d.ts:11-21
Timestamp: 2025-03-12T13:10:05.044Z
Learning: In `packages/loot-core/src/types/models/bank-sync.d.ts`, when defining TypeScript types for data received from the server, maintain the field names as they are in the server response, even if they don't follow TypeScript naming conventions.
packages/desktop-client/src/components/budget/envelope/budgetsummary/BudgetSummary.tsx (1)
Learnt from: UnderKoen
PR: actualbudget/actual#3381
File: packages/desktop-client/src/components/budget/envelope/EnvelopeBudgetComponents.tsx:274-274
Timestamp: 2025-03-12T13:10:12.174Z
Learning: UnderKoen prefers not to include changes that would significantly alter many files in a PR focusing on specific functionality.
packages/desktop-client/src/components/budget/index.tsx (1)
Learnt from: UnderKoen
PR: actualbudget/actual#3381
File: packages/desktop-client/src/components/budget/envelope/EnvelopeBudgetComponents.tsx:274-274
Timestamp: 2025-03-12T13:10:12.174Z
Learning: UnderKoen prefers not to include changes that would significantly alter many files in a PR focusing on specific functionality.
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Functional
- GitHub Check: Visual regression
- GitHub Check: build (windows-latest)
- GitHub Check: Build Docker image (ubuntu)
- GitHub Check: Build Docker image (alpine)
🔇 Additional comments (21)
packages/desktop-client/src/components/reports/spreadsheets/filterEmptyRows.ts (1)
1-4: Import path updated correctly.The import path for
balanceTypeOpTypeandGroupedEntitytypes has been updated to use the broader module path'loot-core/types/models'instead of the more specific path. This change aligns with the broader effort to standardize type imports across the codebase and doesn't affect the functionality of thefilterEmptyRowsfunction.packages/desktop-client/src/components/reports/ReportSidebar.tsx (1)
17-22: Import statement consolidation looks good.The refactoring of import statements consolidates multiple type imports from specific model files into a single import statement from the centralized path. This change maintains the same functionality while improving code organization.
packages/loot-core/src/types/models/index.d.ts (1)
2-2: LGTM on type exports.These additional exports for bank, openid, pluggyai, and simplefin modules look good. They correctly follow the established pattern in the codebase for exporting type definitions. The added exports will support the new delete transaction functionality while maintaining proper TypeScript typing across the application.
Also applies to: 9-9, 11-11, 15-15
packages/desktop-client/src/components/reports/spreadsheets/grouped-spreadsheet.ts (1)
5-5: Import path update is aligned with type reorganization.This change updates the import path for the
GroupedEntitytype from a specific module to a more centralized location. This aligns with the broader pattern of centralizing type definitions in the codebase, making them more accessible across different components while maintaining the same functionality.packages/loot-core/src/shared/user.ts (1)
1-4: Looks good: Role constants moved to shared location.This change appropriately moves the
PossibleRolesconstant to a shared location, making it more accessible across the codebase while maintaining a single source of truth for user roles.packages/desktop-client/src/components/admin/UserDirectory/UserDirectoryRow.tsx (1)
9-10: Imports properly updated to reflect new constant location.The import paths have been correctly updated to use the relocated
PossibleRolesconstant from the shared directory. Using thetypemodifier forUserEntityis also a good TypeScript practice that helps with tree-shaking.packages/desktop-client/src/components/reports/spreadsheets/custom-spreadsheet.ts (1)
8-19: Import paths consolidated appropriatelyThe change simplifies the type import structure by consolidating the imports from
loot-core/types/modelsinstead of using a more specific path. This restructuring is consistent with changes in other files mentioned in the summary and improves codebase organization.packages/desktop-client/src/components/budget/tracking/budgetsummary/BudgetSummary.tsx (1)
32-32: Makemonthprop required instead of optional.The
monthprop is being changed from optional to required, which enforces that it must be provided when using this component. This is a type-safety improvement as it prevents potential runtime errors when the prop is undefined.packages/desktop-client/src/components/budget/envelope/budgetsummary/BudgetSummary.tsx (3)
1-1: Addedmemoimport for component optimization.Good addition of the
memoimport to enable component memoization.
31-31: Convert to memoized function component.Changing the component from a regular function declaration to a memoized constant improves performance by preventing unnecessary re-renders when props haven't changed.
Also applies to: 293-293
295-295: Add display name for debugging.Setting the display name to 'EnvelopeBudgetSummary' is helpful for component identification in React DevTools, improving the debugging experience.
packages/desktop-client/src/components/budget/index.tsx (3)
2-2: Remove unusedmemoimport.Good cleanup to remove the unused
memoimport as it's now being imported directly in the component files.
52-52: Update type definition to reference direct component.Updated the
SummaryComponenttype to reference the module component directly, maintaining type consistency with the implementation changes.
413-413: Use direct component reference.Changed to directly use the memoized component from the module instead of a wrapper, which reduces unnecessary component nesting and improves performance.
packages/component-library/package.json (2)
6-7: Peer Dependencies Update for React and React-DOM
The addition of"react-dom": ">=18.2"alongside"react": ">=18.2"in the peerDependencies section is appropriate. This ensures that any consumer of the component library will use versions of React and React-DOM that are compatible with the library’s requirements.
16-18: Dev Dependencies Alignment with Peer Dependencies
The updates in the devDependencies—updating@types/reactto"^18.2.21"and explicitly setting"react": "18.2.0"and adding"react-dom": "18.2.0"—ensure consistency with the peerDependencies. This alignment helps in maintaining type safety and provides a controlled environment during development and testing.packages/desktop-client/src/components/autocomplete/FilterAutocomplete.tsx (1)
4-4:✅ Verification successful
Import path modification looks good.
The import path for
TransactionFilterEntityhas been updated from a specific module path (loot-core/types/models/transaction-filter) to a more general one (loot-core/types/models). This change aligns with the broader effort to consolidate type imports across the codebase as mentioned in the PR summary.To verify this change doesn't break anything, you can check if the build process completes successfully with:
🏁 Script executed:
#!/bin/bash # Check if the build process runs without type errors npm run build -- --dry-run 2>&1 | grep -i "error TS" || echo "No TypeScript errors found"Length of output: 112
Approval: Import path change validated successfully
The import path change for
TransactionFilterEntity—fromloot-core/types/models/transaction-filtertoloot-core/types/models—is correct and aligns with our consolidation effort. The build process verification ("No TypeScript errors found") confirms that this update does not introduce any issues.packages/desktop-client/package.json (4)
18-18: New Dependency: @testing-library/dom
The addition of@testing-library/domat version9.2.0enhances the DOM testing capabilities of the project. Make sure that its usage is consistent with your current testing framework.
24-24: Updated Type Definitions for React
Upgrading@types/reactto^18.2.21is a minor version bump that may introduce improved type definitions or bug fixes. Please verify that the React components and related tooling remain compatible with this version.
49-49: Added Runtime Type Checking with prop-types
Introducingprop-typesat version^15.8.1provides extra runtime type checking for React components. Ensure that components which benefit from this validation employprop-typesto catch potential prop-related issues early in development.
83-83: New Build Tool: webpack
The inclusion ofwebpackat version^5.98.0suggests further reliance on Webpack-specific tooling (for example, in tandem withwebpack-bundle-analyzer). Verify that the webpack configuration is correctly integrated and that there is no conflict with the Vite-based build process already defined in the scripts.
|
Where you able to solve the problem with some rules losing the delete mark @youngcw ? |
|
I have not looked into why the table only updates sometimes |
No description provided.