Conversation
actual-github-bot
bot
changed the title
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for actualbudget-website ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Bundle Statsdesktop-clientTotal
Changeset
View detailed bundle breakdownAdded Removed Bigger
Smaller
Unchanged
loot-coreTotal
Changeset
View detailed bundle breakdownAdded
Removed
Bigger Smaller Unchanged apiTotal
Changeset
View detailed bundle breakdownAdded Removed Bigger
Smaller Unchanged |
|
✅ VRT screenshots have been automatically updated. |
Auto-generated by VRT workflow PR: actualbudget#6157
mannkind
force-pushed
the
payee_locations
branch
2 times, most recently
from
de5980a to
c93ade6
Compare
|
✅ VRT screenshots have been automatically updated. |
Auto-generated by VRT workflow PR: actualbudget#6157
|
Welp. I think I've addressed all the initial CI linting/errors/feedback. I'm removing the WIP tag. But if I've still done something wrong, please do let me know. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds payee locations end-to-end: DB schema and migration, server API handlers and queries (nearby lookup), location service and adapters, Redux support and thunks, frontend hooks and UI for nearby payees (autocomplete, modal, transaction edit), YNAB5 import, and feature-flagged location permission handling. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant UI as Client UI
participant LocSvc as LocationService
participant API as Backend API
participant DB as Database
User->>UI: open autocomplete / grant location
UI->>LocSvc: getCurrentPosition()
LocSvc->>LocSvc: return cached or request geolocation
LocSvc-->>UI: coordinates
UI->>API: api/payees-get-nearby {latitude, longitude, maxDistance}
API->>DB: query payee_locations (Haversine, rank, limit)
DB-->>API: nearby payees with distance
API-->>UI: nearby payees
UI->>UI: render Nearby Payees section
User->>UI: select nearby payee
UI->>API: api/payee-location-create / updatePayeeLocationIfNeeded
API->>DB: insert/update/delete payee_location
DB-->>API: confirm
API-->>UI: success
UI->>User: update selection
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/loot-core/src/server/api-models.ts (1)
102-121: Inconsistency between type and implementation.
APIPayeeEntitynow includeslocationin the type definition (line 104), buttoExternal(lines 110-116) doesn't serialize thelocationfield fromPayeeEntity. This creates a mismatch where the type signature promises a field that the implementation doesn't provide.Apply this diff to include location in the serialization:
toExternal(payee: PayeeEntity) { return { id: payee.id, name: payee.name, transfer_acct: payee.transfer_acct, + location: payee.location, }; },packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx (1)
583-597: Highlighted index comment vs behavior when nearby payees and “new” item coexist
getHighlightedIndexnow does:// If we have nearby payees, highlight the first nearby payee if (filteredNearbyPayees.length > 0) { return 0; }However, when there is a non-empty input and thus a
"new"item present,0will still correspond to the first suggestion insuggestions(often"new"), not necessarily the first nearby payee. The comment and actual behavior can diverge.Depending on how
AutocompleteusesgetHighlightedIndex, you might want to:
- Adjust the calculation to account for how nearby items are injected into the rendered list, or
- Update the comment to reflect the real behavior if highlighting the first suggestion is intended.
This is subtle but can affect keyboard selection expectations.
🧹 Nitpick comments (17)
packages/loot-core/src/server/importers/ynab5.ts (2)
217-219: Consider using logger.error for error messages.For consistency with logging best practices, use
logger.errorinstead oflogger.logwhen logging error messages.Apply this diff:
- logger.log( + logger.error( `Failed to import location for payee ${actualPayeeId}: ${error.message}`, );
568-636: ZIP file handling looks solid with one optional refinement.The ZIP detection using magic numbers is correct, the case-insensitive file search is appropriate, and error handling gracefully degrades when payee locations are missing or invalid. The normalization logic maintains backward compatibility with the original JSON-only implementation.
Consider making the payee locations file search more precise to avoid potential false matches:
const payeeLocationsEntry = entries.find( entry => - entry.entryName.toLowerCase().includes('payee_location') && + (entry.entryName.toLowerCase().endsWith('payee_locations.json') || + entry.entryName.toLowerCase().includes('/payee_locations.json')) && entry.entryName.toLowerCase().endsWith('.json'), );packages/loot-core/src/shared/constants.ts (1)
1-1: Document the unit of measurement for DEFAULT_MAX_DISTANCE.The constant value
500lacks clarity about its unit of measurement. Based on the PR context, this appears to be used for distance-based payee filtering, but it's unclear whether this represents meters, miles, kilometers, or another unit.Consider one of these approaches:
Option 1: Add a comment documenting the unit:
+// Maximum distance in meters for nearby payee lookup export const DEFAULT_MAX_DISTANCE = 500;Option 2: Make the unit explicit in the constant name:
-export const DEFAULT_MAX_DISTANCE = 500; +export const DEFAULT_MAX_DISTANCE_METERS = 500;packages/loot-core/migrations/1763316795000_add_payee_locations.sql (1)
12-16: Consider a composite index for geospatial queries.The
getNearbyPayeesquery filters by geographic proximity using the Haversine formula onlatitudeandlongitudecolumns (lines 247-252 of packages/loot-core/src/server/payees/app.ts). A composite index on(latitude, longitude)or(payee_id, latitude, longitude)would help the query optimizer reduce the number of rows evaluated before applying the distance calculation. For optimal performance, consider also adding bounding box filtering (e.g.,WHERE latitude BETWEEN ? AND ? AND longitude BETWEEN ? AND ?) to the query before the expensive Haversine computation.packages/loot-core/src/shared/location-utils.ts (1)
1-71: Distance utilities look correct; only tiny naming nit
calculateDistanceimplements Haversine correctly with a reasonable Earth radius, andformatDistance’s metric/imperial thresholds and rounding rules are clear and match the intended UX. Only minor nit is thedeltaLamdavariable name (could bedeltaLambdafor readability), but that’s cosmetic.packages/desktop-client/src/payees/payeesSlice.ts (1)
25-44: Nearby‑payees state and reducers are wired consistentlyThe added
nearbyPayeesslice state and the pending/fulfilled/rejected handlers forgetNearbyPayees/reloadNearbyPayeesmirror the existing patterns for common payees and regular payees, and markingisNearbyPayeesDirtyinmarkPayeesDirty,createPayee.fulfilled,updatePayeeLocationIfNeeded.fulfilled, anddeletePayeeLocation.fulfilledis a sensible way to keep nearby suggestions in sync. Theif (action.payload)check increatePayee.fulfilledis effectively always true (id string), but harmless.Also applies to: 57-76, 126-148, 363-384
packages/loot-core/src/shared/location.ts (2)
35-52: Preserve original error details when rethrowing fromgetCurrentPositionCurrently the catch block wraps the original error in
new Error(\Location error: ${error}`), which can lose stack/metadata and stringify as[object Object]`.Consider preserving the original error (or at least its message) explicitly:
- } catch (error) { - console.warn('Geolocation error:', error); - throw new Error(`Location error: ${error}`); - } + } catch (error) { + console.warn('Geolocation error:', error); + const message = + error instanceof Error ? error.message : String(error ?? 'Unknown'); + throw new Error(`Location error: ${message}`); + }This keeps logs readable while preserving a meaningful error message.
96-131: Consider avoiding noisyconsole.loginsavePayeeLocationIfNeeded
savePayeeLocationIfNeededlogs on every successful save and on all background failures. Given this can be called frequently, it may be noisy in production logs.You might want to either:
- Gate these under a debug flag, or
- Use a less verbose channel (e.g., a dedicated logger with log levels) and drop the success
console.log.Behavior is otherwise solid.
packages/loot-core/src/shared/location-adapters.ts (1)
68-99: Consider reusing response type ingetNearbyPayees
getNearbyPayeesnormalizes falsy responses to[], which is good. If other API client methods start needing similar normalization, you may want a small helper:async function sendOrEmpty<T>(name: string, args: unknown): Promise<T[]> { const result = await send(name, args); return result || []; }Not required now, but could reduce repetition later.
packages/loot-core/src/server/payees/app.ts (2)
144-162:createPayeeLocationimplementation is correct; consider index supportThe insert logic (uuid id,
Date.now()forcreated_at, parameterized query) looks correct and safe.To keep nearby lookups and payee-specific queries fast as the table grows, ensure there are indexes on at least
(payee_id, created_at)and possibly(latitude, longitude)or a composite suited for distance queries. That’s likely handled in migrations, but worth double‑checking.
190-312: Nearby payees SQL is correct but could be simplified for maintainabilityThe window-function query correctly:
- Computes a Haversine distance in meters,
- Filters by
maxDistance,- Picks the closest location per payee with
ROW_NUMBER() OVER (PARTITION BY pl.payee_id ORDER BY distance),- Limits to 10 closest payees,
- Maps into
PayeeEntitywith embeddedlocation&distance.Two suggestions:
- Reduce duplication of the Haversine expression
It's currently duplicated three times (SELECT, ORDER BY, WHERE). Extracting to a CTE or reusing the computed
distanceexpression (e.g., computing once in the inner SELECT and reusing that inWHEREandROW_NUMBER()ordering) would aid maintainability and reduce risk of copy‑paste divergence.
- Clarify units in code comments
The query multiplies by
1000to convert km to meters whileDEFAULT_MAX_DISTANCEis documented in meters. The comment is correct but you might want a brief comment near the function signature noting “maxDistancein meters” to keep server and client usage aligned.Functionally this looks solid.
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx (3)
1055-1120: Absolute-positioned “forget” button may layout unpredictablyThe “forget” button is absolutely positioned:
<Button variant="bare" onPress={onForgetLocation} style={{ position: 'absolute', top: '75%', right: '20px', ... }} >Because the parent container (
<View>around the payee field) is not explicitlyposition: 'relative', the button’s absolute positioning is relative to the nearest positioned ancestor, which may not be the payee field container. This can lead to overlapping or misaligned UI on different devices.Consider either:
- Making the immediate wrapper
position: 'relative'and aligning within that, or- Using flexbox layout in the row to place the forget button next to the payee field without absolute positioning.
1361-1432: Nearby-payee lookup effect: scope and error handling look reasonableThe
useEffectthat, onlocationAccess, callslocationService.getCurrentPosition()andgetNearbyPayees, then stores the nearest payee and clearsshouldShowForgetLocation, is well-contained and catches errors so it won’t block transaction entry.You may later consider:
- Adding an
isAdding.currentcheck so we don’t do auto-nearest-payee work when editing existing transactions, if that’s not desired.- Debouncing or caching server calls beyond what
LocationServicealready does, if you expect frequent toggling of location permission.Not strictly necessary for MVP.
1624-1658:onForgetLocationshould avoid unnecessaryDateconversionsThe forget-location logic is sound: it gets the most recent location for the current payee, checks if it is within
DEFAULT_MAX_DISTANCEof the current position, and deletes it via the thunk.You could simplify and avoid
Dateobject allocation:- const mostRecentLocation = payeeLocations.sort( - (a, b) => - new Date(b.created_at).getTime() - new Date(a.created_at).getTime(), - )[0]; + const mostRecentLocation = payeeLocations.sort( + (a, b) => b.created_at - a.created_at, + )[0];Since
created_atis already a numeric timestamp, this is equivalent and cheaper.packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx (3)
193-252: PayeeList grouping and indexing logic now includes nearby payees cleanlyThe
useMemothat partitions items intonewPayee,nearbyPayees,suggestedPayees,payees, andtransferPayees, and assigns ahighlightedIndexin render order, is a nice way to keep the grouping logic local.One thing to double‑check:
itemspassed intoPayeeListare now[...filteredNearbyPayees, ...items], wherefilteredNearbyPayeesare not part of the underlyingAutocompletesuggestions array. IfgetItemPropsdepends on identity or index within the original suggestions list, this could desync Downshift’s internal index tracking from what’s rendered.If
getItemPropsonly relies on the passeditemand not its position in the originalsuggestions, you’re fine. Otherwise, you may need to:
- Include nearby payees in
suggestionsearlier, or- Extend
renderItemsto pass an explicit index togetItemPropsinstead of relying on identity.Please verify against
Autocomplete’s implementation to avoid subtle keyboard/selection bugs.
485-515: Create / select logic with location updates is correct but may need testsIn
handleSelect:
- New payees are created via
createPayee({ name, locationAccess }), which matches the thunk signature and lets the thunk decide whether to save a location.- Existing payees, when
locationAccessis true, triggerupdatePayeeLocationIfNeeded(payeeId)inside a try/catch so errors don’t block selection.This is the right place to hook location updates. It would be good to have tests around:
- Selecting an existing payee with and without
locationAccess,- Creating a new payee when
locationAccessis true/false.Behaviorally this looks sound.
806-930:NearbyPayeeItem: avoid redundant permission hooks and fix translation usageThe
NearbyPayeeItemcomponent introduces a nice UI for nearby entries (bookmark icon, distance display, and forget button), but there are a couple of points worth tightening:
- Redundant and semantically odd use of
useLocationPermissionconst locationAccess = useLocationPermission(); const narrowStyle = locationAccess ? { ...styles.mobileMenuItem, ... } : {}; const iconSize = locationAccess ? 14 : 8;
useLocationPermissionis about geolocation permission, not viewport width. Using it to choose mobile styles is indirect and also runs the relatively heavy permission logic once per nearby item.Prefer:
- Passing
locationAccess(or better,isNarrowWidth) from the parent, or- Using
useResponsive()here, likePayeeItemdoes, to determine narrow styles and icon size.
- Translation pattern
<button ...> <Trans>{t('forget')}</Trans> <SvgLocationCurrent ... /> </button>Wrapping
t('forget')in<Trans>is redundant and can look up a translated string as a key. Use either:<Trans>forget</Trans>or
{t('forget')}directly.
Addressing these keeps the component lighter and avoids translation confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
packages/desktop-client/e2e/settings.mobile.test.ts-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-4-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.ts-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-5-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.mobile.test.ts-snapshots/Mobile-Settings-checks-that-settings-page-can-be-opened-6-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.test.ts-snapshots/Settings-checks-the-page-visuals-1-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.test.ts-snapshots/Settings-checks-the-page-visuals-2-chromium-linux.pngis excluded by!**/*.pngpackages/desktop-client/e2e/settings.test.ts-snapshots/Settings-checks-the-page-visuals-3-chromium-linux.pngis excluded by!**/*.pngupcoming-release-notes/6157.mdis excluded by!**/*.md
📒 Files selected for processing (29)
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.test.tsx(1 hunks)packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx(14 hunks)packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx(14 hunks)packages/desktop-client/src/components/modals/PayeeAutocompleteModal.tsx(3 hunks)packages/desktop-client/src/components/modals/manager/ImportYNAB5Modal.tsx(3 hunks)packages/desktop-client/src/components/settings/Format.tsx(3 hunks)packages/desktop-client/src/hooks/useLocationPermission.ts(1 hunks)packages/desktop-client/src/hooks/usePayees.ts(2 hunks)packages/desktop-client/src/hooks/useUnitOfMeasurementFormat.ts(1 hunks)packages/desktop-client/src/modals/modalsSlice.ts(1 hunks)packages/desktop-client/src/payees/payeesSlice.ts(10 hunks)packages/loot-core/migrations/1763316795000_add_payee_locations.sql(1 hunks)packages/loot-core/src/server/api-models.ts(1 hunks)packages/loot-core/src/server/api.ts(1 hunks)packages/loot-core/src/server/aql/schema/index.ts(1 hunks)packages/loot-core/src/server/importers/ynab5-types.ts(2 hunks)packages/loot-core/src/server/importers/ynab5.ts(4 hunks)packages/loot-core/src/server/payees/app.ts(4 hunks)packages/loot-core/src/shared/constants.ts(1 hunks)packages/loot-core/src/shared/location-adapters.ts(1 hunks)packages/loot-core/src/shared/location-integration.test.ts(1 hunks)packages/loot-core/src/shared/location-utils.test.ts(1 hunks)packages/loot-core/src/shared/location-utils.ts(1 hunks)packages/loot-core/src/shared/location.ts(1 hunks)packages/loot-core/src/types/api-handlers.ts(2 hunks)packages/loot-core/src/types/models/index.ts(1 hunks)packages/loot-core/src/types/models/payee-location.ts(1 hunks)packages/loot-core/src/types/models/payee.ts(2 hunks)packages/loot-core/src/types/prefs.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (49)
📚 Learning: 2025-01-12T20:26:15.249Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 4141
File: packages/loot-core/src/server/aql/schema/index.ts:145-145
Timestamp: 2025-01-12T20:26:15.249Z
Learning: In the schema definitions in `packages/loot-core/src/server/aql/schema/index.ts`, field definitions should follow the existing pattern using only the common properties (`type`, `default`, `required`, `ref`) for consistency.
Applied to files:
packages/loot-core/src/server/aql/schema/index.tspackages/loot-core/src/types/models/index.ts
📚 Learning: 2024-11-04T14:14:10.698Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 3581
File: packages/loot-core/src/types/models/bank-sync.d.ts:11-21
Timestamp: 2024-11-04T14:14:10.698Z
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.
Applied to files:
packages/loot-core/src/server/aql/schema/index.tspackages/loot-core/src/types/models/index.tspackages/loot-core/src/types/prefs.tspackages/loot-core/src/server/api-models.tspackages/loot-core/src/types/models/payee-location.tspackages/loot-core/src/types/models/payee.tspackages/loot-core/src/server/api.tspackages/loot-core/src/server/importers/ynab5-types.tspackages/loot-core/src/types/api-handlers.tspackages/loot-core/src/server/payees/app.ts
📚 Learning: 2024-10-08T23:41:32.134Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 3609
File: packages/loot-core/src/server/db/index.ts:583-598
Timestamp: 2024-10-08T23:41:32.134Z
Learning: In `packages/loot-core/src/server/db/index.ts`, the `orphanedPayeesQuery` uses a subquery with `json_each(r.conditions) AS cond` to loop through JSON conditions because the "description" field may not be the first index. This implementation must remain as is to ensure correct functionality.
Additionally, table aliases are already used consistently in the SQL queries within this file.
Applied to files:
packages/loot-core/src/server/aql/schema/index.tspackages/loot-core/migrations/1763316795000_add_payee_locations.sql
📚 Learning: 2025-10-12T04:07:06.002Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 5786
File: packages/api/tsconfig.dist.json:14-14
Timestamp: 2025-10-12T04:07:06.002Z
Learning: In the Actual Budget codebase, when rootDir is removed from packages/loot-core/tsconfig.api.json to allow referencing files outside the loot-core directory, the declaration output structure changes. The path alias for loot-core in packages/api/tsconfig.dist.json must be updated from "./types/loot-core/src/*" to "./types/loot-core/loot-core/src/*" to match the new emitted declaration paths, as TypeScript preserves the full directory structure from the project root when rootDir is not specified.
Applied to files:
packages/loot-core/src/server/aql/schema/index.tspackages/loot-core/src/shared/constants.tspackages/loot-core/src/types/models/index.tspackages/loot-core/src/shared/location-utils.test.tspackages/loot-core/src/server/api-models.tspackages/loot-core/src/types/models/payee-location.tspackages/loot-core/src/types/models/payee.tspackages/loot-core/src/shared/location-integration.test.tspackages/loot-core/src/server/api.tspackages/loot-core/src/server/importers/ynab5-types.tspackages/loot-core/src/shared/location-adapters.tspackages/loot-core/src/shared/location.tspackages/loot-core/src/types/api-handlers.tspackages/loot-core/src/server/payees/app.tspackages/loot-core/src/shared/location-utils.ts
📚 Learning: 2025-01-02T18:25:14.566Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 4041
File: packages/loot-core/src/types/util.d.ts:13-16
Timestamp: 2025-01-02T18:25:14.566Z
Learning: In `packages/loot-core/src/types/util.d.ts`, `TransObjectLiteral` remains typed as `any` due to the react-i18next issue (https://github.com/i18next/react-i18next/issues/1483) not being resolved yet.
Applied to files:
packages/loot-core/src/types/models/index.ts
📚 Learning: 2024-10-04T08:48:55.161Z
Learnt from: MikesGlitch
Repo: actualbudget/actual PR: 3553
File: packages/desktop-electron/package.json:88-88
Timestamp: 2024-10-04T08:48:55.161Z
Learning: In the `desktop-electron` module, `loot-core`'s essential files are intentionally copied during packaging using `electron-builder`. Therefore, references to `loot-core` in the codebase are expected and should remain, even though the `loot-core` dependency is removed from `package.json`.
Applied to files:
packages/loot-core/src/types/models/index.ts
📚 Learning: 2024-10-25T05:04:40.902Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3689
File: packages/loot-core/src/platform/server/fs/index.web.ts:319-340
Timestamp: 2024-10-25T05:04:40.902Z
Learning: In `packages/loot-core/src/platform/server/fs/index.web.ts`, errors are already handled in other locations, so additional error handling in `copyFile` is unnecessary.
Applied to files:
packages/loot-core/src/types/models/index.tspackages/loot-core/src/server/api.ts
📚 Learning: 2025-08-16T16:42:08.306Z
Learnt from: sjones512
Repo: actualbudget/actual PR: 5554
File: packages/desktop-client/src/components/reports/spreadsheets/crossover-spreadsheet.ts:7-8
Timestamp: 2025-08-16T16:42:08.306Z
Learning: In the Actual Budget codebase, it's the established pattern and acceptable to import useSpreadsheet as a type-only import and then use ReturnType<typeof useSpreadsheet> in type annotations. This pattern is used consistently across all spreadsheet files including custom-spreadsheet.ts, grouped-spreadsheet.ts, spending-spreadsheet.ts, net-worth-spreadsheet.ts, cash-flow-spreadsheet.tsx, calendar-spreadsheet.ts, summary-spreadsheet.ts, and crossover-spreadsheet.ts.
Applied to files:
packages/loot-core/src/types/models/index.ts
📚 Learning: 2025-06-16T17:41:34.452Z
Learnt from: misu-dev
Repo: actualbudget/actual PR: 5167
File: packages/desktop-client/src/components/spreadsheet/useFormat.ts:64-72
Timestamp: 2025-06-16T17:41:34.452Z
Learning: The format function in useFormat.ts is widely used across the application and currently needs to accept both string and number inputs for financial format types. While strict type checking is preferred long-term, string parsing is maintained as a pragmatic compromise during the currency feature development phase.
Applied to files:
packages/desktop-client/src/hooks/useUnitOfMeasurementFormat.tspackages/desktop-client/src/components/settings/Format.tsx
📚 Learning: 2025-01-16T14:30:36.518Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 4166
File: packages/loot-core/src/client/data-hooks/transactions.ts:155-158
Timestamp: 2025-01-16T14:30:36.518Z
Learning: In packages/loot-core/src/client/data-hooks/transactions.ts, the `upcomingLength` preference is always stored as a number in string format, so no additional type checking is needed when using `parseInt`.
Applied to files:
packages/desktop-client/src/hooks/useUnitOfMeasurementFormat.tspackages/loot-core/src/types/prefs.ts
📚 Learning: 2025-06-14T21:20:46.938Z
Learnt from: misu-dev
Repo: actualbudget/actual PR: 5167
File: packages/desktop-client/src/components/spreadsheet/useFormat.ts:161-166
Timestamp: 2025-06-14T21:20:46.938Z
Learning: In the useFormat hook's formatDisplay function, there's an intentional separation between displayDecimalPlaces (used for Intl.NumberFormat display formatting) and activeCurrency.decimalPlaces (used for integerToCurrency conversion logic). These should not be mixed as displayDecimalPlaces controls how many decimals to show while activeCurrency.decimalPlaces controls the mathematical conversion from integer amounts to decimal amounts.
Applied to files:
packages/desktop-client/src/hooks/useUnitOfMeasurementFormat.ts
📚 Learning: 2025-01-08T20:54:04.680Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 4108
File: tsconfig.json:22-22
Timestamp: 2025-01-08T20:54:04.680Z
Learning: Jest testing framework is used in multiple packages (api, crdt, loot-core) while desktop-client uses Vitest. Both Jest and Vitest types should be kept in the root tsconfig.json.
Applied to files:
packages/loot-core/src/shared/location-utils.test.tspackages/loot-core/src/shared/location-integration.test.ts
📚 Learning: 2024-09-17T20:04:47.663Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 3458
File: packages/loot-core/src/client/state-types/prefs.d.ts:5-5
Timestamp: 2024-09-17T20:04:47.663Z
Learning: In future reviews, ensure that changes related to `PrefsState` in `prefs.d.ts` do not incorrectly assume necessary updates in other parts of the codebase. Verify the impact thoroughly before making suggestions.
Applied to files:
packages/loot-core/src/types/prefs.ts
📚 Learning: 2024-10-02T14:56:29.404Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 3544
File: packages/loot-core/src/client/state-types/prefs.d.ts:13-18
Timestamp: 2024-10-02T14:56:29.404Z
Learning: When action types like `SetPrefsAction` are updated with new properties, ensure that reducers are correctly handling these properties before suggesting a potential issue.
Applied to files:
packages/loot-core/src/types/prefs.tspackages/desktop-client/src/payees/payeesSlice.ts
📚 Learning: 2024-10-13T23:24:09.179Z
Learnt from: tim-smart
Repo: actualbudget/actual PR: 3343
File: packages/desktop-client/src/components/common/Modal.tsx:86-86
Timestamp: 2024-10-13T23:24:09.179Z
Learning: In `packages/desktop-client/src/components/common/Modal.tsx`, setting `willChange: 'transform'` forces the browser to render the layer with GPU acceleration, improving performance.
Applied to files:
packages/desktop-client/src/modals/modalsSlice.tspackages/desktop-client/src/components/modals/PayeeAutocompleteModal.tsx
📚 Learning: 2024-10-25T04:49:31.861Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3689
File: packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx:144-156
Timestamp: 2024-10-25T04:49:31.861Z
Learning: In `packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx`, styles for buttons may differ for each button in the future, so avoid suggesting extraction of common styles in this file.
Applied to files:
packages/desktop-client/src/modals/modalsSlice.tspackages/desktop-client/src/components/modals/manager/ImportYNAB5Modal.tsx
📚 Learning: 2024-10-08T15:46:15.739Z
Learnt from: UnderKoen
Repo: actualbudget/actual PR: 3365
File: packages/loot-core/src/types/models/rule.d.ts:4-4
Timestamp: 2024-10-08T15:46:15.739Z
Learning: In `packages/loot-core/src/server/accounts/transaction-rules.ts`, the `stage` property can have legacy values `'cleanup'` and `'modify'`, which are converted to `'pre'`. The type remains `string` to accommodate these values and ensure correct usage.
Applied to files:
packages/loot-core/src/server/api-models.ts
📚 Learning: 2025-11-04T00:47:00.968Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 6065
File: packages/desktop-client/src/components/transactions/TransactionList.tsx:197-205
Timestamp: 2025-11-04T00:47:00.968Z
Learning: In packages/desktop-client/src/components/transactions/TransactionList.tsx and similar schedule creation code, when using schedule/create with conditions and then updating the rule's actions, it's correct to filter rule.actions to keep only 'link-schedule' and append custom actions. Transactions created by schedules are defined by a mix of conditions (which create the base transaction with date, amount, payee, account) and actions (which apply additional modifications like category, notes, splits). The filtering pattern replaces any auto-generated actions with the desired custom actions.
Applied to files:
packages/desktop-client/src/payees/payeesSlice.tspackages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-10-09T20:17:46.493Z
Learnt from: UnderKoen
Repo: actualbudget/actual PR: 3619
File: packages/loot-core/src/server/accounts/transaction-rules.ts:551-0
Timestamp: 2024-10-09T20:17:46.493Z
Learning: When finalizing transactions that involve inserting or retrieving payees, avoid using `Promise.all` as it may result in duplicate payees due to concurrent operations. Sequential processing ensures payees are correctly handled without duplication.
Applied to files:
packages/desktop-client/src/payees/payeesSlice.tspackages/loot-core/src/server/importers/ynab5.tspackages/desktop-client/src/components/autocomplete/PayeeAutocomplete.test.tsxpackages/loot-core/src/server/payees/app.tspackages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsxpackages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-10-24T17:05:41.415Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-10-24T17:05:41.415Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
Applied to files:
packages/loot-core/src/types/models/payee.tspackages/desktop-client/src/components/autocomplete/PayeeAutocomplete.test.tsxpackages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsxpackages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsxpackages/desktop-client/src/components/modals/PayeeAutocompleteModal.tsx
📚 Learning: 2025-01-03T03:27:48.005Z
Learnt from: dkhalife
Repo: actualbudget/actual PR: 3984
File: packages/desktop-client/src/components/modals/SelectLinkedAccountsModal.tsx:43-68
Timestamp: 2025-01-03T03:27:48.005Z
Learning: We have 2 types for accounts: `NormalizedAccount` (bank account ID field is `id`) and `AccountEntity` (database ID field is `id` plus a `account_id` that stores the bank’s ID). Accordingly, `account_id` shouldn't be replaced by `id` in `AccountEntity`.
Applied to files:
packages/loot-core/src/types/models/payee.ts
📚 Learning: 2025-10-12T04:00:51.978Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 5786
File: packages/plugins-core/vite.config.mts:27-35
Timestamp: 2025-10-12T04:00:51.978Z
Learning: In the Actual Budget plugins architecture, the actual-app/plugins-core package is designed to be self-contained. Plugin authors should only need to install actual-app/plugins-core, which bundles the internal monorepo packages (actual-app/shared-types, actual-app/query, actual-app/components) at build time. Only common dependencies like React, React-DOM, and i18next are kept as peerDependencies/externals to avoid duplication with the host application.
Applied to files:
packages/loot-core/src/server/importers/ynab5.ts
📚 Learning: 2024-10-23T07:38:25.699Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3689
File: packages/loot-core/src/server/main.ts:1807-1809
Timestamp: 2024-10-23T07:38:25.699Z
Learning: In the function `delete-budget` in `packages/loot-core/src/server/main.ts`, the function `removeAllBackups(id)` handles its own errors internally, so additional error handling when calling it is unnecessary.
Applied to files:
packages/loot-core/src/server/api.ts
📚 Learning: 2024-10-24T05:09:44.115Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3689
File: packages/loot-core/src/server/backups.web.ts:203-207
Timestamp: 2024-10-24T05:09:44.115Z
Learning: In `packages/loot-core/src/server/backups.web.ts`, developers prefer to keep the error handling for cloud storage uploads inline rather than extracting it into a separate function. Avoid suggesting this refactoring in future reviews.
Applied to files:
packages/loot-core/src/server/api.ts
📚 Learning: 2024-10-12T19:13:25.005Z
Learnt from: jfdoming
Repo: actualbudget/actual PR: 3641
File: packages/loot-core/src/server/accounts/rules.test.ts:524-536
Timestamp: 2024-10-12T19:13:25.005Z
Learning: In `packages/loot-core/src/server/accounts/rules.test.ts`, prefer explicit action definitions over refactoring similar actions into loops or helper functions, even when actions are similar.
Applied to files:
packages/loot-core/src/server/api.tspackages/loot-core/src/server/payees/app.ts
📚 Learning: 2024-10-08T15:46:15.739Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 3570
File: packages/desktop-client/src/components/modals/ImportTransactionsModal/utils.ts:120-130
Timestamp: 2024-10-08T15:46:15.739Z
Learning: Avoid overcomplicating the `ImportTransaction` type; prefer to keep it simple.
Applied to files:
packages/loot-core/src/types/api-handlers.ts
📚 Learning: 2024-10-09T20:30:39.127Z
Learnt from: UnderKoen
Repo: actualbudget/actual PR: 3619
File: packages/loot-core/src/server/accounts/transaction-rules.ts:795-801
Timestamp: 2024-10-09T20:30:39.127Z
Learning: In the `finalizeTransactionForRules` function within `packages/loot-core/src/server/accounts/transaction-rules.ts`, potential race conditions when inserting payees are handled in the method that calls this function, so additional safeguards within `finalizeTransactionForRules` are unnecessary.
Applied to files:
packages/loot-core/src/server/payees/app.ts
📚 Learning: 2025-10-06T15:26:36.914Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 5870
File: packages/loot-core/src/server/transactions/transaction-rules.ts:345-348
Timestamp: 2025-10-06T15:26:36.914Z
Learning: In packages/loot-core/src/server/rules/rule.ts, Rule.execActions<T>(object) returns a Partial<T> diff object (not an array). When actions create splits, the top-level execActions returns a grouped transaction with a subtransactions array; the Rule.execActions diff includes subtransactions. Merging via Object.assign preserves splits.
Applied to files:
packages/loot-core/src/server/payees/app.ts
📚 Learning: 2024-11-26T13:07:02.794Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3891
File: packages/loot-core/src/shared/rules.ts:209-212
Timestamp: 2024-11-26T13:07:02.794Z
Learning: The file `packages/loot-core/src/shared/rules.ts` is not yet translated, so internationalization using the `t()` function is not required here.
Applied to files:
packages/loot-core/src/shared/location-utils.ts
📚 Learning: 2024-09-27T14:15:46.637Z
Learnt from: jfdoming
Repo: actualbudget/actual PR: 3402
File: packages/desktop-client/src/components/accounts/Account.tsx:8-8
Timestamp: 2024-09-27T14:15:46.637Z
Learning: In the `AllTransactions` component of `Account.tsx`, the `useLayoutEffect` hook is appropriately used to dispatch an action that closes splits for parent transactions when `prependTransactions` changes, ensuring this occurs synchronously before the component is painted.
Applied to files:
packages/desktop-client/src/hooks/usePayees.tspackages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsxpackages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-10-05T10:58:55.008Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 3570
File: packages/desktop-client/src/components/modals/ImportTransactionsModal/utils.ts:16-24
Timestamp: 2024-10-05T10:58:55.008Z
Learning: In `packages/desktop-client/src/components/modals/ImportTransactionsModal/utils.ts`, the `parseDate` function's `str` parameter should maintain its current type `string | number | null | Array<unknown> | object`, as narrowing it to `string | null` is not suitable.
Applied to files:
packages/desktop-client/src/components/modals/manager/ImportYNAB5Modal.tsx
📚 Learning: 2024-10-16T03:51:04.683Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3593
File: packages/desktop-client/src/components/sidebar/BudgetName.tsx:114-136
Timestamp: 2024-10-16T03:51:04.683Z
Learning: In 'packages/desktop-client/src/components/sidebar/BudgetName.tsx', empty budget names are handled elsewhere, so additional error handling within the 'EditableBudgetName' component is unnecessary.
Applied to files:
packages/desktop-client/src/components/modals/manager/ImportYNAB5Modal.tsxpackages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-01-16T14:28:00.133Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 4166
File: packages/loot-core/src/client/data-hooks/transactions.ts:258-258
Timestamp: 2025-01-16T14:28:00.133Z
Learning: Variables defined within React hooks like `useMemo` or `useEffect` are not dependencies of other hooks. Only external variables and values from the component scope need to be included in the dependency arrays.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2024-10-22T05:34:56.976Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx:261-277
Timestamp: 2024-10-22T05:34:56.976Z
Learning: In React components (TypeScript), avoid using hooks like `useCallback` inside callbacks or nested functions. Hooks must be called at the top level of functional components.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2024-10-22T05:32:30.530Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/desktop-client/src/components/schedules/index.tsx:37-39
Timestamp: 2024-10-22T05:32:30.530Z
Learning: In our React function components, we should include `dispatch` in the dependency array of `useCallback` hooks to comply with ESLint rules, even though `dispatch` is a stable function.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2025-05-22T05:17:51.422Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 4955
File: packages/component-library/src/Input.tsx:52-64
Timestamp: 2025-05-22T05:17:51.422Z
Learning: React 19 has simplified ref handling, eliminating the need for `React.forwardRef()` in functional components. Components can now receive and destructure the ref directly from props rather than requiring the second parameter in forwardRef.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2025-01-16T19:31:27.020Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 4169
File: packages/component-library/src/icons/Loading.tsx:8-8
Timestamp: 2025-01-16T19:31:27.020Z
Learning: The Loading component in actual-app/components package is a direct copy from its previous location, and improvements to its accessibility and security are planned to be addressed in a separate PR.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2025-05-22T05:17:51.422Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 4955
File: packages/component-library/src/Input.tsx:52-64
Timestamp: 2025-05-22T05:17:51.422Z
Learning: In React 19, function components can receive refs directly through props without requiring React.forwardRef(). This simplified ref forwarding feature allows directly destructuring the ref prop and passing it along to DOM elements or other components.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2025-02-11T18:44:28.613Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 0
File: :0-0
Timestamp: 2025-02-11T18:44:28.613Z
Learning: The Button2 component is deprecated. Use actual-app/components/button instead.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2024-10-16T03:46:34.277Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3593
File: packages/desktop-client/src/components/sidebar/BudgetName.tsx:151-165
Timestamp: 2024-10-16T03:46:34.277Z
Learning: In `packages/desktop-client/src/components/sidebar/BudgetName.tsx`, `media` is not implemented on `<Button>` components at this time.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2024-10-10T02:29:05.655Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3593
File: packages/desktop-client/src/components/sidebar/Sidebar.tsx:112-116
Timestamp: 2024-10-10T02:29:05.655Z
Learning: In `packages/desktop-client/src/components/sidebar/BudgetName.tsx`, the `BudgetName` component consists of three parts: `BudgetName`, `EditBudgetName`, and the Menu. Keeping `EditBudgetName` as a separate component helps maintain cleaner code by separating concerns.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2024-10-14T09:03:37.410Z
Learnt from: minajevs
Repo: actualbudget/actual PR: 3274
File: packages/desktop-client/src/components/mobile/transactions/TransactionEdit.jsx:0-0
Timestamp: 2024-10-14T09:03:37.410Z
Learning: In `packages/desktop-client/src/components/mobile/transactions/TransactionEdit.jsx`, within the `onEditField` function, the `unserializedTransaction` variable is always expected to be found when accessing its properties. Adding a null check may not be necessary.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-11-09T20:18:28.468Z
Learnt from: csenel
Repo: actualbudget/actual PR: 3810
File: packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx:150-161
Timestamp: 2024-11-09T20:18:28.468Z
Learning: In `packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx`, prefer to keep the implementation of checks consistent with similar patterns elsewhere in the codebase, even if alternative implementations are more concise.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-10-05T10:58:13.598Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 3570
File: packages/desktop-client/src/components/modals/ImportTransactionsModal/Transaction.tsx:83-90
Timestamp: 2024-10-05T10:58:13.598Z
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`.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-03-14T15:11:36.220Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 4484
File: packages/desktop-client/src/components/mobile/budget/ExpenseGroup.tsx:0-0
Timestamp: 2025-03-14T15:11:36.220Z
Learning: In the Actual Budget app, the callback functions `onEditCategory` and `onBudgetAction` are already properly memoized with useCallback in the Budget component (packages/desktop-client/src/components/mobile/budget/index.tsx).
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-03-14T15:11:36.220Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 4484
File: packages/desktop-client/src/components/mobile/budget/ExpenseGroup.tsx:0-0
Timestamp: 2025-03-14T15:11:36.220Z
Learning: In the Actual Budget mobile app, the callback functions `onEditCategory` (implemented as `onOpenCategoryMenuModal`) and `onBudgetAction` are already properly memoized with useCallback in the Budget component (packages/desktop-client/src/components/mobile/budget/index.tsx).
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-11-12T18:18:07.282Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3828
File: packages/desktop-client/src/components/reports/reports/Calendar.tsx:575-631
Timestamp: 2024-11-12T18:18:07.282Z
Learning: In `Calendar.tsx`, transaction-related callbacks such as `onBatchDelete`, `onBatchDuplicate`, `onCreateRule`, and `onScheduleAction` are intentionally left as empty functions because these operations should not be usable on that page.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-06-21T04:15:23.727Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 5207
File: packages/desktop-client/src/components/transactions/TransactionsTable.tsx:2194-2201
Timestamp: 2025-06-21T04:15:23.727Z
Learning: In TransactionsTable.tsx, the balance calculation using find() to locate the first non-scheduled transaction is acceptable for performance because: 1) the component only renders when adding transactions (limited usage), 2) find() short-circuits on first match, and 3) scheduled transactions are typically sparse in the transaction list.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-10-22T02:08:48.162Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/loot-core/src/client/data-hooks/transactions.ts:32-88
Timestamp: 2024-10-22T02:08:48.162Z
Learning: In the React `useTransactions` hook defined in `packages/loot-core/src/client/data-hooks/transactions.ts`, changes to `options` (e.g., `options.pageCount`) should be applied on the next query without adding them to the `useEffect` dependency array, to avoid unnecessary rerenders when options change.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
🧬 Code graph analysis (20)
packages/desktop-client/src/hooks/useLocationPermission.ts (2)
packages/component-library/src/hooks/useResponsive.ts (1)
useResponsive(5-23)packages/loot-core/src/shared/location.ts (1)
locationService(144-147)
packages/desktop-client/src/hooks/useUnitOfMeasurementFormat.ts (1)
packages/desktop-client/src/hooks/useSyncedPref.ts (1)
useSyncedPref(12-29)
packages/loot-core/src/shared/location-utils.test.ts (2)
packages/loot-core/src/shared/location-utils.ts (2)
calculateDistance(21-40)formatDistance(48-72)packages/loot-core/src/shared/location.ts (1)
formatDistance(31-33)
packages/loot-core/src/server/api-models.ts (1)
packages/loot-core/src/types/models/payee.ts (1)
PayeeEntity(4-12)
packages/desktop-client/src/payees/payeesSlice.ts (4)
packages/loot-core/src/types/models/payee.ts (1)
PayeeEntity(4-12)packages/loot-core/src/shared/location.ts (3)
deletePayeeLocation(75-82)getNearbyPayees(84-94)locationService(144-147)packages/loot-core/src/shared/location-adapters.ts (1)
getNearbyPayees(88-98)packages/desktop-client/src/redux/index.ts (1)
createAppAsyncThunk(12-15)
packages/loot-core/src/types/models/payee.ts (1)
packages/loot-core/src/types/models/payee-location.ts (1)
PayeeLocationEntity(1-8)
packages/loot-core/src/shared/location-integration.test.ts (5)
packages/loot-core/src/shared/location-adapters.ts (2)
GeolocationAdapter(12-14)LocationApiClient(19-30)packages/loot-core/src/shared/location-utils.ts (1)
LocationCoordinates(10-13)packages/loot-core/src/types/models/payee-location.ts (1)
PayeeLocationEntity(1-8)packages/loot-core/src/types/models/payee.ts (1)
PayeeEntity(4-12)packages/loot-core/src/shared/location.ts (2)
locationService(144-147)LocationService(21-141)
packages/loot-core/src/server/importers/ynab5.ts (2)
packages/loot-core/src/server/importers/ynab5-types.ts (2)
Budget(1-12)PayeeLocationsData(14-18)packages/loot-core/src/platform/server/log/index.ts (1)
logger(11-43)
packages/loot-core/src/server/api.ts (1)
packages/loot-core/src/server/main.ts (1)
handlers(51-51)
packages/loot-core/src/shared/location-adapters.ts (3)
packages/loot-core/src/shared/location-utils.ts (1)
LocationCoordinates(10-13)packages/loot-core/src/types/models/payee-location.ts (1)
PayeeLocationEntity(1-8)packages/loot-core/src/types/models/payee.ts (1)
PayeeEntity(4-12)
packages/loot-core/src/shared/location.ts (5)
packages/loot-core/src/shared/location-utils.ts (3)
LocationCoordinates(10-13)formatDistance(48-72)calculateDistance(21-40)packages/loot-core/src/shared/location-adapters.ts (4)
GeolocationAdapter(12-14)LocationApiClient(19-30)BrowserGeolocationAdapter(35-63)SendApiLocationClient(68-99)packages/loot-core/src/types/models/payee-location.ts (1)
PayeeLocationEntity(1-8)packages/loot-core/src/shared/constants.ts (1)
DEFAULT_MAX_DISTANCE(1-1)packages/loot-core/src/types/models/payee.ts (1)
PayeeEntity(4-12)
packages/desktop-client/src/components/settings/Format.tsx (4)
packages/loot-core/src/types/prefs.ts (1)
SyncedPrefs(14-54)packages/desktop-client/src/hooks/useSyncedPref.ts (1)
useSyncedPref(12-29)packages/desktop-client/src/components/settings/UI.tsx (1)
Column(100-123)packages/component-library/src/Select.tsx (1)
Select(44-138)
packages/loot-core/src/types/api-handlers.ts (2)
packages/loot-core/src/types/models/payee-location.ts (1)
PayeeLocationEntity(1-8)packages/loot-core/src/server/api-models.ts (1)
APIPayeeEntity(102-105)
packages/loot-core/src/server/payees/app.ts (6)
packages/loot-core/src/shared/location.ts (3)
getPayeeLocations(66-73)deletePayeeLocation(75-82)getNearbyPayees(84-94)packages/desktop-client/src/payees/payeesSlice.ts (2)
deletePayeeLocation(205-216)getNearbyPayees(262-288)packages/loot-core/src/shared/location-adapters.ts (1)
getNearbyPayees(88-98)packages/loot-core/src/types/models/payee.ts (1)
PayeeEntity(4-12)packages/loot-core/src/types/models/payee-location.ts (1)
PayeeLocationEntity(1-8)packages/loot-core/src/shared/constants.ts (1)
DEFAULT_MAX_DISTANCE(1-1)
packages/loot-core/src/shared/location-utils.ts (1)
packages/loot-core/src/shared/location.ts (1)
formatDistance(31-33)
packages/desktop-client/src/hooks/usePayees.ts (3)
packages/desktop-client/src/redux/index.ts (2)
useDispatch(18-18)useSelector(19-19)packages/loot-core/src/shared/location.ts (2)
locationService(144-147)getNearbyPayees(84-94)packages/desktop-client/src/payees/payeesSlice.ts (1)
getNearbyPayees(262-288)
packages/desktop-client/src/components/modals/manager/ImportYNAB5Modal.tsx (1)
packages/component-library/src/Paragraph.tsx (1)
Paragraph(12-32)
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx (8)
packages/loot-core/src/types/models/payee.ts (1)
PayeeEntity(4-12)packages/desktop-client/src/hooks/usePayees.ts (2)
usePayees(66-78)useNearbyPayees(31-64)packages/desktop-client/src/components/autocomplete/Autocomplete.tsx (1)
defaultFilterSuggestion(96-102)packages/desktop-client/src/payees/payeesSlice.ts (3)
deletePayeeLocation(205-216)createPayee(169-189)updatePayeeLocationIfNeeded(191-203)packages/desktop-client/src/hooks/useLocationPermission.ts (1)
useLocationPermission(12-58)packages/desktop-client/src/hooks/useUnitOfMeasurementFormat.ts (1)
useUnitOfMeasurementFormat(3-6)packages/loot-core/src/shared/location-utils.ts (1)
formatDistance(48-72)packages/component-library/src/TextOneLine.tsx (1)
TextOneLine(7-22)
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx (8)
packages/component-library/src/Button.tsx (1)
Button(139-185)packages/component-library/src/Text.tsx (1)
Text(19-28)packages/desktop-client/src/hooks/useLocationPermission.ts (1)
useLocationPermission(12-58)packages/loot-core/src/types/models/payee.ts (1)
PayeeEntity(4-12)packages/loot-core/src/shared/location.ts (2)
locationService(144-147)deletePayeeLocation(75-82)packages/loot-core/src/shared/location-utils.ts (1)
calculateDistance(21-40)packages/loot-core/src/shared/constants.ts (1)
DEFAULT_MAX_DISTANCE(1-1)packages/desktop-client/src/payees/payeesSlice.ts (1)
deletePayeeLocation(205-216)
packages/desktop-client/src/components/modals/PayeeAutocompleteModal.tsx (1)
packages/desktop-client/src/hooks/usePayees.ts (2)
usePayees(66-78)useNearbyPayees(31-64)
packages/desktop-client/src/components/modals/manager/ImportYNAB5Modal.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/desktop-client/src/hooks/useLocationPermission.ts (1)
24-25: Consider checking API existence before calling.While the catch block handles unsupported APIs, checking
navigator.permissionsexistence first would be more defensive and make the intent clearer.+ if (!navigator.permissions) { + setLocationAccess(false); + return; + } + navigator.permissions .query({ name: 'geolocation' })packages/desktop-client/src/payees/payeesSlice.ts (2)
373-381: Consider translating nearby payees for consistency.Unlike
_loadCommonPayees(Line 367) and_loadPayees(Line 384),_loadNearbyPayeesdoes not calltranslatePayeeson the payee list. While nearby payees are unlikely to include the special "Starting Balance" payee that requires translation, applyingtranslatePayeesensures consistency and defensive programming.Apply this diff to align with the established pattern:
function _loadNearbyPayees( state: PayeesState, nearbyPayees: PayeesState['nearbyPayees'], ) { - state.nearbyPayees = nearbyPayees; + state.nearbyPayees = translatePayees(nearbyPayees) as PayeeEntity[]; state.isNearbyPayeesLoading = false; state.isNearbyPayeesLoaded = true; state.isNearbyPayeesDirty = false; }Alternatively, if nearby payees are guaranteed never to include special payees, add a comment explaining why translation is skipped.
176-185: Update comment to reflect actual behavior.The comment on lines 176-177 states: "If locationAccess is enabled and we're on mobile (narrow width)", but the code only checks the
locationAccessboolean parameter without verifying whether the device is mobile or has a narrow width. This could be misleading for future maintainers.Apply this diff to clarify the comment:
- // If locationAccess is enabled and we're on mobile (narrow width), - // attempt to save the current location for this payee + // If locationAccess is enabled, attempt to save the current location for this payee + // (locationAccess is typically set based on user permission and device capabilities)Or, if the mobile/narrow-width check happens in the caller, reference that in the comment.
packages/loot-core/src/server/importers/ynab5.ts (2)
184-222: Validate imported coordinates before creating payee locationsThe import flow and ordering (after payees, before transactions) look solid, and it’s good that deleted locations and unknown payees are skipped. One robustness gap is that latitude/longitude are parsed and forwarded without validation. If the YNAB export ever contains empty or malformed coordinate values, you could end up trying to create locations with
NaNcoordinates.Consider validating that both coordinates are finite numbers before sending the create message, and logging/skipping invalid entries. For example:
- try { - // Create the payee location in Actual - await send('payee-location-create', { - payee_id: actualPayeeId, - latitude: parseFloat(location.latitude), - longitude: parseFloat(location.longitude), - }); - } catch (error) { - logger.log( - `Failed to import location for payee ${actualPayeeId}: ${error.message}`, - ); - } + const latitude = Number(location.latitude); + const longitude = Number(location.longitude); + + if (!Number.isFinite(latitude) || !Number.isFinite(longitude)) { + logger.warn( + `Skipping location with invalid coordinates for payee ${actualPayeeId}`, + ); + continue; + } + + try { + await send('payee-location-create', { + payee_id: actualPayeeId, + latitude, + longitude, + }); + } catch (error) { + logger.log( + `Failed to import location for payee ${actualPayeeId}: ${error.message}`, + ); + }This keeps the happy path unchanged while avoiding persisting obviously bad coordinates.
Also applies to: 553-557
569-625: Confirm ZIP budget entry matching covers all YNAB5 export variantsThe ZIP handling and normalization look good and keep the non‑ZIP code path intact. One thing to double‑check is the
budgetEntrypredicate:const budgetEntry = entries.find(entry => entry.entryName.toLowerCase().endsWith('budget.json') || entry.entryName.toLowerCase().endsWith('data.json') || entry.entryName.toLowerCase() === 'budget.json' || entry.entryName.toLowerCase() === 'data.json', );If all YNAB5 exports you care about always contain a
budget.jsonordata.json(possibly in a subdirectory), this is perfectly fine. If there are variants that use different filenames or extensions, this will throw even though the budget file is present.Please sanity‑check this against a couple of real YNAB5 ZIP exports; if you do see other patterns, it may be worth broadening the predicate (e.g., allowing more filename variants or making the target entry configurable) before relying on this in production.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx(14 hunks)packages/desktop-client/src/hooks/useLocationPermission.ts(1 hunks)packages/desktop-client/src/hooks/usePayees.ts(2 hunks)packages/desktop-client/src/payees/payeesSlice.ts(10 hunks)packages/loot-core/src/server/importers/ynab5.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/desktop-client/src/hooks/usePayees.ts
- packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
🧰 Additional context used
🧠 Learnings (13)
📚 Learning: 2024-10-25T18:30:24.287Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3309
File: packages/desktop-client/src/components/ColumnWidthContext.jsx:17-17
Timestamp: 2024-10-25T18:30:24.287Z
Learning: In `packages/desktop-client/src/components/ColumnWidthContext.jsx`, replacing `useState` with `useRef` for `positionAccumulator` causes the resize action to become unpredictable. Therefore, it's preferable to keep using `useState` for `positionAccumulator` in this context.
Applied to files:
packages/desktop-client/src/hooks/useLocationPermission.ts
📚 Learning: 2024-09-17T20:04:47.663Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 3458
File: packages/loot-core/src/client/state-types/prefs.d.ts:5-5
Timestamp: 2024-09-17T20:04:47.663Z
Learning: In future reviews, ensure that changes related to `PrefsState` in `prefs.d.ts` do not incorrectly assume necessary updates in other parts of the codebase. Verify the impact thoroughly before making suggestions.
Applied to files:
packages/desktop-client/src/hooks/useLocationPermission.ts
📚 Learning: 2025-10-12T04:07:06.002Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 5786
File: packages/api/tsconfig.dist.json:14-14
Timestamp: 2025-10-12T04:07:06.002Z
Learning: In the Actual Budget codebase, when rootDir is removed from packages/loot-core/tsconfig.api.json to allow referencing files outside the loot-core directory, the declaration output structure changes. The path alias for loot-core in packages/api/tsconfig.dist.json must be updated from "./types/loot-core/src/*" to "./types/loot-core/loot-core/src/*" to match the new emitted declaration paths, as TypeScript preserves the full directory structure from the project root when rootDir is not specified.
Applied to files:
packages/loot-core/src/server/importers/ynab5.ts
📚 Learning: 2024-10-22T08:57:47.316Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3689
File: packages/loot-core/src/server/main.ts:1860-1863
Timestamp: 2024-10-22T08:57:47.316Z
Learning: When duplicating a budget in `packages/loot-core/src/server/main.ts`, `fs.copyFile` can be used to copy `db.sqlite` because it already handles SQL files correctly, making the use of `_copySqlFile` unnecessary.
Applied to files:
packages/loot-core/src/server/importers/ynab5.ts
📚 Learning: 2024-11-26T13:07:02.794Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3891
File: packages/loot-core/src/shared/rules.ts:209-212
Timestamp: 2024-11-26T13:07:02.794Z
Learning: The file `packages/loot-core/src/shared/rules.ts` is not yet translated, so internationalization using the `t()` function is not required here.
Applied to files:
packages/loot-core/src/server/importers/ynab5.ts
📚 Learning: 2024-10-09T20:30:39.127Z
Learnt from: UnderKoen
Repo: actualbudget/actual PR: 3619
File: packages/loot-core/src/server/accounts/transaction-rules.ts:795-801
Timestamp: 2024-10-09T20:30:39.127Z
Learning: In the `finalizeTransactionForRules` function within `packages/loot-core/src/server/accounts/transaction-rules.ts`, potential race conditions when inserting payees are handled in the method that calls this function, so additional safeguards within `finalizeTransactionForRules` are unnecessary.
Applied to files:
packages/loot-core/src/server/importers/ynab5.ts
📚 Learning: 2024-10-24T04:33:06.425Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3689
File: packages/loot-core/src/platform/server/fs/index.web.ts:377-385
Timestamp: 2024-10-24T04:33:06.425Z
Learning: In `packages/loot-core/src/platform/server/fs/index.web.ts`, the `getModifiedTime` function is not used on the web platform.
Applied to files:
packages/loot-core/src/server/importers/ynab5.ts
📚 Learning: 2024-10-24T05:09:44.115Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3689
File: packages/loot-core/src/server/backups.web.ts:203-207
Timestamp: 2024-10-24T05:09:44.115Z
Learning: In `packages/loot-core/src/server/backups.web.ts`, developers prefer to keep the error handling for cloud storage uploads inline rather than extracting it into a separate function. Avoid suggesting this refactoring in future reviews.
Applied to files:
packages/loot-core/src/server/importers/ynab5.ts
📚 Learning: 2024-10-05T10:58:55.008Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 3570
File: packages/desktop-client/src/components/modals/ImportTransactionsModal/utils.ts:16-24
Timestamp: 2024-10-05T10:58:55.008Z
Learning: In `packages/desktop-client/src/components/modals/ImportTransactionsModal/utils.ts`, the `parseDate` function's `str` parameter should maintain its current type `string | number | null | Array<unknown> | object`, as narrowing it to `string | null` is not suitable.
Applied to files:
packages/loot-core/src/server/importers/ynab5.ts
📚 Learning: 2025-10-12T04:00:51.978Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 5786
File: packages/plugins-core/vite.config.mts:27-35
Timestamp: 2025-10-12T04:00:51.978Z
Learning: In the Actual Budget plugins architecture, the actual-app/plugins-core package is designed to be self-contained. Plugin authors should only need to install actual-app/plugins-core, which bundles the internal monorepo packages (actual-app/shared-types, actual-app/query, actual-app/components) at build time. Only common dependencies like React, React-DOM, and i18next are kept as peerDependencies/externals to avoid duplication with the host application.
Applied to files:
packages/loot-core/src/server/importers/ynab5.ts
📚 Learning: 2024-10-09T20:17:46.493Z
Learnt from: UnderKoen
Repo: actualbudget/actual PR: 3619
File: packages/loot-core/src/server/accounts/transaction-rules.ts:551-0
Timestamp: 2024-10-09T20:17:46.493Z
Learning: When finalizing transactions that involve inserting or retrieving payees, avoid using `Promise.all` as it may result in duplicate payees due to concurrent operations. Sequential processing ensures payees are correctly handled without duplication.
Applied to files:
packages/loot-core/src/server/importers/ynab5.tspackages/desktop-client/src/payees/payeesSlice.ts
📚 Learning: 2025-11-04T00:47:00.968Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 6065
File: packages/desktop-client/src/components/transactions/TransactionList.tsx:197-205
Timestamp: 2025-11-04T00:47:00.968Z
Learning: In packages/desktop-client/src/components/transactions/TransactionList.tsx and similar schedule creation code, when using schedule/create with conditions and then updating the rule's actions, it's correct to filter rule.actions to keep only 'link-schedule' and append custom actions. Transactions created by schedules are defined by a mix of conditions (which create the base transaction with date, amount, payee, account) and actions (which apply additional modifications like category, notes, splits). The filtering pattern replaces any auto-generated actions with the desired custom actions.
Applied to files:
packages/desktop-client/src/payees/payeesSlice.ts
📚 Learning: 2024-10-02T14:56:29.404Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 3544
File: packages/loot-core/src/client/state-types/prefs.d.ts:13-18
Timestamp: 2024-10-02T14:56:29.404Z
Learning: When action types like `SetPrefsAction` are updated with new properties, ensure that reducers are correctly handling these properties before suggesting a potential issue.
Applied to files:
packages/desktop-client/src/payees/payeesSlice.ts
🧬 Code graph analysis (3)
packages/desktop-client/src/hooks/useLocationPermission.ts (2)
packages/component-library/src/hooks/useResponsive.ts (1)
useResponsive(5-23)packages/loot-core/src/shared/location.ts (1)
locationService(144-147)
packages/loot-core/src/server/importers/ynab5.ts (3)
packages/loot-core/src/server/importers/ynab5-types.ts (1)
Budget(1-12)packages/loot-core/src/platform/server/log/index.ts (1)
logger(11-43)packages/loot-core/src/platform/server/connection/index.ts (1)
send(124-133)
packages/desktop-client/src/payees/payeesSlice.ts (4)
packages/loot-core/src/types/models/payee.ts (1)
PayeeEntity(4-12)packages/loot-core/src/shared/location.ts (3)
deletePayeeLocation(75-82)getNearbyPayees(84-94)locationService(144-147)packages/loot-core/src/shared/location-adapters.ts (1)
getNearbyPayees(88-98)packages/desktop-client/src/redux/index.ts (1)
createAppAsyncThunk(12-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Generate VRT Updates
- GitHub Check: web
- GitHub Check: validate-cli
- GitHub Check: test
- GitHub Check: build (ubuntu-22.04)
- GitHub Check: build (windows-latest)
- GitHub Check: build (macos-latest)
- GitHub Check: Functional Desktop App
- GitHub Check: Analyze
- GitHub Check: Wait for Netlify build to finish
- GitHub Check: compare
🔇 Additional comments (1)
packages/desktop-client/src/hooks/useLocationPermission.ts (1)
17-19: LGTM! Mobile-only activation and permission prompt are well designed.The narrow width check appropriately limits location features to mobile devices, and triggering
getCurrentPosition()when the permission status is 'prompt' correctly initiates the browser's permission dialog.Also applies to: 39-41
|
✅ VRT screenshots have been automatically updated. |
Auto-generated by VRT workflow PR: actualbudget#6157
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/desktop-client/src/payees/payeesSlice.ts (1)
243-306: LGTM: Thunks correctly use locationService and follow established patterns.Both thunks properly call
locationService.getNearbyPayees(lines 259, 287), addressing the concern from the previous review. The pattern of having separatereloadandgetthunks (with the latter having a condition check) is consistent withgetPayees/reloadPayeesandgetCommonPayees/reloadCommonPayees.Optional: Extract locationParams type.
The
locationParamstype is defined inline in both thunks. While this works, extracting it to a shared type definition could improve maintainability if this structure is used elsewhere.Example:
type LocationParams = { latitude: number; longitude: number; maxDistance?: number; }; export const reloadNearbyPayees = createAppAsyncThunk( `${sliceName}/reloadNearbyPayees`, async (locationParams?: LocationParams) => { // ... }, );
🧹 Nitpick comments (1)
packages/desktop-client/src/payees/payeesSlice.ts (1)
176-185: Consider using console.error for consistency.Lines 183 and 198 use
console.logfor errors, whilelocationServicemethods useconsole.error(see relevant code snippets). For consistency, consider usingconsole.errorhere as well.Apply this diff:
} catch (error) { // Silently handle location errors - don't block payee creation - console.log('Could not save location for new payee:', error); + console.error('Could not save location for new payee:', error); }Also applies to: 198, 212
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/desktop-client/e2e/reports.test.ts-snapshots/Reports-loads-cash-flow-graph-and-checks-visuals-1-chromium-linux.pngis excluded by!**/*.png
📒 Files selected for processing (2)
packages/desktop-client/src/hooks/useLocationPermission.ts(1 hunks)packages/desktop-client/src/payees/payeesSlice.ts(10 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2024-10-25T18:30:24.287Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3309
File: packages/desktop-client/src/components/ColumnWidthContext.jsx:17-17
Timestamp: 2024-10-25T18:30:24.287Z
Learning: In `packages/desktop-client/src/components/ColumnWidthContext.jsx`, replacing `useState` with `useRef` for `positionAccumulator` causes the resize action to become unpredictable. Therefore, it's preferable to keep using `useState` for `positionAccumulator` in this context.
Applied to files:
packages/desktop-client/src/hooks/useLocationPermission.ts
📚 Learning: 2024-09-17T20:04:47.663Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 3458
File: packages/loot-core/src/client/state-types/prefs.d.ts:5-5
Timestamp: 2024-09-17T20:04:47.663Z
Learning: In future reviews, ensure that changes related to `PrefsState` in `prefs.d.ts` do not incorrectly assume necessary updates in other parts of the codebase. Verify the impact thoroughly before making suggestions.
Applied to files:
packages/desktop-client/src/hooks/useLocationPermission.ts
📚 Learning: 2024-11-23T01:45:33.007Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3878
File: packages/desktop-client/src/auth/AuthProvider.tsx:21-30
Timestamp: 2024-11-23T01:45:33.007Z
Learning: In the `AuthProvider` component in `AuthProvider.tsx`, when the server URL is not set (the application is running locally), the permission check should grant permissions without checking user data.
Applied to files:
packages/desktop-client/src/hooks/useLocationPermission.ts
📚 Learning: 2024-11-09T20:18:28.468Z
Learnt from: csenel
Repo: actualbudget/actual PR: 3810
File: packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx:150-161
Timestamp: 2024-11-09T20:18:28.468Z
Learning: In `packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx`, prefer to keep the implementation of checks consistent with similar patterns elsewhere in the codebase, even if alternative implementations are more concise.
Applied to files:
packages/desktop-client/src/payees/payeesSlice.ts
📚 Learning: 2024-10-09T20:17:46.493Z
Learnt from: UnderKoen
Repo: actualbudget/actual PR: 3619
File: packages/loot-core/src/server/accounts/transaction-rules.ts:551-0
Timestamp: 2024-10-09T20:17:46.493Z
Learning: When finalizing transactions that involve inserting or retrieving payees, avoid using `Promise.all` as it may result in duplicate payees due to concurrent operations. Sequential processing ensures payees are correctly handled without duplication.
Applied to files:
packages/desktop-client/src/payees/payeesSlice.ts
📚 Learning: 2024-10-09T20:30:39.127Z
Learnt from: UnderKoen
Repo: actualbudget/actual PR: 3619
File: packages/loot-core/src/server/accounts/transaction-rules.ts:795-801
Timestamp: 2024-10-09T20:30:39.127Z
Learning: In the `finalizeTransactionForRules` function within `packages/loot-core/src/server/accounts/transaction-rules.ts`, potential race conditions when inserting payees are handled in the method that calls this function, so additional safeguards within `finalizeTransactionForRules` are unnecessary.
Applied to files:
packages/desktop-client/src/payees/payeesSlice.ts
📚 Learning: 2024-11-04T00:34:13.035Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 3581
File: packages/loot-core/src/client/actions/account.ts:180-194
Timestamp: 2024-11-04T00:34:13.035Z
Learning: In `packages/loot-core/src/client/actions/account.ts`, within the `syncAccounts` function, the batch sync request for SimpleFin accounts handles errors by returning error objects instead of throwing exceptions. Therefore, wrapping this block in a try-catch is unnecessary.
Applied to files:
packages/desktop-client/src/payees/payeesSlice.ts
📚 Learning: 2025-11-04T00:47:00.968Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 6065
File: packages/desktop-client/src/components/transactions/TransactionList.tsx:197-205
Timestamp: 2025-11-04T00:47:00.968Z
Learning: In packages/desktop-client/src/components/transactions/TransactionList.tsx and similar schedule creation code, when using schedule/create with conditions and then updating the rule's actions, it's correct to filter rule.actions to keep only 'link-schedule' and append custom actions. Transactions created by schedules are defined by a mix of conditions (which create the base transaction with date, amount, payee, account) and actions (which apply additional modifications like category, notes, splits). The filtering pattern replaces any auto-generated actions with the desired custom actions.
Applied to files:
packages/desktop-client/src/payees/payeesSlice.ts
📚 Learning: 2024-10-02T14:56:29.404Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 3544
File: packages/loot-core/src/client/state-types/prefs.d.ts:13-18
Timestamp: 2024-10-02T14:56:29.404Z
Learning: When action types like `SetPrefsAction` are updated with new properties, ensure that reducers are correctly handling these properties before suggesting a potential issue.
Applied to files:
packages/desktop-client/src/payees/payeesSlice.ts
🧬 Code graph analysis (2)
packages/desktop-client/src/hooks/useLocationPermission.ts (2)
packages/component-library/src/hooks/useResponsive.ts (1)
useResponsive(5-23)packages/loot-core/src/shared/location.ts (1)
locationService(144-147)
packages/desktop-client/src/payees/payeesSlice.ts (4)
packages/loot-core/src/types/models/payee.ts (1)
PayeeEntity(4-12)packages/loot-core/src/shared/location.ts (3)
deletePayeeLocation(75-82)getNearbyPayees(84-94)locationService(144-147)packages/loot-core/src/shared/location-adapters.ts (1)
getNearbyPayees(88-98)packages/desktop-client/src/redux/index.ts (1)
createAppAsyncThunk(12-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: validate-cli
- GitHub Check: build (windows-latest)
- GitHub Check: build (ubuntu-22.04)
- GitHub Check: build (macos-latest)
- GitHub Check: test
- GitHub Check: Wait for Netlify build to finish
- GitHub Check: Functional Desktop App
- GitHub Check: Generate VRT Updates
- GitHub Check: web
- GitHub Check: Analyze
- GitHub Check: compare
🔇 Additional comments (6)
packages/desktop-client/src/hooks/useLocationPermission.ts (1)
12-66: Previous critical issues have been successfully resolved!The implementation correctly addresses both previously flagged critical issues:
✅ Memory leak fix:
handleChangeis now properly scoped and the same function reference is used for bothaddEventListenerandremoveEventListener.✅ Race condition fix: The
isMountedflag correctly prevents state updates and event listener attachment after component unmount.The overall logic is sound:
- Permission checking is appropriately scoped to narrow widths (mobile)
- Event listener lifecycle is properly managed
- Auto-prompting when
state === 'prompt'(line 44-46) is a reasonable UX choice for mobile location features- Dependency array is correct
packages/desktop-client/src/payees/payeesSlice.ts (5)
16-44: LGTM: State extensions follow established patterns.The nearbyPayees state properties are consistently structured with the existing commonPayees and payees patterns, including loading/loaded/dirty flags.
191-216: LGTM: Thunks are well-structured.Both thunks properly wrap
locationServicecalls with error handling and return meaningful result objects that enable the reducers to update state correctly.
398-402: LGTM: Correctly marks all payee types as dirty.The addition of
isNearbyPayeesDirtyensures that nearby payees are reloaded whenever any payee data changes, maintaining consistency across all payee lists.
356-367: LGTM: New actions properly exported.All four new location-related actions are correctly added to the exported actions object.
381-389: The original review comment is incorrect and should be disregarded.The review assumes that nearbyPayees can include the "Starting Balance" payee, but this is not possible. The
getNearbyPayeesserver function queries thepayee_locationstable and only returns payees that have location entries. The "Starting Balance" system payee is never assigned a location entry (no evidence ofcreatePayeeLocationcalls for system payees), so it can never appear in the nearbyPayees result set.Additionally,
translatePayeesonly translates the "Starting Balance" payee name to its i18next equivalent. Since "Starting Balance" cannot appear in nearbyPayees, not callingtranslatePayeesin_loadNearbyPayeesis correct and consistent with the data flow.Likely an incorrect or invalid review comment.
|
✅ VRT screenshots have been automatically updated. |
Auto-generated by VRT workflow PR: actualbudget#6157
|
✅ VRT screenshots have been automatically updated. |
Auto-generated by VRT workflow PR: actualbudget#6157
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx (4)
152-167:ItemTypes/determineItemTypenow carry anisNearbyflag that isn’t actually used
determineItemTypegained anisNearbyparameter and a'nearby_payee'branch, but nearby items are still typed by directly settingitemType: 'nearby_payee'innearbyPayeesWithType. This makes the signature more complex without being exercised.Either:
- Use
determineItemType(item, /*isCommon=*/false, /*isNearby=*/true)when constructing nearby items, or- Drop the
isNearbyparameter and special-case from the nearby pipeline only, to keep the helper focused.Also applies to: 452-463
376-413: ClarifynearbyPayeessource and avoid treating an explicit empty array as “no override”The current wiring:
- Always calls
useNearbyPayees(locationAccess)(line 405) and then- Falls back with
if (!nearbyPayees) { nearbyPayees = retrievedNearbyPayees; }(lines 410–412),- Then derives
nearbyPayeesWithTypeandfilteredNearbyPayees(lines 452–476),- And uses
handleForgetLocationanddeletePayeeLocationfor the “forget” button (lines 479–485, 634–635).Two implications:
- A caller that passes
nearbyPayees: []cannot currently suppress nearby suggestions, because!nearbyPayeestreats an empty array as falsy and you fall back to the store-derivedretrievedNearbyPayees.useNearbyPayeesstill runs even if a caller intends to provide its ownnearbyPayeeslist (the hook will no‑op whenlocationAccessis false, but that intent isn’t visible here).A tighter pattern would be:
- Distinguish “not provided” from “explicitly empty”:
- const retrievedNearbyPayees = useNearbyPayees(locationAccess) || []; + const hasExternalNearby = nearbyPayees !== undefined; + const retrievedNearbyPayees = useNearbyPayees( + locationAccess && !hasExternalNearby, + ) || []; @@ - if (!nearbyPayees) { - nearbyPayees = retrievedNearbyPayees; - } + if (nearbyPayees === undefined) { + nearbyPayees = retrievedNearbyPayees; + }This (a) lets parents explicitly disable nearby suggestions by passing
[], and (b) avoids unnecessary location fetches when they provide their own list, while preserving the default behavior.Also applies to: 452-476, 479-485, 491-512, 604-605, 634-635
193-252: Highlighted index vs rendered order for nearby payees is inconsistentWith nearby payees enabled:
renderItemspasses[...filteredNearbyPayees, ...items]intoPayeeList(line 604), soitemsinsidePayeeListnow include nearby entries in addition to Downshift’s suggestions.- The
useMemoat lines 193–252 re-groups items and assigns a synthetichighlightedIndexin render order:newPayee(0), then nearby payees, then suggested/payees/transfer.getHighlightedIndexreturns0wheneverfilteredNearbyPayees.length > 0(lines 587–590) but that index is still relative to the base suggestions list, which doesn’t know about the prepended nearby items.- When the user has typed something, the “new” sentinel item also exists; in that case,
highlightedIndex === 0will highlight the “Create payee” row (which you also assign index 0 inPayeeList), not the first nearby payee as the comment suggests. Keyboard navigation may also end up selecting a different item than the one visually highlighted.If you don’t want to rework Autocomplete’s internal item list right now, a minimal fix to avoid misleading behavior is to drop the nearby‑specific branch and reuse the previous logic:
- getHighlightedIndex={suggestions => { - // If we have nearby payees, highlight the first nearby payee - if (filteredNearbyPayees.length > 0) { - return 0; - } - - // Otherwise use original logic for suggestions + getHighlightedIndex={suggestions => { if (suggestions.length === 0) { return null; } else if (suggestions[0].id === 'new') { // Highlight the first payee since the create payee option is at index 0. return suggestions.length > 1 ? 1 : 0; } return 0; }}Longer‑term, making nearby items part of the actual
suggestionslist that Autocomplete/Downshift tracks (rather than only adding them inrenderItems) would keep keyboard highlight and selection fully aligned with what’s rendered.Please manually verify keyboard navigation (arrow keys + Enter) when nearby payees are present and text has been typed, to ensure the selected item matches the visually highlighted row.
Also applies to: 275-287, 587-600, 604-605
809-933: RefineNearbyPayeeItemhooks, layout behavior, and button stylingThe new
NearbyPayeeItemis a nice addition, but a few details can be tightened:
useLocationPermissionis called per row (line 822) and its boolean is used only to switch between mobile vs desktop styling (lines 824–833) and icon size (line 831). That couples layout to permission state and re-queries the Permissions API for every nearby item.
- Prefer
useResponsive()here (likePayeeItemandCreatePayeeButton) and passlocationAccessdown fromPayeeAutocompletewhen you actually need permission semantics.- Button colors are hard-coded (
#dc3545,#c82333, lines 910–919). For consistency and theming/dark-mode support, it would be better to use palette entries fromtheme(e.g., an error/destructive color).<Trans>{t('forget')}</Trans>(lines 923–923) is redundant: you’re callingtand then wrapping its result inTrans. Either plain<Trans>Forget location</Trans>ort('forget_location')alone would be clearer and avoid double work.- There’s a native
<button>nested inside a wrapperdivwithrole="button"(lines 861–863, 905–930). You correctly stop propagation inhandleForgetClick, but nested interactive roles can still be confusing for assistive tech; if feasible, consider making the outer container non-interactive and relying on Downshift’sgetItemPropsfor selection, with the “forget” button as the only explicit control.Example of tightening hooks/layout:
- const { t } = useTranslation(); - const locationAccess = useLocationPermission(); - const unitOfMeasurementFormat = useUnitOfMeasurementFormat(); - const narrowStyle = locationAccess + const { t } = useTranslation(); + const { isNarrowWidth } = useResponsive(); + const unitOfMeasurementFormat = useUnitOfMeasurementFormat(); + const narrowStyle = isNarrowWidth @@ - const iconSize = locationAccess ? 14 : 8; + const iconSize = isNarrowWidth ? 14 : 8;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/desktop-client/e2e/reports.test.ts-snapshots/Reports-loads-cash-flow-graph-and-checks-visuals-1-chromium-linux.pngis excluded by!**/*.png
📒 Files selected for processing (1)
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx(14 hunks)
🧰 Additional context used
🧠 Learnings (12)
📚 Learning: 2024-10-24T17:05:41.415Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-10-24T17:05:41.415Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2025-01-16T14:28:00.133Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 4166
File: packages/loot-core/src/client/data-hooks/transactions.ts:258-258
Timestamp: 2025-01-16T14:28:00.133Z
Learning: Variables defined within React hooks like `useMemo` or `useEffect` are not dependencies of other hooks. Only external variables and values from the component scope need to be included in the dependency arrays.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2024-10-22T05:34:56.976Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx:261-277
Timestamp: 2024-10-22T05:34:56.976Z
Learning: In React components (TypeScript), avoid using hooks like `useCallback` inside callbacks or nested functions. Hooks must be called at the top level of functional components.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2024-10-22T05:32:30.530Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/desktop-client/src/components/schedules/index.tsx:37-39
Timestamp: 2024-10-22T05:32:30.530Z
Learning: In our React function components, we should include `dispatch` in the dependency array of `useCallback` hooks to comply with ESLint rules, even though `dispatch` is a stable function.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2025-05-22T05:17:51.422Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 4955
File: packages/component-library/src/Input.tsx:52-64
Timestamp: 2025-05-22T05:17:51.422Z
Learning: React 19 has simplified ref handling, eliminating the need for `React.forwardRef()` in functional components. Components can now receive and destructure the ref directly from props rather than requiring the second parameter in forwardRef.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2024-09-27T14:15:46.637Z
Learnt from: jfdoming
Repo: actualbudget/actual PR: 3402
File: packages/desktop-client/src/components/accounts/Account.tsx:8-8
Timestamp: 2024-09-27T14:15:46.637Z
Learning: In the `AllTransactions` component of `Account.tsx`, the `useLayoutEffect` hook is appropriately used to dispatch an action that closes splits for parent transactions when `prependTransactions` changes, ensuring this occurs synchronously before the component is painted.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2025-01-16T19:31:27.020Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 4169
File: packages/component-library/src/icons/Loading.tsx:8-8
Timestamp: 2025-01-16T19:31:27.020Z
Learning: The Loading component in actual-app/components package is a direct copy from its previous location, and improvements to its accessibility and security are planned to be addressed in a separate PR.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2025-05-22T05:17:51.422Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 4955
File: packages/component-library/src/Input.tsx:52-64
Timestamp: 2025-05-22T05:17:51.422Z
Learning: In React 19, function components can receive refs directly through props without requiring React.forwardRef(). This simplified ref forwarding feature allows directly destructuring the ref prop and passing it along to DOM elements or other components.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2025-02-11T18:44:28.613Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 0
File: :0-0
Timestamp: 2025-02-11T18:44:28.613Z
Learning: The Button2 component is deprecated. Use actual-app/components/button instead.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2024-10-16T03:46:34.277Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3593
File: packages/desktop-client/src/components/sidebar/BudgetName.tsx:151-165
Timestamp: 2024-10-16T03:46:34.277Z
Learning: In `packages/desktop-client/src/components/sidebar/BudgetName.tsx`, `media` is not implemented on `<Button>` components at this time.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2024-10-10T02:29:05.655Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3593
File: packages/desktop-client/src/components/sidebar/Sidebar.tsx:112-116
Timestamp: 2024-10-10T02:29:05.655Z
Learning: In `packages/desktop-client/src/components/sidebar/BudgetName.tsx`, the `BudgetName` component consists of three parts: `BudgetName`, `EditBudgetName`, and the Menu. Keeping `EditBudgetName` as a separate component helps maintain cleaner code by separating concerns.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2024-10-09T20:17:46.493Z
Learnt from: UnderKoen
Repo: actualbudget/actual PR: 3619
File: packages/loot-core/src/server/accounts/transaction-rules.ts:551-0
Timestamp: 2024-10-09T20:17:46.493Z
Learning: When finalizing transactions that involve inserting or retrieving payees, avoid using `Promise.all` as it may result in duplicate payees due to concurrent operations. Sequential processing ensures payees are correctly handled without duplication.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
🧬 Code graph analysis (1)
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx (7)
packages/loot-core/src/types/models/payee.ts (1)
PayeeEntity(4-12)packages/desktop-client/src/hooks/usePayees.ts (2)
usePayees(75-87)useNearbyPayees(31-73)packages/desktop-client/src/components/autocomplete/Autocomplete.tsx (1)
defaultFilterSuggestion(96-102)packages/desktop-client/src/payees/payeesSlice.ts (3)
deletePayeeLocation(205-216)createPayee(169-189)updatePayeeLocationIfNeeded(191-203)packages/desktop-client/src/hooks/useLocationPermission.ts (1)
useLocationPermission(12-66)packages/desktop-client/src/hooks/useUnitOfMeasurementFormat.ts (1)
useUnitOfMeasurementFormat(3-6)packages/loot-core/src/shared/location-utils.ts (1)
formatDistance(48-72)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Functional Desktop App
- GitHub Check: Wait for Netlify build to finish
- GitHub Check: validate-cli
- GitHub Check: test
- GitHub Check: Analyze
- GitHub Check: build (ubuntu-22.04)
- GitHub Check: build (macos-latest)
- GitHub Check: Generate VRT Updates
- GitHub Check: build (windows-latest)
- GitHub Check: web
- GitHub Check: compare
🔇 Additional comments (1)
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx (1)
487-517: Location-aware create/update/delete flows look solidThe async flows in
handleSelectandhandleForgetLocationare well-contained:
- New payees go through
createPayee({ name: payeeName, locationAccess })(lines 491–495), so location saving is opt‑in and tied to permission.- Existing payees trigger
updatePayeeLocationIfNeededonly whenlocationAccessis true (lines 505–512), with errors logged but not blocking selection.- Forgetting a location dispatches
deletePayeeLocation(locationId).unwrap()with proper try/catch (lines 479–485), keeping UI behavior decoupled from backend failure.This keeps the autocomplete responsive while still opportunistically persisting location data.
Also applies to: 479-485
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/loot-core/src/server/api-models.ts (1)
101-104: APIPayeeEntity now includeslocation; consider clarifying/aligning payeeModel behaviorExposing
locationonAPIPayeeEntitymakes sense for nearby‑payee flows, butpayeeModel.toExternalstill only returns{ id, name, transfer_acct }, andfromExternalblindly casts the input toPayeeEntity. This means:
- Generic endpoints (
api/payees-get,api/common-payees-get) will never populatelocation, even though the type now allows it.api/payee-updatetechnically could accept alocationfield infields, but it will just be passed through topayees-batch-changewithout any explicit translation/validation.If the intent is that
locationis read‑only and only populated by specific handlers (e.g.,api/payees-get-nearby), it might be worth either:
- Documenting this in a comment on
APIPayeeEntity, and/or- Narrowing what
payeeModel.fromExternalaccepts/forwards solocationisn’t accidentally treated as a writable field.Can you confirm the intended semantics here and, if needed, tighten the model to avoid confusion for API consumers?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/loot-core/src/server/api-models.ts(1 hunks)packages/loot-core/src/server/api.ts(1 hunks)packages/loot-core/src/types/api-handlers.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Always runyarn typecheckbefore committing to ensure type safety
Use TypeScript for all code in the project
Prefertypeoverinterfacefor type definitions in TypeScript
Avoidenum- use objects or maps instead
Avoidanyorunknownunless absolutely necessary; prefer specific types
Avoid type assertions (as,!) - prefersatisfiesfor type narrowing
Use inline type imports:import { type MyType } from '...'in TypeScript
Don't useReact.*patterns - use named imports instead (e.g.,import { useState }notReact.useState)
Files:
packages/loot-core/src/server/api-models.tspackages/loot-core/src/server/api.tspackages/loot-core/src/types/api-handlers.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Runyarn lint:fixto automatically fix linting errors before committing
Use descriptive variable names with auxiliary verbs (e.g.,isLoaded,hasError)
Use named exports for components and utilities; avoid default exports except in specific cases
Prefer functional and declarative programming patterns - avoid classes
Use thefunctionkeyword for pure functions rather than arrow functions or classes
Prefer iteration and modularization over code duplication
Organize imports in order: React, Node.js modules, external packages, Actual packages, parent imports, sibling imports, index imports; maintain newlines between groups
Never useconsole.*- use logger instead (enforced by ESLintactual/prefer-logger-over-console)
Import fromuuidwith destructuring:import { v4 as uuidv4 } from 'uuid'
Never import colors directly - use theme instead
Don't directly reference platform-specific imports (.api,.web,.electron); use conditional exports inloot-coreinstead
Use all user-facing strings with i18n translations - follow ESLint ruleactual/no-untranslated-strings
Custom ESLint rules include:no-untranslated-strings,prefer-trans-over-t,prefer-logger-over-console,typography,prefer-if-statement
The codebase is being migrated from classes to functions - prefer functional patterns over class-based implementations
Legacy React patterns withReact.*are being removed - use named imports instead
Files:
packages/loot-core/src/server/api-models.tspackages/loot-core/src/server/api.tspackages/loot-core/src/types/api-handlers.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Structure component files as: exported component/page, helpers, static content, types
Files:
packages/loot-core/src/server/api-models.tspackages/loot-core/src/server/api.tspackages/loot-core/src/types/api-handlers.ts
packages/loot-core/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import
@actual-app/web/*inloot-coreto maintain platform independence
Files:
packages/loot-core/src/server/api-models.tspackages/loot-core/src/server/api.tspackages/loot-core/src/types/api-handlers.ts
packages/loot-core/src/types/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Look for existing type definitions in
packages/loot-core/src/types/before creating new ones
Files:
packages/loot-core/src/types/api-handlers.ts
🧠 Learnings (9)
📚 Learning: 2024-11-04T14:14:10.698Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 3581
File: packages/loot-core/src/types/models/bank-sync.d.ts:11-21
Timestamp: 2024-11-04T14:14:10.698Z
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.
Applied to files:
packages/loot-core/src/server/api-models.tspackages/loot-core/src/server/api.tspackages/loot-core/src/types/api-handlers.ts
📚 Learning: 2025-10-12T04:07:06.002Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 5786
File: packages/api/tsconfig.dist.json:14-14
Timestamp: 2025-10-12T04:07:06.002Z
Learning: In the Actual Budget codebase, when rootDir is removed from packages/loot-core/tsconfig.api.json to allow referencing files outside the loot-core directory, the declaration output structure changes. The path alias for loot-core in packages/api/tsconfig.dist.json must be updated from "./types/loot-core/src/*" to "./types/loot-core/loot-core/src/*" to match the new emitted declaration paths, as TypeScript preserves the full directory structure from the project root when rootDir is not specified.
Applied to files:
packages/loot-core/src/server/api-models.tspackages/loot-core/src/server/api.tspackages/loot-core/src/types/api-handlers.ts
📚 Learning: 2024-10-08T15:46:15.739Z
Learnt from: UnderKoen
Repo: actualbudget/actual PR: 3365
File: packages/loot-core/src/types/models/rule.d.ts:4-4
Timestamp: 2024-10-08T15:46:15.739Z
Learning: In `packages/loot-core/src/server/accounts/transaction-rules.ts`, the `stage` property can have legacy values `'cleanup'` and `'modify'`, which are converted to `'pre'`. The type remains `string` to accommodate these values and ensure correct usage.
Applied to files:
packages/loot-core/src/server/api-models.ts
📚 Learning: 2024-10-23T07:38:25.699Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3689
File: packages/loot-core/src/server/main.ts:1807-1809
Timestamp: 2024-10-23T07:38:25.699Z
Learning: In the function `delete-budget` in `packages/loot-core/src/server/main.ts`, the function `removeAllBackups(id)` handles its own errors internally, so additional error handling when calling it is unnecessary.
Applied to files:
packages/loot-core/src/server/api.ts
📚 Learning: 2024-10-24T05:09:44.115Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3689
File: packages/loot-core/src/server/backups.web.ts:203-207
Timestamp: 2024-10-24T05:09:44.115Z
Learning: In `packages/loot-core/src/server/backups.web.ts`, developers prefer to keep the error handling for cloud storage uploads inline rather than extracting it into a separate function. Avoid suggesting this refactoring in future reviews.
Applied to files:
packages/loot-core/src/server/api.ts
📚 Learning: 2024-10-25T05:04:40.902Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3689
File: packages/loot-core/src/platform/server/fs/index.web.ts:319-340
Timestamp: 2024-10-25T05:04:40.902Z
Learning: In `packages/loot-core/src/platform/server/fs/index.web.ts`, errors are already handled in other locations, so additional error handling in `copyFile` is unnecessary.
Applied to files:
packages/loot-core/src/server/api.ts
📚 Learning: 2024-10-12T19:13:25.005Z
Learnt from: jfdoming
Repo: actualbudget/actual PR: 3641
File: packages/loot-core/src/server/accounts/rules.test.ts:524-536
Timestamp: 2024-10-12T19:13:25.005Z
Learning: In `packages/loot-core/src/server/accounts/rules.test.ts`, prefer explicit action definitions over refactoring similar actions into loops or helper functions, even when actions are similar.
Applied to files:
packages/loot-core/src/server/api.ts
📚 Learning: 2025-11-24T22:26:33.975Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T22:26:33.975Z
Learning: Applies to packages/loot-core/src/types/**/*.{ts,tsx} : Look for existing type definitions in `packages/loot-core/src/types/` before creating new ones
Applied to files:
packages/loot-core/src/types/api-handlers.ts
📚 Learning: 2024-10-08T15:46:15.739Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 3570
File: packages/desktop-client/src/components/modals/ImportTransactionsModal/utils.ts:120-130
Timestamp: 2024-10-08T15:46:15.739Z
Learning: Avoid overcomplicating the `ImportTransaction` type; prefer to keep it simple.
Applied to files:
packages/loot-core/src/types/api-handlers.ts
🧬 Code graph analysis (3)
packages/loot-core/src/server/api-models.ts (1)
packages/loot-core/src/types/models/payee.ts (1)
PayeeEntity(4-12)
packages/loot-core/src/server/api.ts (1)
packages/loot-core/src/server/main.ts (1)
handlers(51-51)
packages/loot-core/src/types/api-handlers.ts (2)
packages/loot-core/src/types/models/payee-location.ts (1)
PayeeLocationEntity(1-8)packages/loot-core/src/server/api-models.ts (1)
APIPayeeEntity(101-104)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: build (ubuntu-22.04)
- GitHub Check: build (windows-latest)
- GitHub Check: build (macos-latest)
- GitHub Check: web
- GitHub Check: test
- GitHub Check: validate-cli
- GitHub Check: Functional Desktop App
- GitHub Check: Wait for Netlify build to finish
- GitHub Check: Generate VRT Updates
- GitHub Check: Analyze
- GitHub Check: compare
🔇 Additional comments (3)
packages/loot-core/src/server/api.ts (1)
742-768: New payee-location handlers follow existing API patterns and look correctThe four new handlers (
payee-location-create,payee-locations-get,payee-location-delete,payees-get-nearby) are wired consistently with the rest of this module:
- Mutating operations are wrapped in
withMutation.- All enforce
checkFileOpen()before delegating.- Argument shapes (
payee_id,latitude,longitude,maxDistance) match the exportedApiHandlerssignatures and the underlying entities.No issues from this side; behavior looks aligned with the rest of the API layer.
packages/loot-core/src/types/api-handlers.ts (2)
17-24: ReusingPayeeLocationEntityhere is appropriatePulling
PayeeLocationEntityinto the API handlers type map keeps the public handler signatures aligned with the core model definition instead of duplicating a shape, which matches the existing typing approach inloot-core.
221-237: New payee-location API handler types align with models and server wiringThe added handler signatures look consistent and type-safe:
payee_iduses the same snake_case asPayeeLocationEntity.payee_id, matching server/DB field naming.api/payee-locations-getreturnsPayeeLocationEntity[], so callers get full location records (including optionaldistance) without redefining the shape.api/payees-get-nearbyreturnsAPIPayeeEntity[], which now includes an optionallocationfield, fitting the nearby-payees use case.These match the implementations in
server/api.tsand the underlying model types; I don’t see any typing or shape mismatches here.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx (1)
1663-1672: Consider restructuring effect to avoid onUpdate in dependencies.The effect includes
onUpdatein its dependency array, which itself depends ontransactions. This creates a pattern where the effect can trigger itself indirectly: effect runs → calls onUpdate → updates transactions → effect runs again. While the guards prevent an infinite loop, this causes unnecessary re-execution.Consider either:
- Moving the auto-selection logic into the effect that fetches nearby payees, or
- Using a ref to track whether auto-selection has occurred to avoid re-triggering
Example approach 1:
useEffect(() => { if (!locationAccess) { return; } let unmounted = false; async function findNearbyPayee() { try { const currentLocation = await locationService.getCurrentPosition(); const nearbyPayees = await locationService.getNearbyPayees(currentLocation); if (!unmounted && nearbyPayees.length > 0) { const transaction = transactions[0]; if (transaction && !transaction.payee) { const updated = { ...transaction, payee: nearbyPayees[0].id }; onUpdate(updated, 'payee'); } setNearestPayee(nearbyPayees[0]); setAutoSelectedPayeeId(nearbyPayees[0].id); setShouldShowForgetLocation(false); } } catch (error) { // error handling } } findNearbyPayee(); return () => { unmounted = true; }; }, [locationAccess, transactions, onUpdate]);This consolidates the logic and avoids the circular dependency pattern.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx(15 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Always runyarn typecheckbefore committing to ensure type safety
Use TypeScript for all code in the project
Prefertypeoverinterfacefor type definitions in TypeScript
Avoidenum- use objects or maps instead
Avoidanyorunknownunless absolutely necessary; prefer specific types
Avoid type assertions (as,!) - prefersatisfiesfor type narrowing
Use inline type imports:import { type MyType } from '...'in TypeScript
Don't useReact.*patterns - use named imports instead (e.g.,import { useState }notReact.useState)
Files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Runyarn lint:fixto automatically fix linting errors before committing
Use descriptive variable names with auxiliary verbs (e.g.,isLoaded,hasError)
Use named exports for components and utilities; avoid default exports except in specific cases
Prefer functional and declarative programming patterns - avoid classes
Use thefunctionkeyword for pure functions rather than arrow functions or classes
Prefer iteration and modularization over code duplication
Organize imports in order: React, Node.js modules, external packages, Actual packages, parent imports, sibling imports, index imports; maintain newlines between groups
Never useconsole.*- use logger instead (enforced by ESLintactual/prefer-logger-over-console)
Import fromuuidwith destructuring:import { v4 as uuidv4 } from 'uuid'
Never import colors directly - use theme instead
Don't directly reference platform-specific imports (.api,.web,.electron); use conditional exports inloot-coreinstead
Use all user-facing strings with i18n translations - follow ESLint ruleactual/no-untranslated-strings
Custom ESLint rules include:no-untranslated-strings,prefer-trans-over-t,prefer-logger-over-console,typography,prefer-if-statement
The codebase is being migrated from classes to functions - prefer functional patterns over class-based implementations
Legacy React patterns withReact.*are being removed - use named imports instead
Files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Structure component files as: exported component/page, helpers, static content, types
Files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
packages/desktop-client/src/components/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Create new components in their own files
Files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/*.tsx: Don't useReact.FunctionComponentorReact.FC- type props directly in function signature
Use<Link>component instead of<a>tags in React components
Avoid unstable nested components - define components at module scope
Use declarative JSX with minimal and readable code
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements
Prefer explicit expressions (condition && <Component />) in JSX
PreferTranscomponent instead oft()function for translations (enforced by ESLintactual/prefer-trans-over-t)
Files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
packages/desktop-client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/desktop-client/src/**/*.{ts,tsx}: Use custom hooks fromsrc/hooksinstead of react-router directly (e.g.,useNavigate()fromsrc/hooks)
UseuseDispatch(),useSelector(),useStore()fromsrc/reduxinstead of react-redux directly
Use absolute imports indesktop-client(enforced by ESLint)
Files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
🧠 Learnings (20)
📚 Learning: 2024-10-14T09:03:37.410Z
Learnt from: minajevs
Repo: actualbudget/actual PR: 3274
File: packages/desktop-client/src/components/mobile/transactions/TransactionEdit.jsx:0-0
Timestamp: 2024-10-14T09:03:37.410Z
Learning: In `packages/desktop-client/src/components/mobile/transactions/TransactionEdit.jsx`, within the `onEditField` function, the `unserializedTransaction` variable is always expected to be found when accessing its properties. Adding a null check may not be necessary.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-11-04T00:47:00.968Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 6065
File: packages/desktop-client/src/components/transactions/TransactionList.tsx:197-205
Timestamp: 2025-11-04T00:47:00.968Z
Learning: In packages/desktop-client/src/components/transactions/TransactionList.tsx and similar schedule creation code, when using schedule/create with conditions and then updating the rule's actions, it's correct to filter rule.actions to keep only 'link-schedule' and append custom actions. Transactions created by schedules are defined by a mix of conditions (which create the base transaction with date, amount, payee, account) and actions (which apply additional modifications like category, notes, splits). The filtering pattern replaces any auto-generated actions with the desired custom actions.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-11-09T20:18:28.468Z
Learnt from: csenel
Repo: actualbudget/actual PR: 3810
File: packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx:150-161
Timestamp: 2024-11-09T20:18:28.468Z
Learning: In `packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx`, prefer to keep the implementation of checks consistent with similar patterns elsewhere in the codebase, even if alternative implementations are more concise.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-10-24T17:05:41.415Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-10-24T17:05:41.415Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-10-05T10:58:13.598Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 3570
File: packages/desktop-client/src/components/modals/ImportTransactionsModal/Transaction.tsx:83-90
Timestamp: 2024-10-05T10:58:13.598Z
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`.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-11-22T17:03:41.823Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 6158
File: packages/desktop-client/src/hooks/useScheduleEdit.ts:163-172
Timestamp: 2025-11-22T17:03:41.823Z
Learning: In packages/desktop-client/src/hooks/useScheduleEdit.ts around lines 163-172, the 'set-transactions' case uses a single-argument sort comparator (`transactions.sort(a => { return action.transactionId === a.id ? -1 : 1; })`), which is not symmetric and can produce unstable ordering. The correct approach is to use a two-argument comparator that returns -1/0/1 based on whether a or b matches action.transactionId. MatissJanis prefers to fix this in a future PR when this code is modified again.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-10-16T03:51:04.683Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3593
File: packages/desktop-client/src/components/sidebar/BudgetName.tsx:114-136
Timestamp: 2024-10-16T03:51:04.683Z
Learning: In 'packages/desktop-client/src/components/sidebar/BudgetName.tsx', empty budget names are handled elsewhere, so additional error handling within the 'EditableBudgetName' component is unnecessary.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-09-27T14:15:46.637Z
Learnt from: jfdoming
Repo: actualbudget/actual PR: 3402
File: packages/desktop-client/src/components/accounts/Account.tsx:8-8
Timestamp: 2024-09-27T14:15:46.637Z
Learning: In the `AllTransactions` component of `Account.tsx`, the `useLayoutEffect` hook is appropriately used to dispatch an action that closes splits for parent transactions when `prependTransactions` changes, ensuring this occurs synchronously before the component is painted.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-10-22T02:08:48.162Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/loot-core/src/client/data-hooks/transactions.ts:32-88
Timestamp: 2024-10-22T02:08:48.162Z
Learning: In the React `useTransactions` hook defined in `packages/loot-core/src/client/data-hooks/transactions.ts`, changes to `options` (e.g., `options.pageCount`) should be applied on the next query without adding them to the `useEffect` dependency array, to avoid unnecessary rerenders when options change.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-10-09T20:30:39.127Z
Learnt from: UnderKoen
Repo: actualbudget/actual PR: 3619
File: packages/loot-core/src/server/accounts/transaction-rules.ts:795-801
Timestamp: 2024-10-09T20:30:39.127Z
Learning: In the `finalizeTransactionForRules` function within `packages/loot-core/src/server/accounts/transaction-rules.ts`, potential race conditions when inserting payees are handled in the method that calls this function, so additional safeguards within `finalizeTransactionForRules` are unnecessary.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-10-09T20:17:46.493Z
Learnt from: UnderKoen
Repo: actualbudget/actual PR: 3619
File: packages/loot-core/src/server/accounts/transaction-rules.ts:551-0
Timestamp: 2024-10-09T20:17:46.493Z
Learning: When finalizing transactions that involve inserting or retrieving payees, avoid using `Promise.all` as it may result in duplicate payees due to concurrent operations. Sequential processing ensures payees are correctly handled without duplication.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-11-12T19:52:52.889Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3792
File: packages/desktop-client/src/components/reports/reports/Summary.tsx:134-161
Timestamp: 2024-11-12T19:52:52.889Z
Learning: In `packages/desktop-client/src/components/reports/reports/Summary.tsx`, API calls like `get-earliest-transaction` are used without explicit error handling to maintain consistency with other components.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-11-24T22:26:33.975Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T22:26:33.975Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Don't directly reference platform-specific imports (`.api`, `.web`, `.electron`); use conditional exports in `loot-core` instead
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-11-24T22:26:33.975Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T22:26:33.975Z
Learning: Applies to packages/desktop-client/src/**/*.{ts,tsx} : Use custom hooks from `src/hooks` instead of react-router directly (e.g., `useNavigate()` from `src/hooks`)
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-11-24T22:26:33.975Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T22:26:33.975Z
Learning: Applies to packages/desktop-client/src/**/*.{ts,tsx} : Use `useDispatch()`, `useSelector()`, `useStore()` from `src/redux` instead of react-redux directly
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-03-14T15:11:36.220Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 4484
File: packages/desktop-client/src/components/mobile/budget/ExpenseGroup.tsx:0-0
Timestamp: 2025-03-14T15:11:36.220Z
Learning: In the Actual Budget mobile app, the callback functions `onEditCategory` (implemented as `onOpenCategoryMenuModal`) and `onBudgetAction` are already properly memoized with useCallback in the Budget component (packages/desktop-client/src/components/mobile/budget/index.tsx).
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-03-14T15:11:36.220Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 4484
File: packages/desktop-client/src/components/mobile/budget/ExpenseGroup.tsx:0-0
Timestamp: 2025-03-14T15:11:36.220Z
Learning: In the Actual Budget app, the callback functions `onEditCategory` and `onBudgetAction` are already properly memoized with useCallback in the Budget component (packages/desktop-client/src/components/mobile/budget/index.tsx).
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-11-12T18:18:07.282Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3828
File: packages/desktop-client/src/components/reports/reports/Calendar.tsx:575-631
Timestamp: 2024-11-12T18:18:07.282Z
Learning: In `Calendar.tsx`, transaction-related callbacks such as `onBatchDelete`, `onBatchDuplicate`, `onCreateRule`, and `onScheduleAction` are intentionally left as empty functions because these operations should not be usable on that page.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-06-21T04:15:23.727Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 5207
File: packages/desktop-client/src/components/transactions/TransactionsTable.tsx:2194-2201
Timestamp: 2025-06-21T04:15:23.727Z
Learning: In TransactionsTable.tsx, the balance calculation using find() to locate the first non-scheduled transaction is acceptable for performance because: 1) the component only renders when adding transactions (limited usage), 2) find() short-circuits on first match, and 3) scheduled transactions are typically sparse in the transaction list.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-10-06T15:26:36.914Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 5870
File: packages/loot-core/src/server/transactions/transaction-rules.ts:345-348
Timestamp: 2025-10-06T15:26:36.914Z
Learning: In packages/loot-core/src/server/rules/rule.ts, Rule.execActions<T>(object) returns a Partial<T> diff object (not an array). When actions create splits, the top-level execActions returns a grouped transaction with a subtransactions array; the Rule.execActions diff includes subtransactions. Merging via Object.assign preserves splits.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: validate-cli
- GitHub Check: test
- GitHub Check: build (ubuntu-22.04)
- GitHub Check: build (macos-latest)
- GitHub Check: web
- GitHub Check: build (windows-latest)
- GitHub Check: Analyze
- GitHub Check: Functional Desktop App
- GitHub Check: Wait for Netlify build to finish
- GitHub Check: Generate VRT Updates
- GitHub Check: compare
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx (1)
1407-1433: Critical: Cleanup function is incorrectly placed inside async function.Lines 1427-1429 return a cleanup function from inside the
findNearbyPayeeasync function, but useEffect cleanup must be returned directly from the effect, not from an async function. This means the cleanup will never execute, andunmountedwill never be set totrue, potentially causing state updates on an unmounted component.Apply this diff:
useEffect(() => { if (!locationAccess) { return; } let unmounted = false; async function findNearbyPayee() { try { const currentLocation = await locationService.getCurrentPosition(); const nearbyPayees = await locationService.getNearbyPayees(currentLocation); if (!unmounted && nearbyPayees.length > 0) { setNearestPayee(nearbyPayees[0]); setAutoSelectedPayeeId(nearbyPayees[0].id); setShouldShowForgetLocation(false); } } catch (error) { // Don't block transaction creation } - return () => { - unmounted = true; - }; } findNearbyPayee(); + + return () => { + unmounted = true; + }; }, [locationAccess]);
🧹 Nitpick comments (1)
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx (1)
149-155: ItemTypes extension anddetermineItemType’sisNearbyflagAdding
'nearby_payee'toItemTypesand plumbing anisNearbyflag intodetermineItemTypematches the new use case. At the moment,isNearbyis not actually passed astrueanywhere in this file (nearby items are instead tagged in a separatenearbyPayeesWithTypemap).If no external callers rely on
determineItemType(..., true, true), consider either:
- wiring callers to use the new flag, or
- dropping the parameter for now to keep the API minimal.
Also applies to: 157-173
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx(14 hunks)packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx(16 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use TypeScript for all code
Use descriptive variable names with auxiliary verbs (e.g., 'isLoaded', 'hasError')
Use named exports for components and utilities; avoid default exports except in specific cases
Use functional and declarative programming patterns - avoid classes
Use the 'function' keyword for pure functions
Structure files with: exported component/page, helpers, static content, types
Organize imports in the following order: React imports, built-in Node.js modules, external packages, Actual packages, parent imports, sibling imports, index imports, with newlines between groups
Never use 'console.*' - use logger instead (enforced by ESLint rule 'actual/prefer-logger-over-console')
Never import from 'uuid' without destructuring - use 'import { v4 as uuidv4 } from 'uuid''
Files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsxpackages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Prefer 'type' over 'interface' for defining types in TypeScript
Avoid 'enum' - use objects or maps instead
Avoid 'any' or 'unknown' unless absolutely necessary
Avoid type assertions ('as', '!') - prefer 'satisfies' for type narrowing
Use inline type imports: 'import { type MyType } from '...''
Look for existing type definitions in 'packages/loot-core/src/types/' before creating new types
Files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsxpackages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{tsx,jsx}: Create new components in their own files
Don't use 'React.FunctionComponent' or 'React.FC' - type props directly
Don't use 'React.*' patterns - use named imports instead
Use '' instead of '' tags in React components
Avoid unstable nested components
Use 'satisfies' for type narrowing in React components
Use declarative JSX that is minimal and readable; avoid unnecessary curly braces in conditionals
Prefer explicit expressions ('condition && ') in JSX
Never import colors directly - use theme instead
Use 'Trans' component instead of 't()' function when possible for internationalization
All user-facing strings must be translated using i18n
Files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsxpackages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
packages/desktop-client/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/desktop-client/src/**/*.{tsx,jsx}: Use custom hooks from 'src/hooks' instead of react-router directly (e.g., 'useNavigate()' from 'src/hooks')
Use 'useDispatch()', 'useSelector()', 'useStore()' from 'src/redux' instead of react-redux
Files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsxpackages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
🧠 Learnings (39)
📚 Learning: 2024-10-14T09:03:37.410Z
Learnt from: minajevs
Repo: actualbudget/actual PR: 3274
File: packages/desktop-client/src/components/mobile/transactions/TransactionEdit.jsx:0-0
Timestamp: 2024-10-14T09:03:37.410Z
Learning: In `packages/desktop-client/src/components/mobile/transactions/TransactionEdit.jsx`, within the `onEditField` function, the `unserializedTransaction` variable is always expected to be found when accessing its properties. Adding a null check may not be necessary.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-11-04T00:47:00.968Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 6065
File: packages/desktop-client/src/components/transactions/TransactionList.tsx:197-205
Timestamp: 2025-11-04T00:47:00.968Z
Learning: In packages/desktop-client/src/components/transactions/TransactionList.tsx and similar schedule creation code, when using schedule/create with conditions and then updating the rule's actions, it's correct to filter rule.actions to keep only 'link-schedule' and append custom actions. Transactions created by schedules are defined by a mix of conditions (which create the base transaction with date, amount, payee, account) and actions (which apply additional modifications like category, notes, splits). The filtering pattern replaces any auto-generated actions with the desired custom actions.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-11-09T20:18:28.468Z
Learnt from: csenel
Repo: actualbudget/actual PR: 3810
File: packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx:150-161
Timestamp: 2024-11-09T20:18:28.468Z
Learning: In `packages/desktop-client/src/components/transactions/SelectedTransactionsButton.tsx`, prefer to keep the implementation of checks consistent with similar patterns elsewhere in the codebase, even if alternative implementations are more concise.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-10-24T17:05:41.415Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/desktop-client/src/components/accounts/Account.tsx:655-665
Timestamp: 2024-10-24T17:05:41.415Z
Learning: The Account component in 'packages/desktop-client/src/components/accounts/Account.tsx' is being rewritten in a separate PR.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsxpackages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2024-10-05T10:58:13.598Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 3570
File: packages/desktop-client/src/components/modals/ImportTransactionsModal/Transaction.tsx:83-90
Timestamp: 2024-10-05T10:58:13.598Z
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`.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-11-22T17:03:41.876Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 6158
File: packages/desktop-client/src/hooks/useScheduleEdit.ts:163-172
Timestamp: 2025-11-22T17:03:41.876Z
Learning: In packages/desktop-client/src/hooks/useScheduleEdit.ts around lines 163-172, the 'set-transactions' case uses a single-argument sort comparator (`transactions.sort(a => { return action.transactionId === a.id ? -1 : 1; })`), which is not symmetric and can produce unstable ordering. The correct approach is to use a two-argument comparator that returns -1/0/1 based on whether a or b matches action.transactionId. MatissJanis prefers to fix this in a future PR when this code is modified again.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-10-16T03:51:04.683Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3593
File: packages/desktop-client/src/components/sidebar/BudgetName.tsx:114-136
Timestamp: 2024-10-16T03:51:04.683Z
Learning: In 'packages/desktop-client/src/components/sidebar/BudgetName.tsx', empty budget names are handled elsewhere, so additional error handling within the 'EditableBudgetName' component is unnecessary.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-09-27T14:15:46.637Z
Learnt from: jfdoming
Repo: actualbudget/actual PR: 3402
File: packages/desktop-client/src/components/accounts/Account.tsx:8-8
Timestamp: 2024-09-27T14:15:46.637Z
Learning: In the `AllTransactions` component of `Account.tsx`, the `useLayoutEffect` hook is appropriately used to dispatch an action that closes splits for parent transactions when `prependTransactions` changes, ensuring this occurs synchronously before the component is painted.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsxpackages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2024-10-22T02:08:48.162Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/loot-core/src/client/data-hooks/transactions.ts:32-88
Timestamp: 2024-10-22T02:08:48.162Z
Learning: In the React `useTransactions` hook defined in `packages/loot-core/src/client/data-hooks/transactions.ts`, changes to `options` (e.g., `options.pageCount`) should be applied on the next query without adding them to the `useEffect` dependency array, to avoid unnecessary rerenders when options change.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-11-26T23:01:06.627Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 6259
File: packages/desktop-client/src/budgetfiles/budgetfilesSlice.ts:320-326
Timestamp: 2025-11-26T23:01:06.627Z
Learning: In packages/desktop-client/src/budgetfiles/budgetfilesSlice.ts around lines 320-326 in the downloadBudget function, there's a potential TypeError when error.meta is null. The check `typeof error.meta === 'object'` passes for null (since typeof null === 'object' in JavaScript), but then `'isMissingKey' in error.meta` throws a TypeError. The safe fix is to add `error.meta != null &&` before the typeof check to handle both null and undefined cases. Remind about this when logic changes are made to this error handling code.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-10-09T20:30:39.127Z
Learnt from: UnderKoen
Repo: actualbudget/actual PR: 3619
File: packages/loot-core/src/server/accounts/transaction-rules.ts:795-801
Timestamp: 2024-10-09T20:30:39.127Z
Learning: In the `finalizeTransactionForRules` function within `packages/loot-core/src/server/accounts/transaction-rules.ts`, potential race conditions when inserting payees are handled in the method that calls this function, so additional safeguards within `finalizeTransactionForRules` are unnecessary.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-10-09T20:17:46.493Z
Learnt from: UnderKoen
Repo: actualbudget/actual PR: 3619
File: packages/loot-core/src/server/accounts/transaction-rules.ts:551-0
Timestamp: 2024-10-09T20:17:46.493Z
Learning: When finalizing transactions that involve inserting or retrieving payees, avoid using `Promise.all` as it may result in duplicate payees due to concurrent operations. Sequential processing ensures payees are correctly handled without duplication.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsxpackages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2025-12-04T19:58:16.030Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T19:58:16.030Z
Learning: Applies to **/*.{tsx,jsx} : Never import colors directly - use theme instead
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-11-12T18:16:38.182Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3828
File: packages/desktop-client/src/style/themes/development.ts:217-217
Timestamp: 2024-11-12T18:16:38.182Z
Learning: In `packages/desktop-client/src/style/themes/development.ts`, the constants `calendarCellBackground` and `calendarBackground` are used for different calendar components, so they may share the same color value.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-10-25T04:49:31.861Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3689
File: packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx:144-156
Timestamp: 2024-10-25T04:49:31.861Z
Learning: In `packages/desktop-client/src/components/modals/manager/DuplicateFileModal.tsx`, styles for buttons may differ for each button in the future, so avoid suggesting extraction of common styles in this file.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-10-16T03:46:34.277Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3593
File: packages/desktop-client/src/components/sidebar/BudgetName.tsx:151-165
Timestamp: 2024-10-16T03:46:34.277Z
Learning: In `packages/desktop-client/src/components/sidebar/BudgetName.tsx`, `media` is not implemented on `<Button>` components at this time.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-12-04T19:58:16.030Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T19:58:16.030Z
Learning: Applies to **/*.{ts,tsx,js,jsx} : Never use 'console.*' - use logger instead (enforced by ESLint rule 'actual/prefer-logger-over-console')
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-11-12T19:52:52.889Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3792
File: packages/desktop-client/src/components/reports/reports/Summary.tsx:134-161
Timestamp: 2024-11-12T19:52:52.889Z
Learning: In `packages/desktop-client/src/components/reports/reports/Summary.tsx`, API calls like `get-earliest-transaction` are used without explicit error handling to maintain consistency with other components.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-11-04T00:34:13.035Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 3581
File: packages/loot-core/src/client/actions/account.ts:180-194
Timestamp: 2024-11-04T00:34:13.035Z
Learning: In `packages/loot-core/src/client/actions/account.ts`, within the `syncAccounts` function, the batch sync request for SimpleFin accounts handles errors by returning error objects instead of throwing exceptions. Therefore, wrapping this block in a try-catch is unnecessary.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-10-24T05:09:44.115Z
Learnt from: tlesicka
Repo: actualbudget/actual PR: 3689
File: packages/loot-core/src/server/backups.web.ts:203-207
Timestamp: 2024-10-24T05:09:44.115Z
Learning: In `packages/loot-core/src/server/backups.web.ts`, developers prefer to keep the error handling for cloud storage uploads inline rather than extracting it into a separate function. Avoid suggesting this refactoring in future reviews.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-12-04T19:58:16.030Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T19:58:16.030Z
Learning: Applies to packages/loot-core/src/**/*.{ts,tsx,js,jsx} : Don't directly reference platform-specific imports (.api, .web, .electron) - use conditional exports in loot-core for platform-specific code
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-12-04T19:58:16.030Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T19:58:16.030Z
Learning: Applies to packages/desktop-client/src/**/*.{tsx,jsx} : Use custom hooks from 'src/hooks' instead of react-router directly (e.g., 'useNavigate()' from 'src/hooks')
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsxpackages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2025-12-04T19:58:16.030Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T19:58:16.030Z
Learning: Applies to packages/desktop-client/src/**/*.{tsx,jsx} : Use 'useDispatch()', 'useSelector()', 'useStore()' from 'src/redux' instead of react-redux
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsxpackages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2025-03-14T15:11:36.220Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 4484
File: packages/desktop-client/src/components/mobile/budget/ExpenseGroup.tsx:0-0
Timestamp: 2025-03-14T15:11:36.220Z
Learning: In the Actual Budget app, the callback functions `onEditCategory` and `onBudgetAction` are already properly memoized with useCallback in the Budget component (packages/desktop-client/src/components/mobile/budget/index.tsx).
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-03-14T15:11:36.220Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 4484
File: packages/desktop-client/src/components/mobile/budget/ExpenseGroup.tsx:0-0
Timestamp: 2025-03-14T15:11:36.220Z
Learning: In the Actual Budget mobile app, the callback functions `onEditCategory` (implemented as `onOpenCategoryMenuModal`) and `onBudgetAction` are already properly memoized with useCallback in the Budget component (packages/desktop-client/src/components/mobile/budget/index.tsx).
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2024-11-12T18:18:07.282Z
Learnt from: lelemm
Repo: actualbudget/actual PR: 3828
File: packages/desktop-client/src/components/reports/reports/Calendar.tsx:575-631
Timestamp: 2024-11-12T18:18:07.282Z
Learning: In `Calendar.tsx`, transaction-related callbacks such as `onBatchDelete`, `onBatchDuplicate`, `onCreateRule`, and `onScheduleAction` are intentionally left as empty functions because these operations should not be usable on that page.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-12-04T19:58:16.030Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T19:58:16.030Z
Learning: Applies to **/*.{tsx,jsx} : Use 'Trans' component instead of 't()' function when possible for internationalization
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsxpackages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2024-10-08T15:46:15.739Z
Learnt from: qedi-r
Repo: actualbudget/actual PR: 3527
File: packages/desktop-client/src/components/accounts/Header.jsx:0-0
Timestamp: 2024-10-08T15:46:15.739Z
Learning: In the Actual Budget project, importing `t` directly from 'i18next' is best avoided, and we should almost always prefer using the useTranslation hook if possible.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-06-21T04:15:23.727Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 5207
File: packages/desktop-client/src/components/transactions/TransactionsTable.tsx:2194-2201
Timestamp: 2025-06-21T04:15:23.727Z
Learning: In TransactionsTable.tsx, the balance calculation using find() to locate the first non-scheduled transaction is acceptable for performance because: 1) the component only renders when adding transactions (limited usage), 2) find() short-circuits on first match, and 3) scheduled transactions are typically sparse in the transaction list.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-10-06T15:26:36.914Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 5870
File: packages/loot-core/src/server/transactions/transaction-rules.ts:345-348
Timestamp: 2025-10-06T15:26:36.914Z
Learning: In packages/loot-core/src/server/rules/rule.ts, Rule.execActions<T>(object) returns a Partial<T> diff object (not an array). When actions create splits, the top-level execActions returns a grouped transaction with a subtransactions array; the Rule.execActions diff includes subtransactions. Merging via Object.assign preserves splits.
Applied to files:
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
📚 Learning: 2025-01-16T14:28:00.133Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 4166
File: packages/loot-core/src/client/data-hooks/transactions.ts:258-258
Timestamp: 2025-01-16T14:28:00.133Z
Learning: Variables defined within React hooks like `useMemo` or `useEffect` are not dependencies of other hooks. Only external variables and values from the component scope need to be included in the dependency arrays.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2025-12-04T19:58:16.030Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T19:58:16.030Z
Learning: Applies to **/*.{tsx,jsx} : Don't use 'React.*' patterns - use named imports instead
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2024-10-22T05:34:56.976Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/desktop-client/src/components/mobile/accounts/AccountTransactions.tsx:261-277
Timestamp: 2024-10-22T05:34:56.976Z
Learning: In React components (TypeScript), avoid using hooks like `useCallback` inside callbacks or nested functions. Hooks must be called at the top level of functional components.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2025-12-04T19:58:16.030Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T19:58:16.030Z
Learning: Applies to **/*.{tsx,jsx} : Don't use 'React.FunctionComponent' or 'React.FC' - type props directly
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2024-10-22T05:32:30.530Z
Learnt from: joel-jeremy
Repo: actualbudget/actual PR: 3685
File: packages/desktop-client/src/components/schedules/index.tsx:37-39
Timestamp: 2024-10-22T05:32:30.530Z
Learning: In our React function components, we should include `dispatch` in the dependency array of `useCallback` hooks to comply with ESLint rules, even though `dispatch` is a stable function.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2025-12-04T19:58:16.030Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T19:58:16.030Z
Learning: Applies to **/*.{tsx,jsx} : Avoid unstable nested components
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2025-05-22T05:17:51.422Z
Learnt from: matt-fidd
Repo: actualbudget/actual PR: 4955
File: packages/component-library/src/Input.tsx:52-64
Timestamp: 2025-05-22T05:17:51.422Z
Learning: React 19 has simplified ref handling, eliminating the need for `React.forwardRef()` in functional components. Components can now receive and destructure the ref directly from props rather than requiring the second parameter in forwardRef.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2025-01-16T19:31:27.020Z
Learnt from: MatissJanis
Repo: actualbudget/actual PR: 4169
File: packages/component-library/src/icons/Loading.tsx:8-8
Timestamp: 2025-01-16T19:31:27.020Z
Learning: The Loading component in actual-app/components package is a direct copy from its previous location, and improvements to its accessibility and security are planned to be addressed in a separate PR.
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
📚 Learning: 2025-12-04T19:58:16.030Z
Learnt from: CR
Repo: actualbudget/actual PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T19:58:16.030Z
Learning: Applies to **/*.{tsx,jsx} : Use '<Link>' instead of '<a>' tags in React components
Applied to files:
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
🧬 Code graph analysis (2)
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx (6)
packages/desktop-client/src/hooks/useLocationPermission.ts (1)
useLocationPermission(12-66)packages/loot-core/src/types/models/payee.ts (1)
PayeeEntity(4-12)packages/loot-core/src/shared/location.ts (2)
locationService(144-147)deletePayeeLocation(75-82)packages/loot-core/src/shared/location-utils.ts (1)
calculateDistance(21-40)packages/loot-core/src/shared/constants.ts (1)
DEFAULT_MAX_DISTANCE(1-1)packages/desktop-client/src/payees/payeesSlice.ts (1)
deletePayeeLocation(205-216)
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx (8)
packages/loot-core/src/types/models/payee.ts (1)
PayeeEntity(4-12)packages/desktop-client/src/hooks/usePayees.ts (2)
usePayees(75-87)useNearbyPayees(31-73)packages/desktop-client/src/components/autocomplete/Autocomplete.tsx (1)
defaultFilterSuggestion(96-102)packages/desktop-client/src/redux/index.ts (1)
useDispatch(18-18)packages/desktop-client/src/payees/payeesSlice.ts (3)
deletePayeeLocation(205-216)createPayee(169-189)updatePayeeLocationIfNeeded(191-203)packages/loot-core/src/server/accounts/payees.ts (1)
createPayee(4-17)packages/desktop-client/src/hooks/useLocationPermission.ts (1)
useLocationPermission(12-66)packages/loot-core/src/shared/location-utils.ts (1)
formatDistance(48-72)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Redirect rules - actualbudget-website
- GitHub Check: Header rules - actualbudget-website
- GitHub Check: Pages changed - actualbudget-website
- GitHub Check: build (windows-latest)
- GitHub Check: build (macos-latest)
- GitHub Check: build (ubuntu-22.04)
- GitHub Check: Analyze
- GitHub Check: Generate VRT Updates
- GitHub Check: Wait for Netlify build to finish
- GitHub Check: api
- GitHub Check: compare
- GitHub Check: autofix
🔇 Additional comments (11)
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx (8)
19-19: LGTM!The location-related imports are properly structured and follow the coding guidelines.
Also applies to: 35-37, 85-85, 95-95
560-562: LGTM!The new props for location access and forget location functionality are properly typed and well-structured.
1362-1368: LGTM!The state declarations for location-related functionality are well-structured and properly typed.
1495-1502: LGTM!The logic to update all fields when the payee changes is appropriate for location-based transaction entry, allowing rules to re-apply and auto-fill fields based on the new payee context.
1533-1540: LGTM!The logic correctly shows the forget location button only when the payee is manually changed by the user (not auto-selected) and location access is granted.
1628-1669: LGTM!The forget location implementation is well-structured:
- Creates a sorted copy to avoid mutation (line 1639)
- Uses distance calculation to verify proximity before deletion
- Provides user feedback via notifications on error
1671-1680: LGTM!The auto-select effect correctly guards against undefined transactions and maintains immutability by creating a new transaction object before updating.
1709-1711: LGTM!The location-related props are correctly forwarded to the inner component.
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx (3)
2-14: Imports and new hook/util wiring look consistentNew imports for icons, location utilities/hooks (
formatDistance,useLocationPermission,useNearbyPayees,useUnitOfMeasurementFormat) and updated payees slice actions (updatePayeeLocationIfNeeded,deletePayeeLocation) are coherent with the added functionality and respect the existing import structure.Also applies to: 19-23, 30-33, 42-49, 52-55
357-378: Nearby payees wiring is sensible; only filtering/highlight behaviour needs confirmingThe overall flow in
PayeeAutocompletelooks good:
nearbyPayees/locationAccesscan be controlled externally but fall back touseNearbyPayees(locationAccess)when omitted.- Nearby payees are tagged with
itemType: 'nearby_payee'and filtered using the samedefaultFilterSuggestionas regular payees.renderItemsprependsfilteredNearbyPayeesin front of the normal list so nearby entries surface above suggested/other payees.Aside from the highlight/indexing concerns already raised, the composition and filtering here seem correct and match the PR description.
Also applies to: 397-413, 452-476, 587-600
175-187: Unable to verify indexing alignment between highlightedIndex and rendered PayeeList itemsCannot confirm the described mismatch between
highlightedIndexfrom<Autocomplete>and the items rendered byPayeeListwhenfilteredNearbyPayeesis present. Repository access is unavailable for verification.
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx (1)
581-599:⚠️ Potential issue | 🟠 MajorBug:
getHighlightedIndexreturns 0 without accounting for the "new" payee item when nearby payees exist.When the user is typing (triggering the "Create payee" item at index 0),
getHighlightedIndexreturns0iffilteredNearbyPayees.length > 0. But inPayeeList, thenewPayeeitem is assignedhighlightedIndex: 0before nearby payees. So the highlight lands on "Create payee" instead of the first nearby payee, contradicting the comment on Line 582.The fix needs to account for the "new" item offset, similar to the existing logic on Lines 590-592:
Proposed fix
getHighlightedIndex={suggestions => { // If we have nearby payees, highlight the first nearby payee if (filteredNearbyPayees.length > 0) { - return 0; + // Account for the "new payee" item at index 0 when user has typed input + const hasNewItem = suggestions.length > 0 && suggestions[0].id === 'new'; + return hasNewItem ? 1 : 0; } // Otherwise use original logic for suggestions#!/bin/bash # Verify how getHighlightedIndex return value is used by Autocomplete ast-grep --pattern 'getHighlightedIndex($$$)'
🧹 Nitpick comments (1)
packages/desktop-client/src/components/autocomplete/PayeeAutocomplete.tsx (1)
812-933: Consider extracting shared styling logic betweenNearbyPayeeItemandPayeeItem.Lines 819–840 of
NearbyPayeeItemduplicate thenarrowStyle,iconSize,paddingLeftOverFromIcon, anditemIconlogic fromPayeeItem(lines 726–746). A small helper (e.g.,usePayeeItemStyles(item)) could reduce this duplication.Not blocking — just a follow-up idea to reduce drift between the two components.
No worries! I'm patient and have been building/running a local container image for a few months now. A few more days, weeks, or months wouldn't hurt me :)
Conflicts resolved. |
|
VRT tests ❌ failed. View the test report. To update the VRT screenshots, comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx`:
- Around line 1661-1709: The onForgetLocation handler currently calls
setShouldShowForgetLocation(false) unconditionally, hiding the button even when
no location was deleted; update onForgetLocation so that
setShouldShowForgetLocation(false) is only called after a confirmed successful
deletion (i.e., after dispatch(deletePayeeLocation(...)).unwrap() returns a
result with result.success true). Use the existing symbols (onForgetLocation,
mostRecentLocation, DEFAULT_MAX_DISTANCE_METERS, deletePayeeLocation,
setShouldShowForgetLocation) to move the state change into the success branch
and leave the button visible (and optionally dispatch an error/info
notification) when there was no recent location or the distance check failed.
- Around line 1108-1135: The inline style for the Button uses an invalid CSS
shorthand "border: theme.errorBorder" (in the TransactionEdit render for
shouldShowForgetLocation and also seen in PayeeAutocomplete.tsx), so replace
that single property with explicit borderWidth, borderStyle, and borderColor
(e.g., borderWidth: '1px', borderStyle: 'solid', borderColor: theme.errorBorder)
or build a full shorthand string including width and style; update the style
object on the Button (and the similar spot in PayeeAutocomplete where
`theme.errorBorder` is used) to use these correct properties so the border
renders properly.
🧹 Nitpick comments (1)
packages/desktop-client/src/payees/payeesSlice.ts (1)
241-290:getNearbyPayeescondition doesn't account for changed coordinates — document the intended usage.The
conditioncallback (lines 282-288) only checksisNearbyPayeesDirty/isNearbyPayeesLoaded, ignoring the actuallocationParams. If a caller invokesgetNearbyPayeeswith different coordinates after the initial load, the thunk will be skipped and stale data returned. This is consistent with thegetPayees/getCommonPayeespattern, but location-based queries are inherently parameter-dependent unlike those other thunks.Ensure that consumers always use
reloadNearbyPayeeswhen coordinates change (not justgetNearbyPayees). A brief code comment ongetNearbyPayeesclarifying this would help prevent misuse.
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
Show resolved
Hide resolved
packages/desktop-client/src/components/mobile/transactions/TransactionEdit.tsx
Show resolved
Hide resolved
Ah. I never noticed this property wasn't working. I guess the button looked OK to me!
MatissJanis
left a comment
MatissJanis
left a comment
There was a problem hiding this comment.
Sorry, just two small nitpicks from me. Apart from that LGTM!
Co-authored-by: Matiss Janis Aboltins <matiss@mja.lv>
Co-authored-by: Matiss Janis Aboltins <matiss@mja.lv>
Guess I shouldn't commit a suggestion after waking up
2 participants
I'd like to open a discussion about payee locations.
This change helps streamline mobile transaction entry by suggesting payees based on stored payee location data. It should reduce the time it takes to select or add the correct payee, specifically when recording expenses on the go.
This feature was a key factor in getting my partner onboard with finally canceling our YNAB5 subscription. We've been using some version of this feature since early October (with many, many rebases!), and it's exciting to finally reach a point where I think it works well enough that it may actually be useful to other people.
For what it's worth we've been using this against a successfully imported YNAB5 budget that has been continuously in-use since 2012/YNAB4.
Quick demo on a testing/local budget:
https://github.com/user-attachments/assets/5ff34a75-c32f-417d-8740-3f620784b2ce
Static Image of Payee Autocomplete w/Nearby Payees:
Feature Flag:
This is my first attempt working on Actual Budget and I've tried to keep the diff relatively small given the scope of the feature.
Bundle Stats
View detailed bundle stats
desktop-client
Total
Changeset
home/runner/work/actual/actual/packages/loot-core/src/shared/location-service.tssrc/hooks/useLocationPermission.tshome/runner/work/actual/actual/packages/loot-core/src/shared/location-adapters.tshome/runner/work/actual/actual/packages/component-library/src/icons/v1/LocationCurrent.tsxhome/runner/work/actual/actual/packages/loot-core/src/shared/location-utils.tshome/runner/work/actual/actual/packages/loot-core/src/shared/location.tshome/runner/work/actual/actual/packages/loot-core/src/shared/constants.tssrc/hooks/usePayees.tssrc/payees/payeesSlice.tssrc/components/autocomplete/PayeeAutocomplete.tsxlocale/ca.jsonsrc/components/modals/PayeeAutocompleteModal.tsxsrc/components/mobile/transactions/TransactionEdit.tsxsrc/hooks/useFeatureFlag.tssrc/components/settings/Experimental.tsxlocale/en.jsonView detailed bundle breakdown
Added
No assets were added
Removed
No assets were removed
Bigger
Smaller
No assets were smaller
Unchanged
loot-core
Total
Changeset
home/runner/work/actual/actual/packages/loot-core/src/shared/constants.tshome/runner/work/actual/actual/packages/loot-core/src/server/payees/app.tshome/runner/work/actual/actual/packages/loot-core/src/server/importers/ynab5.tshome/runner/work/actual/actual/packages/loot-core/src/server/api.tshome/runner/work/actual/actual/packages/loot-core/src/server/aql/schema/index.tshome/runner/work/actual/actual/packages/loot-core/src/server/api-models.tsView detailed bundle breakdown
Added
Removed
Bigger
No assets were bigger
Smaller
No assets were smaller
Unchanged
No assets were unchanged
api
Total
Changeset
src/shared/constants.tssrc/server/payees/app.tssrc/server/importers/ynab5.tssrc/server/api.tssrc/server/aql/schema/index.tssrc/server/api-models.tsView detailed bundle breakdown
Added
No assets were added
Removed
No assets were removed
Bigger
Smaller
No assets were smaller
Unchanged
No assets were unchanged