Issue #3055097 by Pancho: Arrays of FillPdfFormFields should always be keyed by pdf_key

src/Controller/HandlePdfController.php

@@ -141,8 +141,7 @@ class HandlePdfController extends ControllerBase {
 
     $mapped_fields = &$field_mapping['fields'];
     $image_data = &$field_mapping['images'];
-    foreach ($fields as $field) {
-      $pdf_key = $field->pdf_key->value;
+    foreach ($fields as $pdf_key => $field) {
       if ($context['sample']) {
         $mapped_fields[$pdf_key] = $pdf_key;
       }

src/Entity/FillPdfForm.php

@@ -169,7 +169,13 @@ class FillPdfForm extends ContentEntityBase implements FillPdfFormInterface {
       ->condition('fillpdf_form', $this->id())
       ->execute();
     $field_storage = \Drupal::entityTypeManager()->getStorage('fillpdf_form_field');
-    return $field_storage->loadMultiple($field_ids);
+    $fields = $field_storage->loadMultiple($field_ids);
+
+    $keyed_fields = [];
+    foreach ($fields as $field) {
+      $keyed_fields[$field->pdf_key->value] = $field;
+    }
+    return $keyed_fields;
   }
 
   /**

src/FillPdfFormInterface.php

@@ -13,7 +13,7 @@ interface FillPdfFormInterface extends ExportableContentEntityInterface {
    * Returns all FillPdfFormFields associated with this FillPdfForm.
    *
    * @return \Drupal\fillpdf\FillPdfFormFieldInterface[]
-   *   An array of FillPdfFormFields.
+   *   Associative array of FillPdfFormFields keyed by the pdf_key.
    */
   public function getFormFields();
 

src/Form/FillPdfFormForm.php

@@ -442,7 +442,7 @@ class FillPdfFormForm extends ContentEntityForm {
       $message[] = $this->t('Your previous field mappings have been transferred to the new PDF template you uploaded.');
 
       // Import previous form field values over new fields.
-      $non_matching_fields = $this->serializer->importFormFieldsByKey($existing_fields, $form_fields);
+      $non_matching_fields = $this->serializer->importFormFields($existing_fields, $form_fields);
       if (count($non_matching_fields)) {
         $message[] = $this->t("These keys couldn't be found in the new PDF:");
       }

src/Serializer.php

@@ -68,6 +68,9 @@ class Serializer implements SerializerInterface {
 
     foreach ($field_json as $normalized_field) {
       $field = $this->serializer->denormalize($normalized_field, 'Drupal\fillpdf\Entity\FillPdfFormField');
+      // @todo: Exported fields are now already keyed by PDF key. For now, we're
+      //   not using the array keys to remain compatible with previous exports,
+      //   but should do so that at some later point.
       $decoded_fields[$field->pdf_key->value] = $field;
     }
 
@@ -98,8 +101,8 @@ class Serializer implements SerializerInterface {
    */
   public function importFormFields(array $keyed_fields, array $existing_fields = []) {
     $existing_fields_by_key = [];
-    foreach ($existing_fields as $existing_field) {
-      $existing_fields_by_key[$existing_field->pdf_key->value] = $existing_field;
+    foreach ($existing_fields as $pdf_key => $existing_field) {
+      $existing_fields_by_key[$pdf_key] = $existing_field;
     }
 
     $existing_field_pdf_keys = array_keys($existing_fields_by_key);
@@ -125,15 +128,28 @@ class Serializer implements SerializerInterface {
   }
 
   /**
-   * {@inheritdoc}
+   * Overwrites empty new field values with previous existing values.
+   *
+   * @param \Drupal\fillpdf\FillPdfFormFieldInterface[] $form_fields
+   *   Associative array of saved FillPdfFormField objects keyed by entity ID.
+   * @param string[] $existing_fields
+   *   (optional) Array of existing PDF keys.
+   *
+   * @return string[]
+   *   Array of unmatched PDF keys.
+   *
+   * @deprecated in fillpdf:8.x-4.7 and will be removed from fillpdf:8.x-5.0.
+   *   Field lists are already keyed by pdf_key now, so rekeying them is
+   *   unnecessary. Use ::importFormFields instead.
+   * @see https://www.drupal.org/project/fillpdf/issues/3055097
+   * @see \Drupal\fillpdf\SerializerInterface::importFormFields()
    */
   public function importFormFieldsByKey(array $form_fields, array $existing_fields = []) {
-    // Key form fields by PDF key, then pass to ::importFormFields().
+    @trigger_error('SerializerInterface::importFormFieldsByKey() is deprecated in fillpdf:8.x-4.7 and will be removed before fillpdf:8.x-5.0. Use \Drupal\fillpdf\SerializerInterface::importFormFields() instead. See https://www.drupal.org/project/fillpdf/issues/3055097.', E_USER_DEPRECATED);
     $keyed_fields = [];
     foreach ($form_fields as $form_field) {
       $keyed_fields[$form_field->pdf_key->value] = $form_field;
     }
-
     return $this->importFormFields($keyed_fields, $existing_fields);
   }
 

src/SerializerInterface.php

@@ -64,19 +64,4 @@ interface SerializerInterface {
    */
   public function importFormFields(array $keyed_fields, array $existing_fields = []);
 
-  /**
-   * Overwrites empty new field values with previous existing values.
-   *
-   * @param \Drupal\fillpdf\FillPdfFormFieldInterface[] $form_fields
-   *   Associative array of saved FillPdfFormField objects keyed by entity ID.
-   * @param string[] $existing_fields
-   *   (optional) Array of existing PDF keys.
-   *
-   * @return string[]
-   *   Array of unmatched PDF keys.
-   *
-   * @see \Drupal\fillpdf\SerializerInterface::importFormFields()
-   */
-  public function importFormFieldsByKey(array $form_fields, array $existing_fields = []);
-
 }

tests/src/Functional/FillPdfFormImportFormTest.php

@@ -2,8 +2,6 @@
 
 namespace Drupal\Tests\fillpdf\Functional;
 
-use Drupal\fillpdf\Entity\FillPdfForm;
-use Drupal\fillpdf\Entity\FillPdfFormField;
 use Drupal\Tests\BrowserTestBase;
 use Drupal\Tests\fillpdf\Traits\TestFillPdfTrait;
 use Drupal\Core\Url;
@@ -63,9 +61,13 @@ class FillPdfFormImportFormTest extends BrowserTestBase {
     $this->drupalPostForm(Url::fromRoute('entity.fillpdf_form.edit_form', ['fillpdf_form' => $form2_id]), $edit, 'Save');
     $this->assertSession()->pageTextContains("FillPDF Form Duplicate of Test has been updated.");
 
-    // Import again and check the admin_title has been imported as well.
+    // Import again.
     $import_url = Url::fromRoute('entity.fillpdf_form.import_form', ['fillpdf_form' => $form2_id]);
     $this->drupalPostForm($import_url, ['code' => $code], 'Import');
+    // Check none of the mappings failed.
+    $this->assertSession()->pageTextContains('Successfully imported FillPDF form configuration and matching PDF field keys.');
+    $this->assertSession()->pageTextNotContains('but it does not exist on this form. Therefore, it was ignored.');
+    // Check the admin_title has been imported as well.
     $this->assertSession()->fieldValueEquals('admin_title[0][value]', 'Test');
     $this->assertSession()->fieldValueEquals('replacements[0][value]', 'y|Yes');
   }

tests/src/Functional/PdfPopulationTest.php

@@ -157,8 +157,8 @@ class PdfPopulationTest extends FillPdfTestBase {
 
     // 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;
+    foreach (array_keys($fillpdf_form->getFormFields()) as $pdf_key) {
+      $actual_keys[] = $pdf_key;
     }
     sort($actual_keys);
 
@@ -220,8 +220,8 @@ class PdfPopulationTest extends FillPdfTestBase {
    *   Array of FillPdfFormFields.
    */
   protected function mapFillPdfFieldsToNodeFields(array $fields) {
-    foreach ($fields as $field) {
-      switch ($field->pdf_key->value) {
+    foreach ($fields as $pdf_key => $field) {
+      switch ($pdf_key) {
         case 'ImageField':
         case 'Button2':
           $field->value = '[node:field_fillpdf_test_image]';

tests/src/Functional/PdfWebformPopulationTest.php

@@ -151,8 +151,8 @@ class PdfWebformPopulationTest extends FillPdfTestBase {
    *   Array of FillPdfFormFields.
    */
   protected function mapFillPdfFieldsToWebformFields(array $fields) {
-    foreach ($fields as $field) {
-      switch ($field->pdf_key->value) {
+    foreach ($fields as $pdf_key => $field) {
+      switch ($pdf_key) {
         case 'ImageField':
           $field->value = '[webform_submission:values:image]';
           break;