From dfebb479646e30c44cadb101c9bb50cf56561241 Mon Sep 17 00:00:00 2001 From: Bernd Oliver Suenderhauf <bos@suenderhauf.de> Date: Mon, 20 May 2019 17:13:47 +0200 Subject: [PATCH] Issue #3055097 by Pancho: Arrays of FillPdfFormFields should always be keyed by pdf_key --- src/Controller/HandlePdfController.php | 3 +-- src/Entity/FillPdfForm.php | 8 +++++- src/FillPdfFormInterface.php | 2 +- src/Form/FillPdfFormForm.php | 2 +- src/Serializer.php | 26 +++++++++++++++---- src/SerializerInterface.php | 15 ----------- .../Functional/FillPdfFormImportFormTest.php | 8 +++--- tests/src/Functional/PdfPopulationTest.php | 8 +++--- .../Functional/PdfWebformPopulationTest.php | 4 +-- 9 files changed, 42 insertions(+), 34 deletions(-) diff --git a/src/Controller/HandlePdfController.php b/src/Controller/HandlePdfController.php index 9a3fa0c..c0382a8 100644 --- a/src/Controller/HandlePdfController.php +++ b/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; } diff --git a/src/Entity/FillPdfForm.php b/src/Entity/FillPdfForm.php index 2d89e19..70977c0 100644 --- a/src/Entity/FillPdfForm.php +++ b/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; } /** diff --git a/src/FillPdfFormInterface.php b/src/FillPdfFormInterface.php index b5fbd9c..bdf8585 100644 --- a/src/FillPdfFormInterface.php +++ b/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(); diff --git a/src/Form/FillPdfFormForm.php b/src/Form/FillPdfFormForm.php index 95e6071..832958a 100644 --- a/src/Form/FillPdfFormForm.php +++ b/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:"); } diff --git a/src/Serializer.php b/src/Serializer.php index 2c0d841..002e396 100644 --- a/src/Serializer.php +++ b/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); } diff --git a/src/SerializerInterface.php b/src/SerializerInterface.php index 901b85d..e4809d9 100644 --- a/src/SerializerInterface.php +++ b/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 = []); - } diff --git a/tests/src/Functional/FillPdfFormImportFormTest.php b/tests/src/Functional/FillPdfFormImportFormTest.php index 3782d53..045bbcc 100644 --- a/tests/src/Functional/FillPdfFormImportFormTest.php +++ b/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'); } diff --git a/tests/src/Functional/PdfPopulationTest.php b/tests/src/Functional/PdfPopulationTest.php index ec2ac0c..2590efe 100644 --- a/tests/src/Functional/PdfPopulationTest.php +++ b/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]'; diff --git a/tests/src/Functional/PdfWebformPopulationTest.php b/tests/src/Functional/PdfWebformPopulationTest.php index 629a7d7..9ad698b 100644 --- a/tests/src/Functional/PdfWebformPopulationTest.php +++ b/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; -- GitLab