From 6165bb1503bfdbbdcbb6623ea814bc034ef43c1b Mon Sep 17 00:00:00 2001
From: Kirill Roskolii <kirill.roskolii@catalyst.net.nz>
Date: Mon, 24 Feb 2025 10:50:47 +1300
Subject: [PATCH 1/4] Support multi-valued fields
---
phpstan-baseline.neon | 6 -
src/DiffEntityComparison.php | 10 +-
src/Plugin/diff/Field/ImageFieldBuilder.php | 27 +++--
tests/src/Functional/DiffPluginFileTest.php | 123 ++++++++++++++------
4 files changed, 110 insertions(+), 56 deletions(-)
diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon
index 6c8a18c..45a349d 100644
--- a/phpstan-baseline.neon
+++ b/phpstan-baseline.neon
@@ -345,11 +345,6 @@ parameters:
count: 1
path: src/Plugin/diff/Field/ImageFieldBuilder.php
- -
- message: "#^Implicit array creation is not allowed \\- variable \\$image_style might not exist\\.$#"
- count: 1
- path: src/Plugin/diff/Field/ImageFieldBuilder.php
-
-
message: "#^Method Drupal\\\\diff\\\\Plugin\\\\diff\\\\Field\\\\ImageFieldBuilder\\:\\:build\\(\\) has parameter \\$field_items with generic interface Drupal\\\\Core\\\\Field\\\\FieldItemListInterface but does not specify its types\\: T$#"
count: 1
@@ -575,4 +570,3 @@ parameters:
message: "#^Property Drupal\\\\Tests\\\\diff\\\\Unit\\\\VisualDiffThemeNegotiatorTest\\:\\:\\$configFactory with generic interface Prophecy\\\\Prophecy\\\\ProphecyInterface does not specify its types\\: T$#"
count: 1
path: tests/src/Unit/VisualDiffThemeNegotiatorTest.php
-
diff --git a/src/DiffEntityComparison.php b/src/DiffEntityComparison.php
index c8c7a03..25668a9 100644
--- a/src/DiffEntityComparison.php
+++ b/src/DiffEntityComparison.php
@@ -116,19 +116,17 @@ class DiffEntityComparison {
$value = $left_values[$delta];
if (isset($value['#thumbnail'])) {
$result['#left_thumbnail'][] = $value['#thumbnail'];
+ $value = $value['data'];
}
- else {
- $result['#left'][] = \is_array($value) ? \implode("\n", $value) : $value;
- }
+ $result['#left'][] = \is_array($value) ? \implode("\n", $value) : $value;
}
if (isset($right_values[$delta])) {
$value = $right_values[$delta];
if (isset($value['#thumbnail'])) {
$result['#right_thumbnail'][] = $value['#thumbnail'];
+ $value = $value['data'];
}
- else {
- $result['#right'][] = \is_array($value) ? \implode("\n", $value) : $value;
- }
+ $result['#right'][] = \is_array($value) ? \implode("\n", $value) : $value;
}
}
diff --git a/src/Plugin/diff/Field/ImageFieldBuilder.php b/src/Plugin/diff/Field/ImageFieldBuilder.php
index 24b920d..b9264bd 100644
--- a/src/Plugin/diff/Field/ImageFieldBuilder.php
+++ b/src/Plugin/diff/Field/ImageFieldBuilder.php
@@ -30,13 +30,14 @@ class ImageFieldBuilder extends FieldDiffBuilderBase {
// Every item from $field_items is of type FieldItemInterface.
foreach ($field_items as $field_key => $field_item) {
if (!$field_item->isEmpty()) {
+ $item_data = [];
$values = $field_item->getValue();
// Compare file names.
if (isset($values['target_id'])) {
/** @var \Drupal\file\Entity\File $image */
$image = $fileManager->load($values['target_id']);
- $result[$field_key][] = $this->t('Image: @image', [
+ $item_data[] = (string) $this->t('Image: @image', [
'@image' => $image->getFilename(),
]);
}
@@ -44,7 +45,7 @@ class ImageFieldBuilder extends FieldDiffBuilderBase {
// Compare Alt fields.
if ($this->configuration['compare_alt_field']) {
if (isset($values['alt'])) {
- $result[$field_key][] = $this->t('Alt: @alt', [
+ $item_data[] = (string) $this->t('Alt: @alt', [
'@alt' => $values['alt'],
]);
}
@@ -53,7 +54,7 @@ class ImageFieldBuilder extends FieldDiffBuilderBase {
// Compare Title fields.
if ($this->configuration['compare_title_field']) {
if (!empty($values['title'])) {
- $result[$field_key][] = $this->t('Title: @title', [
+ $item_data[] = (string) $this->t('Title: @title', [
'@title' => $values['title'],
]);
}
@@ -62,32 +63,38 @@ class ImageFieldBuilder extends FieldDiffBuilderBase {
// Compare file id.
if ($this->configuration['show_id']) {
if (isset($values['target_id'])) {
- $result[$field_key][] = $this->t('File ID: @fid', [
+ $item_data[] = (string) $this->t('File ID: @fid', [
'@fid' => $values['target_id'],
]);
}
}
- $separator = $this->configuration['property_separator'] == 'nl' ? "\n" : $this->configuration['property_separator'];
- $result[$field_key] = \implode($separator, $result[$field_key]);
-
// EXPERIMENTAL: Attach thumbnail image data.
if ($this->configuration['show_thumbnail']) {
if (isset($values['target_id'])) {
$storage = $this->entityTypeManager->getStorage('entity_form_display');
$display = $storage->load($field_items->getFieldDefinition()->getTargetEntityTypeId() . '.' . $field_items->getEntity()->bundle() . '.default');
if ($image_field = $display->getComponent($field_item->getFieldDefinition()->getName())) {
+ /** @var \Drupal\file\Entity\File $image */
$image = $fileManager->load($values['target_id']);
-
- $image_style[$field_key]['#thumbnail'] = [
+ $thumbnail = [
'#theme' => 'image_style',
'#uri' => $image->getFileUri(),
'#style_name' => $image_field['settings']['preview_image_style'],
];
- $result = \array_merge($result, $image_style);
}
}
}
+
+ $separator = $this->configuration['property_separator'] == 'nl' ? "\n" : $this->configuration['property_separator'];
+ $properties = \implode($separator, $item_data);
+ if (isset($thumbnail)) {
+ $result[$field_key]['#thumbnail'] = $thumbnail;
+ $result[$field_key]['data'] = $properties;
+ }
+ else {
+ $result[$field_key] = $properties;
+ }
}
}
diff --git a/tests/src/Functional/DiffPluginFileTest.php b/tests/src/Functional/DiffPluginFileTest.php
index 40f40b4..1d5eeff 100644
--- a/tests/src/Functional/DiffPluginFileTest.php
+++ b/tests/src/Functional/DiffPluginFileTest.php
@@ -126,44 +126,13 @@ class DiffPluginFileTest extends DiffPluginTestBase {
}
/**
- * Tests the Image plugin.
+ * Tests the Image plugin for singe value fields.
*
* @see \Drupal\diff\Plugin\diff\Field\ImageFieldBuilder
*/
- public function testImagePlugin(): void {
+ public function testImagePluginSingleValue(): void {
// Add image field to the article content type.
- $image_field_name = 'field_image';
- FieldStorageConfig::create([
- 'field_name' => $image_field_name,
- 'entity_type' => 'node',
- 'type' => 'image',
- 'settings' => [],
- 'cardinality' => 1,
- ])->save();
-
- $field_config = FieldConfig::create([
- 'field_name' => $image_field_name,
- 'label' => 'Image',
- 'entity_type' => 'node',
- 'bundle' => 'article',
- 'required' => FALSE,
- 'settings' => ['alt_field' => 1],
- ]);
- $field_config->save();
-
- $this->formDisplay->load('node.article.default')
- ->setComponent($image_field_name, [
- 'type' => 'image_image',
- 'settings' => [],
- ])
- ->save();
-
- $this->viewDisplay->load('node.article.default')
- ->setComponent($image_field_name, [
- 'type' => 'image',
- 'settings' => [],
- ])
- ->save();
+ $this->setUpImageField('field_image');
// Create an article.
$node = $this->drupalCreateNode([
@@ -296,4 +265,90 @@ class DiffPluginFileTest extends DiffPluginTestBase {
$this->assertSession()->responseNotContains($image_url);
}
+ /**
+ * Tests the Image plugin for multi value fields.
+ *
+ * @see \Drupal\diff\Plugin\diff\Field\ImageFieldBuilder
+ */
+ public function testImagePluginMultiValue(): void {
+ // Add image field to the article content type.
+ $this->setUpImageField('field_image', 10);
+
+ // Create an article.
+ $node = $this->drupalCreateNode([
+ 'type' => 'article',
+ 'title' => 'Test article',
+ ]);
+
+ // Upload an image to the article.
+ $test_files = $this->drupalGetTestFiles('image');
+ $edit = ['files[field_image_0][]' => $this->fileSystem->realpath($test_files['1']->uri)];
+ $this->drupalPostNodeForm('node/' . $node->id() . '/edit', $edit, 'Save');
+ $edit = [
+ 'field_image[0][alt]' => 'Image alt',
+ 'revision' => TRUE,
+ ];
+ $this->submitForm($edit, 'Save');
+ $node = $this->drupalGetNodeByTitle('Test article', TRUE);
+
+ // Upload new image to the article.
+ $edit = ['files[field_image_1][]' => $this->fileSystem->realpath($test_files['2']->uri)];
+ $this->drupalPostNodeForm('node/' . $node->id() . '/edit', $edit, 'Save');
+ $edit = [
+ 'field_image[1][alt]' => 'Image two alt',
+ 'revision' => TRUE,
+ ];
+ $this->submitForm($edit, 'Save');
+
+ $this->drupalGet('node/' . $node->id() . '/revisions');
+ $this->submitForm([], 'Compare selected revisions');
+ // Assert alt text of both images are visible, and only once.
+ $this->assertSession()->pageTextMatchesCount(1, 'Alt: Image alt');
+ $this->assertSession()->pageTextMatchesCount(1, 'Alt: Image two alt');
+ }
+
+ /**
+ * Sets image with on the "article".
+ *
+ * @param string $field_name
+ * @param int $cardinality
+ * @param string $label
+ *
+ * @return void
+ * @throws \Drupal\Core\Entity\EntityStorageException
+ */
+ protected function setUpImageField(string $field_name, int $cardinality = 1, string $label = 'Image'): void {
+ FieldStorageConfig::create([
+ 'field_name' => $field_name,
+ 'entity_type' => 'node',
+ 'type' => 'image',
+ 'settings' => [],
+ 'cardinality' => $cardinality,
+ ])->save();
+
+ $field_config = FieldConfig::create([
+ 'field_name' => $field_name,
+ 'label' => $label,
+ 'entity_type' => 'node',
+ 'bundle' => 'article',
+ 'required' => FALSE,
+ 'settings' => ['alt_field' => 1],
+ ]);
+ $field_config->save();
+
+ $this->formDisplay->load('node.article.default')
+ ->setComponent($field_name, [
+ 'type' => 'image_image',
+ 'settings' => [],
+ ])
+ ->save();
+
+ $this->viewDisplay->load('node.article.default')
+ ->setComponent($field_name, [
+ 'type' => 'image',
+ 'settings' => [],
+ ])
+ ->save();
+ }
+
}
--
GitLab
From 2c27a2c57fcbb5a7a0de7f4863369a450830dd33 Mon Sep 17 00:00:00 2001
From: Kirill Roskolii <kirill.roskolii@catalyst.net.nz>
Date: Tue, 25 Feb 2025 11:20:54 +1300
Subject: [PATCH 2/4] Code style
---
tests/src/Functional/DiffPluginFileTest.php | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/src/Functional/DiffPluginFileTest.php b/tests/src/Functional/DiffPluginFileTest.php
index 1d5eeff..a5fd4c8 100644
--- a/tests/src/Functional/DiffPluginFileTest.php
+++ b/tests/src/Functional/DiffPluginFileTest.php
@@ -311,11 +311,11 @@ class DiffPluginFileTest extends DiffPluginTestBase {
* Sets image with on the "article".
*
* @param string $field_name
+ * Field name.
* @param int $cardinality
+ * Field cardinality.
* @param string $label
- *
- * @return void
- * @throws \Drupal\Core\Entity\EntityStorageException
+ * Field label.
*/
protected function setUpImageField(string $field_name, int $cardinality = 1, string $label = 'Image'): void {
FieldStorageConfig::create([
--
GitLab
From e503d511fb0dfb69cff1ead34a33be942e6665c6 Mon Sep 17 00:00:00 2001
From: Kirill Roskolii <kirill.roskolii@catalyst.net.nz>
Date: Tue, 25 Feb 2025 11:22:40 +1300
Subject: [PATCH 3/4] Format correctly
---
tests/src/Functional/DiffPluginFileTest.php | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/src/Functional/DiffPluginFileTest.php b/tests/src/Functional/DiffPluginFileTest.php
index a5fd4c8..02da470 100644
--- a/tests/src/Functional/DiffPluginFileTest.php
+++ b/tests/src/Functional/DiffPluginFileTest.php
@@ -303,8 +303,8 @@ class DiffPluginFileTest extends DiffPluginTestBase {
$this->drupalGet('node/' . $node->id() . '/revisions');
$this->submitForm([], 'Compare selected revisions');
// Assert alt text of both images are visible, and only once.
- $this->assertSession()->pageTextMatchesCount(1, 'Alt: Image alt');
- $this->assertSession()->pageTextMatchesCount(1, 'Alt: Image two alt');
+ $this->assertSession()->pageTextMatchesCount(1, '/Alt: Image alt/');
+ $this->assertSession()->pageTextMatchesCount(1, '/Alt: Image two alt/');
}
/**
--
GitLab
From 858a30fc344892bb21a0c908ef70743e3e9908e8 Mon Sep 17 00:00:00 2001
From: Kirill Roskolii <kirill.roskolii@catalyst.net.nz>
Date: Tue, 25 Feb 2025 11:39:41 +1300
Subject: [PATCH 4/4] Re-structure test
---
tests/src/Functional/DiffPluginFileTest.php | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/tests/src/Functional/DiffPluginFileTest.php b/tests/src/Functional/DiffPluginFileTest.php
index 02da470..9c6d120 100644
--- a/tests/src/Functional/DiffPluginFileTest.php
+++ b/tests/src/Functional/DiffPluginFileTest.php
@@ -280,7 +280,7 @@ class DiffPluginFileTest extends DiffPluginTestBase {
'title' => 'Test article',
]);
- // Upload an image to the article.
+ // Upload first image to the article.
$test_files = $this->drupalGetTestFiles('image');
$edit = ['files[field_image_0][]' => $this->fileSystem->realpath($test_files['1']->uri)];
$this->drupalPostNodeForm('node/' . $node->id() . '/edit', $edit, 'Save');
@@ -289,7 +289,6 @@ class DiffPluginFileTest extends DiffPluginTestBase {
'revision' => TRUE,
];
$this->submitForm($edit, 'Save');
- $node = $this->drupalGetNodeByTitle('Test article', TRUE);
// Upload new image to the article.
$edit = ['files[field_image_1][]' => $this->fileSystem->realpath($test_files['2']->uri)];
@@ -302,8 +301,10 @@ class DiffPluginFileTest extends DiffPluginTestBase {
$this->drupalGet('node/' . $node->id() . '/revisions');
$this->submitForm([], 'Compare selected revisions');
- // Assert alt text of both images are visible, and only once.
- $this->assertSession()->pageTextMatchesCount(1, '/Alt: Image alt/');
+ // Assert alt text of both images are visible. First image added in previous
+ // revision would have its alt text visible twice on both sides as context.
+ // Alt text of the second image should only be present once on right.
+ $this->assertSession()->pageTextMatchesCount(2, '/Alt: Image alt/');
$this->assertSession()->pageTextMatchesCount(1, '/Alt: Image two alt/');
}
--
GitLab