Document transaction print#7715
Conversation
…on-print # Conflicts: # frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/TransactionTabs.tsx
There was a problem hiding this comment.
Sorry @Khuslen122, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughAdds a complete transaction printing subsystem: Mongolian number-to-words, shared print helpers, per-journal printable components with selectable variants, GraphQL detail expansion for product info, routing/registry updates, and a print page with variant selector and preview integration. ChangesTransaction Printing System
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (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 |
🌗 Pull Request OverviewThis PR implements comprehensive document printing functionality for the accounting module, adding support for various Mongolian financial document formats including invoices, cash receipts, bank payment orders, and inventory transaction forms. It introduces a flexible variant system allowing users to select different layouts per journal type, and adds a utility for converting amounts to Mongolian text. Reviewed Changes Show a summary per file
📋 Review Findings📄
|
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/plugins/accounting_ui/src/pages/TransactionPrintPage.tsx (1)
14-15:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHonor
inPreview=truebefore auto-printing.
TransactionTabsappendsinPreview=true, but this page ignores it and always triggerswindow.print(). That breaks preview-first flow and forces the dialog before layout selection.🐛 Proposed fix
const query = new URLSearchParams(useLocation().search); const transactionId = query.get('_id'); + const inPreview = query.get('inPreview') === 'true'; @@ useEffect(() => { - if (!loading && transaction && !hasPrintedRef.current) { + if (!loading && transaction && !inPreview && !hasPrintedRef.current) { hasPrintedRef.current = true; @@ } - }, [loading, transaction]); + }, [loading, transaction, inPreview]);Also applies to: 37-45
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/plugins/accounting_ui/src/pages/TransactionPrintPage.tsx` around lines 14 - 15, The page always calls window.print() regardless of the inPreview flag; update the auto-print logic in TransactionPrintPage to read the location query (useLocation()/URLSearchParams) and skip calling window.print() when query.get('inPreview') (or 'inPreview') is truthy/equals 'true' so the preview flow from TransactionTabs is honored; locate the code that derives transactionId from query and the place where window.print() is invoked (and the similar block at lines ~37-45) and wrap/guard those print calls with a check that inPreview !== 'true' before triggering window.print().
🧹 Nitpick comments (5)
frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/numberToWords.ts (1)
62-63: ⚡ Quick winUse
functiondeclarations for pure utilities in this fileThe pure helpers/formatter are declared as arrow-function constants; this file’s TS guideline requires
functionkeyword for pure functions.As per coding guidelines, **"Use the 'function' keyword for pure functions"**.♻️ Proposed refactor
-const belowThousand = (num: number, attr: boolean): string => { +function belowThousand(num: number, attr: boolean): string { const parts: string[] = []; ... -}; +} -const integerToMongolianText = (value: number): string => { +function integerToMongolianText(value: number): string { ... -}; +} -export const amountToMongolianText = (amount: number): string => { +export function amountToMongolianText(amount: number): string { ... -}; +}Also applies to: 88-89, 117-117
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/numberToWords.ts` around lines 62 - 63, Replace pure helper arrow-function constants in this module with function declarations: change the const belowThousand = (num: number, attr: boolean): string => { ... } to a function belowThousand(num: number, attr: boolean): string { ... } and do the same for the other pure helper constants in this file (the two remaining arrow-function helpers declared later). Ensure signatures, exported names (if any), and internal behavior remain identical while switching to the function keyword to follow the TS guideline.frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/variants.ts (1)
3-6: ⚡ Quick winUse path aliases instead of relative imports
Switch these local relative imports to the project’s absolute alias style for consistency and easier refactors.
As per coding guidelines, “Always use absolute paths when importing” and “Use path aliases for imports: ~/* for service root,@/* for modules directory, and erxes-api-shared/* for shared library in backend services”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/variants.ts` around lines 3 - 6, The imports for CashVariant, InvIncomeVariant, InvMoveVariant and InvSaleVariant use relative paths; change them to the project's absolute alias style (e.g., use `@/`* for modules) so they follow the coding guideline "Always use absolute paths when importing." Update the four import statements to use the module alias (for example replacing './cash' etc. with the appropriate `@/`... alias for this module) ensuring each symbol (CashVariant, InvIncomeVariant, InvMoveVariant, InvSaleVariant) still resolves correctly and the project build/tsconfig path mappings remain satisfied.frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/TransactionDocument.tsx (1)
5-5: ⚡ Quick winAlign imports and prop typing with project TS/TSX conventions
Use absolute-path imports and extract prop shapes into named interfaces (with
Iprefix) instead of inline object types for components/helpers.
As per coding guidelines, “Always use absolute paths when importing”, “Use path aliases for imports: ~/* for service root,@/* for modules directory, and erxes-api-shared/* for shared library in backend services”, and “Use TypeScript interface naming convention: prefix interfaces with I (e.g.,IUser,IUserDetails)”.Also applies to: 30-38, 167-171
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/TransactionDocument.tsx` at line 5, Update the import and prop typing to follow project TS conventions: change the relative import "import { amountToMongolianText } from './numberToWords';" to the absolute/path-alias variant (e.g., import from "`@/modules/transactions/transaction-form/components/documents/numberToWords`" or the project's ~/@ alias as appropriate) and extract any inline prop types used by the TransactionDocument component into a named interface with an I prefix (for example declare interface ITransactionDocumentProps { /* fields previously inline */ } and use it on the TransactionDocument function/component). Also create named interface(s) for any helper argument shapes returned/consumed by amountToMongolianText (e.g., IAmountToMongolianArgs) and update references to use those interfaces; then replace inline object types in the component and helper signatures with these I-prefixed interfaces.frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/index.tsx (2)
16-23: ⚡ Quick winUse an interface for
PrintBodyprops (IPrintBodyProps) instead of an inline type literal.This keeps prop contracts reusable and aligned with the project TS conventions.
♻️ Proposed refactor
+interface IPrintBodyProps { + transaction: ITransaction + variant?: string +} + export const PrintBody = ({ transaction, variant, -}: { - transaction: ITransaction; - // Selected layout for journals that offer several — see DOCUMENT_VARIANTS. - variant?: string; -}) => { +}: IPrintBodyProps) => {As per coding guidelines, "Use TypeScript for all code; prefer interfaces over types" and "Use TypeScript interface naming convention: prefix interfaces with
I(e.g.,IUser,IUserDetails)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/index.tsx` around lines 16 - 23, Replace the inline prop type for the PrintBody component with a named interface: create an interface IPrintBodyProps that declares transaction: ITransaction and optional variant?: string (add a brief comment if desired referencing DOCUMENT_VARIANTS), then update the PrintBody signature to accept props: IPrintBodyProps; ensure any existing usages or imports still compile and export the new interface if it should be reused elsewhere.
33-37: ⚡ Quick winAvoid duplicating default variant literals in the dispatcher.
Defaults are hardcoded here while variant defaults are also managed in the variants registry flow. Centralizing on one default source avoids drift and inconsistent first render behavior when
variantis empty.♻️ Proposed refactor
import { asCashVariant, asInvIncomeVariant, asInvMoveVariant, asInvSaleVariant, + getDefaultVariant, } from './variants'; export const PrintBody = ({ transaction, variant, }: IPrintBodyProps) => { + const fallbackVariant = getDefaultVariant(transaction.journal) + const selectedVariant = variant || fallbackVariant + if (transaction.journal === TrJournalEnum.CASH) { return ( <PrintCashDocument transaction={transaction} - variant={asCashVariant(variant || 'lined')} + variant={asCashVariant(selectedVariant)} /> ); } @@ if (transaction.journal === TrJournalEnum.INV_MOVE) { return ( <PrintInvMoveDocument transaction={transaction} - variant={asInvMoveVariant(variant || 'standard')} + variant={asInvMoveVariant(selectedVariant)} /> ); } @@ if (transaction.journal === TrJournalEnum.INV_SALE) { return ( <PrintInvSaleDocument transaction={transaction} - variant={asInvSaleVariant(variant || 'numbered')} + variant={asInvSaleVariant(selectedVariant)} /> ); } @@ if (transaction.journal === TrJournalEnum.INV_INCOME) { return ( <PrintInvIncomeDocument transaction={transaction} - variant={asInvIncomeVariant(variant || 'numbered')} + variant={asInvIncomeVariant(selectedVariant)} /> ); }Also applies to: 42-46, 51-55, 60-64
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/index.tsx` around lines 33 - 37, The component is duplicating the default variant literal ('lined') when calling the variant coercion helpers, which conflicts with the central variants registry defaults; remove the local fallback and pass the raw variant through so the registry-provided default is used. Concretely, update the branches that render PrintCashDocument, PrintCardDocument, PrintChequeDocument, PrintTransferDocument (identify by transaction.journal checks and components PrintCashDocument/PrintCardDocument/PrintChequeDocument/PrintTransferDocument) to call asCashVariant/asCardVariant/asChequeVariant/asTransferVariant with the original variant value (variant) instead of using variant || 'lined' (and analogous literal fallbacks). Ensure no other local hardcoded default literals remain in those render calls so the variants registry flow controls the default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invIncome.tsx`:
- Around line 81-83: The header fields in invIncome.tsx are using static
placeholders instead of the transaction data; update the JSX that renders
"Огноо", "Хэнээс(хаанаас)" and "Утга" to bind to the transaction object (e.g.,
use transaction.date formatted for the date, transaction.party or
transaction.from for the sender, and transaction.description or transaction.note
for the text) with safe fallbacks to the existing placeholder text; apply the
same change to the other occurrences referenced (the blocks around the other
line ranges) so each header displays transaction.date, transaction.from/party
and transaction.description rather than hardcoded placeholders.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invMove.tsx`:
- Line 31: The toAccount field in invMove.tsx is being hardcoded to an empty
string (toAccount: '') so the “Дансанд” column never displays; update the places
that set toAccount (the initial object at the top and the similar block around
lines 127-129) to populate it from the actual source on the movement/line item
(e.g., use the movement/toAccount or item.toAccount property used elsewhere in
this component/context) instead of ''. Locate the object literal(s) where
toAccount is defined and replace the empty string with the appropriate value
from the incoming props/state (for example props.movement.toAccount or
currentRow.toAccount) so the UI can render the real destination account.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invoice.tsx`:
- Around line 124-126: The invoice view currently renders empty placeholders for
phone and email; update the JSX that renders the "Утас, факс:" and "Э-шуудан:"
lines to output the computed values (use the existing variables
counterpartyPhone and counterpartyEmail) instead of the hardcoded empty
strings—e.g., replace the empty-string expressions with the variables (falling
back to an empty string if null/undefined) so printed invoices include contact
details.
- Around line 83-89: The counterpartyName expression currently returns
transaction.customer?.firstName before attempting the full name, so lastName is
never used; update the logic around the counterpartyName variable to construct
and prefer the full name (e.g., join firstName and lastName when either exists)
then fall back to transaction.customer?.firstName, then
transaction.customer?.code, then ''. Locate and modify the counterpartyName
assignment in invoice.tsx to build an array [transaction.customer?.firstName,
transaction.customer?.lastName].filter(Boolean).join(' ') first (if non-empty),
otherwise use the single-field fallbacks.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invOut.tsx`:
- Line 71: Replace the misspelled form code in the FormHeader call inside
invOut.tsx (the line using FormHeader code="НМХмаяг МХ1") with the correct form
code string; update the code prop on FormHeader (component FormHeader) to the
proper text "НМХ маяг МХ1" so the printed header matches the official document
formatting.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSale.tsx`:
- Around line 92-94: The rendered UI shows only static labels instead of the
computed values; update the JSX to output the computed variables partyName and
description (e.g., replace the static label divs "Хэнд(хаана):" and "Утга:" with
divs that render {partyName} and {description}) and do the same for the other
occurrences where the same labels are printed (the other label blocks in this
component). Ensure you safely render them (e.g., {partyName || ''}, {description
|| ''}) so missing metadata doesn't break rendering.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/numberToWords.ts`:
- Around line 120-122: The current split computes tugrug and mongo from
safeAmount separately and can yield mongo === 100 for values like 1.999; change
the logic to round total cents first (e.g., totalCents = Math.round(safeAmount *
100)) and then derive tugrug = Math.floor(totalCents / 100) and mongo =
totalCents % 100 so mongo is always 0..99; update the variables in
numberToWords.ts accordingly where tugrug, mongo and safeAmount are used.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/TransactionDocument.tsx`:
- Around line 173-183: For inventory journals the totalAmount currently sums all
transaction.details but ProductTable only renders product rows, causing
mismatch; update the isInventory branch that computes totalAmount (and the
similar logic at the other occurrence) to first filter details to the same
subset ProductTable renders (e.g., details.filter(d => d.productId || d.type ===
'product' or whatever predicate ProductTable uses such as presence of
count/unitPrice) and then reduce over that filtered array (summing d.amount ??
(d.count ?? 0) * (d.unitPrice ?? 0)) so the printed total and amount-in-words
match the visible ProductTable rows.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/variants.ts`:
- Around line 57-67: The helpers asCashVariant, asInvMoveVariant,
asInvSaleVariant and asInvIncomeVariant currently cast any string to the variant
types unsafely; update each to validate the input string against
DOCUMENT_VARIANTS (or a per-journal allowlist) before narrowing and return the
journal default when validation fails; specifically, replace the direct
assertions with a check like "if DOCUMENT_VARIANTS.includes(variant) (or
allowlist.has(variant)) return variant as <Type> else return journalDefault" so
only allowed values are narrowed and invalid strings fall back to the default.
---
Outside diff comments:
In `@frontend/plugins/accounting_ui/src/pages/TransactionPrintPage.tsx`:
- Around line 14-15: The page always calls window.print() regardless of the
inPreview flag; update the auto-print logic in TransactionPrintPage to read the
location query (useLocation()/URLSearchParams) and skip calling window.print()
when query.get('inPreview') (or 'inPreview') is truthy/equals 'true' so the
preview flow from TransactionTabs is honored; locate the code that derives
transactionId from query and the place where window.print() is invoked (and the
similar block at lines ~37-45) and wrap/guard those print calls with a check
that inPreview !== 'true' before triggering window.print().
---
Nitpick comments:
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/index.tsx`:
- Around line 16-23: Replace the inline prop type for the PrintBody component
with a named interface: create an interface IPrintBodyProps that declares
transaction: ITransaction and optional variant?: string (add a brief comment if
desired referencing DOCUMENT_VARIANTS), then update the PrintBody signature to
accept props: IPrintBodyProps; ensure any existing usages or imports still
compile and export the new interface if it should be reused elsewhere.
- Around line 33-37: The component is duplicating the default variant literal
('lined') when calling the variant coercion helpers, which conflicts with the
central variants registry defaults; remove the local fallback and pass the raw
variant through so the registry-provided default is used. Concretely, update the
branches that render PrintCashDocument, PrintCardDocument, PrintChequeDocument,
PrintTransferDocument (identify by transaction.journal checks and components
PrintCashDocument/PrintCardDocument/PrintChequeDocument/PrintTransferDocument)
to call asCashVariant/asCardVariant/asChequeVariant/asTransferVariant with the
original variant value (variant) instead of using variant || 'lined' (and
analogous literal fallbacks). Ensure no other local hardcoded default literals
remain in those render calls so the variants registry flow controls the default.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/numberToWords.ts`:
- Around line 62-63: Replace pure helper arrow-function constants in this module
with function declarations: change the const belowThousand = (num: number, attr:
boolean): string => { ... } to a function belowThousand(num: number, attr:
boolean): string { ... } and do the same for the other pure helper constants in
this file (the two remaining arrow-function helpers declared later). Ensure
signatures, exported names (if any), and internal behavior remain identical
while switching to the function keyword to follow the TS guideline.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/TransactionDocument.tsx`:
- Line 5: Update the import and prop typing to follow project TS conventions:
change the relative import "import { amountToMongolianText } from
'./numberToWords';" to the absolute/path-alias variant (e.g., import from
"`@/modules/transactions/transaction-form/components/documents/numberToWords`" or
the project's ~/@ alias as appropriate) and extract any inline prop types used
by the TransactionDocument component into a named interface with an I prefix
(for example declare interface ITransactionDocumentProps { /* fields previously
inline */ } and use it on the TransactionDocument function/component). Also
create named interface(s) for any helper argument shapes returned/consumed by
amountToMongolianText (e.g., IAmountToMongolianArgs) and update references to
use those interfaces; then replace inline object types in the component and
helper signatures with these I-prefixed interfaces.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/variants.ts`:
- Around line 3-6: The imports for CashVariant, InvIncomeVariant, InvMoveVariant
and InvSaleVariant use relative paths; change them to the project's absolute
alias style (e.g., use `@/`* for modules) so they follow the coding guideline
"Always use absolute paths when importing." Update the four import statements to
use the module alias (for example replacing './cash' etc. with the appropriate
`@/`... alias for this module) ensuring each symbol (CashVariant,
InvIncomeVariant, InvMoveVariant, InvSaleVariant) still resolves correctly and
the project build/tsconfig path mappings remain satisfied.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 879705fb-da44-4417-84e5-9ab6a5dd22af
📒 Files selected for processing (16)
frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/TransactionTabs.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/TransactionDocument.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/bank.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/cash.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/index.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invIncome.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invMove.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invOut.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSale.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSaleReturn.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invoice.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/numberToWords.tsfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/variants.tsfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/contants/printDocuments.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/graphql/queries/accTransactionDetail.tsfrontend/plugins/accounting_ui/src/pages/TransactionPrintPage.tsx
| from: d.branch?.title || '', | ||
| fromAccount: d.account?.code || '', | ||
| to: d.department?.title || '', | ||
| toAccount: '', |
There was a problem hiding this comment.
Populate destination account instead of hardcoding blank values.
toAccount is always '', so the “Дансанд” column can never show data.
Proposed fix
- toAccount: '',
+ toAccount:
+ d?.extra?.toAccount ||
+ transaction?.extraData?.toAccount ||
+ '',Also applies to: 127-129
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invMove.tsx`
at line 31, The toAccount field in invMove.tsx is being hardcoded to an empty
string (toAccount: '') so the “Дансанд” column never displays; update the places
that set toAccount (the initial object at the top and the similar block around
lines 127-129) to populate it from the actual source on the movement/line item
(e.g., use the movement/toAccount or item.toAccount property used elsewhere in
this component/context) instead of ''. Locate the object literal(s) where
toAccount is defined and replace the empty string with the appropriate value
from the incoming props/state (for example props.movement.toAccount or
currentRow.toAccount) so the UI can render the real destination account.
| const counterpartyName = | ||
| transaction.customer?.firstName || | ||
| [transaction.customer?.firstName, transaction.customer?.lastName] | ||
| .filter(Boolean) | ||
| .join(' ') || | ||
| transaction.customer?.code || | ||
| ''; |
There was a problem hiding this comment.
Fix counterparty full-name fallback order.
Current logic returns firstName before attempting full name, so lastName is never included when firstName exists.
Proposed fix
- const counterpartyName =
- transaction.customer?.firstName ||
- [transaction.customer?.firstName, transaction.customer?.lastName]
- .filter(Boolean)
- .join(' ') ||
- transaction.customer?.code ||
- '';
+ const fullName = [transaction.customer?.firstName, transaction.customer?.lastName]
+ .filter(Boolean)
+ .join(' ')
+ const counterpartyName = fullName || transaction.customer?.code || ''🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invoice.tsx`
around lines 83 - 89, The counterpartyName expression currently returns
transaction.customer?.firstName before attempting the full name, so lastName is
never used; update the logic around the counterpartyName variable to construct
and prefer the full name (e.g., join firstName and lastName when either exists)
then fall back to transaction.customer?.firstName, then
transaction.customer?.code, then ''. Locate and modify the counterpartyName
assignment in invoice.tsx to build an array [transaction.customer?.firstName,
transaction.customer?.lastName].filter(Boolean).join(' ') first (if non-empty),
otherwise use the single-field fallbacks.
|
|
||
| return ( | ||
| <div className="flex-1"> | ||
| <FormHeader code="НМХмаяг МХ1" /> |
There was a problem hiding this comment.
Correct form code typo in header label.
The printed form code text has a typo (НМХмаяг МХ1), which can invalidate/undermine official document formatting.
Proposed fix
- <FormHeader code="НМХмаяг МХ1" />
+ <FormHeader code="НХМаягт МХ1" />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <FormHeader code="НМХмаяг МХ1" /> | |
| <FormHeader code="НХМаягт МХ1" /> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invOut.tsx`
at line 71, Replace the misspelled form code in the FormHeader call inside
invOut.tsx (the line using FormHeader code="НМХмаяг МХ1") with the correct form
code string; update the code prop on FormHeader (component FormHeader) to the
proper text "НМХ маяг МХ1" so the printed header matches the official document
formatting.
| const tugrug = Math.floor(safeAmount); | ||
| const mongo = Math.round((safeAmount - tugrug) * 100); | ||
|
|
There was a problem hiding this comment.
Handle cent overflow after rounding (mongo === 100)
The current split can output invalid text like “100 мөнгө” for values such as 1.999. Round total cents first, then derive integer/fractional parts from that rounded value.
💡 Proposed fix
- const tugrug = Math.floor(safeAmount);
- const mongo = Math.round((safeAmount - tugrug) * 100);
+ const totalMongo = Math.round(safeAmount * 100)
+ const tugrug = Math.floor(totalMongo / 100)
+ const mongo = totalMongo % 100🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/numberToWords.ts`
around lines 120 - 122, The current split computes tugrug and mongo from
safeAmount separately and can yield mongo === 100 for values like 1.999; change
the logic to round total cents first (e.g., totalCents = Math.round(safeAmount *
100)) and then derive tugrug = Math.floor(totalCents / 100) and mongo =
totalCents % 100 so mongo is always 0..99; update the variables in
numberToWords.ts accordingly where tugrug, mongo and safeAmount are used.
| const details = transaction?.details || []; | ||
| const isInventory = INVENTORY_JOURNALS.has(journal); | ||
|
|
||
| const title = DOCUMENT_TITLES[journal] || 'Гүйлгээний баримт'; | ||
|
|
||
| const totalAmount = isInventory | ||
| ? details.reduce( | ||
| (sum, d) => sum + (d.amount ?? (d.count ?? 0) * (d.unitPrice ?? 0)), | ||
| 0, | ||
| ) | ||
| : details.reduce((sum, d) => sum + (d.currencyAmount ?? d.amount ?? 0), 0); |
There was a problem hiding this comment.
Keep inventory total source consistent with rendered rows
For inventory journals, totalAmount is computed from all details, but ProductTable renders only product rows. This can print a total/amount-in-words that doesn’t match the visible table.
Suggested fix
- const details = transaction?.details || [];
+ const details = transaction?.details || [];
+ const inventoryRows = details.filter((d) => d.productId || d.product);
const totalAmount = isInventory
- ? details.reduce(
+ ? inventoryRows.reduce(
(sum, d) => sum + (d.amount ?? (d.count ?? 0) * (d.unitPrice ?? 0)),
0,
)
: details.reduce((sum, d) => sum + (d.currencyAmount ?? d.amount ?? 0), 0);
- {isInventory ? (
- <ProductTable details={details} />
+ {isInventory ? (
+ <ProductTable details={inventoryRows} />
) : (
<AccountTable details={details} />
)}Also applies to: 225-227
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/TransactionDocument.tsx`
around lines 173 - 183, For inventory journals the totalAmount currently sums
all transaction.details but ProductTable only renders product rows, causing
mismatch; update the isInventory branch that computes totalAmount (and the
similar logic at the other occurrence) to first filter details to the same
subset ProductTable renders (e.g., details.filter(d => d.productId || d.type ===
'product' or whatever predicate ProductTable uses such as presence of
count/unitPrice) and then reduce over that filtered array (summing d.amount ??
(d.count ?? 0) * (d.unitPrice ?? 0)) so the printed total and amount-in-words
match the visible ProductTable rows.
| export const asCashVariant = (variant: string): CashVariant => | ||
| variant as CashVariant; | ||
|
|
||
| export const asInvMoveVariant = (variant: string): InvMoveVariant => | ||
| variant as InvMoveVariant; | ||
|
|
||
| export const asInvSaleVariant = (variant: string): InvSaleVariant => | ||
| variant as InvSaleVariant; | ||
|
|
||
| export const asInvIncomeVariant = (variant: string): InvIncomeVariant => | ||
| variant as InvIncomeVariant; |
There was a problem hiding this comment.
Avoid unchecked string-to-variant assertions
These helpers erase runtime safety: any arbitrary string is accepted as a valid variant. Validate against DOCUMENT_VARIANTS (or a per-journal allowlist) before narrowing, and fallback to the journal default when invalid.
Suggested fix
+const hasVariant = (journal: TrJournalEnum, variant: string) =>
+ (DOCUMENT_VARIANTS[journal] ?? []).some((item) => item.value === variant)
export const asCashVariant = (variant: string): CashVariant =>
- variant as CashVariant;
+ (hasVariant(TrJournalEnum.CASH, variant)
+ ? variant
+ : getDefaultVariant(TrJournalEnum.CASH)) as CashVariant;
export const asInvMoveVariant = (variant: string): InvMoveVariant =>
- variant as InvMoveVariant;
+ (hasVariant(TrJournalEnum.INV_MOVE, variant)
+ ? variant
+ : getDefaultVariant(TrJournalEnum.INV_MOVE)) as InvMoveVariant;
export const asInvSaleVariant = (variant: string): InvSaleVariant =>
- variant as InvSaleVariant;
+ (hasVariant(TrJournalEnum.INV_SALE, variant)
+ ? variant
+ : getDefaultVariant(TrJournalEnum.INV_SALE)) as InvSaleVariant;
export const asInvIncomeVariant = (variant: string): InvIncomeVariant =>
- variant as InvIncomeVariant;
+ (hasVariant(TrJournalEnum.INV_INCOME, variant)
+ ? variant
+ : getDefaultVariant(TrJournalEnum.INV_INCOME)) as InvIncomeVariant;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/variants.ts`
around lines 57 - 67, The helpers asCashVariant, asInvMoveVariant,
asInvSaleVariant and asInvIncomeVariant currently cast any string to the variant
types unsafely; update each to validate the input string against
DOCUMENT_VARIANTS (or a per-journal allowlist) before narrowing and return the
journal default when validation fails; specifically, replace the direct
assertions with a check like "if DOCUMENT_VARIANTS.includes(variant) (or
allowlist.has(variant)) return variant as <Type> else return journalDefault" so
only allowed values are narrowed and invalid strings fall back to the default.
🌗 Pull Request OverviewThis PR implements a comprehensive document printing system for accounting transactions, adding support for multiple document types (cash, bank, inventory, invoices) with various layout variants. It includes a new number-to-words utility for Mongolian currency formatting and introduces a variant selection system for different document layouts. Reviewed Changes Show a summary per file
📋 Review Findings📄
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invIncome.tsx (1)
12-12: ⚡ Quick winUse absolute import path for shared document helpers.
Please replace the relative import with the project alias path for consistency with frontend import conventions.
As per coding guidelines, "Always use absolute paths when importing".Proposed fix
-} from './shared'; +} from '~/modules/transactions/transaction-form/components/documents/shared';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invIncome.tsx` at line 12, In invIncome.tsx replace the relative import string './shared' with the project's absolute alias import used elsewhere in the frontend (i.e. use the same alias pattern as other imports in this module) so the shared document helpers are imported via the project alias instead of a relative path; update the import statement that currently reads "from './shared'" to use the absolute alias equivalent to keep import conventions consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invIncome.tsx`:
- Around line 160-162: Clamp the discount so totals never go negative: instead
of computing grandTotal as "grandTotal = total - discount" use a clampedDiscount
= Math.min(Math.max(discount, 0), total) and then compute grandTotal = total -
clampedDiscount (or simply grandTotal = Math.max(0, total - discount)); apply
this change where totals are rendered (references: variables discount, total,
grandTotal in invIncome.tsx and the analogous totals block later in the file) so
the displayed payable amount can never be negative.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSale.tsx`:
- Around line 178-180: The payable calculation can become negative when discount
> subtotal; update the logic in the invSale component so payable is clamped to a
minimum of zero (e.g., compute payable as the non-negative result of total minus
discount or clamp discount to at most total) instead of letting total - discount
produce negatives; apply the same change to the duplicate logic block around the
other occurrence (the second payable/discount calculation at the later section)
so both uses of discount, payable, total are protected against negative payable
values.
---
Nitpick comments:
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invIncome.tsx`:
- Line 12: In invIncome.tsx replace the relative import string './shared' with
the project's absolute alias import used elsewhere in the frontend (i.e. use the
same alias pattern as other imports in this module) so the shared document
helpers are imported via the project alias instead of a relative path; update
the import statement that currently reads "from './shared'" to use the absolute
alias equivalent to keep import conventions consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ca5c7aa1-b279-4530-a58a-c84e48ec4eaf
📒 Files selected for processing (6)
frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invIncome.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invMove.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invOut.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSale.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSaleReturn.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/shared.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invMove.tsx
| const discount = transaction?.extraData?.discount ?? 0; | ||
| const grandTotal = total - discount; | ||
| const filled = padRows(rows, 4); |
There was a problem hiding this comment.
Guard against negative grand total when discount exceeds subtotal.
grandTotal = total - discount can go negative and print an invalid payable amount. Clamp the applied discount before rendering totals.
Proposed fix
- const discount = transaction?.extraData?.discount ?? 0;
- const grandTotal = total - discount;
+ const rawDiscount = Number(transaction?.extraData?.discount ?? 0);
+ const discount = Math.max(0, Number.isFinite(rawDiscount) ? rawDiscount : 0);
+ const appliedDiscount = Math.min(discount, total);
+ const grandTotal = total - appliedDiscount;Also applies to: 233-241
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invIncome.tsx`
around lines 160 - 162, Clamp the discount so totals never go negative: instead
of computing grandTotal as "grandTotal = total - discount" use a clampedDiscount
= Math.min(Math.max(discount, 0), total) and then compute grandTotal = total -
clampedDiscount (or simply grandTotal = Math.max(0, total - discount)); apply
this change where totals are rendered (references: variables discount, total,
grandTotal in invIncome.tsx and the analogous totals block later in the file) so
the displayed payable amount can never be negative.
| const discount = transaction?.extraData?.discount ?? 0; | ||
| const payable = total - discount; | ||
| const filled = padRows(rows, 5); |
There was a problem hiding this comment.
Prevent negative payable when discount is greater than subtotal.
payable = total - discount can render an invalid negative amount on the document.
Proposed fix
- const discount = transaction?.extraData?.discount ?? 0;
- const payable = total - discount;
+ const rawDiscount = Number(transaction?.extraData?.discount ?? 0);
+ const discount = Math.max(0, Number.isFinite(rawDiscount) ? rawDiscount : 0);
+ const appliedDiscount = Math.min(discount, total);
+ const payable = total - appliedDiscount;Also applies to: 253-261
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSale.tsx`
around lines 178 - 180, The payable calculation can become negative when
discount > subtotal; update the logic in the invSale component so payable is
clamped to a minimum of zero (e.g., compute payable as the non-negative result
of total minus discount or clamp discount to at most total) instead of letting
total - discount produce negatives; apply the same change to the duplicate logic
block around the other occurrence (the second payable/discount calculation at
the later section) so both uses of discount, payable, total are protected
against negative payable values.
🌗 Pull Request OverviewThis PR implements document printing functionality for accounting transactions with multiple layout variants. It adds new document components for various journal types (cash, bank, inventory income/out/move/sale/return, invoices), shared UI components, a number-to-Mongolian-words utility, and a print toolbar with variant selection. Reviewed Changes Show a summary per file
📋 Review Findings📄
|
There was a problem hiding this comment.
♻️ Duplicate comments (5)
frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/shared.tsx (1)
197-220:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBind shared receipt header fields to transaction metadata instead of placeholders.
Both reusable receipt components ignore available
date,partyName, anddescription, so printed documents lose real transaction context.Proposed fix
export const TwinReceipt = ({ transaction, labels, minRows, }: { transaction: ITransaction; labels: IReceiptLabels; minRows: number; }) => { + const { date, partyName, description } = getMeta(transaction) const rows = buildRows(transaction); const filled = padRows(rows, minRows); return ( <div className="flex-1"> <FormHeader code={labels.formCode} /> @@ - <div className="text-[11px] font-bold">Огноо: 20.../.../...</div> - <div className="mt-1 text-[11px] font-bold">{labels.partyLabel}</div> - <div className="mt-1 mb-2 text-[11px] font-bold">Утга:</div> + <div className="text-[11px] font-bold"> + Огноо: {date || '20.../.../...'} + </div> + <div className="mt-1 text-[11px] font-bold"> + {labels.partyLabel} {partyName || ''} + </div> + <div className="mt-1 mb-2 text-[11px] font-bold"> + Утга: {description || ''} + </div> @@ export const SimpleReceipt = ({ transaction, labels, minRows, }: { transaction: ITransaction; labels: IReceiptLabels; minRows: number; }) => { + const { date, partyName, description } = getMeta(transaction) const rows = buildRows(transaction); const filled = padRows(rows, minRows); return ( @@ - <div className="font-bold">Огноо: 20.../.../...</div> - <div className="mt-1 font-bold">{labels.partyLabel}</div> - <div className="mt-1 mb-2 font-bold">Утга:</div> + <div className="font-bold">Огноо: {date || '20.../.../...'}</div> + <div className="mt-1 font-bold"> + {labels.partyLabel} {partyName || ''} + </div> + <div className="mt-1 mb-2 font-bold">Утга: {description || ''}</div>Also applies to: 234-257
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/shared.tsx` around lines 197 - 220, The TwinReceipt component is rendering placeholder header values (e.g. "Огноо: 20.../.../...", empty Утга) instead of using transaction metadata; update TwinReceipt to bind its date, party name and description fields to the transaction object (use transaction.date, transaction.partyName and transaction.description or the correct ITransaction property names) where currently labels or static placeholders are rendered (references: TwinReceipt, buildRows, padRows, labels.formCode, labels.orgLabel, labels.partyLabel); make the same change in the sibling receipt component around the 234-257 region so both reusable receipt components show real transaction data.frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invIncome.tsx (2)
48-49:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClamp and sanitize discount before computing
grandTotal.Current logic can print negative totals (and
NaNfor bad discount payloads).Proposed fix
- const discount = transaction?.extraData?.discount ?? 0; - const grandTotal = total - discount; + const rawDiscount = Number(transaction?.extraData?.discount ?? 0) + const discount = Number.isFinite(rawDiscount) ? Math.max(0, rawDiscount) : 0 + const appliedDiscount = Math.min(discount, total) + const grandTotal = total - appliedDiscount @@ - <td className={`${TD} text-right`}>{formatNumber(discount)}</td> + <td className={`${TD} text-right`}> + {formatNumber(appliedDiscount)} + </td>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invIncome.tsx` around lines 48 - 49, The discount pulled from transaction?.extraData?.discount is not sanitized and can be NaN or negative, producing negative or invalid grandTotal; parse and coerce discount to a finite number (e.g. Number(...) or parseFloat), fall back to 0 when not finite, then clamp it between 0 and total (use Math.max(0, Math.min(total, discount))) before computing grandTotal = total - discount; update the logic around the discount variable and the grandTotal calculation (references: discount, transaction?.extraData?.discount, grandTotal, total) so grandTotal cannot be NaN or negative.
61-63:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRender receipt metadata fields from
transactioninstead of fixed placeholders.These variants currently drop real date/party/description values on print.
Proposed fix
- const { documentNo } = getMeta(transaction); + const { documentNo, date, partyName, description } = getMeta(transaction); @@ - <div className="font-bold">Огноо: 20.../.../...</div> - <div className="mt-1 font-bold">Хэнээс(хаанаас):</div> - <div className="mt-1 mb-2 font-bold">Утга:</div> + <div className="font-bold">Огноо: {date || '20.../.../...'}</div> + <div className="mt-1 font-bold"> + Хэнээс(хаанаас): {partyName || ''} + </div> + <div className="mt-1 mb-2 font-bold">Утга: {description || ''}</div>- const { documentNo } = getMeta(transaction); + const { documentNo, date, partyName, description } = getMeta(transaction); @@ - <div className="font-bold">20... он ... сар ... өдөр</div> - <div className="mt-1 font-bold">Бэлтгэн нийлүүлэгчийн нэр:</div> - <div className="mt-1 mb-2 font-bold">Утга:</div> + <div className="font-bold">{date || '20... он ... сар ... өдөр'}</div> + <div className="mt-1 font-bold"> + Бэлтгэн нийлүүлэгчийн нэр: {partyName || ''} + </div> + <div className="mt-1 mb-2 font-bold">Утга: {description || ''}</div>Also applies to: 159-161
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invIncome.tsx` around lines 61 - 63, The JSX in invIncome.tsx currently renders hard-coded placeholders for date/party/description; replace those with the actual transaction fields (e.g., transaction.date formatted for display, transaction.party or transaction.from / transaction.fromName, and transaction.description or transaction.note) instead of the static strings. Update the render lines inside the invIncome component to output the formatted transaction.date (use an existing formatDate helper or new Date(transaction.date).toLocaleDateString()), transaction.party/name, and transaction.description with optional chaining and sensible fallbacks (e.g., transaction?.date ?? '', transaction?.party ?? '', transaction?.description ?? ''). Do the same replacement for the duplicate block noted at lines 159-161. Ensure you reference the component's transaction prop and keep null-safe access.frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSale.tsx (2)
118-120:⚠️ Potential issue | 🟠 Major | ⚡ Quick winProtect payable from going negative and sanitize discount input.
total - discountcan render invalid negative payable values.Proposed fix
- const discount = transaction?.extraData?.discount ?? 0; - const payable = total - discount; + const rawDiscount = Number(transaction?.extraData?.discount ?? 0) + const discount = Number.isFinite(rawDiscount) ? Math.max(0, rawDiscount) : 0 + const appliedDiscount = Math.min(discount, total) + const payable = total - appliedDiscount @@ - <td className={`${TD} text-right`}>{formatNumber(discount)}</td> + <td className={`${TD} text-right`}> + {formatNumber(appliedDiscount)} + </td>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSale.tsx` around lines 118 - 120, The payable calculation can go negative because discount is taken directly from transaction?.extraData?.discount; sanitize discount by coercing it to a finite non-negative number and cap it at total before computing payable (e.g., derive a safeDiscount from transaction?.extraData?.discount that uses Number()/parseFloat, defaults to 0 for invalid values, clamps to >=0 and <= total), then compute payable = total - safeDiscount and ensure payable is at least 0; update the variables near discount, payable, total and keep padRows(rows, 5) unchanged.
50-51:⚠️ Potential issue | 🟠 Major | ⚡ Quick winShow party/description fields from transaction metadata in all sale variants.
These blocks currently print only labels, which leaves documents incomplete.
Proposed fix
- const { date } = getMeta(transaction); + const { date, partyName, description } = getMeta(transaction); @@ - <div className="mt-1 font-bold">Хэнд(хаана):</div> - <div className="mt-1 mb-2 font-bold">Утга:</div> + <div className="mt-1 font-bold">Хэнд(хаана): {partyName || ''}</div> + <div className="mt-1 mb-2 font-bold">Утга: {description || ''}</div>- const { date } = getMeta(transaction); + const { date, partyName, description } = getMeta(transaction); @@ - <div className="mt-1 font-bold">Хэнд(хаана):</div> - <div className="mt-1 mb-2 font-bold">Утга:</div> + <div className="mt-1 font-bold">Хэнд(хаана): {partyName || ''}</div> + <div className="mt-1 mb-2 font-bold">Утга: {description || ''}</div>- const { documentNo, date } = getMeta(transaction); + const { documentNo, date, partyName, description } = getMeta(transaction); @@ - <div className="mt-1 font-bold">(Худалдан авагчийн нэр):</div> - <div className="mt-1 mb-2 font-bold">Утга:</div> + <div className="mt-1 font-bold"> + (Худалдан авагчийн нэр): {partyName || ''} + </div> + <div className="mt-1 mb-2 font-bold">Утга: {description || ''}</div>Also applies to: 132-133, 232-233
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSale.tsx` around lines 50 - 51, The JSX in invSale.tsx currently renders only labels for "Хэнд(хаана):" and "Утга:" (and the analogous blocks at the other two locations) but never outputs the corresponding values from the transaction metadata; update the component to read and render the metadata fields (e.g., transaction.metadata.party and transaction.metadata.description or the equivalent keys used in your transaction object) alongside the labels in all sale variants (the blocks around the existing labels and the similar blocks at the other two occurrences) so the party and description from the transaction are displayed when present, falling back to an empty string or a localized "N/A" when missing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invIncome.tsx`:
- Around line 48-49: The discount pulled from transaction?.extraData?.discount
is not sanitized and can be NaN or negative, producing negative or invalid
grandTotal; parse and coerce discount to a finite number (e.g. Number(...) or
parseFloat), fall back to 0 when not finite, then clamp it between 0 and total
(use Math.max(0, Math.min(total, discount))) before computing grandTotal = total
- discount; update the logic around the discount variable and the grandTotal
calculation (references: discount, transaction?.extraData?.discount, grandTotal,
total) so grandTotal cannot be NaN or negative.
- Around line 61-63: The JSX in invIncome.tsx currently renders hard-coded
placeholders for date/party/description; replace those with the actual
transaction fields (e.g., transaction.date formatted for display,
transaction.party or transaction.from / transaction.fromName, and
transaction.description or transaction.note) instead of the static strings.
Update the render lines inside the invIncome component to output the formatted
transaction.date (use an existing formatDate helper or new
Date(transaction.date).toLocaleDateString()), transaction.party/name, and
transaction.description with optional chaining and sensible fallbacks (e.g.,
transaction?.date ?? '', transaction?.party ?? '', transaction?.description ??
''). Do the same replacement for the duplicate block noted at lines 159-161.
Ensure you reference the component's transaction prop and keep null-safe access.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSale.tsx`:
- Around line 118-120: The payable calculation can go negative because discount
is taken directly from transaction?.extraData?.discount; sanitize discount by
coercing it to a finite non-negative number and cap it at total before computing
payable (e.g., derive a safeDiscount from transaction?.extraData?.discount that
uses Number()/parseFloat, defaults to 0 for invalid values, clamps to >=0 and <=
total), then compute payable = total - safeDiscount and ensure payable is at
least 0; update the variables near discount, payable, total and keep
padRows(rows, 5) unchanged.
- Around line 50-51: The JSX in invSale.tsx currently renders only labels for
"Хэнд(хаана):" and "Утга:" (and the analogous blocks at the other two locations)
but never outputs the corresponding values from the transaction metadata; update
the component to read and render the metadata fields (e.g.,
transaction.metadata.party and transaction.metadata.description or the
equivalent keys used in your transaction object) alongside the labels in all
sale variants (the blocks around the existing labels and the similar blocks at
the other two occurrences) so the party and description from the transaction are
displayed when present, falling back to an empty string or a localized "N/A"
when missing.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/shared.tsx`:
- Around line 197-220: The TwinReceipt component is rendering placeholder header
values (e.g. "Огноо: 20.../.../...", empty Утга) instead of using transaction
metadata; update TwinReceipt to bind its date, party name and description fields
to the transaction object (use transaction.date, transaction.partyName and
transaction.description or the correct ITransaction property names) where
currently labels or static placeholders are rendered (references: TwinReceipt,
buildRows, padRows, labels.formCode, labels.orgLabel, labels.partyLabel); make
the same change in the sibling receipt component around the 234-257 region so
both reusable receipt components show real transaction data.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 245cc001-4f8b-4c65-a14c-d388428e247f
📒 Files selected for processing (5)
frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invIncome.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSale.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSaleReturn.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invoice.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/shared.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSaleReturn.tsx
- frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invoice.tsx
🌗 Pull Request OverviewThis PR implements document printing functionality for accounting transactions, adding multiple print layout variants for different journal types (cash, bank, inventory, sales, etc.). It introduces reusable document components, Mongolian number-to-words conversion, and a print preview interface with layout selection. Reviewed Changes Show a summary per file
📋 Review Findings📄
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invIncome.tsx (1)
60-61:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBind numbered receipt header fields to transaction metadata.
This variant still prints static header placeholders even though
getMetaalready provides date/party/description for the transaction.💡 Proposed fix
- const { documentNo } = getMeta(transaction); + const { documentNo, date, partyName, description } = getMeta(transaction); @@ - <div className="font-bold">20... он ... сар ... өдөр</div> - <div className="mt-1 font-bold">Бэлтгэн нийлүүлэгчийн нэр:</div> - <div className="mt-1 mb-2 font-bold">Утга:</div> + <div className="font-bold"> + {date || '20... он ... сар ... өдөр'} + </div> + <div className="mt-1 font-bold"> + Бэлтгэн нийлүүлэгчийн нэр: {partyName || ''} + </div> + <div className="mt-1 mb-2 font-bold"> + Утга: {description || ''} + </div>Also applies to: 75-77
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invIncome.tsx` around lines 60 - 61, The NumberedReceipt component is rendering static header placeholders instead of using transaction metadata from getMeta; update NumberedReceipt to extract and bind all relevant metadata (documentNo, date, party, description returned by getMeta(transaction)) into the header JSX elements instead of hard-coded text, and do the same replacement for the other receipt/header variant in the same file (the similar block around lines 75-77) so both components render dynamic transaction metadata.frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/shared.tsx (2)
279-282:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRender document metadata values instead of static placeholders.
These shared receipt variants still print label-only lines for date/party/description, so printed docs lose available transaction metadata.
💡 Proposed fix
export const TwinReceipt = ({ transaction, labels, minRows, }: { transaction: ITransaction; labels: IReceiptLabels; minRows: number; }) => { + const { date, partyName, description } = getMeta(transaction) const rows = buildRows(transaction); const filled = padRows(rows, minRows); return ( @@ - <div className="text-[11px] font-bold">Огноо: 20.../.../...</div> - <div className="mt-1 text-[11px] font-bold">{labels.partyLabel}</div> - <div className="mt-1 mb-2 text-[11px] font-bold">Утга:</div> + <div className="text-[11px] font-bold"> + Огноо: {date || '20.../.../...'} + </div> + <div className="mt-1 text-[11px] font-bold"> + {labels.partyLabel} {partyName || ''} + </div> + <div className="mt-1 mb-2 text-[11px] font-bold"> + Утга: {description || ''} + </div>Also applies to: 316-319, 442-444
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/shared.tsx` around lines 279 - 282, The component currently renders static placeholders for document metadata (date, party, description) instead of real values; update the JSX in shared.tsx to output the actual transaction/document fields (e.g., use the component's props such as transaction.date or document.date, transaction.party or document.party, and transaction.description/document.description) in place of the hardcoded "Огноо: 20.../.../..." and the empty "Утга:" line, while keeping the existing labels like labels.partyLabel; make the same replacements in the other occurrences referenced (lines around 316-319 and 442-444) so printed docs show real metadata.
428-430:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize and clamp discount before computing payable.
discountis used raw, so non-numeric values can renderNaN, and discounts above subtotal can print negative payable amounts.💡 Proposed fix
- const discount = transaction?.extraData?.discount ?? 0; - const payable = total - discount; + const rawDiscount = Number(transaction?.extraData?.discount ?? 0) + const normalizedDiscount = + Number.isFinite(rawDiscount) ? Math.max(0, rawDiscount) : 0 + const discount = Math.min(normalizedDiscount, total) + const payable = total - discount;Also applies to: 508-516
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/shared.tsx` around lines 428 - 430, Normalize the discount value before computing payable: coerce transaction?.extraData?.discount to a numeric value (e.g., Number or parseFloat fallback) and clamp it between 0 and total/subtotal so payable = total - discount cannot become NaN or negative; update the calculation where discount is defined (the variable using transaction?.extraData?.discount) and any other identical locations (the second block around the same pattern) to ensure payable uses the sanitized discount, leaving padRows(rows, config.minRows) unchanged.frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSale.tsx (1)
38-39:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPopulate party/description from metadata in sale receipt headers.
Both local sale variants render label-only lines, leaving party and description blank in output despite having transaction metadata available.
💡 Proposed fix
- const { date } = getMeta(transaction); + const { date, partyName, description } = getMeta(transaction); @@ - <div className="mt-1 font-bold">Хэнд(хаана):</div> - <div className="mt-1 mb-2 font-bold">Утга:</div> + <div className="mt-1 font-bold">Хэнд(хаана): {partyName || ''}</div> + <div className="mt-1 mb-2 font-bold">Утга: {description || ''}</div>- const { documentNo, date } = getMeta(transaction); + const { documentNo, date, partyName, description } = getMeta(transaction); @@ - <div className="mt-1 font-bold">(Худалдан авагчийн нэр):</div> - <div className="mt-1 mb-2 font-bold">Утга:</div> + <div className="mt-1 font-bold"> + (Худалдан авагчийн нэр): {partyName || ''} + </div> + <div className="mt-1 mb-2 font-bold"> + Утга: {description || ''} + </div>Also applies to: 52-53, 107-108, 122-123
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSale.tsx` around lines 38 - 39, The sale receipt headers are rendered with label-only rows because the header objects created in buildRows (used where rows is assigned) don't use transaction metadata; call getMeta(transaction) to extract the metadata (e.g., party and description) and populate those fields on the header row objects for the local sale variants before returning rows. Concretely, in the code path where rows are built for sales (the buildRows function and the place where const { date } = getMeta(transaction) is used), assign header.party = meta.party (or meta.name) and header.description = meta.description (or meta.notes) so the rendered header uses those values instead of leaving them blank.
🧹 Nitpick comments (2)
frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSale.tsx (1)
19-19: ⚡ Quick winUse absolute import path for shared document helpers.
Please replace this relative import with an absolute alias import to keep import style consistent with repository standards.
As per coding guidelines, "Always use absolute paths when importing".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSale.tsx` at line 19, Replace the relative import "from './shared'" in invSale.tsx with the repository's absolute alias import for the shared document helpers (i.e., update the import that currently references './shared' to the absolute path your project uses for modules—such as the project's alias for the transactions/transaction-form/components/documents/shared module); locate the import statement in invSale.tsx that imports from './shared' and change it to the absolute alias form to match repository standards.frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invIncome.tsx (1)
19-19: ⚡ Quick winUse absolute import path for shared document helpers.
Please switch this relative import to the module alias path to match repo import conventions.
As per coding guidelines, "Always use absolute paths when importing".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invIncome.tsx` at line 19, Replace the relative import "from './shared'" in invIncome.tsx with the repository's absolute/module-alias import for the shared document helpers (i.e., use the project's alias path that points to the same shared module instead of './shared'); update the import statement that currently brings in symbols from './shared' to the corresponding absolute path so it matches the repo's "always use absolute paths" convention.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/shared.tsx`:
- Line 142: Replace truthy fallbacks that hide numeric zeroes with nullish
checks: where the JSX uses expressions like value || ' ' or row?.count ? ... : '
', change them to use the nullish coalescing operator (e.g., value ?? ' ' ) or
an explicit undefined/null test (e.g., row?.count ?? ' '). Update all similar
occurrences in this module (shared.tsx) so numerical 0 is rendered instead of
being treated as falsy.
---
Duplicate comments:
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invIncome.tsx`:
- Around line 60-61: The NumberedReceipt component is rendering static header
placeholders instead of using transaction metadata from getMeta; update
NumberedReceipt to extract and bind all relevant metadata (documentNo, date,
party, description returned by getMeta(transaction)) into the header JSX
elements instead of hard-coded text, and do the same replacement for the other
receipt/header variant in the same file (the similar block around lines 75-77)
so both components render dynamic transaction metadata.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSale.tsx`:
- Around line 38-39: The sale receipt headers are rendered with label-only rows
because the header objects created in buildRows (used where rows is assigned)
don't use transaction metadata; call getMeta(transaction) to extract the
metadata (e.g., party and description) and populate those fields on the header
row objects for the local sale variants before returning rows. Concretely, in
the code path where rows are built for sales (the buildRows function and the
place where const { date } = getMeta(transaction) is used), assign header.party
= meta.party (or meta.name) and header.description = meta.description (or
meta.notes) so the rendered header uses those values instead of leaving them
blank.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/shared.tsx`:
- Around line 279-282: The component currently renders static placeholders for
document metadata (date, party, description) instead of real values; update the
JSX in shared.tsx to output the actual transaction/document fields (e.g., use
the component's props such as transaction.date or document.date,
transaction.party or document.party, and
transaction.description/document.description) in place of the hardcoded "Огноо:
20.../.../..." and the empty "Утга:" line, while keeping the existing labels
like labels.partyLabel; make the same replacements in the other occurrences
referenced (lines around 316-319 and 442-444) so printed docs show real
metadata.
- Around line 428-430: Normalize the discount value before computing payable:
coerce transaction?.extraData?.discount to a numeric value (e.g., Number or
parseFloat fallback) and clamp it between 0 and total/subtotal so payable =
total - discount cannot become NaN or negative; update the calculation where
discount is defined (the variable using transaction?.extraData?.discount) and
any other identical locations (the second block around the same pattern) to
ensure payable uses the sanitized discount, leaving padRows(rows,
config.minRows) unchanged.
---
Nitpick comments:
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invIncome.tsx`:
- Line 19: Replace the relative import "from './shared'" in invIncome.tsx with
the repository's absolute/module-alias import for the shared document helpers
(i.e., use the project's alias path that points to the same shared module
instead of './shared'); update the import statement that currently brings in
symbols from './shared' to the corresponding absolute path so it matches the
repo's "always use absolute paths" convention.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSale.tsx`:
- Line 19: Replace the relative import "from './shared'" in invSale.tsx with the
repository's absolute alias import for the shared document helpers (i.e., update
the import that currently references './shared' to the absolute path your
project uses for modules—such as the project's alias for the
transactions/transaction-form/components/documents/shared module); locate the
import statement in invSale.tsx that imports from './shared' and change it to
the absolute alias form to match repository standards.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 906b9508-34ac-4841-94c6-efc8972dd630
📒 Files selected for processing (6)
frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/TransactionDocument.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/bank.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invIncome.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSale.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSaleReturn.tsxfrontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/shared.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/TransactionDocument.tsx
- frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/invSaleReturn.tsx
- frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/bank.tsx
| <div className="w-[42%] shrink-0 bg-black/4 px-2 py-1 font-medium"> | ||
| {label} | ||
| </div> | ||
| <div className="flex-1 px-2 py-1">{value || ' '}</div> |
There was a problem hiding this comment.
Use nullish checks so numeric zero values are printed.
Current truthy checks hide valid 0 values (value || ' ', row?.count ? ... : ' '), which can silently blank numeric fields.
💡 Proposed fix
- <div className="flex-1 px-2 py-1">{value || ' '}</div>
+ <div className="flex-1 px-2 py-1">{value ?? ' '}</div>
@@
- {row?.count ? row.count.toLocaleString() : ' '}
+ {row ? row.count.toLocaleString() : ' '}
@@
- {row?.count ? fixNum(row.count, 2).toLocaleString() : ' '}
+ {row ? fixNum(row.count, 2).toLocaleString() : ' '}
@@
- {row?.count ? row.count.toLocaleString() : ' '}
+ {row ? row.count.toLocaleString() : ' '}Also applies to: 211-212, 376-377, 484-485
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@frontend/plugins/accounting_ui/src/modules/transactions/transaction-form/components/documents/shared.tsx`
at line 142, Replace truthy fallbacks that hide numeric zeroes with nullish
checks: where the JSX uses expressions like value || ' ' or row?.count ? ... : '
', change them to use the nullish coalescing operator (e.g., value ?? ' ' ) or
an explicit undefined/null test (e.g., row?.count ?? ' '). Update all similar
occurrences in this module (shared.tsx) so numerical 0 is rendered instead of
being treated as falsy.
🌗 Pull Request OverviewThis PR adds comprehensive document printing functionality for various transaction types (cash, bank, inventory, invoices) in the accounting module. It introduces multiple print layout variants, a number-to-Mongolian-words converter, and a print preview interface with layout selection. Reviewed Changes Show a summary per file
📋 Review Findings📄
|
|



Summary by CodeRabbit
New Features
Improvements
Bug Fixes