Skip to content

feat: add manual bank transaction entry page#1041

Merged
TaprootFreak merged 8 commits intodevelopfrom
feat/sepa-manual-entry
Apr 10, 2026
Merged

feat: add manual bank transaction entry page#1041
TaprootFreak merged 8 commits intodevelopfrom
feat/sepa-manual-entry

Conversation

@TaprootFreak
Copy link
Copy Markdown
Collaborator

Summary

  • Add "Manual entry" button on /sepa page linking to /sepa/manuell
  • New form at /sepa/manuell for manual bank transaction entry with fields: booking date, value date, amount, currency, direction (CRDT/DBIT), counterparty details (name, address, IBAN), and remittance info
  • Generates CAMT.053 XML client-side and submits via existing POST /bankTx endpoint
  • Admin-only access: useAdminGuard() on frontend, RoleGuard(BANKING_BOT) on API (ADMIN role inherits BANKING_BOT access)

Test plan

  • Navigate to /sepa and verify "Manual entry" button appears
  • Click button and verify redirect to /sepa/manuell
  • Fill out form and submit — verify bank transaction is created
  • Verify non-admin users cannot access /sepa/manuell

Add a form-based alternative to XML file upload for creating bank
transactions. The form generates a CAMT.053 XML and submits it via
the existing POST /bankTx endpoint. Admin-only access enforced via
useAdminGuard() on frontend and RoleGuard(BANKING_BOT) on API.
- Fix escapeXml crash on undefined values for optional address fields
- Extract escapeXml, buildPartyXml, buildCamt053Xml outside component
- Break long inline XML strings into readable multi-line structure
- Only emit address XML tags for fields that have values
@TaprootFreak TaprootFreak marked this pull request as ready for review April 9, 2026 13:32
@TaprootFreak TaprootFreak requested a review from bernd2022 April 9, 2026 13:32
@bernd2022
Copy link
Copy Markdown
Collaborator

Code Review

Kritisch

1. Hardcoded Bankdaten im Frontend (sepa-manual.screen.tsx:43-45)

const ACCOUNT_IBAN = 'CH7780808002608614092';
const ACCOUNT_OWNER = 'DFX AG';
const ACCOUNT_BANK = 'Raiffeisenbank Waldkirch';

Echte IBAN und Bankdaten sind direkt im Frontend-Code. Auch wenn der Screen admin-geschützt ist: Das landet im JS-Bundle und ist für jeden im Browser-DevTools einsehbar. Diese Werte sollten vom Backend kommen oder zumindest aus einer Environment-Config.


2. Fehlende IBAN-Validierung (sepa-manual.screen.tsx:281)

Die IBAN wird nur mit Validations.Required geprüft. Im Rest der Codebase (z.B. add-bank-account.tsx:79) wird Validations.Iban verwendet. Hier kann jeder beliebige String als IBAN eingegeben werden, was zu ungültigem XML und Fehlern im Backend führt.

// Ist:
iban: [Validations.Required],
// Sollte:
iban: [Validations.Required, Validations.Iban(...)],

3. bookingDate / valueDate nicht escaped (sepa-manual.screen.tsx:123-124, 168-172)

Diese Felder werden direkt ins XML eingesetzt ohne escapeXml(). Zwar kommen sie aus einem type="date" Input, aber das ist kein Schutz — der Browser erzwingt das Format nicht bei programmatischer Manipulation. Konsistenz und Defense-in-Depth verlangen hier Escaping.


Hoch

4. async Funktion mit .then()/.catch() statt await (sepa-manual.screen.tsx:243-267)

onSubmit ist als async deklariert, verwendet aber Promise-Chaining. Entweder async/await mit try/catch/finally oder .then()/.catch() ohne async — nicht beides.


5. parseFloat(data.amount) ohne NaN-Check (sepa-manual.screen.tsx:82)

amount ist im Interface als string typisiert. parseFloat kann NaN zurückgeben, was dann als NaN in das XML geschrieben wird.


6. Komponente überschreitet 250-Zeilen-Limit (369 Zeilen)

Die XML-Builder-Logik (Zeilen 47-217, ~170 Zeilen) sollte in eine eigene Utility-Datei extrahiert werden, z.B. src/utils/camt053-builder.ts. Das würde die Komponente auf ~200 Zeilen bringen und die Builder-Logik testbar machen.


Mittel

7. Duplizierter Code zwischen sepa.screen.tsx und sepa-manual.screen.tsx

Folgende Patterns sind quasi identisch kopiert:

  • State-Deklarationen (isUploading, showNotification, error)
  • toggleNotification Funktion (identisch)
  • Upload-Logik (call({ url: 'bankTx', ... }))
  • Notification-UI (das gesamte JSX-Block mit DfxIcon/CHECK)

Ein Custom Hook useBankTxUpload oder zumindest useNotification würde die Duplikation reduzieren.


8. Schwache AcctSvcrRef-Generierung (sepa-manual.screen.tsx:81)

Date.now() hat Millisekunden-Auflösung. Bei schnellen aufeinanderfolgenden Einträgen gibt es Kollisionen. Besser: crypto.randomUUID().


9. country-Feld ohne Format-Validierung (sepa-manual.screen.tsx:334)

Freitext-Eingabe für einen ISO 3166-1 Alpha-2 Country Code. Ein Dropdown mit gültigen Ländercodes oder zumindest eine Längen-/Format-Validierung wäre sicherer.


Niedrig

10. Button-Label "Next" unpassend (sepa-manual.screen.tsx:360)

Der Submit-Button sagt "Next", aber es gibt keinen nächsten Schritt — es ist ein direkter Upload. "Upload" oder "Submit" wäre treffender.

…urable

- Rename buildingNumber→houseNumber, postalCode→zip to match codebase
- Use StyledHorizontalStack for Street+Nr and ZIP+City rows
- Replace plain Country input with StyledSearchDropdown<Country>
- Add autocomplete attributes matching existing screens
- Replace hardcoded ACCOUNT_IBAN/OWNER/BANK with form fields
- Fix amount formatting to 2 decimal places
- Remove unused async from onSubmit
- Extract XML builder to src/util/camt053-builder.ts (#6)
- Add IBAN validation via Validations.Iban for both IBAN fields (#2)
- Escape bookingDate/valueDate/currency in XML output (#3)
- Add NaN guard for parseFloat on amount (#5)
- Use crypto.randomUUID() for AcctSvcrRef (#8)
- Change button label from "Next" to "Upload" (#10)
@TaprootFreak
Copy link
Copy Markdown
Collaborator Author

@davidleomay

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 10, 2026

🤖 PR Review Bot

❌ Security: 1 critical vulnerabilities


This is an automated review. Please address the issues above.

Copy link
Copy Markdown
Member

@davidleomay davidleomay left a comment

Choose a reason for hiding this comment

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

It would be easier to add a JSON endpoint to create bank transactions to the API instead of adding a CAMT053 builder here 😁

@TaprootFreak TaprootFreak merged commit 6588da9 into develop Apr 10, 2026
6 checks passed
@TaprootFreak TaprootFreak deleted the feat/sepa-manual-entry branch April 10, 2026 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants