Issue #3116806 by wizonesolutions: Updating PDF template does not transfer field mappings

src/Form/FillPdfFormForm.php

@@ -502,6 +502,9 @@ class FillPdfFormForm extends ContentEntityForm {
 
   /**
    * {@inheritdoc}
+   *
+   * @throws \Drupal\Core\Entity\EntityStorageException
+   * @throws \Drupal\Core\Entity\EntityMalformedException
    */
   public function save(array $form, FormStateInterface $form_state) {
     /** @var \Drupal\fillpdf\FillPdfFormInterface $entity */
@@ -521,17 +524,23 @@ class FillPdfFormForm extends ContentEntityForm {
       $entity->file = $new_file;
       $new_fields = $this->inputHelper->parseFields($entity);
 
-      // Save new fields, delete existing ones.
+      // Enrich the new field objects with existing values. Note that we pass
+      // them in seemingly-reverse order since we want to import the EXISTING
+      // fields into the NEW fields.
+      $non_matching_fields = $this->serializer->importFormFields($existing_fields, $new_fields, FALSE);
+
+      // Save new fields.
+      /** @var \Drupal\fillpdf\FillPdfFormFieldInterface $field */
       foreach ($new_fields as $field) {
         $field->save();
       }
+      // Delete existing fields. Importing the new fields saved them.
+      /** @var \Drupal\fillpdf\FillPdfFormFieldInterface $field */
       foreach ($existing_fields as $field) {
         $field->delete();
       }
 
-      $non_matching_fields = array_diff_key($existing_fields, $new_fields);
-
-      if (count($existing_fields) > count($non_matching_fields)) {
+      if (count($existing_fields)) {
         $this->messenger()->addStatus($this->t('Your previous field mappings have been transferred to the new PDF template you uploaded.'));
       }
       if (count($non_matching_fields)) {

src/Serializer.php

@@ -99,9 +99,10 @@ class Serializer implements SerializerInterface {
   /**
    * {@inheritdoc}
    */
-  public function importFormFields(array $imported_fields, array $existing_fields = []) {
+  public function importFormFields(array $keyed_fields, array &$existing_fields = [], $save_existing_fields = TRUE): array {
+    $affected_fields = [];
     $unmatched_pdf_keys = [];
-    foreach ($imported_fields as $pdf_key => $keyed_field) {
+    foreach ($keyed_fields as $pdf_key => $keyed_field) {
       // If the imported field's PDF key matching the PDF key of the
       // existing field, then copy the constituent entity properties.
       if (in_array($pdf_key, array_keys($existing_fields), TRUE)) {
@@ -111,12 +112,16 @@ class Serializer implements SerializerInterface {
             $existing_fields[$pdf_key]->{$keyed_field_name} = $keyed_field_data;
           }
         }
-        $existing_fields[$pdf_key]->save();
+        $affected_fields[] = $pdf_key;
       }
       else {
         $unmatched_pdf_keys[] = $pdf_key;
       }
     }
+    // Save changed fields.
+    foreach ($affected_fields as $pdf_key) {
+      $existing_fields[$pdf_key]->save();
+    }
     return $unmatched_pdf_keys;
   }
 

src/SerializerInterface.php

@@ -10,38 +10,38 @@ namespace Drupal\fillpdf;
 interface SerializerInterface {
 
   /**
-   * Serializes a FillPdfForm for export.
+   * Serializes a FillPDF form for export.
    *
    * @param \Drupal\fillpdf\FillPdfFormInterface $fillpdf_form
-   *   The FillPdf Form to serialize.
+   *   The FillPDF Form to serialize.
    *
    * @return string
-   *   The serialized FillPdfForm.
+   *   The serialized FillPDF form.
    */
   public function getFormExportCode(FillPdfFormInterface $fillpdf_form);
 
   /**
-   * Deserializes a serialized FillPdfForm for import.
+   * Deserializes a serialized FillPDF form for import.
    *
    * @param string $code
-   *   The serialized FillPdfForm.
+   *   The serialized FillPDF form.
    *
    * @return array
-   *   Associative array containing the deserialized FillPdfForm object keyed
-   *   with 'form' and an array of deserialized FillPdfFormField objects keyed
+   *   Associative array containing the deserialized FillPDF form object keyed
+   *   with 'form' and an array of deserialized FillPDF form objects keyed
    *   with 'fields'.
    */
   public function deserializeForm($code);
 
   /**
-   * Imports a FillPdfForm.
+   * Imports a FillPDF form..
    *
    * @param \Drupal\fillpdf\FillPdfFormInterface $fillpdf_form
-   *   The existing FillPdfForm.
+   *   The existing FillPDF form.
    * @param \Drupal\fillpdf\FillPdfFormInterface $imported_form
-   *   The existing FillPdfForm.
+   *   The FillPDF form being imported, usually from import code.
    * @param \Drupal\fillpdf\FillPdfFormFieldInterface[] $imported_fields
-   *   Array of FillPdfFormField objects to import.
+   *   Array of FillPDF form objects to import.
    *
    * @return string[]
    *   Array of unmatched PDF keys.
@@ -49,19 +49,21 @@ interface SerializerInterface {
   public function importForm(FillPdfFormInterface $fillpdf_form, FillPdfFormInterface $imported_form, array $imported_fields);
 
   /**
-   * Imports FillPdfFormFields.
+   * Imports FillPDF form fields.
    *
-   * Overwrites empty field values imported from export code with previous
-   * existing values.
+   * Overwrites empty field values with previous existing field values.
    *
    * @param \Drupal\fillpdf\FillPdfFormFieldInterface[] $keyed_fields
-   *   Associative array of unsaved FillPdfFormField objects keyed by PDF key.
-   * @param string[] $existing_fields
+   *   Associative array of unsaved FillPDF Form objects keyed by PDF key.
+   * @param \Drupal\fillpdf\FillPdfFormFieldInterface[] $existing_fields
    *   (optional) Array of existing PDF keys.
+   * @param bool $save_existing_fields
+   *   Whether to save the form fields in $existing_fields after updating them.
+   *   If you pass FALSE, you will have to save them yourself.
    *
    * @return string[]
    *   Array of unmatched PDF keys.
    */
-  public function importFormFields(array $keyed_fields, array $existing_fields = []);
+  public function importFormFields(array $keyed_fields, array &$existing_fields = [], $save_existing_fields = TRUE): array;
 
 }

tests/src/Functional/FillPdfFormFormTest.php

@@ -106,7 +106,9 @@ class FillPdfFormFormTest extends FillPdfUploadTestBase {
     $this->assertNotUploadTextFile(self::OP_UPLOAD);
     $this->assertNotUploadTextFile(self::OP_SAVE);
     $this->assertUploadPdfFile(self::OP_UPLOAD, TRUE);
-    $this->assertUploadPdfFile(self::OP_SAVE, TRUE);
+    $pdf_fields = $latest_fillpdf_form->getFormFields();
+    FillPdfTestBase::mapFillPdfFieldsToEntityFields('node', $pdf_fields);
+    $this->assertUploadPdfFile(self::OP_SAVE, TRUE, $latest_fillpdf_form);
   }
 
   /**

tests/src/Functional/FillPdfTestBase.php

@@ -54,6 +54,50 @@ abstract class FillPdfTestBase extends FileFieldTestBase {
    */
   protected $testImage;
 
+  /**
+   * Maps FillPdf fields to entity fields.
+   *
+   * @param string $entity_type
+   *   The entity type.
+   * @param \Drupal\fillpdf\Entity\FillPdfFormField[] $fields
+   *   Array of FillPdfFormFields.
+   */
+  public static function mapFillPdfFieldsToEntityFields($entity_type, array $fields) {
+    /** @var \Drupal\token\TokenEntityMapperInterface $token_entity_mapper */
+    $token_entity_mapper = \Drupal::service('token.entity_mapper');
+    $token_type = $token_entity_mapper->getTokenTypeForEntityType($entity_type);
+
+    foreach ($fields as $pdf_key => $field) {
+      switch ($pdf_key) {
+        case 'ImageField':
+        case 'Button2':
+        case 'TestButton':
+          $field->value = "[$token_type:field_fillpdf_test_image]";
+          break;
+
+        case 'TextField1':
+        case 'Text1':
+          $label_key = \Drupal::entityTypeManager()
+            ->getDefinition($entity_type)
+            ->getKey('label');
+          $field->value = "[$token_type:$label_key]";
+          $field->replacements = 'Hello & how are you?|Hello & how are you doing?';
+          break;
+
+        case 'TextField2':
+        case 'Text2':
+          if ($token_type == 'node') {
+            $field->value = '[node:body]';
+          }
+          elseif ($token_type == 'term') {
+            $field->value = '[term:description]';
+          }
+          break;
+      }
+      $field->save();
+    }
+  }
+
   /**
    * {@inheritdoc}
    */

tests/src/Functional/FillPdfUploadTestBase.php

@@ -7,6 +7,7 @@ use Drupal\Component\Version\Constraint;
 use Drupal\file\Entity\File;
 use Drupal\fillpdf\Component\Utility\FillPdf;
 use Drupal\Component\Render\FormattableMarkup;
+use Drupal\fillpdf\FillPdfFormInterface;
 
 /**
  * Allows testing everything around uploading PDF template files.
@@ -82,10 +83,26 @@ abstract class FillPdfUploadTestBase  extends FillPdfTestBase {
    * @param bool $filename_preexists
    *   (optional) Whether the testfile has previously been uploaded, so a file
    *   with the same filename preexists. Defaults to FALSE.
+   * @param \Drupal\fillpdf\FillPdfFormInterface $form
+   *   The FillPDF Form that is being updated. Needed so we can make some
+   *   assertions against the fields.
+   *
+   * @throws \Behat\Mink\Exception\ExpectationException
+   * @throws \Behat\Mink\Exception\ResponseTextException
    */
-  protected function assertUploadPdfFile($op = self::OP_UPLOAD, $filename_preexists = FALSE) {
+  protected function assertUploadPdfFile($op = self::OP_UPLOAD, $filename_preexists = FALSE, FillPdfFormInterface $form = NULL) {
     $previous_file_id = $this->getLastFileId();
 
+    if ($op === self::OP_SAVE) {
+      // Record the mappings in the FillPdfFormFields before overwriting the
+      // file. We may need to compare them later.
+      $existing_fields = $form->getFormFields();
+      $existing_mappings = [];
+      foreach ($existing_fields as $existing_field) {
+        $existing_mappings[$existing_field->pdf_key->value] = $existing_field->value->value;
+      }
+    }
+
     // Upload PDF test file.
     $edit = ['files[upload_pdf]' => $this->getTestPdfPath('fillpdf_test_v3.pdf')];
     $this->drupalPostForm(NULL, $edit, $op);
@@ -161,6 +178,17 @@ abstract class FillPdfUploadTestBase  extends FillPdfTestBase {
         $this->assertFileIsPermanent($new_file);
         $expected_file_uri = FillPdf::buildFileUri($this->config('fillpdf.settings')->get('template_scheme'), "fillpdf/{$new_filename}");
         $this->assertEquals($new_file->getFileUri(), $expected_file_uri);
+
+        $new_fields = $form->getFormFields();
+        $new_mappings = [];
+        foreach ($new_fields as $new_field) {
+          $new_mappings[$new_field->pdf_key->value] = $new_field->value->value;
+        }
+
+        /** @var array $existing_mappings */
+        foreach ($existing_mappings as $field_name => $existing_mapping) {
+          $this->assertEquals($existing_mapping, $new_mappings[$field_name], 'New mapping value matches old mapping value.');
+        }
         break;
     }
   }

tests/src/Functional/PdfPopulationTest.php

@@ -8,10 +8,10 @@ use Drupal\fillpdf\Entity\FillPdfForm;
 use Drupal\fillpdf\FieldMapping\ImageFieldMapping;
 use Drupal\fillpdf\FieldMapping\TextFieldMapping;
 use Drupal\fillpdf_test\Plugin\FillPdfBackend\TestFillPdfBackend;
+use Drupal\Tests\taxonomy\Traits\TaxonomyTestTrait;
 use Drupal\user\Entity\Role;
 
 // When 8.7.x is fully EOL, this can be removed.
-use Drupal\Tests\taxonomy\Traits\TaxonomyTestTrait;
 
 if (!trait_exists('\Drupal\Tests\taxonomy\Traits\TaxonomyTestTrait')) {
   class_alias('\Drupal\Tests\taxonomy\Functional\TaxonomyTestTrait', '\Drupal\Tests\taxonomy\Traits\TaxonomyTestTrait');
@@ -79,7 +79,7 @@ class PdfPopulationTest extends FillPdfTestBase {
 
     // Get the field definitions for the form that was created and configure
     // them.
-    $this->mapFillPdfFieldsToEntityFields('node', $fillpdf_form->getFormFields());
+    FillPdfTestBase::mapFillPdfFieldsToEntityFields('node', $fillpdf_form->getFormFields());
 
     // Create a node to populate the FillPdf Form.
     $node = $this->createNode([
@@ -172,7 +172,7 @@ class PdfPopulationTest extends FillPdfTestBase {
 
       // Get the field definitions for the form that was created and configure
       // them.
-      $this->mapFillPdfFieldsToEntityFields($entity_type, $fillpdf_form->getFormFields());
+      FillPdfTestBase::mapFillPdfFieldsToEntityFields($entity_type, $fillpdf_form->getFormFields());
 
       // Hit the generation route, check results from the test backend plugin.
       $url = $this->linkManipulator->generateLink([
@@ -289,7 +289,7 @@ class PdfPopulationTest extends FillPdfTestBase {
 
     $original_pdf = file_get_contents($this->getTestPdfPath('fillpdf_test_v3.pdf'));
 
-    $this->mapFillPdfFieldsToEntityFields('node', $fillpdf_form->getFormFields());
+    FillPdfTestBase::mapFillPdfFieldsToEntityFields('node', $fillpdf_form->getFormFields());
 
     // Create a node to populate the FillPdf Form.
     // The content of this node is not important; we just need an entity to
@@ -323,48 +323,6 @@ class PdfPopulationTest extends FillPdfTestBase {
     self::assertEquals("PDF form fields don't accept any HTML.\n", $merge_state['field_mapping']['fields']['TextField2']);
   }
 
-  /**
-   * Maps FillPdf fields to entity fields.
-   *
-   * @param string $entity_type
-   *   The entity type.
-   * @param \Drupal\fillpdf\Entity\FillPdfFormField[] $fields
-   *   Array of FillPdfFormFields.
-   */
-  protected function mapFillPdfFieldsToEntityFields($entity_type, array $fields) {
-    /** @var \Drupal\token\TokenEntityMapperInterface $token_entity_mapper */
-    $token_entity_mapper = \Drupal::service('token.entity_mapper');
-    $token_type = $token_entity_mapper->getTokenTypeForEntityType($entity_type);
-
-    foreach ($fields as $pdf_key => $field) {
-      switch ($pdf_key) {
-        case 'ImageField':
-        case 'Button2':
-        case 'TestButton':
-          $field->value = "[$token_type:field_fillpdf_test_image]";
-          break;
-
-        case 'TextField1':
-        case 'Text1':
-          $label_key = \Drupal::entityTypeManager()->getDefinition($entity_type)->getKey('label');
-          $field->value = "[$token_type:$label_key]";
-          $field->replacements = 'Hello & how are you?|Hello & how are you doing?';
-          break;
-
-        case 'TextField2':
-        case 'Text2':
-          if ($token_type == 'node') {
-            $field->value = '[node:body]';
-          }
-          elseif ($token_type == 'term') {
-            $field->value = '[term:description]';
-          }
-          break;
-      }
-      $field->save();
-    }
-  }
-
   /**
    * Tests PDF population using local service.
    *
@@ -418,7 +376,7 @@ class PdfPopulationTest extends FillPdfTestBase {
     // Get the field definitions for the form that was created and configure
     // them.
     $fields = $fillpdf_form->getFormFields();
-    $this->mapFillPdfFieldsToEntityFields('node', $fields);
+    FillPdfTestBase::mapFillPdfFieldsToEntityFields('node', $fields);
 
     // Set up a test node.
     $node = $this->createNode([