Feature: Beleg Nummern halten interne PDF Links zu den Belegen#454
Feature: Beleg Nummern halten interne PDF Links zu den Belegen#454
Conversation
📝 WalkthroughWalkthroughAdds per-cell internal PDF link support and implements it end-to-end. The Column interface gains an optional internalPdfLinkSegments callback to emit text/targetId segments. PDFDrawer tracks pendingInternalLinks and defers creating link annotations until receipts are attached and target pages are known; drawTable collects segments and populates pending links. drawLink and pending-link materialization use addAnnot rather than direct Annots injection. reportPrinter adds a helper to produce receipt link segments and wires internalPdfLinkSegments into receipt-related columns. Tests were added to verify annotations for table-rendered receipt numbers and that drawLink appends annotations. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
common/print/reportPrinter.ts (1)
501-513: 🧹 Nitpick | 🔵 TrivialGenerate the receipt text and link segments from one source of truth.
drawTableresolves link rectangles by searching for each segment text inside the rendered cell. Here the display string andinternalPdfLinkSegmentsare built by separate traversals in two columns, so any future formatting drift will silently drop links. A small helper that returns[{ text, targetId }]once and derivesfnfrom it would keep the mapping stable.Also applies to: 561-566
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/print/reportPrinter.ts` around lines 501 - 513, Create a single helper that produces an array of { text, targetId } for a Cost using receiptMap (e.g., getReceiptSegments(cost: Cost): {text: string, targetId: string}[]), then replace the current anonymous fn and internalPdfLinkSegments logic so both use that helper: have fn map the helper result to texts and join(', ') and have internalPdfLinkSegments return the helper result (with number.toString() for text and id for targetId); apply the same change to the analogous code block around the other occurrence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@common/print/printer.spec.ts`:
- Around line 21-62: The test currently only checks a single-line cell; update
it to include a wrapped-cell case for internalPdfLinkSegments by forcing the
second column to wrap (e.g., reduce its width and/or increase the text length
via fn) and provide internalPdfLinkSegments that return segments spanning
multiple lines so the production code will iterate multiText.lines; use the same
rows/receipt objects but ensure the drawer.drawTable call uses the narrower
column (or larger font) so the cell text wraps, then keep the existing
assertions that two internal link annotations are created and their targets
match pdf.getPage(1) and pdf.getPage(2) to verify line-by-line matching.
In `@common/print/printer.ts`:
- Around line 555-570: The code currently calls column callbacks (column.fn,
column.internalPdfLinkSegments, and column.countryCodeForFlag) before checking
for undefined cells, which causes throws for missing data; change the logic in
the loop that builds cell values (using datum = data[i][column.key]) so that if
datum === undefined you set cell = EMPTY_CELL and skip all column callbacks (do
not call column.fn, column.internalPdfLinkSegments, or
column.countryCodeForFlag) and also skip countryCodeForFlag handling for empty
cells; keep the existing opts.firstRow handling but ensure the undefined guard
runs before any callback invocations (apply the same fix for the second block
around lines 611-618).
---
Outside diff comments:
In `@common/print/reportPrinter.ts`:
- Around line 501-513: Create a single helper that produces an array of { text,
targetId } for a Cost using receiptMap (e.g., getReceiptSegments(cost: Cost):
{text: string, targetId: string}[]), then replace the current anonymous fn and
internalPdfLinkSegments logic so both use that helper: have fn map the helper
result to texts and join(', ') and have internalPdfLinkSegments return the
helper result (with number.toString() for text and id for targetId); apply the
same change to the analogous code block around the other occurrence.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 50c1cf31-2cd0-48cc-9d36-b9a268a93273
📒 Files selected for processing (3)
common/print/printer.spec.tscommon/print/printer.tscommon/print/reportPrinter.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@common/print/printer.ts`:
- Around line 415-431: When iterating pendingInternalLinks, add a log when
receiptFirstPageMap[link.targetId] is undefined so missing target pages aren't
silently skipped; inside the loop where targetPage is checked, emit a warning
that includes the missing link.targetId (and optional context like link.page or
link coordinates) before continuing. Update the block around
pendingInternalLinks/receiptFirstPageMap/link.targetId to call the existing
logger (e.g., this.logger.warn or processLogger.warn) with a clear message, then
continue as before.
In `@common/print/reportPrinter.ts`:
- Around line 511-516: The code calls getReceiptNumberLinkSegments twice per
Cost (once in the column fn and once in internalPdfLinkSegments), causing
duplicate work; modify getReceiptNumberLinkSegments to memoize results per Cost
using a WeakMap (keyed by the Cost object) so repeated calls return the cached
segments, keeping the existing APIs (fn and internalPdfLinkSegments) unchanged;
store computed segment arrays in the WeakMap inside getReceiptNumberLinkSegments
and return the cached array on subsequent calls to avoid repeated string
conversions and map lookups.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f224e94c-143e-4fe6-b61e-5f7902f4939e
📒 Files selected for processing (2)
common/print/printer.tscommon/print/reportPrinter.ts
1 participant
Summary by CodeRabbit
New Features
Tests