Bug: Check deposit page crashes with DB_ERROR_SYNTAX — filter passed to select_comptes() not migrated to USF syntax

[nagoz]

Bug

Since 24.0, Form::select_comptes() runs its $filtre parameter through forgeSQLFromUniversalSearchCriteria() (Universal Search Filter syntax), but the caller in htdocs/compta/paiement/cheque/card.php still passes the legacy raw SQL string 'courant <> 2'.

forgeSQLFromUniversalSearchCriteria() does not throw on invalid input — it returns the error message as a string, which is then concatenated straight into the SQL:

SELECT rowid, label, bank, clos as status, currency_code
FROM llx_bank_account
WHERE entity IN (1) AND clos = 0Filter error - Bad syntax of the search string ORDER BY label

Result: DB_ERROR_SYNTAX, and the "New check deposit" page is completely unusable.

Faulty line

htdocs/compta/paiement/cheque/card.php:440 (identical on 24.0 and develop)

php
$form->select_comptes($filteraccountid, 'accountid', 0, 'courant <> 2', 1);
Suggested fix
php
$form->select_comptes($filteraccountid, 'accountid', 0, '(courant:!=:2)', 1);

!= is mapped to <> by dolForgeSQLCriteriaCallback(), so the generated SQL is strictly equivalent to the original intent (exclude cash registers, courant = 2, from the list).

I checked every other select_comptes() call in htdocs/ — this is the only remaining caller passing a non-USF filter.

Root cause / regression window
Branch select_comptes() filter handling card.php argument Broken?
18.0 → 23.0 $sql .= " AND ".$filtre; (// TODO Support USF) 'courant <> 2' No
24.0 (stable) $sql .= forgeSQLFromUniversalSearchCriteria($filtre); 'courant <> 2' Yes
develop (25.0.0-alpha) same as 24.0 'courant <> 2' Yes

The // TODO Support USF was resolved in 24.0 and the PHPDoc updated to Must use USF syntax., but this call site was not migrated. The fix should target the 24.0 branch and be propagated to develop.

This is not related to MAIN_FEATURES_LEVEL: the faulty line is unconditional, so the crash occurs identically with MAIN_FEATURES_LEVEL = 0.

Side note (separate concern)

select_comptes() silently concatenates whatever forgeSQLFromUniversalSearchCriteria() returns. When the filter is malformed, an English error message ends up inside the SQL query instead of raising a proper error. Passing $noerror = 1 (which returns 1 = 2) or checking the return value before concatenation would fail more gracefully. Same pattern may exist in other select_*() helpers.

Dolibarr Version

24.0.0 (stable). Code inspection confirms the faulty line is also present on develop (25.0.0-alpha). Not affected: 18.0 → 23.0.

Environment PHP

8.4.22 (Apache, Linux x86_64)

Environment Database

MySQL / MariaDB via mysqli

Steps to reproduce the behavior and expected behavior

Enable the Bank/Cash module.
Go to Bank | Cash → Check deposits (/compta/paiement/cheque/index.php).
Click New check deposit (/compta/paiement/cheque/card.php?action=new&mainmenu=bank).

Expected: the check deposit creation form is displayed, with a bank account dropdown excluding cash registers.

Actual: Dolibarr error page — DB_ERROR_SYNTAX.

Attached files

Url: /compta/paiement/cheque/card.php?leftmenu=checks_bis&action=new&mainmenu=bank
Referer: /compta/paiement/cheque/index.php?leftmenu=checks&mainmenu=bank

Last DB access in error:
SELECT rowid, label, bank, clos as status, currency_code
FROM llx_bank_account
WHERE entity IN (1) AND clos = 0Filter error - Bad syntax of the search string ORDER BY label

Return code: DB_ERROR_SYNTAX

MySQL message:
You have an error in your SQL syntax; check the manual that corresponds to your MySQL server
version for the right syntax to use near 'error - Bad syntax of the search string ORDER BY label'
at line 1

[pixodeo]

Confirmed your analysis in full and sent #39708 against 24.0.

Reproduced by calling the real select_comptes() both ways: with 'courant <> 2' no select is rendered and MariaDB reports DB_ERROR_SYNTAX near 'error - Bad syntax of the search string ORDER BY label', with '(courant:!=:2)' the dropdown renders normally.

Also checked the equivalence you suggested: forgeSQLFromUniversalSearchCriteria('(courant:!=:2)') returns " AND ((courant <> 2))", and both WHERE clauses return the same rows, so cash registers stay excluded exactly as before.

Your regression window is right too: select_comptes() only started using USF in 24.0, 21.0 to 23.0 concatenated the filter directly. And your survey of the other callers holds, they all use the (courant:=:N) form already, with takepos/pay.php passing an empty string which skips the filter entirely.

Thanks for the very precise report, it made this a five minute check.