Secondary Currency Enhancement - #4535

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 `@app/Controllers/Config.php`:
- Around line 479-483: The secondary_currency_rate value is being sanitized with
FILTER_SANITIZE_NUMBER_FLOAT which breaks locale decimal separators; update the
handling in the controller (postSaveGeneral) to pass the raw post value for
'secondary_currency_rate' through the existing parse_decimals() helper instead
of FILTER_SANITIZE_NUMBER_FLOAT so locale formats like "1,2345" are parsed
correctly; locate the array entry for 'secondary_currency_rate' in Config.php
and replace its getPost(...) sanitization with
parse_decimals($this->request->getPost('secondary_currency_rate')) so stored
rates are locale-aware and consistent with other fields using parse_decimals().

In `@app/Views/configs/receipt_config.php`:
- Around line 185-195: The form checkbox rendering for
receipt_show_secondary_currency currently uses
$config['receipt_show_secondary_currency'] == 1 which throws an "Undefined array
key" if that config is absent; update the checked expression in the
form_checkbox block (name/id 'receipt_show_secondary_currency') to use a
null-coalescing default like ($config['receipt_show_secondary_currency'] ?? 0)
== 1 so missing keys default safely to 0.

In `@app/Views/reports/tabular_details.php`:
- Around line 34-40: The view reads $secondaryCurrency['show'] without guarding
it; update the conditional inside the foreach over $overall_summary_display_data
to check existence safely (e.g. use ($secondaryCurrency['show'] ?? false) or
isset($secondaryCurrency) && !empty($secondaryCurrency['show'])) so the block
only runs when $secondaryCurrency is defined and truthy; keep the loop and
esc($summary_row['secondary']) usage the same but wrap the current if
($secondaryCurrency['show']) condition in a null-coalescing/isset guard.

In `@app/Views/sales/invoice_email.php`:
- Line 141: The template renders the secondary-currency cell using
to_currency($total) as a fallback which shows the primary currency when
secondaryTotalDisplay is missing; change the fallback to a secondary-currency
formatted value instead by replacing the fallback expression so it uses the
secondary-currency formatter (e.g., call to_secondary_currency($total) or pass a
flag to to_currency to force secondary formatting) for the element with id
"total_secondary_currency" and ensure you still escape the result with esc()
while keeping the existing variables secondaryTotalDisplay and $total.

In `@app/Views/sales/quote_email.php`:
- Around line 141-152: The template blindly dereferences
$secondaryCurrency['show'] and $secondaryCurrency['rate']; update the
conditional and expressions to guard against a missing or non-array
$secondaryCurrency by first checking isset($secondaryCurrency) &&
isset($secondaryCurrency['show']) (or is_array($secondaryCurrency) &&
!empty($secondaryCurrency['show'])) before rendering the two rows, and when
calling secondary_currency_rate_display use the guarded value (only access
$secondaryCurrency['rate'] after the same isset/is_array check) so the fallback
on the null coalescing operator is not evaluated on a missing array; adjust the
blocks that output id="total_secondary_currency" and id="currency_rate"
accordingly to only execute when the guard passes.

In `@app/Views/sales/quote.php`:
- Line 181: The current fallback uses to_currency($total) which renders the
primary currency under the secondary label; update the expression for the
element with id "total_secondary_currency" so that when $secondaryTotalDisplay
is missing it converts/formats $total into the secondary currency instead of the
primary one — e.g. replace to_currency($total) with the existing helper that
formats secondary amounts (use to_secondary_currency($total) if available, or
to_currency($total, $secondary_currency) / convertToSecondary($total) according
to your codebase) so the fallback always shows the secondary-currency value.

In `@app/Views/sales/receipt_default.php`:
- Around line 20-22: The toggle mutates $secondaryCurrency['show'] but template
still unconditionally uses any controller-provided secondary*Display values
(e.g., secondaryPriceDisplay, secondarySubtotalDisplay, secondaryTotalDisplay),
so turn-off still prints secondary amounts and duplicates the primary total;
locate every usage of variables matching secondary*Display in the receipt view
and either wrap them with a check if ($secondaryCurrency['show']) before
rendering or route them through the existing helper that applies this flag,
ensuring the fallback logic that prints primary totals only runs when
$secondaryCurrency['show'] is true.

In `@app/Views/sales/receipt_email.php`:
- Line 131: The secondary-currency row currently falls back to the primary
formatter by calling to_currency($total) inside esc(...); change the fallback so
it uses the secondary-currency formatter instead (i.e., replace the
to_currency($total) fallback with the view's secondary currency formatter used
elsewhere for this template—use the same helper used for secondary totals or
labels) so that esc($secondaryTotalDisplay ?? ...) formats $total in the
secondary currency rather than the primary.

In `@app/Views/sales/receipt_short.php`:
- Around line 122-127: The current row uses to_currency($total) as a fallback
when $secondaryTotalDisplay is unset, which mislabels the primary total as the
secondary currency; update the template in receipt_short.php to render either
esc($secondaryTotalDisplay) when set or an empty string otherwise (e.g., use a
conditional around $secondaryTotalDisplay or a ternary checking
isset($secondaryTotalDisplay) instead of falling back to to_currency($total)),
keeping the row visible only when $secondaryCurrency['show'] is true and
ensuring esc(...) wraps the output.

In `@app/Views/sales/register.php`:
- Around line 839-842: The updateSecondaryRows function is currently inserting
derived currency strings with .html(), which can introduce stored-XSS; change
the DOM writes in updateSecondaryRows to use .text() for
"#sale_total_secondary_currency" and "#sale_amount_due_secondary_currency"
(replace .html() calls with .text()), and ensure any server-side-rendered values
used to compute totalDisplay/amountDueDisplay are passed through the esc()
helper before being emitted into the page.

In `@app/Views/sales/tax_invoice.php`:
- Line 196: The secondary total cell (id="total_secondary_currency") currently
falls back to to_currency($total) which can show the primary currency amount;
remove that fallback and output only the secondary total display (escape
secondaryTotalDisplay) or an empty string if missing—i.e., replace
esc($secondaryTotalDisplay ?? to_currency($total)) with
esc($secondaryTotalDisplay ?? '') or use the proper secondary total variable
(e.g., $secondaryTotal) formatted with to_currency if available.

---

Nitpick comments:
In `@app/Views/sales/quote_email.php`:
- Line 106: The template inconsistently wraps the fallback currency output with
esc() causing potential double-encoding for secondaryPriceDisplay; update the
cell to match surrounding rows by emitting the formatted value the same way (use
secondaryPriceDisplay directly or call to_currency($item['price']) without
esc()) and, if secondaryPriceDisplay can contain user-controlled input, move
sanitization into the currency helper (to_currency) so the template remains
consistent; adjust the expression referencing secondaryPriceDisplay and
to_currency accordingly.

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 `@app/Controllers/Reports.php`:
- Around line 1563-1577: The _average_secondary_rate function currently returns
an unweighted mean; change it to compute a weighted average using each row's
base amount (e.g., the column that represents the base-currency total per row —
locate the rows' amount field used elsewhere in reports, e.g., "base_amount" or
"amount") by summing rate*amount and dividing by sum(amount), skipping rows with
missing/zero amounts or rates; if no valid weighted data exists (or when
multiple snapshots are detected and weighting is impossible), return null to
suppress a misleading single rate. Ensure you still respect the $field parameter
for the rate, validate numeric values, and handle empty arrays to avoid division
by zero in _average_secondary_rate.
- Around line 646-653: The tabular discounts report ($data array in Reports.php)
is missing the per-field secondary-currency summary so it falls back to a single
rate; compute a summary_secondary_data by calling the existing
_build_secondary_summary_display_map(...) (same approach used by
graphical_summary_discounts()) and include it in the $data array (add
'summary_secondary_data' => $summary_secondary_data) so the tabular report uses
the correct per-field secondary currency values when rendering the
summary/footer.

In
`@app/Database/Migrations/20260508000001_AlterSecondaryCurrencyRatePrecision.php`:
- Around line 14-17: The up() migration currently changes
secondary_currency_rate to DECIMAL(15,0) which truncates fractional exchange
rates; update the ALTER TABLE call in the up() method (inside the class
20260508000001_AlterSecondaryCurrencyRatePrecision) to use DECIMAL(15,6) via the
existing $this->db->query(...) for `secondary_currency_rate`, and ensure the
down() method reverts it back to DECIMAL(15,0); also fix the reversed up/down
logic if present so up() increases precision to 15,6 and down() restores 15,0.

In `@app/Models/Expense.php`:
- Around line 268-270: The code in Expense.php currently forces
secondary_currency_rate to an integer using `(int) round((float) ...)`, which
loses decimal precision; change the assignment to preserve decimals by casting
to float and optionally rounding to a sensible precision (e.g. 4 decimals)
instead of to int — e.g. set `$expense_data['secondary_currency_rate'] = (float)
round((float) (config(OSPOS::class)->settings['secondary_currency_rate'] ?? 0),
4)` or simply `$expense_data['secondary_currency_rate'] = (float)
(config(OSPOS::class)->settings['secondary_currency_rate'] ?? 0)` so the
`secondary_currency_rate` key in the Expense model keeps fractional exchange
rates.

In `@app/Models/Reports/Detailed_sales.php`:
- Around line 129-130: The expanded sale row built in getDataBySaleId() is
missing the persisted secondary_currency_rate field added to getData(); update
getDataBySaleId() to include the snapshot's secondary_currency_rate when
assembling the expanded sale object (mirror the same field selection/aggregation
used in getData, e.g., include MAX(secondary_currency_rate) AS
secondary_currency_rate or otherwise pull the stored snapshot rate) so the
drawer renders the persisted rate rather than the current config value.

In `@app/Models/Reports/Summary_report.php`:
- Around line 85-87: The query in Summary_report.php is averaging
sales.secondary_currency_rate at the sales_items grain which overweights
multi-line sales; change the SQL to compute sale-level rates first (e.g., a
subquery or CTE that selects sale_id and its header secondary_currency_rate,
e.g., SELECT sale_id, secondary_currency_rate FROM sales GROUP BY sale_id or
SELECT DISTINCT sale_id, secondary_currency_rate) and join that result
(referencing the subquery alias, e.g., sale_rates.secondary_currency_rate) into
the main query, then replace AVG(sales.secondary_currency_rate) with
AVG(sale_rates.secondary_currency_rate) so the average is computed across sales
rather than items.

In `@app/Models/Sale.php`:
- Around line 559-566: The secondary_currency_rate is being forcibly cast to int
in both the new-sale and update branches (see secondary_currency_rate, $sale_id
== NEW_ENTRY and $existing_secondary_currency_rate) which strips decimal
precision; change both conversions to preserve decimals by using float casting
and/or round to the intended precision (e.g., (float) and round($value,
<required_decimal_places>)) instead of (int) round((float)...), ensuring
existing_secondary_currency_rate is likewise treated as a float so updates do
not re-truncate stored rates.

In `@app/Views/reports/graphical.php`:
- Around line 41-44: The view currently renders $summary_secondary_data_1[$name]
even when secondary currency is disabled; change the conditional so the
precomputed secondary row only renders when both
!empty($summary_secondary_data_1[$name]) and $secondaryCurrency['show'] are true
(or nest the check for $secondaryCurrency['show'] around the whole
secondary-rendering branch), ensuring $summary_secondary_data_1[$name] is gated
behind $secondaryCurrency['show'] before calling
secondary_currency_display_label or outputting the row.

In `@app/Views/reports/tabular.php`:
- Around line 46-49: The cached secondary summary
($summary_secondary_data[$name]) is being rendered even when the secondary
currency feature is disabled; update the conditional around the summary row in
tabular.php so you only consider or print any secondary summary when
$secondaryCurrency['show'] is true — e.g., check $secondaryCurrency['show']
first and then if (!empty($summary_secondary_data[$name])) use
esc(secondary_currency_display_label(...)) with the cached value, otherwise use
esc(secondary_currency_render_amount(...)); ensure no branch emits
$summary_secondary_data[$name] when $secondaryCurrency['show'] is false.

---

Nitpick comments:
In `@app/Controllers/Reports.php`:
- Line 1460: Rename the new private helper methods in Reports controller from
underscore-prefixed style to camelCase (e.g., change _append_secondary_currency
to appendSecondaryCurrency) and update every local call site in this file to the
new names; do the same for the other private helpers introduced (the ones listed
around the commented lines) so all private function names follow PSR-12
camelCase conventions. Ensure method declarations (private function ...) and all
usages within the file are consistently renamed and tests/usage still compile.

In
`@app/Database/Migrations/20260508000001_AlterSecondaryCurrencyRatePrecision.php`:
- Around line 14-17: Replace the raw ALTER TABLE SQL in the migration class
(20260508000001_AlterSecondaryCurrencyRatePrecision) that uses
$this->db->query(...) to modify `secondary_currency_rate` with CodeIgniter 4
Forge API: build a $fields array for 'secondary_currency_rate' using 'type' =>
'DECIMAL', 'constraint' => '15,0', 'null' => true, 'default' => null and call
$this->forge->modifyColumn($table, $fields); apply the same refactor in both the
up() and down() methods where the raw SQL occurs so the migration uses
modifyColumn() instead of direct SQL.

In `@app/Models/Reports/Detailed_receivings.php`:
- Around line 80-85: Remove the redundant INNER JOIN to 'receivings' that
re-selects columns already projected into receivings_items_temp; in
Detailed_receivings.php, inside the query builder in the method that builds the
report (the block using $builder->join('receivings', ...) and the other
occurrence in getData()), delete those join calls and any references that
qualify columns via the receivings table, and ensure SELECT/GROUP BY use the
fields from receivings_items_temp (e.g., secondary_currency_rate,
receiving_time, comment, reference, payment_type) to avoid ambiguity and
potential row loss.

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
`@app/Database/Migrations/20260508000001_AlterSecondaryCurrencyRatePrecision.php`:
- Around line 14-17: Replace the raw ALTER TABLE string built with
$this->db->query(...) for each $table with a call to
$this->forge->modifyColumn(...), targeting the prefixed table name and updating
the secondary_currency_rate column definition to DECIMAL(15,6) NULL DEFAULT
NULL; specifically remove usages of $this->db->query and instead call
$this->forge->modifyColumn($this->db->prefixTable($table),
['secondary_currency_rate' => ['type' => 'DECIMAL', 'constraint' => '15,6',
'null' => true, 'default' => null]]), doing this for every table handled in the
migration so SQL is produced via the Forge API rather than string concatenation.

In `@app/Models/Reports/Summary_report.php`:
- Around line 80-86: The subquery building $secondary_rate_subquery in
Summary_report (and the similar logic around lines 94-95) only applies date
filtering and omits the rest of the report filters from __common_where(),
causing averaged secondary_currency_rate to come from a broader set; update the
subquery to derive its sale_ids from the same filtered set as the outer query
(use the same $where or call __common_where()/its components) — e.g., first
select distinct sale_id from sales applying the full filter (date, location_id,
sale_type, etc.) and then compute AVG(MAX(...)) over those sale_ids — so the
secondary_currency_rate average is computed over the identical filtered dataset
as the main summary.

---

Duplicate comments:
In `@app/Views/sales/invoice.php`:
- Line 148: The invoice per-item price cell incorrectly always evaluates
secondary_currency_render_amount and never shows the primary price when
secondary pricing is disabled; update the expression that renders the cell (the
one using $item['secondaryPriceDisplay'] and secondary_currency_render_amount)
to first check $secondaryCurrency['show'] and, if true, render
$item['secondaryPriceDisplay'] (or call secondary_currency_render_amount when
that key is missing), otherwise render the primary price display
($item['priceDisplay'] or the raw $item['price']) so primary prices are shown
when secondary currency is turned off; reference the invoice.php cell that uses
$item['secondaryPriceDisplay'], secondary_currency_render_amount, $item['price']
and $secondaryCurrency['show'] to implement the conditional.

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 `@app/Controllers/Config.php`:
- Around line 482-486: When saving secondary currency, add validation in Config
controller: after reading secondary_currency_enabled, if true validate
secondary_currency_code is non-empty (trim and alphanumeric/symbols as needed),
secondary_currency_rate parsed by parse_decimals is a positive number (>0), and
secondary_currency_decimals is an integer in an allowed range (e.g. 0–8); if any
check fails, stop persistence and return validation errors (set
flashdata/response) instead of saving. Use the existing symbols
secondary_currency_enabled, secondary_currency_code, secondary_currency_rate,
secondary_currency_decimals and parse_decimals to perform checks, sanitize
inputs with esc()/htmlspecialchars for stored values, and ensure you do not call
the save/update routine when validation fails.

---

Nitpick comments:
In `@app/Models/Reports/Summary_report.php`:
- Around line 44-66: The duplicated raw-SQL filter block that builds
$secondary_rate_where should be removed and replaced by reusing the same filter
logic from __common_where(): build (or clone) the query builder that
__common_where() populates to produce a SELECT DISTINCT sales.sale_id subquery,
then use that subquery (or join its results) inside the secondary-rate query so
the inner sale_id set shares the exact filters; make __common_where() accessible
to the method (or add a thin protected wrapper that returns the builder) and
reference the builder/SELECT DISTINCT sales.sale_id when constructing the rate
subquery instead of reconstructing sale_type/location/sale_status logic in
$secondary_rate_where.
- Around line 104-112: The duplicated filter logic used to build
$secondary_rate_where and the identical conditions used elsewhere should be
centralized to avoid maintenance drift: extract the shared where-clause
construction into a single reusable method (e.g., create a protected method
buildSecondaryRateWhere() or extend __common_where() to return the same filter
string/array) and replace direct uses in the $secondary_rate_subquery and the
other block with calls to that method (reference symbols:
$secondary_rate_subquery, $secondary_rate_where, __common_where()). Ensure the
new method returns the same format expected by both consumers and update places
that previously constructed the filters inline to call it so future changes are
made in one place.

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 `@app/Models/Reports/Detailed_receivings.php`:
- Line 46: The label for the secondary_rate column is using
lang('Reports.selling_rate'), which is semantically incorrect for a
receivings/purchasing report; change the translation key used in the
$summaryColumns entry (in Detailed_receivings.php where $summaryColumns[] is set
for 'secondary_rate') to a more appropriate key such as
lang('Reports.exchange_rate') or lang('Reports.rate') and also add/update the
corresponding key/value in the Reports translation file so the new label appears
correctly.
- Around line 46-47: The $summaryColumns entry uses the wrong key
'secondary_rate' which doesn't match the SQL alias produced by
getDataByReceivingId() and getData() ('secondary_currency_rate'); update the key
in the $summaryColumns array to 'secondary_currency_rate' and keep the display
label call secondary_currency_display_label(lang('Reports.total'),
$secondaryCurrency) unchanged so views that index rows by column keys receive
the actual SQL column value.

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 `@app/Commands/RefreshSecondaryCurrency.php`:
- Around line 48-66: persistStatus currently swallows batch_save failures
because it only catches exceptions; change it to detect Appconfig::batch_save
returning false and throw an exception so the failure propagates to run().
Specifically, in persistStatus (function persistStatus) capture the return value
from $appconfig->batch_save($payload), and if it is === false throw a
RuntimeException (or other Throwable) containing a useful message (include the
payload/error context). This ensures run() observes the failure and doesn't
report a successful refresh when the Appconfig update failed.

In `@app/Config/Routes.php`:
- Around line 75-76: The routes $routes->post('config/saveScale',
'Config::postSaveScale') and $routes->post('config/saveShortcuts',
'Config::postSaveShortcuts') reference missing controller methods; either add
public methods postSaveScale(Request $request) and postSaveShortcuts(Request
$request) to the Config controller (implementing request validation, processing,
and appropriate response) or remove/disable these route definitions if the
endpoints are not intended to exist; ensure method names exactly match
Config::postSaveScale and Config::postSaveShortcuts so the router can resolve
them.
- Around line 84-120: Several sales routes that mutate server state are
registered with $routes->add() and thus allow GET; change them to verb-specific
registrations (mostly $routes->post() or $routes->delete() as appropriate) so
CSRF-prone actions are not callable via GET. Replace $routes->add(...) with
$routes->post(...) for: 'sales/selectCustomer' (Sales::postSelectCustomer),
'sales/changeMode' (Sales::postChangeMode), 'sales/setComment'
(Sales::postSetComment), 'sales/setInvoiceNumber' (Sales::postSetInvoiceNumber),
'sales/setPaymentType' (Sales::postSetPaymentType), 'sales/setPrintAfterSale'
(Sales::postSetPrintAfterSale), 'sales/setPriceWorkOrders'
(Sales::postSetPriceWorkOrders), 'sales/setEmailReceipt'
(Sales::postSetEmailReceipt), 'sales/addPayment' (Sales::postAddPayment),
'sales/add' (Sales::postAdd), 'sales/editItem/(:segment)'
(Sales::postEditItem/$1), 'sales/complete' (Sales::postComplete),
'sales/delete/(:num)' (Sales::postDelete/$1) —and change other mutating
endpoints to appropriate verbs too (e.g., 'sales/deletePayment/(:any)'
Sales::getDeletePayment/$1 and 'sales/deleteItem/(:num)' Sales::getDeleteItem/$1
should be converted to $routes->post() or $routes->delete(),
'sales/removeCustomer' Sales::getRemoveCustomer to $routes->post(),
'sales/save/(:num)' Sales::postSave/$1 to $routes->post() if not already,
'sales/cancel' Sales::postCancel to $routes->post(), 'sales/suspend'
Sales::postSuspend and 'sales/unsuspend' Sales::postUnsuspend to
$routes->post(); keep true read-only handlers (e.g., receipt/invoice/PDF
display) as GET. Ensure you update only the registrations (route helper) leaving
controller method names intact.

In `@app/Controllers/Config.php`:
- Around line 476-523: Normalize the primary and secondary currency codes (e.g.
trim and strtoupper) and reject when they are identical: update the validation
block that runs when $secondary_currency_enabled to normalize
$secondary_currency_code and the posted currency code
(getPost('currency_code')), and if they match add a validation error (push to
$validation_errors) so saving is blocked; also ensure the value you write into
$batch_save_data['secondary_currency_code'] is the normalized code so the stored
value is consistent with the check.

In `@app/Controllers/Sales.php`:
- Around line 1743-1761: The refresh handler in Sales.php currently returns on
$result['success'] false without updating secondary_currency_last_error and
ignores the boolean return of model(Appconfig::class)->batch_save on success;
update the logic so that on refresh failure you call $appconfig->batch_save(...)
to set secondary_currency_last_error to the failure message (and
secondary_currency_last_synced_at as appropriate), and on the success path check
the boolean result of $appconfig->batch_save (the call that sets
secondary_currency_rate/last_synced_at/last_error) — if batch_save returns
false, persist a non-empty secondary_currency_last_error and return a failure
JSON via $this->response->setJSON; use the same $result['message'] or a clear
error string when writing last_error so the DB always reflects the actual stored
state.

In `@app/Language/ar-LB/Sales.php`:
- Line 115: The localization entry for the key "rate" currently uses a
nonstandard spelling "الصيرفه"; update the value for the "rate" key to a
standard Arabic term—either "الصيرفة" for the generic term or "سعر الصرف" if
this label specifically represents an exchange rate—so replace the value
associated with "rate" accordingly in the array.

In `@app/Language/bg/Config.php`:
- Line 83: The array entry line containing "secondary_currency_rate_required"
and "current_employee_only" has two entries jammed together causing a PHP parse
error; split them into two distinct array entries so each key => value pair is
on its own entry (ensure there's a trailing comma after the
"secondary_currency_rate_required" entry and that "current_employee_only" has
its own quoted key and value), locating the entries by the keys
"secondary_currency_rate_required" and "current_employee_only" in the language
Config.php array.

In `@app/Language/cs/Config.php`:
- Line 83: There is a PHP array syntax error because two entries
("secondary_currency_rate_required" and "current_employee_only") are on the same
line; edit the array in Config.php so each associative entry is a separate
element (put "secondary_currency_rate_required" => "Secondary currency rate must
be a positive number." on its own line followed by a comma, then add
"current_employee_only" => "" on the next line with a trailing comma) so the
array keys/values are properly separated and the file parses.

In `@app/Language/de-DE/Sales.php`:
- Around line 115-116: Translate the two new German language keys: replace the
English values for "rate" and "secondary_currency_update_live_rate_tooltip" with
appropriate German strings (e.g., "Kurs" or "Wechselkurs" for "rate" and
"Aktuellen Wechselkurs der Zweitwährung jetzt abrufen." for
"secondary_currency_update_live_rate_tooltip") and move these entries next to
the other currency/financial keys in the Sales language array to keep semantic
grouping; update only the values for the keys "rate" and
"secondary_currency_update_live_rate_tooltip" and ensure the keys themselves
remain unchanged.

In `@app/Language/es-ES/Config.php`:
- Around line 62-83: The Spanish locale file currently contains English literals
for the new secondary-currency keys (e.g., "secondary_currency_auto_enable",
"secondary_currency_feed_http_error", "secondary_currency_feed_invalid_json",
"secondary_currency_refresh_interval_invalid",
"secondary_currency_rate_required", etc.); replace those English text values
with empty strings ("") so the app's Language::getLine() fallback will use the
English default and translators can track missing entries; leave any keys
already translated in Spanish as-is and ensure each new secondary_currency_* key
uses "" when a Spanish translation is not yet available.

In `@app/Language/es-ES/Sales.php`:
- Around line 115-116: Translate the two new keys in
app/Language/es-ES/Sales.php: replace the English values for "rate" and
"secondary_currency_update_live_rate_tooltip" with Spanish equivalents (for
example "Tasa" or "Tipo de cambio" for "rate" and "Obtener la última tasa de la
moneda secundaria ahora." for "secondary_currency_update_live_rate_tooltip") and
move these entries so they sit with the other currency/financial keys in the
Sales.php array for consistent organization.

In `@app/Language/es-MX/Config.php`:
- Line 83: The array entry line in Config.php incorrectly places two keys on the
same line; split the entries "secondary_currency_rate_required" and
"current_employee_only" into separate array elements (one per line) and ensure
the first entry ends with a comma so the PHP array syntax is valid (adjust
surrounding indentation to match the array style in the file).

In `@app/Language/es-MX/Sales.php`:
- Around line 115-116: The Spanish (Mexico) language file contains untranslated
keys "rate" and "secondary_currency_update_live_rate_tooltip"; update their
values to Spanish (e.g., "rate" => "Tasa" or "Tipo de cambio", and
"secondary_currency_update_live_rate_tooltip" => "Obtener la última tasa de la
moneda secundaria ahora.") and move these entries to the section with other
currency/financial keys so they’re grouped logically (look for nearby keys
dealing with currency, exchange rate, or finance to place them).

In `@app/Language/fa/Sales.php`:
- Around line 115-116: Translate the two new keys "rate" and
"secondary_currency_update_live_rate_tooltip" into Persian (provide appropriate
Persian strings) in app/Language/fa/Sales.php and move their entries from
between "key_help_modal" and "key_in" into the currency/financial block near
"amount_due", "change_due", and "sales_tax" to preserve semantic grouping;
ensure the keys remain unchanged and only their values and position are updated.

In `@app/Language/it/Sales.php`:
- Around line 115-116: The Italian language file contains English text for the
keys "rate" and "secondary_currency_update_live_rate_tooltip"; change their
values to empty strings ("") so the custom Language::getLine() fallback will use
the English translations—update the entries for "rate" and
"secondary_currency_update_live_rate_tooltip" in the app/Language/it/Sales.php
array to "" instead of the English phrases.

In `@app/Language/pl/Config.php`:
- Line 83: The array in app/Language/pl/Config.php has two entries on the same
line causing a parse error; split the entries so each key/value pair is on its
own line by moving "current_employee_only" => "" onto the next line after
"secondary_currency_rate_required" => "...", ensuring proper comma placement and
alignment within the array declaration (look for the array containing the keys
"secondary_currency_rate_required" and "current_employee_only" to update).

In `@app/Language/pl/Sales.php`:
- Line 109: Fix the typo in the translation string for the array key
"key_finish_quote": change the value from "Finish Quote/Invoice witdout payment"
to "Finish Quote/Invoice without payment" so the user-facing text reads
correctly; update the string in the Language/pl/Sales.php localization array
entry for key_finish_quote.

In `@app/Language/ur/Config.php`:
- Line 83: The array line containing "secondary_currency_rate_required" and
"current_employee_only" is malformed because two entries are on the same line;
split them into two separate array entries so each key => value pair is its own
element (ensure there's a trailing comma after
"secondary_currency_rate_required" and that "current_employee_only" is on its
own line), preserving the exact strings "secondary_currency_rate_required" and
"current_employee_only" so the PHP array parses correctly.

In `@app/Libraries/SecondaryCurrencyFeedLib.php`:
- Around line 74-92: The feed URL returned by buildFeedUrl($config) (driven by
secondary_currency_feed_url) must be validated before issuing
Services::curlrequest and $client->get: ensure the URL uses only http or https
schemes, reject or normalize other schemes, and verify the host does not resolve
to private/internal addresses (or enforce a whitelist of allowed hosts) to
prevent SSRF. Implement checks using parse_url/filter_var to confirm scheme and
host, resolve the host and block RFC1918/loopback addresses (or compare against
an allowed-host list), and only proceed to call getCertificateBundlePath(),
create the client via Services::curlrequest, and $client->get when the URL
passes validation; otherwise log and abort the request.

---

Nitpick comments:
In `@app/Helpers/tabular_helper.php`:
- Around line 853-861: The foreach introduces an unused $key and shadows the
outer $expense; change the loop from "foreach ($expense->getResult() as $key =>
$expense)" to use a single, non-shadowing variable (e.g., "foreach
($expense->getResult() as $expenseRow)") and update all uses inside the loop
($expense->amount, $expense->tax_amount, $expense->secondary_currency_rate) to
use $expenseRow so $sum_amount_expense, $sum_tax_amount_expense and
$sum_secondary_amount_expense calculations remain correct.

In `@app/Language/bs/Sales.php`:
- Around line 115-116: Translate the two untranslated keys in the Bosnian locale
by replacing the English values for "rate" and
"secondary_currency_update_live_rate_tooltip" with their Bosnian equivalents;
update the entries for "rate" and "secondary_currency_update_live_rate_tooltip"
in the app/Language/bs/Sales.php locale array to provide Bosnian text (e.g.,
replace "Rate" and "Fetch the latest secondary currency rate now." with
appropriate Bosnian translations).

In `@app/Language/hr-HR/Sales.php`:
- Around line 115-116: The Croatian locale file contains two keys with English
values: "rate" and "secondary_currency_update_live_rate_tooltip"; update their
values to Croatian by replacing "Rate" with an appropriate Croatian word (e.g.,
"Tečaj" or "Stopa") and translate the tooltip to Croatian (e.g., "Preuzmi
najnoviji tečaj za sekundarnu valutu sada."), ensuring the keys "rate" and
"secondary_currency_update_live_rate_tooltip" remain unchanged.

In `@app/Language/hu/Sales.php`:
- Around line 115-116: Translate the two untranslated keys in the Hungarian
locale: replace the English values for "rate" and
"secondary_currency_update_live_rate_tooltip" in the Sales.php locale array with
appropriate Hungarian strings (e.g., "Árfolyam" for "rate" and a Hungarian
sentence for the tooltip) ensuring they match the style and grammar of existing
translations in the file; update the values for the array entries "rate" and
"secondary_currency_update_live_rate_tooltip".

In `@app/Language/id/Config.php`:
- Around line 62-83: The new secondary_currency_* language entries (e.g.,
"secondary_currency_disabled", "secondary_currency_auto_enable",
"secondary_currency_feed_invalid_json",
"secondary_currency_refresh_interval_invalid", etc.) in
app/Language/id/Config.php are left as English; update each of these keys to
either a proper Indonesian translation or set the value to an explicit empty
string ("") so fallback to English is intentional and consistent (follow the
existing pattern used by "current_employee_only"); ensure every
"secondary_currency_*" key is handled this way to avoid mixed-language UI.
- Line 83: The array entry on the same line contains two keys
("secondary_currency_rate_required" and "current_employee_only") in
app/Language/id/Config.php; split them so each key/value pair is on its own line
(one line for "secondary_currency_rate_required" => "Secondary currency rate
must be a positive number.", and a separate line for "current_employee_only" =>
""), preserving the surrounding array formatting, commas, and alignment to
reduce merge conflicts and improve readability.

In `@app/Language/it/Sales.php`:
- Around line 115-116: The two translation keys "rate" and
"secondary_currency_update_live_rate_tooltip" are misplaced among keyboard
shortcut entries; move the "rate" key so it's alphabetically grouped with other
"r" entries (e.g., near "register" or "return") and move
"secondary_currency_update_live_rate_tooltip" to the "s" section near the other
"s" entries (around the existing keys at lines ~183-192) in Sales.php so logical
grouping and alphabetical ordering are preserved.

In `@app/Language/ml/Sales.php`:
- Around line 115-116: The new keys "rate" and
"secondary_currency_update_live_rate_tooltip" are placed between keyboard
shortcut keys; move these two entries into the existing currency/financial
translations block (where other currency keys like "currency", "exchange_rate",
or similar exist) so they're semantically grouped with related strings; ensure
you preserve their current translations/empty-string fallbacks and maintain the
same array ordering/formatting used by the surrounding entries in the
Language/ml/Sales.php file.

In `@app/Language/pl/Config.php`:
- Around line 62-83: Translate the English values for the secondary currency
localization keys shown (e.g. "secondary_currency_disabled",
"secondary_currency_auto_enable", "secondary_currency_auto_enable_tooltip",
"secondary_currency_auto_refresh_disabled",
"secondary_currency_auto_refresh_disabled_force",
"secondary_currency_base_quote_required", "secondary_currency_codes_required",
"secondary_currency_codes_must_differ", "secondary_currency_feed_http_error",
"secondary_currency_feed_invalid_json", "secondary_currency_feed_invalid_rate",
"secondary_currency_feed_request_failed", "secondary_currency_feed_url",
"secondary_currency_feed_url_tooltip", "secondary_currency_last_error",
"secondary_currency_last_synced_at", "secondary_currency_refresh_failed",
"secondary_currency_refresh_successful", "secondary_currency_refresh_interval",
"secondary_currency_refresh_interval_tooltip",
"secondary_currency_refresh_interval_invalid",
"secondary_currency_rate_required") into Polish in the same associative array
(preserve placeholders like {0}, {base}, {quote} and literal flags like
--force), replacing the English strings with proper Polish translations so the
Polish locale no longer falls back to English.

In `@app/Language/th/Sales.php`:
- Around line 115-116: Translate the two English strings in the Thai locale for
the Sales module: replace the value for the "rate" key and the value for
"secondary_currency_update_live_rate_tooltip" with appropriate Thai
translations, keeping the keys unchanged; update the values in the array where
"rate" and "secondary_currency_update_live_rate_tooltip" are defined so the
tooltip reads naturally in Thai and the single-word "rate" is translated to the
correct Thai term.

In `@app/Language/tr/Sales.php`:
- Around line 115-116: Translate the two untranslated keys in the Turkish Sales
locale: replace the English values for "rate" and
"secondary_currency_update_live_rate_tooltip" in the Sales.php translations
array with appropriate Turkish strings (e.g., "Oran" or context-appropriate term
for "rate" and a Turkish sentence for the tooltip) so the keys "rate" and
"secondary_currency_update_live_rate_tooltip" are fully localized.

In `@app/Language/ur/Sales.php`:
- Around line 115-116: The added localization keys "rate" and
"secondary_currency_update_live_rate_tooltip" are in English and misplaced among
shortcut keys; either provide Urdu translations for these keys (replace the
English strings with proper Urdu text for "rate" and for "Fetch the latest
secondary currency rate now.") or relocate the key-value pairs to the locale
file/section that holds payment/currency-related strings so they sit with other
financial keys; update any references if you move them so the application still
reads the same keys.

In `@app/Language/vi/Sales.php`:
- Around line 115-116: Translate the two English values for the localization
keys "rate" and "secondary_currency_update_live_rate_tooltip" in the Vietnamese
language array: replace the value for "rate" with the Vietnamese equivalent
(e.g., "Tỷ giá" or context-appropriate term) and translate
"secondary_currency_update_live_rate_tooltip" into Vietnamese (e.g., "Lấy tỷ giá
ngoại tệ phụ mới nhất ngay bây giờ."), keeping the keys unchanged.

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 `@app/Controllers/Config.php`:
- Around line 531-533: The feed URL is being HTML-escaped (htmlspecialchars)
which can corrupt valid URLs; instead validate and sanitize the incoming
$secondary_currency_feed_url for a proper URL format and only use it if valid.
Update the code that sets the 'secondary_currency_feed_url' (the variable
$secondary_currency_feed_url and the array entry) to trim the input, validate
with a URL validator (e.g., filter_var(..., FILTER_VALIDATE_URL) or an
equivalent validation helper), and fall back to the default
'https://open.er-api.com/v6/latest/{base}' when validation fails; do not apply
htmlspecialchars() to the value used for HTTP requests.

---

Duplicate comments:
In `@app/Controllers/Config.php`:
- Around line 486-491: When $secondary_currency_enabled is true, add a format
check for $secondary_currency_code (after trim/strtoupper) to ensure it matches
ISO 4217 three-letter codes using a regex like /^[A-Z]{3}$/; if it fails, push a
validation error onto $validation_errors (e.g.,
lang('Config.secondary_currency_code_invalid')), and optionally normalize
$secondary_currency_code = strtoupper(trim($secondary_currency_code)) before
further use to sanitize input; update any downstream uses to rely on the
normalized value.

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 `@app/Language/he/Config.php`:
- Around line 62-83: Replace the English text values for the secondary currency
translation keys in app/Language/he/Config.php (keys such as
"secondary_currency_disabled", "secondary_currency_auto_enable",
"secondary_currency_auto_enable_tooltip", "secondary_currency_feed_http_error",
"secondary_currency_refresh_interval_invalid",
"secondary_currency_rate_required", etc.) with proper Hebrew translations or, if
unavailable, set them to empty strings ("") so the MY_Language.php fallback to
English kicks in; update all 22 keys in the block currently containing English
(the secondary_currency_* entries between the existing keys) so localization is
correct and avoids duplicating the source text.

In `@app/Libraries/SecondaryCurrencyFeedLib.php`:
- Around line 24-25: Normalize currency codes to a consistent case before
encoding and comparing: replace current rawurlencode(trim((string)
($config['currency_code'] ?? ''))) and rawurlencode(trim((string)
($config['secondary_currency_code'] ?? ''))) with
rawurlencode(strtoupper(trim((string) ($config['currency_code'] ?? '')))) and
rawurlencode(strtoupper(trim((string) ($config['secondary_currency_code'] ??
'')))). Ensure any equality check that enforces “must differ” compares these
normalized values ($baseCurrency, $secondaryCurrency) and apply the same
strtoupper-trim normalization to the other similar block (the code handling the
alternative parsing around the later section) so case-insensitive matches like
"usd" vs "USD" are treated as equal.
- Around line 75-76: The warning currently logs the raw user-supplied $url when
isSafeFeedUrl($url) fails; instead remove the verbatim URL from the log and log
a sanitized/redacted value or a generic rejection message. Update the
log_message call in SecondaryCurrencyFeedLib (the branch that checks
$this->isSafeFeedUrl($url)) to either log only the host (extracted and escaped
via esc()) or a fixed string like "Secondary currency feed URL rejected:
redacted" and do not include the raw $url; ensure any output uses the esc()
helper per coding guidelines.
- Around line 102-105: The catch block in SecondaryCurrencyFeedLib (the
Throwable handler returning ['success'=>false,'message'=>lang(...,
[$throwable->getMessage()])]) leaks transport details; instead, log the full
$throwable internally (e.g., via the class logger or logger()->error with the
exception) and return a generic localized failure string from
lang('Config.secondary_currency_feed_request_failed') without including
$throwable->getMessage() or other exception data; update the catch in the method
inside SecondaryCurrencyFeedLib to remove the message parameter and add an
internal log call referencing the caught $throwable.

---

Nitpick comments:
In `@app/Language/fa/Config.php`:
- Around line 62-83: The new Persian locale entries are left in English but
should follow the project's untranslated-key convention: replace the English
text for each added key (e.g., "secondary_currency_disabled",
"secondary_currency_auto_enable", "secondary_currency_auto_enable_tooltip",
"secondary_currency_auto_refresh_disabled",
"secondary_currency_auto_refresh_disabled_force",
"secondary_currency_base_quote_required", "secondary_currency_codes_required",
"secondary_currency_codes_must_differ", "secondary_currency_feed_http_error",
"secondary_currency_feed_invalid_json", "secondary_currency_feed_invalid_rate",
"secondary_currency_feed_request_failed", "secondary_currency_feed_url",
"secondary_currency_feed_url_tooltip", "secondary_currency_last_error",
"secondary_currency_last_synced_at", "secondary_currency_refresh_failed",
"secondary_currency_refresh_successful", "secondary_currency_refresh_interval",
"secondary_currency_refresh_interval_tooltip",
"secondary_currency_refresh_interval_invalid",
"secondary_currency_rate_required") with empty-string values ("") so
Language::getLine() will correctly fall back to English.