[IP-988]: template layout redesign

[nielsdrost7]

Summary by CodeRabbit

• New Features

  • Improved invoice PDF template structure with enhanced layout and visual hierarchy for overdue and paid invoices
  • Expanded CSS utility classes for consistent styling across web and PDF interfaces

• Bug Fixes

  • Enhanced database query parameter handling for improved data integrity
  • Strengthened input sanitization for better security logging

[nielsdrost7]

@copilot this was a template layout redesign
Lint the living crap out of this PR, I had some merge conflicts.

It's just a template redesign, see if you can improve stuff, based on the new incoming source from the last 3 years (that's how old the PR was)

[Copilot]

@nielsdrost7 I've opened a new pull request, #1409, to work on those changes. Once the pull request is ready, I'll request review from you.

[nielsdrost7]

@copilot

In application/views/invoice_templates/pdf/InvoicePlane - overdue.php around
lines 148 to 187, the item name and description are rendered into the wrong
table cells (description printed under "Item" and name under "Description"); fix
by swapping the two echo calls so the Item column outputs the escaped item name
(use the existing _htmlsc($item->item_name) call) and the Description column
outputs the escaped, nl2br-formatted item description (use
nl2br(htmlsc($item->item_description))); preserve existing classes, escaping and
formatting for both cells.

In application/views/invoice_templates/pdf/InvoicePlane - paid.php around lines
78 to 84, the "Invoiced To" string is hardcoded on line 79; replace it with the
translation helper call (e.g., use _trans('bill_to') instead of ('Invoiced To'))
so the label uses the existing localization system — update the echo/print to
output _trans('bill_to') within the

element and keep surrounding markup
unchanged.

In application/views/invoice_templates/pdf/InvoicePlane - paid.php around lines
132 to 167, the item name and description are placed under the wrong headers
(description is under "item" and name is under "description"); swap the cell
outputs so the "item" column shows the item name and the "description" column
shows the item description — specifically, render the item name in the first
cell using proper escaping (e.g. _htmlsc($item->item_name)) and render the
description in the second cell using nl2br(htmlsc($item->item_description));
keep existing formatting, line breaks and escaping functions but reverse their
positions so columns match their headers.

In application/views/invoice_templates/pdf/InvoicePlane - paid.php around lines
164 to 166, the invoice row currently echoes $item->item_total (which includes
item-level tax) but the invoice summary is computed from subtotals and tax
breakdowns; change the template to output $item->item_subtotal for the row total
so each line matches the subtotal-based summary math and avoids double-counting
tax.

In application/views/invoice_templates/pdf/InvoicePlane.php around lines 99 to
104, the header text "Invoiced To" is hardcoded which bypasses localization;
replace the literal string with the translation helper (for example call
_trans('bill_to') or the existing translation key used elsewhere) and echo that
value instead, making sure to use the same escaping pattern as other outputs
(e.g., wrap with _htmlsc if needed) so the PDF header is translatable and safe.

In application/views/invoice_templates/pdf/InvoicePlane.php around lines 152 to
168, the item and description cells are inverted: the code currently prints
item_description under the "item" header and item_name under the "description"
header. Fix by swapping those two contents so the cell under the "item"
column echoes the item name (escaped, e.g., htmlsc($item->item_name)) and the
cell under the "description" column echoes the description (escaped and
formatted, e.g., nl2br(htmlsc($item->item_description))). Ensure you keep the
existing CSS classes and escaping functions when moving the values.

In application/views/invoice_templates/pdf/InvoicePlane.php around lines
184–186, the code prints $item->item_total which includes item-level taxes;
replace this with $item->item_subtotal so the row total excludes item taxes and
matches the invoice summary calculations; keep the existing format_currency()
wrapper (i.e., call format_currency($item->item_subtotal)) so formatting remains
consistent.

In application/views/invoice_templates/public/InvoicePlane_Web.php around lines
124-133, fix the typo "Inovoiced To" and make it localizable: replace the
hard-coded, misspelled string with the properly spelled "Invoiced To" and emit
it via the localization helper (use _trans(...) for the displayed label) so the
public view is translated for non-English users.

In application/views/quote_templates/public/InvoicePlane_Web.php around lines 89
to 97, there's an unmatched closing

(line 96) that must be removed; update
the template by deleting that stray closing paragraph tag so all remaining tags
are balanced and the conditional phone/mobile blocks remain unchanged.

In application/views/quote_templates/public/InvoicePlane_Web.php around lines
257-303, the quote notes are being rendered twice (once in the column block at
~257-261 and again in the table block at ~287-301); remove the duplicate output
by keeping only one rendering path — either delete the later table block that
outputs $quote->notes or change the second conditional to an else (or else-if)
so notes are rendered only once, ensuring the remaining rendering preserves the
intended HTML sanitization (nl2br/htmlsc or strip_tags) you prefer.

[Copilot]

@nielsdrost7 I've opened a new pull request, #1410, to work on those changes. Once the pull request is ready, I'll request review from you.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4e75cb2b-4901-4a7c-b7ec-a790461f9159

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• ✅ Review completed - (🔄 Check again to review again)

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/988-template-layout-redesign

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[nielsdrost7]

@copilot
Lint the living crap out of this PR, I had some merge conflicts.

It's just a template redesign, see if you can improve stuff, based on the new incoming source from the last 3 years (that's how old the PR was)

I had some merge conflicts again

[Copilot]

@nielsdrost7 I've opened a new pull request, #1420, to work on those changes. Once the pull request is ready, I'll request review from you.

[coderabbitai]

Actionable comments posted: 17

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

application/views/invoice_templates/pdf/InvoicePlane.php (1)

269-591: ⚠️ Potential issue | 🔴 Critical

Remove the stale old template body after the new layout.

The file now outputs a full new document, then continues into the previous template markup/PHP. This will duplicate invoice sections and produce malformed HTML with extra closing tags. Move any still-needed pieces, such as QR code and terms/footer handling, into the new layout and delete the old block.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@application/views/invoice_templates/pdf/InvoicePlane.php` around lines 269 -
591, The template appends the old/full-document markup after the new layout,
causing duplicated sections and malformed HTML; remove the stale legacy block
after the new layout and migrate only required pieces (QR code block that calls
invoice_qrcode(htmlsc($invoice->invoice_id)), the terms/notes section using
$invoice->invoice_terms, and the footer/QR settings like get_setting(...) and
<sethtmlpagefooter name="defaultFooter" ...>) into the new layout where
appropriate, and delete the leftover old header/body/footer markup (including
duplicated closing tags and the old invoice table duplication driven by
$add_table_and_head_for_sums/$colspan) while keeping helper calls such as
discount_global_print_in_pdf(...) intact in their correct places.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.junie/PR-1441-security-dry-analysis.md:
- Around line 1-9: The markdown file "Security and DRY Analysis for PR `#1441`"
contains stale PR metadata (title, PR number, file-count, and overview) that do
not match the current PR (added in PR `#1408`); either update the front-matter and
heading to reference PR `#1408` and correct title/file-count, or remove this
analysis from the template-layout PR entirely; specifically edit the document
heading and the lines that read "PR `#1441`", "Refactor input sanitization..." and
the "Files Modified: 5 files, +72 insertions, -23 deletions" summary so they
reflect the correct PR or delete the file from this PR.

In `@application/modules/reports/models/Mdl_reports.php`:
- Line 564: The condition uses an inconsistent (int) cast around
$this->db->escape($minQuantity); remove the redundant (int) cast and use
$this->db->escape($minQuantity) to match the other usages of
$minQuantity/$maxQuantity in the Mdl_reports class (same method where the SQL
fragments are built), ensuring the comparison expression follows the same
pattern as the sibling lines.

In `@application/third_party/MX/Controller.php`:
- Line 60: The log call uses the unsanitized $class variable; sanitize $class
with sanitize_for_logging() before passing it to log_message to prevent log
injection (e.g., create a sanitized variable via sanitize_for_logging($class)
and use that in the log_message call in the MX_Controller initialization block).
Ensure you reference the log_message invocation and the $class variable and
replace it with the sanitized value throughout this initialization path.
- Around line 54-58: Replace the loose comparison and the no-op str_replace:
change the condition to use strict comparison against null
(CI::$APP->config->item('controller_suffix') === null) and when it is null
assign the class name directly (use get_class($this)) instead of calling
str_replace('', '', get_class($this)); otherwise keep the existing branch that
strips CI::$APP->config->item('controller_suffix') via str_replace and assign to
$class.

In `@application/views/invoice_templates/pdf/InvoicePlane` - overdue.php:
- Line 7: The template emits raw fields (e.g., $invoice->invoice_number and
company/client VAT/tax code fields) without HTML-escaping; update all raw
outputs in this view (including the instances around invoice numbering and the
company/client VAT and tax code outputs referenced in this file) to use the
project’s escaping helper (html_escape() or htmlsc()/_htmlsc()) before echoing
so they are safely encoded in the PDF view; locate usages of
$invoice->invoice_number, any company/client VAT/tax variables, and replace
direct echoes with their escaped equivalents.
- Around line 191-240: Replace the duplicated inline discount rows in the
template (the checks and <tr> blocks that reference
$invoice->invoice_discount_percent and $invoice->invoice_discount_amount and use
$show_item_discounts) with a call to the invoice helper that renders invoice
discounts (create or reuse a function in invoice_helper.php such as
render_invoice_discount_rows or invoice_discount_html); move the business logic
that decides ordering and whether to show percent vs amount into that helper,
and inside the helper use strict comparisons (=== / !==) when checking
$invoice->invoice_discount_percent and $invoice->invoice_discount_amount and
preserve the legacy-calculation ordering so the displayed discounts match
invoice calculation mode.
- Around line 164-186: Replace the inflated pre-discount value printed in the
total column by using the actual line total field instead of item_subtotal:
change the template line that calls format_currency($item->item_subtotal) to use
the true line total (e.g. $item->item_total) and keep the same format_currency
wrapper so discounts and quantity are reflected correctly; locate the snippet
around the foreach over $items where format_currency is used and update the
reference from $item->item_subtotal to the item_total property (or compute
quantity*(price-discount) if your model uses a different field).
- Around line 274-281: Restore the QR-code rendering block that the base invoice
template used by inserting the same QR-code include/block immediately before the
closing </footer> in this overdue template so invoices with QR-code settings
enabled show the payment QR; ensure the inserted block matches the base
template’s QR-code block (the same include/partial or block used to render the
invoice payment QR and that references $invoice/payment settings) and is placed
after the terms block but before </footer>.

In `@application/views/invoice_templates/pdf/InvoicePlane` - paid.php:
- Line 7: The invoice view is emitting raw fields like $invoice->invoice_number
and various VAT IDs/tax codes without escaping; update all occurrences (e.g.,
the header line showing get_setting(...), $invoice->invoice_number and the
fields flagged around lines 31-36 and 116-121) to use the project's HTML-escape
helpers (html_escape() or htmlsc()/ _htmlsc()) consistently so all company,
client and invoice fields are encoded before rendering in the PDF view.
- Around line 164-186: The total column is printing the pre-total subtotal
($item->item_subtotal) causing discounted lines to show an inflated value;
update the template inside the foreach over $items to render the actual line
total instead (use the model's final total field if available, e.g.
$item->item_total) or compute the line total as (quantity * price - discount)
and pass that result to format_currency, preserving the existing quantity
display (format_amount) and unit behavior; replace the
format_currency($item->item_subtotal) call in the total cell with the correct
computed or model total.
- Around line 190-240: Replace the duplicated inline discount display logic with
the existing invoice discount helper from invoice_helper.php: remove the two
conditional blocks that check $invoice->invoice_discount_percent and
$invoice->invoice_discount_amount and instead call the centralized invoice
discount helper (the invoice discount helper in invoice_helper.php) to render
the appropriate discount row(s); also change the loose comparisons to strict
checks (use !== '0.00' or === '0.00' as appropriate) when determining whether
discounts should be shown so behavior matches the invoice calculation mode.
- Around line 273-280: The paid PDF template drops the base template's QR-code
rendering by ending before it; restore the QR-code block invocation just before
the closing </footer> so paid invoices render payment QR codes again — re-add
the same QR-code block used by the base template (the pdf_qr_code block /
QR-code view) into InvoicePlane - paid.php, ensuring it receives the same
$invoice/payment context as the base template (so settings and payment info are
honored).

In `@application/views/invoice_templates/pdf/InvoicePlane.php`:
- Line 7: The template emits raw values (e.g., $invoice->invoice_number, VAT
IDs, tax codes) while other fields use htmlsc/_htmlsc; update the PDF view to
always html-escape these outputs using html_escape() (or the existing htmlsc
wrapper) before rendering: replace raw echoes of $invoice->invoice_number and
any raw company/client fields (VAT IDs, tax_code, etc.) in the InvoicePlane.php
view and use html_escape(get_setting(...)) where appropriate so all
user-controlled data is consistently encoded.
- Around line 191-240: The subtotal/discount rows duplicate logic — replace the
inline percent/amount blocks that check $invoice->invoice_discount_percent and
$invoice->invoice_discount_amount with a single call to the invoice discount
helper in invoice_helper.php (e.g., invoice_discount_rows($invoice,
$show_item_discounts)) so the display follows the same legacy-calculation
ordering; ensure the helper uses strict comparisons (=== / !==) against '0.00'
and accepts $show_item_discounts to set the correct colspan and formatting.
- Around line 165-186: The line-total cell inside the foreach over $items
currently echoes $item->item_subtotal (which is pre-discount); change it to echo
the actual line total property (use $item->item_total) so discounted rows show
the correct amount, keeping the same format_currency call and leaving the
surrounding $show_item_discounts logic and quantity formatting unchanged.

In `@assets/core/css/custom-pdf.css`:
- Line 6: The closing CSS comment terminator is missing a space before "*/",
causing Stylelint to fail; update the comment block that starts with "/*" so its
closing token is " */" (i.e., insert a single space before the "*/" terminator)
so the stylesheet lint passes.

In `@assets/core/css/custom.css`:
- Line 4: The block comment closing in assets/core/css/custom.css contains no
space before the closing sequence (the line with
"===========================================================================*/"),
which triggers Stylelint spacing rules; update that closing line to include a
single space before the `*/` (e.g., change
"===========================================================================*/"
to "===========================================================================
*/") so the comment spacing conforms to Stylelint.

---

Outside diff comments:
In `@application/views/invoice_templates/pdf/InvoicePlane.php`:
- Around line 269-591: The template appends the old/full-document markup after
the new layout, causing duplicated sections and malformed HTML; remove the stale
legacy block after the new layout and migrate only required pieces (QR code
block that calls invoice_qrcode(htmlsc($invoice->invoice_id)), the terms/notes
section using $invoice->invoice_terms, and the footer/QR settings like
get_setting(...) and <sethtmlpagefooter name="defaultFooter" ...>) into the new
layout where appropriate, and delete the leftover old header/body/footer markup
(including duplicated closing tags and the old invoice table duplication driven
by $add_table_and_head_for_sums/$colspan) while keeping helper calls such as
discount_global_print_in_pdf(...) intact in their correct places.

🪄 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: 3560e4fb-83bf-4345-bf49-bf3d9225525d

📥 Commits

Reviewing files that changed from the base of the PR and between 15ea0a8 and ec3b08f.

📒 Files selected for processing (8)

• .junie/PR-1441-security-dry-analysis.md
• application/modules/reports/models/Mdl_reports.php
• application/third_party/MX/Controller.php
• application/views/invoice_templates/pdf/InvoicePlane - overdue.php
• application/views/invoice_templates/pdf/InvoicePlane - paid.php
• application/views/invoice_templates/pdf/InvoicePlane.php
• assets/core/css/custom-pdf.css
• assets/core/css/custom.css

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Update or remove the stale PR metadata.

This document is being added in PR #1408, but the filename/title/body identify it as PR #1441 with a different title and file-count summary. Either retarget the document to PR #1408 or keep it out of this template-layout PR to avoid misleading project history.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.junie/PR-1441-security-dry-analysis.md around lines 1 - 9, The markdown
file "Security and DRY Analysis for PR `#1441`" contains stale PR metadata (title,
PR number, file-count, and overview) that do not match the current PR (added in
PR `#1408`); either update the front-matter and heading to reference PR `#1408` and
correct title/file-count, or remove this analysis from the template-layout PR
entirely; specifically edit the document heading and the lines that read "PR
`#1441`", "Refactor input sanitization..." and the "Files Modified: 5 files, +72
insertions, -23 deletions" summary so they reflect the correct PR or delete the
file from this PR.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Inconsistent (int) cast wrapping db->escape($minQuantity).

This line wraps the escape call with (int), while the five sibling usages of $minQuantity/$maxQuantity (lines 326, 336, 449, 574, 687) use a plain $this->db->escape(...). Since $minQuantity is already cast to int at line 214, the extra cast here is redundant, and — more importantly — the asymmetry is suspicious (likely a copy/paste artifact). Please align with the other branches.

🔧 Proposed fix

-                                    AND ' . (int) $this->db->escape($minQuantity) . ' <=
+                                    AND ' . $this->db->escape($minQuantity) . ' <=

📝 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.

Suggested change

	AND ' . (int) $this->db->escape($minQuantity) . ' <=
	AND ' . $this->db->escape($minQuantity) . ' <=

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@application/modules/reports/models/Mdl_reports.php` at line 564, The
condition uses an inconsistent (int) cast around
$this->db->escape($minQuantity); remove the redundant (int) cast and use
$this->db->escape($minQuantity) to match the other usages of
$minQuantity/$maxQuantity in the Mdl_reports class (same method where the SQL
fragments are built), ensuring the comparison expression follows the same
pattern as the sibling lines.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Fix loose comparison and logic error.

Two issues in this conditional block:

1. Line 54: Uses loose comparison (==) instead of strict comparison (===). As per coding guidelines, PHP code should use strict comparison.

2. Line 55: Contains a critical logic error - str_replace('', '', get_class($this)) replaces an empty string with an empty string, which is a no-op. When controller_suffix is null, the class name should be used as-is.

🔧 Proposed fix

-if (CI::$APP->config->item('controller_suffix') == null) {
-        $class = str_replace('', '', get_class($this));
+if (CI::$APP->config->item('controller_suffix') === null) {
+        $class = get_class($this);
 } else {
         $class = str_replace(CI::$APP->config->item('controller_suffix'), '', get_class($this));
 }

Note: This is a third-party library file. Consider whether modifications should be submitted upstream or documented as local patches to avoid issues during library updates.

📝 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.

Suggested change

	if (CI::$APP->config->item('controller_suffix') == null) {
	$class = str_replace('', '', get_class($this));
	$class = str_replace('', '', get_class($this));
	} else {
	$class = str_replace(CI::$APP->config->item('controller_suffix'), '', get_class($this));
	$class = str_replace(CI::$APP->config->item('controller_suffix'), '', get_class($this));
	}
	if (CI::$APP->config->item('controller_suffix') === null) {
	$class = get_class($this);
	} else {
	$class = str_replace(CI::$APP->config->item('controller_suffix'), '', get_class($this));
	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@application/third_party/MX/Controller.php` around lines 54 - 58, Replace the
loose comparison and the no-op str_replace: change the condition to use strict
comparison against null (CI::$APP->config->item('controller_suffix') === null)
and when it is null assign the class name directly (use get_class($this))
instead of calling str_replace('', '', get_class($this)); otherwise keep the
existing branch that strips CI::$APP->config->item('controller_suffix') via
str_replace and assign to $class.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Sanitize data before logging.

The $class variable should be sanitized before logging to prevent log injection. As per coding guidelines, always sanitize data before logging using sanitize_for_logging().

🛡️ Proposed fix

-log_message('debug', $class . ' MX_Controller Initialized');
+log_message('debug', sanitize_for_logging($class) . ' MX_Controller Initialized');

📝 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.

Suggested change

	log_message('debug', $class . ' MX_Controller Initialized');
	log_message('debug', sanitize_for_logging($class) . ' MX_Controller Initialized');

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@application/third_party/MX/Controller.php` at line 60, The log call uses the
unsanitized $class variable; sanitize $class with sanitize_for_logging() before
passing it to log_message to prevent log injection (e.g., create a sanitized
variable via sanitize_for_logging($class) and use that in the log_message call
in the MX_Controller initialization block). Ensure you reference the log_message
invocation and the $class variable and replace it with the sanitized value
throughout this initialization path.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Escape raw invoice, company, and client fields before rendering.

invoice_number, VAT IDs, and tax codes are emitted raw while adjacent fields use htmlsc()/_htmlsc(). These values should be encoded consistently in the PDF view.

Proposed fix

-                <?php echo get_setting('custom_title', 'InvoicePlane', true); ?> - <?php _trans('invoice'); ?> <?php echo $invoice->invoice_number; ?>
+                <?php echo get_setting('custom_title', 'InvoicePlane', true); ?> - <?php _trans('invoice'); ?> <?php _htmlsc($invoice->invoice_number); ?>

-                                echo trans('vat_id_short') . ': ' . $invoice->user_vat_id . '<br>';
+                                echo trans('vat_id_short') . ': ' . htmlsc($invoice->user_vat_id) . '<br>';

-                                echo trans('tax_code_short') . ': ' . $invoice->user_tax_code . '<br>';
+                                echo trans('tax_code_short') . ': ' . htmlsc($invoice->user_tax_code) . '<br>';

-                                echo trans('vat_id_short') . ': ' . $invoice->client_vat_id . '<br>';
+                                echo trans('vat_id_short') . ': ' . htmlsc($invoice->client_vat_id) . '<br>';

-                                echo trans('tax_code_short') . ': ' . $invoice->client_tax_code . '<br>';
+                                echo trans('tax_code_short') . ': ' . htmlsc($invoice->client_tax_code) . '<br>';

As per coding guidelines, Always use html_escape() for output encoding in views to prevent XSS vulnerabilities.

Also applies to: 31-36, 117-122

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@application/views/invoice_templates/pdf/InvoicePlane` - overdue.php at line
7, The template emits raw fields (e.g., $invoice->invoice_number and
company/client VAT/tax code fields) without HTML-escaping; update all raw
outputs in this view (including the instances around invoice numbering and the
company/client VAT and tax code outputs referenced in this file) to use the
project’s escaping helper (html_escape() or htmlsc()/_htmlsc()) before echoing
so they are safely encoded in the PDF view; locate usages of
$invoice->invoice_number, any company/client VAT/tax variables, and replace
direct echoes with their escaped equivalents.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Escape raw invoice, company, and client fields before rendering.

invoice_number, VAT IDs, and tax codes are emitted raw while adjacent fields use htmlsc()/_htmlsc(). These values should be encoded consistently in the PDF view.

Proposed fix

-                <?php echo get_setting('custom_title', 'InvoicePlane', true); ?> - <?php _trans('invoice'); ?> <?php echo $invoice->invoice_number; ?>
+                <?php echo get_setting('custom_title', 'InvoicePlane', true); ?> - <?php _trans('invoice'); ?> <?php _htmlsc($invoice->invoice_number); ?>

-                                echo trans('vat_id_short') . ': ' . $invoice->user_vat_id . '<br>';
+                                echo trans('vat_id_short') . ': ' . htmlsc($invoice->user_vat_id) . '<br>';

-                                echo trans('tax_code_short') . ': ' . $invoice->user_tax_code . '<br>';
+                                echo trans('tax_code_short') . ': ' . htmlsc($invoice->user_tax_code) . '<br>';

-                                echo trans('vat_id_short') . ': ' . $invoice->client_vat_id . '<br>';
+                                echo trans('vat_id_short') . ': ' . htmlsc($invoice->client_vat_id) . '<br>';

-                                echo trans('tax_code_short') . ': ' . $invoice->client_tax_code . '<br>';
+                                echo trans('tax_code_short') . ': ' . htmlsc($invoice->client_tax_code) . '<br>';

As per coding guidelines, Always use html_escape() for output encoding in views to prevent XSS vulnerabilities.

Also applies to: 31-36, 117-122

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@application/views/invoice_templates/pdf/InvoicePlane.php` at line 7, The
template emits raw values (e.g., $invoice->invoice_number, VAT IDs, tax codes)
while other fields use htmlsc/_htmlsc; update the PDF view to always html-escape
these outputs using html_escape() (or the existing htmlsc wrapper) before
rendering: replace raw echoes of $invoice->invoice_number and any raw
company/client fields (VAT IDs, tax_code, etc.) in the InvoicePlane.php view and
use html_escape(get_setting(...)) where appropriate so all user-controlled data
is consistently encoded.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Render the actual line total, not the pre-total subtotal.

The total column now prints item_subtotal, so discounted item rows can show an inflated total. Keep quantity formatting and line-total semantics aligned with the invoice data.

Proposed fix

-                                    <?php echo format_amount($item->item_quantity); ?>
+                                    <?php echo format_quantity($item->item_quantity); ?>

-                                    <?php echo format_currency($item->item_subtotal); ?>
+                                    <?php echo format_currency($item->item_total); ?>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@application/views/invoice_templates/pdf/InvoicePlane.php` around lines 165 -
186, The line-total cell inside the foreach over $items currently echoes
$item->item_subtotal (which is pre-discount); change it to echo the actual line
total property (use $item->item_total) so discounted rows show the correct
amount, keeping the same format_currency call and leaving the surrounding
$show_item_discounts logic and quantity formatting unchanged.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Reuse the invoice discount helper instead of duplicating discount display logic.

The inline percent/amount rows bypass the existing legacy-calculation ordering and use loose comparisons. Reuse the helper path so discounts stay consistent with invoice calculation mode.

Proposed direction

+                        <?php if (! $legacy_calculation) : ?>
+                            <?php discount_global_print_in_pdf($invoice, $show_item_discounts); ?>
+                        <?php endif; ?>
+
                         <tr>
                             <td <?php echo $show_item_discounts ? 'colspan="5"' : 'colspan="4"'; ?> class="text-right px-1 py-2 bt">
                                 <?php _trans('subtotal'); ?>
                             </td>
                             <td class="text-right py-2 px-1 bt"><?php echo format_currency($invoice->invoice_item_subtotal); ?></td>
                         </tr>
@@
-                        <?php if ($invoice->invoice_discount_percent != '0.00') : ?>
-                            <tr>
-                                <td <?php echo $show_item_discounts ? 'colspan="5"' : 'colspan="4"'; ?> class="text-right py-2 px-1 bt">
-                                    <?php _trans('discount'); ?>
-                                </td>
-                                <td class="text-right py-2 px-1 bt">
-                                    <?php echo format_amount($invoice->invoice_discount_percent); ?>%
-                                </td>
-                            </tr>
-                        <?php endif; ?>
-                        <?php if ($invoice->invoice_discount_amount != '0.00') : ?>
-                            <tr>
-                                <td <?php echo $show_item_discounts ? 'colspan="5"' : 'colspan="4"'; ?> class="text-right py-2 px-1 bt">
-                                    <?php _trans('discount'); ?>
-                                </td>
-                                <td class="text-right py-2 px-1 bt">
-                                    <?php echo format_currency($invoice->invoice_discount_amount); ?>
-                                </td>
-                            </tr>
-                        <?php endif; ?>
+                        <?php if ($legacy_calculation) : ?>
+                            <?php discount_global_print_in_pdf($invoice, $show_item_discounts); ?>
+                        <?php endif; ?>

Based on learnings, Place invoice-specific business logic in invoice_helper.php. As per coding guidelines, Use strict comparison (===, !==) instead of loose comparison.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@application/views/invoice_templates/pdf/InvoicePlane.php` around lines 191 -
240, The subtotal/discount rows duplicate logic — replace the inline
percent/amount blocks that check $invoice->invoice_discount_percent and
$invoice->invoice_discount_amount with a single call to the invoice discount
helper in invoice_helper.php (e.g., invoice_discount_rows($invoice,
$show_item_discounts)) so the display follows the same legacy-calculation
ordering; ensure the helper uses strict comparisons (=== / !==) against '0.00'
and accepts $show_item_discounts to set the correct colspan and formatting.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Fix the Stylelint comment spacing failure.

Line 6 needs a space before the closing */, otherwise the stylesheet lint step fails.

Proposed fix

-   ============================================================================*/
+   ============================================================================ */

📝 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.

Suggested change

	============================================================================*/
	============================================================================ */

🧰 Tools

🪛 Stylelint (17.7.0)

[error] 6-6: Expected whitespace before "*/" (comment-whitespace-inside)

(comment-whitespace-inside)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@assets/core/css/custom-pdf.css` at line 6, The closing CSS comment terminator
is missing a space before "*/", causing Stylelint to fail; update the comment
block that starts with "/*" so its closing token is " */" (i.e., insert a single
space before the "*/" terminator) so the stylesheet lint passes.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Fix the Stylelint comment spacing failure.

Line 4 needs a space before the closing */, otherwise the stylesheet lint step fails.

Proposed fix

-   ============================================================================*/
+   ============================================================================ */

📝 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.

Suggested change

	============================================================================*/
	============================================================================ */

🧰 Tools

🪛 Stylelint (17.7.0)

[error] 4-4: Expected whitespace before "*/" (comment-whitespace-inside)

(comment-whitespace-inside)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@assets/core/css/custom.css` at line 4, The block comment closing in
assets/core/css/custom.css contains no space before the closing sequence (the
line with
"===========================================================================*/"),
which triggers Stylelint spacing rules; update that closing line to include a
single space before the `*/` (e.g., change
"===========================================================================*/"
to "===========================================================================
*/") so the comment spacing conforms to Stylelint.