- 🇺🇸United States smustgrave
This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.
Sounds like last comment #46 there is a new approach we would like to take?
There was a test failure for 9.5.x also. (which imagine would go away with new tests approach)Thanks.
- 🇬🇷Greece perarg
Regardless of the tests, the patch #45 is working in Drupal Core 9.5.3. I have tested with Commerce 2, using product variations with image linked to content.
- First commit to issue fork.
- 🇧🇪Belgium borisson_ Mechelen, 🇧🇪
I tried to create an issue fork, but looks like I did that on an old version of drupal.
I disagree with #46 that we need to use functional tests.There are a couple of failures on the latest version of drupal with this patch, so it needs to be rerolled to fix those.
-
+++ b/core/modules/image/tests/src/Kernel/ImageFormatterTranslationTest.php @@ -0,0 +1,207 @@ + // Chunk of code from ImageFormatterTest::setUp(), + // But we also add an untranslatable image field.
This comment is not helpful imho.
I think we should say:
Install file entity schema en file usage schema -
+++ b/core/modules/image/tests/src/Kernel/ImageFormatterTranslationTest.php @@ -0,0 +1,207 @@ + $renderer = $this->container->get('renderer'); ... + $build = $this->display->build($entity_test); ... + $output = $renderer->renderRoot($build[$field_name][0]); + $this->assertContains('<a href="' . $entity_test->toUrl()->toString(), (string) $output);
I think this is a smart way to test this bug. :+1:
-
- 🇮🇳India mohit_aghera Rajkot
mohit_aghera → changed the visibility of the branch 11.x to hidden.
- 🇮🇳India mohit_aghera Rajkot
mohit_aghera → changed the visibility of the branch 2645922-image-field-generates to hidden.
- Merge request !5781Fix Image field generates only default language URL → (Open) created by mohit_aghera
- Status changed to Needs review
about 1 year ago 2:43pm 12 December 2023 - 🇮🇳India mohit_aghera Rajkot
Pipeline is green now.
Good to get the initial review.
Updated the issue summary with the necessary details.Hiding all the patches in favour of PR approach.
Hidden a couple of old branches as well as I couldn't update those with latest 11.x due to merge conflicts.
Created a new branch instead. - Status changed to RTBC
about 1 year ago 7:42pm 12 December 2023 - 🇺🇸United States smustgrave
Following steps in issue summary can confirm the issue.
Running test-only feature I get
There was 1 failure: 1) Drupal\Tests\image\Kernel\ImageFormatterTranslationTest::testTranslatedUntranslatableImage Failed asserting that ' <a href="/en/entity_test/1" hreflang="en"><img loading="lazy" src="/vfs://root/sites/simpletest/53240262/files/2023-12/generateImage_nVZkTD.jpg" width="525" height="213" alt="Blandit cogo dignissim elit facilisi lenis uxor." title="Esse gemino illum laoreet mauris paulatim pneum quidne sagaciter singularis." />\n </a>\n ' contains "<a href="/l0/entity_test/1". /builds/issue/drupal-2645922/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:121 /builds/issue/drupal-2645922/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55 /builds/issue/drupal-2645922/core/modules/image/tests/src/Kernel/ImageFormatterTranslationTest.php:214 /builds/issue/drupal-2645922/core/modules/image/tests/src/Kernel/ImageFormatterTranslationTest.php:181 /builds/issue/drupal-2645922/vendor/phpunit/phpunit/src/Framework/TestResult.php:728 FAILURES! Tests: 4, Assertions: 92, Failures: 1.
Which also shows the issue.
Applying the MR does address the issue.
Change to the 2 image formatters LGTM!
- Status changed to Needs work
12 months ago 6:04am 29 December 2023 - 🇳🇿New Zealand quietone
I'm triaging RTBC issues → . I read the IS and the comments. I didn't find any unanswered questions.
There was discussion about using a Functional test because it would be clearer, which @borisson_ disagrees with. I will go look at the MR now. ... I too think a Kernel test is fine. I have left quite a few comments in the MR. And I now see that #55.1 was not addressed.
I am setting to NW.