No actionable comments were generated in the recent review. 🎉

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 help to get the list of available commands and usage tips.}

@bhojkamal As I understand it, this change is just adding the dropdown options for these payment types and not actually implementing the payment types themselves. There is a flaw in our implementation of the payment types in that they are being stored in the table as the string representation of whatever language the logged in employee was using at time of sale. We ought to have instead created a static table containing an id and name of the payment type then linked them using a foreign key and the payment method id is what gets stored in the table so you don't have inconsistent data.

Good: We leave your implementation as is and @CodeRabbit creates an issue describing the need I've just explained.
Better: We implement your change but also refactor the code to correctly model the data.

I'll leave it up to you what you want to do but the issue I brought up needs to be addressed at some point.

#!/bin/bash
gh issue create \
  --repo opensourcepos/opensourcepos \
  --title "Refactor: Store payment types using a foreign key instead of localized strings" \
  --body "## Summary

Payment types (e.g. Cash, Debit Card, Bank Transfer, Wallet, etc.) are currently stored in the sales table as the **string representation** of the payment type label in whatever language the logged-in employee was using at the time of sale.

This creates a data integrity problem: a sale recorded by an English-speaking employee is stored as \`\"Cash\"\`, while the same payment type recorded by a Spanish-speaking employee is stored as \`\"Efectivo\"\`. Querying, filtering, and reporting across these records becomes inconsistent and unreliable.

## Proposed Solution

1. Create a new static **\`payment_types\`** table with at minimum the following columns:
   - \`id\` (primary key, integer)
   - \`name\` (canonical/internal identifier, e.g. \`cash\`, \`debit_card\`, \`bank_transfer\`, \`wallet\`)

2. Update the sales (and receiving) tables to store the **\`payment_type_id\`** (foreign key → \`payment_types.id\`) instead of the localized string.

3. Update all queries, filters, and reports that currently match on the localized string (e.g. \`LIKE lang('Sales.bank_transfer')\`) to instead use the numeric foreign key.

4. Keep the language files for display/UI purposes only — translation happens at the presentation layer, not at the storage layer.

## Impact

- \`app/Models/Sale.php\` — \`get_payments_summary()\` and \`add_filters_to_query()\` use \`LIKE lang('Sales.*')\` conditions that are locale-dependent.
- \`app/Controllers/Reports.php\` — \`get_payment_type()\` returns localized labels used for filtering.
- \`app/Controllers/Sales.php\` — filter keys map to locale-dependent query conditions.
- \`app/Helpers/locale_helper.php\` — \`get_payment_options()\` builds options from language strings.
- \`app/Models/Receiving.php\` — \`get_payment_options()\` similarly builds options from language strings.

## Related PR

This issue was identified during the review of `#4547` (Bank Transfer and Wallet payment options), raised by `@objecttothis`."

@objecttothis The issue has been created: #4548

Here's a summary of what was captured in the issue:

• Root cause: Payment types are stored as locale-dependent strings (e.g. "Cash" vs "Efectivo") rather than a stable identifier, leading to inconsistent data across employees using different languages.
• Proposed fix: Introduce a static payment_types table (id, name) and store the foreign key payment_type_id in the sales/receiving tables instead. Translation stays at the presentation layer only.
• Affected files called out: app/Models/Sale.php, app/Controllers/Reports.php, app/Controllers/Sales.php, app/Helpers/locale_helper.php, and app/Models/Receiving.php.

The issue also links back to this PR (#4547) and credits @objecttothis as the reporter.

Also, I think the change as you have it is a core code change but likely implementing those payment methods (stripe, QR code, etc) we might want to do as a plugin rather than core code, since those plugins may require PCI Compliance work that I don't really want to deal with but plugin authors can manage if they want to develop them. @jekkos, what do you think?

@objecttothis
I’ve added bank_transfer and wallet to the static payment types list. I have used these keys in my CI3 application for the past 4-5 years without issue, so I am implementing them here in CI4 to match the existing static structure. The database keys remain consistent, while the display labels handled by the localization.

The missing initialization is fixed too.
In Controllers/Sales.php in line 90-100 - $data['filters'] is now:
'only_cash' => lang('Sales.cash_filter'),
'only_due' => lang('Sales.due_filter'),
'only_check' => lang('Sales.check_filter'),
'only_creditcard' => lang('Sales.credit_filter'),
'only_debit' => lang('Sales.debit'),
'only_bank_transfer' => lang('Sales.bank_transfer'),
'only_wallet' => lang('Sales.wallet'),

The snapshot is below.

I think, it can be merge now.

@objecttothis I’ve added bank_transfer and wallet to the static payment types list. I have used these keys in my CI3 application for the past 4-5 years without issue, so I am implementing them here in CI4 to match the existing static structure. The database keys remain consistent, while the display labels handled by the localization.

The missing initialization is fixed too. In Controllers/Sales.php in line 90-100 - $data['filters'] is now: 'only_cash' => lang('Sales.cash_filter'), 'only_due' => lang('Sales.due_filter'), 'only_check' => lang('Sales.check_filter'), 'only_creditcard' => lang('Sales.credit_filter'), 'only_debit' => lang('Sales.debit'), 'only_bank_transfer' => lang('Sales.bank_transfer'), 'only_wallet' => lang('Sales.wallet'),

I think, it can be merge now.

For the purpose of this PR, this is good enough, but it doesn't resolve the issue I raised in #4548. For example, if you log in as another employee with a different language enabled (that has translations for those phrases) and complete a sale, the transaction history report will show different languages. It was probably done this way before because originally we didn't have per-employee language selection and the flaw only becomes apparent in situations where more than one language is being used. In our use case there are three languages used by our employees, so it becomes obvious quickly.

All that said, we can resolve #4548 in another PR. I'll do a code review right now.