enhance: add ability to control category learning per payee and globally #4081
Conversation
✅ 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 |
|
While I feel this PR is ready on my eyes, I've placed it as draft because I would like an opinion on the following things:
|
|
/update-vrt |
I think DB is good for this.
I think the case added is good. But I personally don't know how I would test this. Could you explain a flow for me @NullScope
It seems fine, but im not much of a designer.
They look good, but could you add a menu option to the context menu (right click)? And a label for on hover of the icon. Addionally if the option is disable, I don't think we should show it in the menu and/or the icon. |
|
@UnderKoen thanks for the feedback!
There are two flows here:
Will do 👍
I'm not sure I understood this part, could you please explain again? |
WalkthroughThis pull request introduces a new feature for category learning in the application. The changes span multiple files across the desktop client and core server packages. A new The implementation includes modifications to various components such as Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (11)
packages/loot-core/src/types/models/payee.d.ts (1)
8-8: Use a boolean type instead of numeric flags for clarity.Defining
learn_categoriesas1 | 0can lead to confusion, since calling code may incorrectly treat those values as numbers. Consider using a boolean type for improved readability and reduced risk of accidental misuse.- learn_categories?: 1 | 0; + learn_categories?: boolean;packages/desktop-client/src/components/settings/LearnCategories.tsx (1)
21-26: Provide explicit context in the button labels.It might be more user-friendly to label the actions as something like "Disable Automatic Category Learning" and "Enable Automatic Category Learning" for clarity, particularly if the user is not familiar with the term "learning".
- {isLearnCategoriesEnabled - ? t('Disable Category Learning') - : t('Enable Category Learning')} + {isLearnCategoriesEnabled + ? t('Disable Automatic Category Learning') + : t('Enable Automatic Category Learning')}packages/desktop-client/src/components/payees/PayeeMenu.tsx (3)
6-6: Avoid importing unused icons.If some icons from the same file (like
SvgBookmark) are not used or no longer needed, consider removing them to reduce bundle size and potential confusion.
27-27: Consistent destructuring.The destructuring is consistent. Make sure all newly introduced functions (e.g.,
onLearn) are documented for maintainers, especially if it has side effects in other parts of the code.
93-99: Add a tooltip or context label.The new menu item is great. For enhanced usability, consider showing a tooltip or short text label when hovering or focusing on this icon, clarifying the purpose of “Category Learning” for new users.
packages/desktop-client/src/components/payees/PayeeTableRow.tsx (2)
180-186: Toggling learn_categories in onMenuSelect.
The approach to store0or1is fine, but consider using booleans if supported system-wide to reduce confusion.
217-231: Red icon for "learning disabled".
While clear, it may be worth checking color accessibility or theming. A more semantic color or an additional style might improve clarity for users with visual impairments.packages/desktop-client/src/components/transactions/TransactionList.jsx (1)
106-107: Potential type mismatch.
String(learnCategories) !== 'false'works, but consider a more robust check (e.g.,Boolean()or!!learnCategories) if the stored pref changes type in the future.packages/desktop-client/src/components/payees/ManagePayees.tsx (1)
243-243: Menu property wiring.
SupplyingonLearntoPayeeMenuis appropriate. Confirm that there's a clear tooltip or label for clarity.packages/loot-core/src/server/accounts/transaction-rules.ts (1)
804-805: Consider indexing or optimizing thep.learn_categorieslookup.
You might want to ensure there's an index on thepayees.learn_categoriescolumn to prevent potential performance bottlenecks when filtering numerous rows, especially if the payee table grows large.packages/loot-core/src/server/accounts/transaction-rules.test.ts (1)
872-893: Remove theexpect(true).toBe(true)or convert it into a meaningful assertion.
Line 881’sexpect(true).toBe(true)is redundant and does not validate any behavior. Consider removing it, or replace with a test that confirms the user-defined payee initializes properly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
packages/desktop-client/e2e/settings.test.js-snapshots/Settings-checks-the-page-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.test.js-snapshots/Settings-checks-the-page-visuals-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.test.js-snapshots/Settings-checks-the-page-visuals-3-chromium-linux.pngis excluded by!**/*.pngupcoming-release-notes/4081.mdis excluded by!**/*.md
📒 Files selected for processing (11)
packages/desktop-client/src/components/payees/ManagePayees.tsx(3 hunks)packages/desktop-client/src/components/payees/PayeeMenu.tsx(5 hunks)packages/desktop-client/src/components/payees/PayeeTableRow.tsx(5 hunks)packages/desktop-client/src/components/settings/LearnCategories.tsx(1 hunks)packages/desktop-client/src/components/settings/index.tsx(2 hunks)packages/desktop-client/src/components/transactions/TransactionList.jsx(4 hunks)packages/loot-core/migrations/1735868660625_add_learn_categories_to_payees.sql(1 hunks)packages/loot-core/src/server/accounts/transaction-rules.test.ts(1 hunks)packages/loot-core/src/server/accounts/transaction-rules.ts(1 hunks)packages/loot-core/src/types/models/payee.d.ts(1 hunks)packages/loot-core/src/types/prefs.d.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/loot-core/migrations/1735868660625_add_learn_categories_to_payees.sql
🔇 Additional comments (18)
packages/desktop-client/src/components/settings/LearnCategories.tsx (2)
15-16: Verify handling for the pref’s string value.
String(learnCategories) !== 'false'could cause confusion iflearnCategoriesisn’t strictlybool | string. Consider an explicit type check or schema validation to ensure thatlearnCategoriesholds a consistent type (e.g., boolean) rather than converting to string.
29-41: Ensure i18n coverage for all text blocks.While most text is already localized via
t(), confirm that the appended' '}spaces and markup are correctly captured in translation. Potentially move them into the translation text for clarity and consistency.packages/loot-core/src/types/prefs.d.ts (1)
34-35: Confirm consistent naming for flags.Adding
| 'learn-categories'extends the possible keys. Make sure that the naming is aligned across the entire codebase so that referencing code does not introduce confusion (some places might expect'learnCategories').packages/desktop-client/src/components/payees/PayeeMenu.tsx (2)
17-17: Props type looks good.Adding
onLearnfollows the established pattern for menu actions, and the callback signature is concise. No issues found.
51-53: Ensure onLearn can handle multiple payees.If multiple payees can be selected, ensure your
onLearnhandler can properly update each selected payee’slearn_categoriesstate. Consider a short comment clarifying that only single-payee operations are allowed or how multi-payee scenarios are handled.packages/desktop-client/src/components/settings/index.tsx (2)
35-35: Good modular addition.
ImportingLearnCategoriesSettingskeeps settings organized in their own component. Looks good!
187-187: Concise integration in settings.
Adding<LearnCategoriesSettings />here ensures seamless navigation and user access to the new feature. No issues found.packages/desktop-client/src/components/payees/PayeeTableRow.tsx (4)
9-9: Additional icon import is valid.
TheSvgLightBulbicon import is straightforward and used consistently. No issues seen.
14-14: Tooltip import recognized.
ImportingTooltipaligns with showing context when learning is disabled. This is good UX.
67-70: Extended editable fields.
Allowing'learn_categories'inEditablePayeeFieldsis necessary for toggling. Implementation is coherent.
165-170: Menu options for enabling/disabling learning.
Suggest verifying that payees withtransfer_acctnever display the learning toggle if unneeded. Otherwise, logic is clear.packages/desktop-client/src/components/transactions/TransactionList.jsx (5)
16-16: New custom hook usage looks fine.
TheuseSyncedPref('learn-categories')integration is consistent with how preferences are handled elsewhere.
41-46: New parameter in saveDiff.
AddinglearnCategoriesto the request payload is logical. No apparent issues. Ensure the server side properly handles this parameter.
53-54: Extended signature for saveDiffAndApply.
AcceptinglearnCategoriespreserves consistency. Confirm that all calls pass the correct value.
130-130: Maintaining top sort order.
UsingDate.now()is a quick fix. Confirm that a repeated call won't cause collisions if transactions are updated very rapidly.
134-139: Consistent usage of learnCategories.
PassingisLearnCategoriesEnabledtosaveDiffAndApplyaligns with the same approach insaveDiff. Looks good.packages/desktop-client/src/components/payees/ManagePayees.tsx (2)
119-119: Enhancement to onUpdate.
Including'learn_categories'in the generic type is a natural extension for toggling. This is consistent with other updated fields.
171-190: Bulk toggling learn_categories.
This logic is straightforward. Double-check that partially mixed states (some enabled, some disabled) are handled by toggling them all to one state, which appears to be the intended behavior.
NullScope
changed the title
NullScope
force-pushed
the
control-auto-categories
branch
from
46148fb to
d0c7c9f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/desktop-client/src/components/transactions/TransactionList.jsx (1)
130-139: Consider more robust preference handling.While the implementation works, consider making the preference handling more explicit and robust:
- Add a default value to handle cases where the preference might not be set
- Use a more explicit boolean conversion
- const [learnCategories] = useSyncedPref('learn-categories'); - const isLearnCategoriesEnabled = String(learnCategories) !== 'false'; + const [learnCategories = true] = useSyncedPref('learn-categories'); + const isLearnCategoriesEnabled = Boolean(learnCategories === true || learnCategories === 'true');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
packages/desktop-client/e2e/settings.test.js-snapshots/Settings-checks-the-page-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.test.js-snapshots/Settings-checks-the-page-visuals-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.test.js-snapshots/Settings-checks-the-page-visuals-3-chromium-linux.pngis excluded by!**/*.pngupcoming-release-notes/4081.mdis excluded by!**/*.md
📒 Files selected for processing (11)
packages/desktop-client/src/components/payees/ManagePayees.tsx(3 hunks)packages/desktop-client/src/components/payees/PayeeMenu.tsx(5 hunks)packages/desktop-client/src/components/payees/PayeeTableRow.tsx(5 hunks)packages/desktop-client/src/components/settings/LearnCategories.tsx(1 hunks)packages/desktop-client/src/components/settings/index.tsx(2 hunks)packages/desktop-client/src/components/transactions/TransactionList.jsx(4 hunks)packages/loot-core/migrations/1735868660625_add_learn_categories_to_payees.sql(1 hunks)packages/loot-core/src/server/accounts/transaction-rules.test.ts(1 hunks)packages/loot-core/src/server/accounts/transaction-rules.ts(1 hunks)packages/loot-core/src/types/models/payee.d.ts(1 hunks)packages/loot-core/src/types/prefs.d.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/desktop-client/src/components/settings/index.tsx
- packages/loot-core/src/server/accounts/transaction-rules.test.ts
- packages/loot-core/migrations/1735868660625_add_learn_categories_to_payees.sql
- packages/loot-core/src/types/models/payee.d.ts
- packages/loot-core/src/types/prefs.d.ts
- packages/loot-core/src/server/accounts/transaction-rules.ts
- packages/desktop-client/src/components/settings/LearnCategories.tsx
- packages/desktop-client/src/components/payees/PayeeTableRow.tsx
- packages/desktop-client/src/components/payees/ManagePayees.tsx
- packages/desktop-client/src/components/payees/PayeeMenu.tsx
🔇 Additional comments (3)
packages/desktop-client/src/components/transactions/TransactionList.jsx (3)
16-16: LGTM! Import of useSyncedPref is appropriate.The addition of
useSyncedPrefhook aligns with the PR's objective of managing synchronized category learning preferences.
41-51: Function signature updates look good and maintain proper separation of concerns.The addition of the
learnCategoriesparameter to both functions is well-implemented:
- Properly propagates the learning preference to the server
- Maintains existing error handling patterns
- Preserves function focus and single responsibility
Also applies to: 53-59
106-107: State management implementation looks good.The use of
useSyncedPreffor managing the learning preference is well-implemented and the boolean conversion is clear.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/desktop-client/src/components/transactions/TransactionList.jsx (1)
Line range hint
119-142: Add missing dependencies to useCallback hooksThe
isLearnCategoriesEnabledvalue is used in callbacks but not included in their dependency arrays.const onSave = useCallback(async transaction => { // ... implementation - }, []); + }, [isLearnCategoriesEnabled]); const onAddSplit = useCallback(id => { // ... implementation - }, []); + }, [isLearnCategoriesEnabled]); const onSplit = useCallback(id => { // ... implementation - }, []); + }, [isLearnCategoriesEnabled]);Also applies to: 144-147, 150-153
🧹 Nitpick comments (3)
packages/desktop-client/src/components/transactions/TransactionList.jsx (3)
106-107: Consider using Boolean parsing for more robust type conversionThe string comparison could be replaced with Boolean parsing for more robust type handling.
- const [learnCategories = 'false'] = useSyncedPref('learn-categories'); - const isLearnCategoriesEnabled = String(learnCategories) === 'true'; + const [learnCategories = false] = useSyncedPref('learn-categories'); + const isLearnCategoriesEnabled = Boolean(learnCategories);
41-46: Add JSDoc for the new parameterThe function changes look good, but would benefit from parameter documentation.
+/** + * @param {Object} diff - The transaction diff to save + * @param {boolean} learnCategories - Whether to enable category learning + * @returns {Promise<{updates?: Array}>} The remote updates if any + */ async function saveDiff(diff, learnCategories) {
Line range hint
53-58: Improve parameter formatting consistencyThe function implementation looks good, but the parameter formatting could be more consistent.
-async function saveDiffAndApply(diff, changes, onChange, learnCategories) { - const remoteDiff = await saveDiff(diff, learnCategories); - onChange( - applyTransactionDiff(changes.newTransaction, remoteDiff), - applyChanges(remoteDiff, changes.data), - ); +async function saveDiffAndApply( + diff, + changes, + onChange, + learnCategories +) { + const remoteDiff = await saveDiff(diff, learnCategories); + onChange( + applyTransactionDiff(changes.newTransaction, remoteDiff), + applyChanges(remoteDiff, changes.data) + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/desktop-client/src/components/settings/LearnCategories.tsx(1 hunks)packages/desktop-client/src/components/transactions/TransactionList.jsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/desktop-client/src/components/settings/LearnCategories.tsx
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: build (macos-latest)
- GitHub Check: Analyze
- GitHub Check: build (windows-latest)
- GitHub Check: Wait for Netlify build to finish
- GitHub Check: build (ubuntu-latest)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/desktop-client/src/components/transactions/TransactionList.jsx (1)
Line range hint
41-58: Consider adding JSDoc comments for the modified functionsWhile the implementation is solid, adding JSDoc comments for
saveDiffandsaveDiffAndApplywould improve code maintainability by documenting the newlearnCategoriesparameter.Example documentation:
+/** + * Saves transaction changes to the backend + * @param {Object} diff - The transaction changes to save + * @param {boolean} learnCategories - Whether to enable category learning for these transactions + * @returns {Promise<Object>} The remote updates if any + */ async function saveDiff(diff, learnCategories) { +/** + * Saves transaction changes and applies them to the local state + * @param {Object} diff - The transaction changes to save + * @param {Object} changes - The local changes to apply + * @param {Function} onChange - Callback to update local state + * @param {boolean} learnCategories - Whether to enable category learning + * @returns {Promise<void>} + */ async function saveDiffAndApply(diff, changes, onChange, learnCategories) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/desktop-client/src/components/transactions/TransactionList.jsx(5 hunks)
🔇 Additional comments (4)
packages/desktop-client/src/components/transactions/TransactionList.jsx (4)
16-16: LGTM! Clean integration of category learning preferencesThe hook usage and state management follow React best practices with proper default value handling and explicit boolean conversion.
Also applies to: 106-107
Line range hint
53-58: LGTM! Consistent parameter forwardingThe learnCategories parameter is correctly propagated to the underlying saveDiff function while preserving existing functionality.
116-116: LGTM! Comprehensive implementation of category learning across all transaction operationsThe learnCategories parameter is now consistently passed to all relevant transaction operations, addressing the previous review comments.
Also applies to: 130-130, 134-139, 147-147, 154-154
41-46: Verify backend handling of learnCategories parameterThe parameter is correctly passed to the backend endpoint. However, we should verify that the backend properly handles this new parameter.
✅ Verification successful
Backend properly handles learnCategories parameter
The backend implementation in
packages/loot-core/src/server/accounts/transactions.tscorrectly handles thelearnCategoriesparameter. The parameter is properly typed, has appropriate default values, and is used to learn from user's categorization activity.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for backend handler of transactions-batch-update ast-grep --pattern 'transactions-batch-update'Length of output: 46
Script:
#!/bin/bash # Search for transaction batch update related patterns rg -i "transactions?.?batch.?update" -A 5 # Search for IPC handlers related to transactions rg -i "ipcMain\.handle.*transactions?" -A 5 # Search in potential backend directories fd -t f . "packages/node-lib" "packages/backend" "packages/server" --exec rg -l "transactions?.?batch"Length of output: 13998
Script:
#!/bin/bash # Look for batchUpdateTransactions implementation ast-grep --pattern 'function batchUpdateTransactions' # Search for the implementation using ripgrep as fallback rg -l "batchUpdateTransactions" packages/loot-core/src/Length of output: 451
Script:
#!/bin/bash # Check the implementation in transactions.ts rg -A 10 "export async function batchUpdateTransactions" packages/loot-core/src/server/accounts/transactions.ts # Check how learnCategories is used rg -A 5 "learnCategories" packages/loot-core/src/server/accounts/transactions.tsLength of output: 1031
NullScope
force-pushed
the
control-auto-categories
branch
from
5a12995 to
73c3b13
Compare
|
/update-vrt |
Sorry I meant in the app manually
If you disable category learning globally the menu item and icon should not show |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/loot-core/src/server/accounts/transaction-rules.ts (1)
804-805: Consider handling NULL values in the learn_categories condition.The LEFT JOIN with payees table combined with the condition
p.learn_categories = 1might exclude transactions with NULL payees. Consider usingCOALESCEto handle NULL values:- LEFT JOIN payees p ON p.id = t.payee - WHERE date >= ? AND date <= ? AND is_parent = 0 AND a.closed = 0 AND p.learn_categories = 1 + LEFT JOIN payees p ON p.id = t.payee + WHERE date >= ? AND date <= ? AND is_parent = 0 AND a.closed = 0 + AND COALESCE(p.learn_categories, 1) = 1packages/desktop-client/src/components/payees/PayeeMenu.tsx (2)
99-105: Consider adding hover tooltip and clarifying disabled state visibility.Based on the PR discussion:
- Add a tooltip to explain the Category Learning feature
- Clarify whether the menu item should be hidden (not just disabled) when the feature is unavailable
Consider adding a tooltip:
{ icon: SvgLightBulb, iconSize: 9, name: 'learn', text: t('Category Learning'), + tooltip: t('Enable or disable automatic category learning for selected payees'), disabled: isDisabled, },
99-105: Consider accessibility improvements for the Category Learning feature.To improve accessibility:
- Add an ARIA label to make the purpose clear to screen readers
- Ensure the disabled state is properly announced
- Consider keyboard navigation for the feature
{ icon: SvgLightBulb, iconSize: 9, name: 'learn', text: t('Category Learning'), + 'aria-label': t('Toggle automatic category learning for selected payees'), disabled: isDisabled, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
packages/desktop-client/e2e/settings.test.js-snapshots/Settings-checks-the-page-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.test.js-snapshots/Settings-checks-the-page-visuals-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.test.js-snapshots/Settings-checks-the-page-visuals-3-chromium-linux.pngis excluded by!**/*.pngupcoming-release-notes/4081.mdis excluded by!**/*.md
📒 Files selected for processing (11)
packages/desktop-client/src/components/payees/ManagePayees.tsx(3 hunks)packages/desktop-client/src/components/payees/PayeeMenu.tsx(5 hunks)packages/desktop-client/src/components/payees/PayeeTableRow.tsx(5 hunks)packages/desktop-client/src/components/settings/LearnCategories.tsx(1 hunks)packages/desktop-client/src/components/settings/index.tsx(2 hunks)packages/desktop-client/src/components/transactions/TransactionList.jsx(5 hunks)packages/loot-core/migrations/1735868660625_add_learn_categories_to_payees.sql(1 hunks)packages/loot-core/src/server/accounts/transaction-rules.test.ts(1 hunks)packages/loot-core/src/server/accounts/transaction-rules.ts(1 hunks)packages/loot-core/src/types/models/payee.d.ts(1 hunks)packages/loot-core/src/types/prefs.d.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/loot-core/src/types/prefs.d.ts
- packages/desktop-client/src/components/settings/index.tsx
- packages/loot-core/src/server/accounts/transaction-rules.test.ts
- packages/desktop-client/src/components/settings/LearnCategories.tsx
- packages/loot-core/src/types/models/payee.d.ts
- packages/loot-core/migrations/1735868660625_add_learn_categories_to_payees.sql
- packages/desktop-client/src/components/payees/ManagePayees.tsx
- packages/desktop-client/src/components/payees/PayeeTableRow.tsx
- packages/desktop-client/src/components/transactions/TransactionList.jsx
🔇 Additional comments (5)
packages/loot-core/src/server/accounts/transaction-rules.ts (1)
804-805: Implementation aligns with PR objectives.The changes effectively implement the per-payee category learning control by:
- Joining with the payees table to access the learn_categories flag
- Filtering transactions based on this flag during category rule updates
This ensures that category learning only occurs for payees where it's enabled, meeting the PR's goal of providing granular control over the feature.
packages/desktop-client/src/components/payees/PayeeMenu.tsx (4)
6-6: LGTM! Icon choice is appropriate.The lightbulb icon is a good metaphor for the learning feature, and using the v1 icons package maintains consistency.
17-17: LGTM! Type definition follows established patterns.The
onLearncallback type is consistent with other menu action callbacks and follows the component's typing conventions.
27-27: LGTM! Prop destructuring is correct.The
onLearnprop is properly added to the component's parameter destructuring.
56-58: LGTM! Switch case implementation is consistent.The 'learn' case follows the established pattern of other menu actions.
Ah, what I've done to test is use the demo file and do the following:
Good idea, will do 👍 |
| @@ -0,0 +1,44 @@ | |||
| import React from 'react'; | |||
| import { useTranslation } from 'react-i18next'; | |||
There was a problem hiding this comment.
| import { useTranslation } from 'react-i18next'; | |
| import { useTranslation, Trans } from 'react-i18next'; |
| <strong>{t('Category Learning')}</strong> | ||
| {t( | ||
| ' will automatically determine the best category for a transaction and create a rule that sets the category for the payee.', | ||
| )}{' '} |
There was a problem hiding this comment.
| <strong>{t('Category Learning')}</strong> | |
| {t( | |
| ' will automatically determine the best category for a transaction and create a rule that sets the category for the payee.', | |
| )}{' '} | |
| <Trans> | |
| <strong>Category Learning</strong> will automatically determine the best category for a transaction and create a rule that sets the category for the payee. | |
| </Trans>{' '} |
| to="https://actualbudget.org/docs/budgeting/rules/#automatic-rules" | ||
| linkColor="purple" | ||
| > | ||
| {t('Learn more…')} |
There was a problem hiding this comment.
| {t('Learn more…')} | |
| <Trans>Learn more</Trans> |
|
I think the global learning disable option should be in the payee page, not the settings page. Sorry if that has been brought up before, but I didn't see it. |
|
This is minor, but it may be nice to try and make the spacing between icons/text more consistent. Maybe the icons can start centered and spread out if both are present?
|
Good idea, probably the same place proposed for the schedule length adjustment setting |
This PR introduces the following enhancements to the Category Learning feature:
Some payees, such as Amazon, frequently involve transactions spanning multiple categories. In such cases, pre-filled categories can lead to inaccuracies, requiring extra effort to correct, update transaction, or delete unhelpful rules. This can result in wasted time or complications, especially during syncing as it can make it harder to spot an incorrect transaction. Allowing greater control over category learning improves flexibility and reduces friction for users.