Skip to content
Snippets Groups Projects

Fix Image field generates only default language URL

18 unresolved threads

Fix the test case failures.

Re-roll the patch. Remove deprecated code from tests.

Closes #2645922

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
1 <?php
2
3 declare(strict_types=1);
4
5 namespace Drupal\Tests\image\Kernel;
6
7 use Drupal\Core\Field\FieldStorageDefinitionInterface;
8 use Drupal\entity_test\Entity\EntityTest;
9 use Drupal\field\Entity\FieldConfig;
10 use Drupal\field\Entity\FieldStorageConfig;
11 use Drupal\KernelTests\Core\Entity\EntityLanguageTestBase;
12
13 /**
14 * Tests language related aspects of image formatter.
  • quietone
    quietone @quietone started a thread on the diff
  • 13 /**
    14 * Tests language related aspects of image formatter.
    15 *
    16 * @group image
    17 */
    18 class ImageFormatterTranslationTest extends EntityLanguageTestBase {
    19
    20 /**
    21 * Modules to enable.
    22 *
    23 * @var array
    24 */
    25 protected static $modules = ['file', 'image', 'content_translation'];
    26
    27 /**
    28 * Our test entity.
  • quietone
    quietone @quietone started a thread on the diff
  • 20 /**
    21 * Modules to enable.
    22 *
    23 * @var array
    24 */
    25 protected static $modules = ['file', 'image', 'content_translation'];
    26
    27 /**
    28 * Our test entity.
    29 *
    30 * @var \Drupal\entity_test\Entity\EntityTest
    31 */
    32 protected $entity;
    33
    34 /**
    35 * Our test entity, but translated.
  • quietone
    quietone @quietone started a thread on the diff
  • 40
    41 /**
    42 * EntityViewDisplayInterface.
    43 *
    44 * @var \Drupal\Core\Entity\Display\EntityViewDisplayInterface
    45 */
    46 protected $display;
    47
    48 /**
    49 * {@inheritdoc}
    50 */
    51 protected function setUp(): void {
    52 parent::setUp();
    53
    54 // Chunk of code from ImageFormatterTest::setUp(),
    55 // But we also add an untranslatable image field.
    • Comment on lines +54 to +55

      I think the comment should just state what is happening in this test and not refer to other test files. This makes me pause and wonder if I should read that test file and also ask why this is not extending from that.

    • Please register or sign in to reply
  • quietone
    quietone @quietone started a thread on the diff
  • 97 ],
    98 ])->save();
    99
    100 $this->display = \Drupal::service('entity_display.repository')
    101 ->getViewDisplay($entityType, $bundle, 'default')
    102 ->setComponent('field_untranslatable_image', [
    103 'type' => 'image',
    104 'label' => 'hidden',
    105 ])
    106 ->setComponent('field_translatable_image', [
    107 'type' => 'image',
    108 'label' => 'hidden',
    109 ]);
    110 $this->display->save();
    111 // End of code from ImageFormatterTest::setUp().
    112 // Chunk of code essentially taken from EntityUrlLanguageTest::setUp().
    • Comment on lines +111 to +112

      Similar here. For me, it makes an expectation that I understand the other file to know why only some parts were copied here.

    • Please register or sign in to reply
  • quietone
    quietone @quietone started a thread on the diff
  • 108 'label' => 'hidden',
    109 ]);
    110 $this->display->save();
    111 // End of code from ImageFormatterTest::setUp().
    112 // Chunk of code essentially taken from EntityUrlLanguageTest::setUp().
    113 \Drupal::service('router.builder')->rebuild();
    114 // In order to reflect the changes for a multilingual site in the container
    115 // we have to rebuild it.
    116 $config = $this->config('language.negotiation');
    117 $config->set('url.prefixes', ['en' => 'en', 'l0' => 'l0'])
    118 ->save();
    119
    120 \Drupal::service('kernel')->rebuildContainer();
    121 $this->languageManager = $this->container->get('language_manager');
    122 $this->languageManager->reset();
    123 // End of code essentially from EntityUrlLanguageTest::setUp().
  • quietone
    quietone @quietone started a thread on the diff
  • 109 ]);
    110 $this->display->save();
    111 // End of code from ImageFormatterTest::setUp().
    112 // Chunk of code essentially taken from EntityUrlLanguageTest::setUp().
    113 \Drupal::service('router.builder')->rebuild();
    114 // In order to reflect the changes for a multilingual site in the container
    115 // we have to rebuild it.
    116 $config = $this->config('language.negotiation');
    117 $config->set('url.prefixes', ['en' => 'en', 'l0' => 'l0'])
    118 ->save();
    119
    120 \Drupal::service('kernel')->rebuildContainer();
    121 $this->languageManager = $this->container->get('language_manager');
    122 $this->languageManager->reset();
    123 // End of code essentially from EntityUrlLanguageTest::setUp().
    124 // Create our entities.
  • quietone
    quietone @quietone started a thread on the diff
  • 117 $config->set('url.prefixes', ['en' => 'en', 'l0' => 'l0'])
    118 ->save();
    119
    120 \Drupal::service('kernel')->rebuildContainer();
    121 $this->languageManager = $this->container->get('language_manager');
    122 $this->languageManager->reset();
    123 // End of code essentially from EntityUrlLanguageTest::setUp().
    124 // Create our entities.
    125 $this->entity = EntityTest::create([
    126 'name' => $this->randomMachineName(),
    127 ]);
    128 $this->entity->{'field_untranslatable_image'}->generateSampleItems(2);
    129 $this->entity->{'field_translatable_image'}->generateSampleItems(2);
    130 $this->entity->save();
    131
    132 $this->translation = $this->entity->addTranslation('l0', ['name' => 'Translated Entity', 'field_translatable_image' => $this->entity->get('field_translatable_image')->getValue()]);
  • quietone
    quietone @quietone started a thread on the diff
  • 124 // Create our entities.
    125 $this->entity = EntityTest::create([
    126 'name' => $this->randomMachineName(),
    127 ]);
    128 $this->entity->{'field_untranslatable_image'}->generateSampleItems(2);
    129 $this->entity->{'field_translatable_image'}->generateSampleItems(2);
    130 $this->entity->save();
    131
    132 $this->translation = $this->entity->addTranslation('l0', ['name' => 'Translated Entity', 'field_translatable_image' => $this->entity->get('field_translatable_image')->getValue()]);
    133 $this->translation->save();
    134 }
    135
    136 /**
    137 * Render translatable field in default language.
    138 */
    139 public function testDefaultTranslatableImage(): void {
    • There are four near identical test here. It looks like this should use a dataprovider. But then, this is testing a single entity and the setup is always the same so I think that all 4 assertions can be in a single test.

    • Please register or sign in to reply
  • quietone
    quietone @quietone started a thread on the diff
  • 130 $this->entity->save();
    131
    132 $this->translation = $this->entity->addTranslation('l0', ['name' => 'Translated Entity', 'field_translatable_image' => $this->entity->get('field_translatable_image')->getValue()]);
    133 $this->translation->save();
    134 }
    135
    136 /**
    137 * Render translatable field in default language.
    138 */
    139 public function testDefaultTranslatableImage(): void {
    140 $this->display
    141 ->setComponent('field_translatable_image', [
    142 'settings' => ['image_link' => 'content'],
    143 ])
    144 ->save();
    145 $this->assertUrlRenderedInCorrectLanguage($this->entity, 'field_translatable_image');
    • Suggested change
      145 $this->assertUrlRenderedInCorrectLanguage($this->entity, 'field_translatable_image');
      145 $this->assertUrlRenderedLanguage($this->entity, 'field_translatable_image');
    • Please register or sign in to reply
  • quietone
    quietone @quietone started a thread on the diff
  • 199 $expected_url_string = "/{$entity_test->language()->getId()}/entity_test/{$entity_test->id()}";
    200 $this->assertEquals($entity_test->toUrl()->toString(), $expected_url_string);
    201
    202 // Override the language.
    203 \Drupal::service('language.default')->set($entity_test->language());
    204 \Drupal::languageManager()->reset();
    205 \Drupal::languageManager()->getCurrentLanguage();
    206
    207 // Now we actually start testing the formatter.
    208 /** @var \Drupal\Core\Render\RendererInterface $renderer */
    209 $renderer = $this->container->get('renderer');
    210
    211 $build = $this->display->build($entity_test);
    212
    213 $output = $renderer->renderRoot($build[$field_name][0]);
    214 $this->assertStringContainsString('<a href="' . $entity_test->toUrl()->toString(), (string) $output);
    • This can be more robust. The expected url should be passed to this method, such as "en/entity_test/1". For example, toUrl()->toString() is also used to generate the rendered output.

    • Please register or sign in to reply
  • quietone
    quietone @quietone started a thread on the diff
  • 170 }
    171
    172 /**
    173 * Render untranslatable field on translated entity.
    174 */
    175 public function testTranslatedUntranslatableImage(): void {
    176 $this->display
    177 ->setComponent('field_untranslatable_image', [
    178 'settings' => ['image_link' => 'content'],
    179 ])
    180 ->save();
    181 $this->assertUrlRenderedInCorrectLanguage($this->translation, 'field_untranslatable_image');
    182 }
    183
    184 /**
    185 * Modeled after ImageFormatterTest::testImageFormatterUrlOptions().
    • This should be begin with a verb and describe this method.

      Suggested change
      185 * Modeled after ImageFormatterTest::testImageFormatterUrlOptions().
      185 * Asserts the language of a rendered image URL.

      Is that sufficient?

    • Please register or sign in to reply
  • quietone
    quietone @quietone started a thread on the diff
  • 175 public function testTranslatedUntranslatableImage(): void {
    176 $this->display
    177 ->setComponent('field_untranslatable_image', [
    178 'settings' => ['image_link' => 'content'],
    179 ])
    180 ->save();
    181 $this->assertUrlRenderedInCorrectLanguage($this->translation, 'field_untranslatable_image');
    182 }
    183
    184 /**
    185 * Modeled after ImageFormatterTest::testImageFormatterUrlOptions().
    186 *
    187 * Consider an entity with an image field. We render the entity in some
    188 * language. The image field uses the image formatter with a link
    189 * to the content. We are checking that the link goes to the content
    190 * in the appropriate language.
  • quietone
    quietone @quietone started a thread on the diff
  • 183
    184 /**
    185 * Modeled after ImageFormatterTest::testImageFormatterUrlOptions().
    186 *
    187 * Consider an entity with an image field. We render the entity in some
    188 * language. The image field uses the image formatter with a link
    189 * to the content. We are checking that the link goes to the content
    190 * in the appropriate language.
    191 *
    192 * @param \Drupal\entity_test\Entity\EntityTest $entity_test
    193 * A test entity in some language.
    194 * @param string $field_name
    195 * The machine name of the image field being rendered.
    196 */
    197 protected function assertUrlRenderedInCorrectLanguage(EntityTest $entity_test, string $field_name): void {
    198 // First, test that the toUrl() function is behaving as expected.
  • quietone
    quietone @quietone started a thread on the diff
  • 190 * in the appropriate language.
    191 *
    192 * @param \Drupal\entity_test\Entity\EntityTest $entity_test
    193 * A test entity in some language.
    194 * @param string $field_name
    195 * The machine name of the image field being rendered.
    196 */
    197 protected function assertUrlRenderedInCorrectLanguage(EntityTest $entity_test, string $field_name): void {
    198 // First, test that the toUrl() function is behaving as expected.
    199 $expected_url_string = "/{$entity_test->language()->getId()}/entity_test/{$entity_test->id()}";
    200 $this->assertEquals($entity_test->toUrl()->toString(), $expected_url_string);
    201
    202 // Override the language.
    203 \Drupal::service('language.default')->set($entity_test->language());
    204 \Drupal::languageManager()->reset();
    205 \Drupal::languageManager()->getCurrentLanguage();
  • quietone
    quietone @quietone started a thread on the diff
  • 105 ])
    106 ->setComponent('field_translatable_image', [
    107 'type' => 'image',
    108 'label' => 'hidden',
    109 ]);
    110 $this->display->save();
    111 // End of code from ImageFormatterTest::setUp().
    112 // Chunk of code essentially taken from EntityUrlLanguageTest::setUp().
    113 \Drupal::service('router.builder')->rebuild();
    114 // In order to reflect the changes for a multilingual site in the container
    115 // we have to rebuild it.
    116 $config = $this->config('language.negotiation');
    117 $config->set('url.prefixes', ['en' => 'en', 'l0' => 'l0'])
    118 ->save();
    119
    120 \Drupal::service('kernel')->rebuildContainer();
    • Comment on lines +114 to +120
      Suggested change
      114 // In order to reflect the changes for a multilingual site in the container
      115 // we have to rebuild it.
      116 $config = $this->config('language.negotiation');
      117 $config->set('url.prefixes', ['en' => 'en', 'l0' => 'l0'])
      118 ->save();
      119
      120 \Drupal::service('kernel')->rebuildContainer();
      114
      115 // In order to reflect the changes for a multilingual site in the container
      116 // we have to rebuild it.
      117 $config = $this->config('language.negotiation');
      118 $config->set('url.prefixes', ['en' => 'en', 'l0' => 'l0'])
      119 ->save();
      120 \Drupal::service('kernel')->rebuildContainer();
    • Please register or sign in to reply
  • quietone
    quietone @quietone started a thread on the diff
  • 114 // In order to reflect the changes for a multilingual site in the container
    115 // we have to rebuild it.
    116 $config = $this->config('language.negotiation');
    117 $config->set('url.prefixes', ['en' => 'en', 'l0' => 'l0'])
    118 ->save();
    119
    120 \Drupal::service('kernel')->rebuildContainer();
    121 $this->languageManager = $this->container->get('language_manager');
    122 $this->languageManager->reset();
    123 // End of code essentially from EntityUrlLanguageTest::setUp().
    124 // Create our entities.
    125 $this->entity = EntityTest::create([
    126 'name' => $this->randomMachineName(),
    127 ]);
    128 $this->entity->{'field_untranslatable_image'}->generateSampleItems(2);
    129 $this->entity->{'field_translatable_image'}->generateSampleItems(2);
  • quietone
    quietone @quietone started a thread on the diff
  • 189 * to the content. We are checking that the link goes to the content
    190 * in the appropriate language.
    191 *
    192 * @param \Drupal\entity_test\Entity\EntityTest $entity_test
    193 * A test entity in some language.
    194 * @param string $field_name
    195 * The machine name of the image field being rendered.
    196 */
    197 protected function assertUrlRenderedInCorrectLanguage(EntityTest $entity_test, string $field_name): void {
    198 // First, test that the toUrl() function is behaving as expected.
    199 $expected_url_string = "/{$entity_test->language()->getId()}/entity_test/{$entity_test->id()}";
    200 $this->assertEquals($entity_test->toUrl()->toString(), $expected_url_string);
    201
    202 // Override the language.
    203 \Drupal::service('language.default')->set($entity_test->language());
    204 \Drupal::languageManager()->reset();
    Please register or sign in to reply
    Loading