Issue #3023330 by Pancho: Consolidate duplicate form field instances

src/InputHelper.php

@@ -93,7 +93,8 @@ class InputHelper implements InputHelperInterface {
           ]
         );
 
-        $form_fields[] = $field;
+        // Use the field name as key, so to consolidate duplicate fields.
+        $form_fields[$arr['name']] = $field;
       }
     }
 

tests/modules/fillpdf_test/src/Plugin/BackendService/Test.php

@@ -72,6 +72,18 @@ class Test extends BackendServiceBase implements ContainerFactoryPluginInterface
     return file_get_contents(drupal_get_path('module', 'fillpdf_test') . '/files/fillpdf_test_v3.pdf');
   }
 
+  /**
+   * Returns a list of fields, as if a PDF file was parsed.
+   *
+   * Note that there is a duplicate field that get consolidated in
+   * InputHelper::attachPdfToForm() at the latest.
+   * The expected number of fields is therefore three, not four.
+   *
+   * @return array
+   *   List of associative arrays representing fields.
+   *
+   * @see \Drupal\fillpdf\InputHelper::attachPdfToForm()
+   */
   public static function getParseResult() {
     return [
       0 => [
@@ -80,7 +92,7 @@ class Test extends BackendServiceBase implements ContainerFactoryPluginInterface
         'type' => 'Pushbutton',
       ],
       1 => [
-        'name' => 'Button',
+        'name' => 'TestButton',
         'value' => '',
         'type' => 'Pushbutton',
       ],
@@ -89,6 +101,11 @@ class Test extends BackendServiceBase implements ContainerFactoryPluginInterface
         'value' => '',
         'type' => 'Text',
       ],
+      3 => [
+        'name' => 'ImageField',
+        'value' => '',
+        'type' => 'Pushbutton',
+      ],
     ];
   }
 

tests/modules/fillpdf_test/src/Plugin/FillPdfBackend/TestFillPdfBackend.php

@@ -81,6 +81,18 @@ class TestFillPdfBackend implements FillPdfBackendPluginInterface, ContainerFact
     return $populated_pdf;
   }
 
+  /**
+   * Returns a list of fields, as if a PDF file was parsed.
+   *
+   * Note that there is a duplicate field that get consolidated in
+   * InputHelper::attachPdfToForm() at the latest.
+   * The expected number of fields is therefore three, not four.
+   *
+   * @return array
+   *   List of associative arrays representing fields.
+   *
+   * @see \Drupal\fillpdf\InputHelper::attachPdfToForm()
+   */
   public static function getParseResult() {
     return [
       0 => [
@@ -89,7 +101,7 @@ class TestFillPdfBackend implements FillPdfBackendPluginInterface, ContainerFact
         'type' => 'Pushbutton',
       ],
       1 => [
-        'name' => 'Button',
+        'name' => 'TestButton',
         'value' => '',
         'type' => 'Pushbutton',
       ],
@@ -98,6 +110,11 @@ class TestFillPdfBackend implements FillPdfBackendPluginInterface, ContainerFact
         'value' => '',
         'type' => 'Text',
       ],
+      3 => [
+        'name' => 'ImageField',
+        'value' => '',
+        'type' => 'Pushbutton',
+      ],
     ];
   }
 

tests/src/Functional/FillPdfUploadTestBase.php

@@ -159,6 +159,13 @@ abstract class FillPdfUploadTestBase extends FileFieldTestBase {
         $this->assertSession()->pageTextContains('New FillPDF form has been created.');
         $this->assertSession()->pageTextNotContains('Your previous field mappings have been transferred to the new PDF template you uploaded.');
 
+        // There should be three fields in the correct order.
+        // @todo: Add some CSS markup to the view so we can test the order.
+        $this->assertSession()->pageTextContainsOnce('ImageField');
+        $this->assertSession()->pageTextContainsOnce('TestButton');
+        $this->assertSession()->pageTextContainsOnce('TextField');
+        $this->assertSession()->elementsCount('css', 'tbody > tr', 3);
+
         // Make sure the file is permanent and correctly placed.
         $this->assertFileIsPermanent($new_file);
         $expected_file_uri = FillPdf::buildFileUri($this->config('fillpdf.settings')->get('template_scheme'), "fillpdf/{$new_filename}");

tests/src/Functional/PdfPopulationTest.php

@@ -104,40 +104,44 @@ class PdfPopulationTest extends FillPdfTestBase {
     $this->uploadTestPdf('fillpdf_test_v3.pdf');
     $fillpdf_form = FillPdfForm::load($this->getLatestFillPdfForm());
 
-    // Get the field definitions for the form that was created and configure
-    // them.
+    // Get the field definitions from the actually created form and sort.
     $actual_keys = [];
     foreach ($fillpdf_form->getFormFields() as $form_field) {
       $actual_keys[] = $form_field->pdf_key->value;
     }
+    sort($actual_keys);
+
+    // Get the fields from the fixture and sort.
     $expected_keys = [];
     foreach (TestFillPdfBackend::getParseResult() as $expected_field) {
       $expected_keys[] = $expected_field['name'];
     }
-    // Sort the arrays before comparing.
     sort($expected_keys);
-    sort($actual_keys);
-    $differences = array_diff($expected_keys, $actual_keys);
 
-    self::assertEmpty($differences, 'Parsed fields and fields in fixture match.');
+    // Now compare. InputHelper::attachPdfToForm() filtered out the duplicate,
+    // so the count differs, but not the actual values.
+    $this->assertCount(4, $expected_keys);
+    $this->assertCount(3, $actual_keys);
+    $differences = array_diff($expected_keys, $actual_keys);
+    self::assertEmpty($differences, 'Parsed fields are in fixture match.');
 
-    // Now create an instance of the backend service.
+    // Now create an instance of the backend service and test directly.
     /** @var \Drupal\fillpdf_test\Plugin\BackendService\Test $backend_service */
     $backend_service = $this->backendServiceManager->createInstance('test');
-
-    // Test the parse method.
     $original_pdf = file_get_contents($this->getTestPdfPath('fillpdf_test_v3.pdf'));
-    $parsed_fields = $backend_service->parse($original_pdf);
+
+    // Get the fields from the backend service and sort.
     $actual_keys = [];
-    foreach ($parsed_fields as $parsed_field) {
+    foreach ($backend_service->parse($original_pdf) as $parsed_field) {
       $actual_keys[] = $parsed_field['name'];
     }
-    // Sort the arrays before comparing.
-    sort($expected_keys);
     sort($actual_keys);
-    $differences = array_diff($expected_keys, $actual_keys);
 
-    self::assertEmpty($differences, 'Parsed fields from plugin and fields in fixture match.');
+    // Compare the values.
+    $this->assertCount(4, $expected_keys);
+    $this->assertCount(4, $actual_keys);
+    $differences = array_diff($expected_keys, $actual_keys);
+    self::assertEmpty($differences, 'Parsed fields from plugin are in fixture match.');
 
     // Test the merge method. We'd normally pass in values for $webform_fields
     // and $options, but since this is a stub anyway, there isn't much point.