Problem/Motivation

\Drupal\Core\Validation\DrupalTranslator uses t() which makes it tricky to write unit tests. It already uses the TranslatableMarkup object so lets replace t().

See #2932462: Add EntityContextDefinition for the 80% use case for an example of a unit test that's made more tricky because of this.

Proposed resolution

Do it

Remaining tasks

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#5 2979944-5.patch2.88 KBalexpott
#5 2-5-interdiff.txt801 bytesalexpott
#2 2979944-2.patch2.87 KBalexpott

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Active » Needs review
StatusFileSize
new 2979944-2.patch2.87 KB
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs review » Reviewed & tested by the community 
+++ b/core/lib/Drupal/Core/Validation/DrupalTranslator.php
@@ -79,7 +82,7 @@ protected function processParameters(array $parameters) {
+        // TranslatableMarkup does not work with objects being passed as replacement strings.

Nit: Longer than 80 characters.

Otherwise, looks good. RTBC from me once green.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Reviewed & tested by the community » Needs review

From #2932462-55: Add EntityContextDefinition for the 80% use case:

And the !function_exists pattern in tests is in 17 other tests as well.

Maybe we should remove all of those in this patch too, in order to prove that the fix actually works.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 2-5-interdiff.txt801 bytes
new 2979944-5.patch2.88 KB

The other 17 are not caused by this one. Patch attached fixes #3

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs review » Reviewed & tested by the community

Okay. Back to RTBC.

  • catch committed 49a45db on 8.6.x 
    Issue #2979944 by alexpott: Remove t() usage in Drupal\Core\Validation
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed 49a45db and pushed to 8.6.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.