feat(imports): Submit confirms by id and resolve names reactively#77
feat(imports): Submit confirms by id and resolve names reactively#77
Conversation
Code Review SummaryThis PR improves the import flow by shifting from name-based resolution to ID-based resolution for wallets, parties, and categories. It introduces reactive matching to handle asynchronous data loading and clarifies the auto-creation logic in the confirmation dialog. 🚀 Key Improvements
💡 Minor Suggestions
|
![sourceant[bot]](https://avatars.githubusercontent.com/in/1161781?s=60&v=4){kind=small}
| watch( | ||
| [suggestions, wallets, parties, categories], | ||
| () => { | ||
| for (const s of suggestions.value) { |
There was a problem hiding this comment.
The current watcher mutates the reactive suggestions array directly inside a loop, which can trigger redundant re-evaluations or side effects. While Vue handles this reasonably well, it is cleaner to batch changes or check if the value actually changed before assignment.
| for (const s of suggestions.value) { | |
| for (const s of suggestions.value) { | |
| if (!s.wallet_id && s.wallet) { | |
| const match = wallets.value.find((w) => w.name === s.wallet); | |
| if (match && s.wallet_id !== match.id) s.wallet_id = match.id; | |
| } | |
| if (!s.party_id && s.party) { | |
| const match = parties.value.find((p) => p.name === s.party); | |
| if (match && s.party_id !== match.id) s.party_id = match.id; | |
| } | |
| if (!s.category_id && s.category) { | |
| const match = categories.value.find((c) => c.name === s.category); | |
| if (match && s.category_id !== match.id) s.category_id = match.id; | |
| } | |
| } |
| // A row with no wallet_id is fine iff its suggestion wallet will be auto-created. | ||
| // If the count of rows missing a wallet_id exceeds the count that auto-create | ||
| // would cover, the user has to intervene before we can submit. | ||
| const walletGapUncovered = computed(() => { |
There was a problem hiding this comment.
The logic for walletGapUncovered assumes that if autoCreateWallets is true, all missing IDs are covered. However, this is only true if the missing IDs have a suggested 'name'. If a row has neither a wallet_id nor a suggested wallet name, the auto-create won't help. Consider refining this to ensure all accepted rows actually have a path to a valid wallet.
| const walletGapUncovered = computed(() => { | |
| const walletGapUncovered = computed(() => { | |
| if (props.missingWalletCount === 0) return false; | |
| // If we have missing IDs, they are only 'covered' if we have names to create and the toggle is on | |
| if (props.newWalletCount > 0 && autoCreateWallets.value && props.newWalletCount === props.missingWalletCount) return false; | |
| return true; | |
| }); |
Deploying webui with
|
| Latest commit: |
f8c1056
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9b40a5c4.webui-9fh.pages.dev |
| Branch Preview URL: | https://fix-import-confirm-id-flow.webui-9fh.pages.dev |
Deploying trakli-dev with
|
| Latest commit: |
f8c1056
|
| Status: | ⚡️ Build in progress... |
The review screen now submits explicit wallet/party/category IDs and pre-fills them from the user's existing records by exact name match. Resolution is reactive: when the wallets/parties/categories caches load after the analyzer response (common on a fresh page open), any suggestion that was sitting unresolved gets re-matched automatically. The confirm dialog only surfaces auto-create toggles when the analyzer produced a name that doesn't match an existing record, so a fully resolved batch shows a clean confirmation. Signed-off-by: nfebe <fenn25.fn@gmail.com>
nfebe
force-pushed
the
fix/import-confirm-id-flow
branch
from
1d61584 to
f8c1056
Compare
| field: 'wallet_id' | 'party_id' | 'category_id', | ||
| value: string | ||
| ) => { | ||
| const id = value === '' ? null : Number(value); |
There was a problem hiding this comment.
Using Number(value) can result in NaN if the value is not numeric. Since IDs are expected to be numbers, it's safer to ensure only valid numbers are emitted, or handle the NaN case to prevent invalid data being sent to the API.
| const id = value === '' ? null : Number(value); | |
| const id = value === '' ? null : Number.parseInt(value, 10); | |
| if (id !== null && isNaN(id)) return; |
| class="inline-edit" | ||
| :class="{ 'inline-edit--new': isNewWallet(suggestion.wallet) }" | ||
| :value="suggestion.wallet || ''" | ||
| :class="{ 'inline-edit--missing': !suggestion.wallet_id }" |
There was a problem hiding this comment.
The wallet selection is missing a visual indicator for error states when a wallet is required but not selected. Adding the inline-edit--missing class based on a logic check (similar to isRowInvalid) improves UX.
| :class="{ 'inline-edit--missing': !suggestion.wallet_id }" | |
| :class="{ 'inline-edit--missing': suggestion.status === 'accepted' && !suggestion.wallet_id }" |
| if (s.party_id != null) item.party_id = s.party_id; | ||
| if (s.category_id != null) item.category_id = s.category_id; | ||
| if (s.edited) { | ||
| if (s.amount != null) item.amount = s.amount; |
There was a problem hiding this comment.
The implementation for 'edited' fields is slightly inconsistent. For optional numeric fields like amount, it is safer to check for undefined or null specifically, while for strings like type or date, a truthiness check is usually sufficient.
| if (s.amount != null) item.amount = s.amount; | |
| if (s.amount !== undefined && s.amount !== null) item.amount = s.amount; | |
| if (s.type) item.type = s.type; | |
| if (s.description !== undefined && s.description !== null) item.description = s.description; | |
| if (s.date) item.date = s.date; |
1 participant
The review screen now submits explicit wallet/party/category IDs and pre-fills them from the user's existing records by exact name match. Resolution is reactive: when the wallets/parties/categories caches load after the analyzer response (common on a fresh page open), any suggestion that was sitting unresolved gets re-matched automatically.
The confirm dialog only surfaces auto-create toggles when the analyzer produced a name that doesn't match an existing record, so a fully resolved batch shows a clean confirmation.