StringDatabaseStorage::deleteStrings() does not work

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to the Core change policies → .

Can the issue summary be flushed out some please

Proposed solution is a little vague, don't need code examples but how is this being solved.

Also recommend leaving all summary headers, if they are blank or NA.

Tried to improve issue summary.

Much better!

I moved the test to a kernel test.

Feedback of switching to a kernel test appears to be addressed

I queued the test-only changes job (which someone should have done and documented before this was RTBC).

The test-only job failed as expected:

https://git.drupalcode.org/issue/drupal-3497647/-/jobs/5279091

Drupal\Core\Database\InvalidQueryException: Calling Drupal\Core\Database\Query\Condition::condition() without an array compatible operator is not supported. See https://www.drupal.org/node/3350985
/builds/issue/drupal-3497647/core/lib/Drupal/Core/Database/Query/Condition.php:115
/builds/issue/drupal-3497647/core/lib/Drupal/Core/Database/Query/QueryConditionTrait.php:27
/builds/issue/drupal-3497647/core/modules/locale/src/StringDatabaseStorage.php:530
/builds/issue/drupal-3497647/core/modules/locale/src/StringDatabaseStorage.php:209
/builds/issue/drupal-3497647/core/modules/locale/tests/src/Kernel/LocaleStringTest.php:277
ERRORS!

I reviewed the relevant APIs, and I agree that this is a good fix. Saving credits.

Sorry, just caught a couple assertion messages creeping back in. @prudloff, are you okay with the inline comments I proposed instead, in place of your assertion messages? If you accept the suggestions, the issue can be moved straight back to RTBC (no need for an extra review cycle for it).

Sorry I didn't know that assertion messages should not be used. (Maybe a phpcs rule delicould catch that?)
I am OK with your changes.

@prudloff, it's one of those "cultural knowledge" things where it's been enforced as a best practice in the core queue and been the subject of repeated massive codebase cleanups, but the policy draft to formalize it has stalled out for years. :) Tried to get the latest version of it moving again today in 🌱 [META] [policy] Remove PHPUnit assertion messages when possible, and standardize remaining messages Active . Unfortunately there are a subset of cases where messages are needed (the policy draft in the linked issue outlines them), so enforcing it with a rule isn't as simple as some.

In any case, thanks for the bug fix! I went back and forth on the priority, but I think this is actually major, given that the method as it stands is basically straight-up broken.

Committed to 11.x, 11.2.x, and 10.5.x as a patch-safe major bug fix. Thanks!

xjm → closed merge request !11306