From 02e1ce7e4d937cf92ad1d62ca898c4e726743b8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Lys=C3=BD?= Date: Thu, 8 Apr 2021 12:39:12 +0200 Subject: [PATCH] Remove getFields method from FormEntity [MAILPOET-3032] --- lib/API/JSON/v1/Forms.php | 2 +- lib/Entities/FormEntity.php | 51 +++++-------------- .../RequiredCustomFieldValidator.php | 11 ++-- .../SubscriberSubscribeController.php | 9 +++- tests/unit/Entities/FormEntityTest.php | 8 +-- 5 files changed, 33 insertions(+), 48 deletions(-) diff --git a/lib/API/JSON/v1/Forms.php b/lib/API/JSON/v1/Forms.php index 603b9f0324..2e63ff78f7 100644 --- a/lib/API/JSON/v1/Forms.php +++ b/lib/API/JSON/v1/Forms.php @@ -233,7 +233,7 @@ class Forms extends APIEndpoint { } // Check Custom HTML block permissions - $customHtmlBlocks = $formEntity->getBlocksByType(FormEntity::HTML_BLOCK_TYPE); + $customHtmlBlocks = $formEntity->getBlocksByTypes([FormEntity::HTML_BLOCK_TYPE]); if (count($customHtmlBlocks) && !$this->wp->currentUserCan('administrator')) { return $this->errorResponse([ Error::FORBIDDEN => __('Only administrator can edit forms containing Custom HTML block.', 'mailpoet'), diff --git a/lib/Entities/FormEntity.php b/lib/Entities/FormEntity.php index 7a252ff79d..eaeba734a7 100644 --- a/lib/Entities/FormEntity.php +++ b/lib/Entities/FormEntity.php @@ -43,6 +43,16 @@ class FormEntity { const COLUMNS_BLOCK_TYPE = 'columns'; const COLUMN_BLOCK_TYPE = 'column'; + public const FORM_FIELD_TYPES = [ + self::CHECKBOX_BLOCK_TYPE, + self::RADIO_BLOCK_TYPE, + self::SEGMENT_SELECTION_BLOCK_TYPE, + self::DATE_BLOCK_TYPE, + self::SELECT_BLOCK_TYPE, + self::TEXT_BLOCK_TYPE, + self::TEXTAREA_BLOCK_TYPE, + ]; + /** * @ORM\Column(type="string") * @var string @@ -162,24 +172,24 @@ class FormEntity { ]; } - public function getBlocksByType(string $type, array $blocks = null): array { + public function getBlocksByTypes(array $types, array $blocks = null): array { $found = []; if ($blocks === null) { $blocks = $this->getBody() ?? []; } foreach ($blocks as $block) { - if ($block['type'] === $type) { + if (isset($block['type']) && in_array($block['type'], $types, true)) { $found[] = $block; } if (isset($block['body']) && is_array($block['body']) && !empty($block['body'])) { - $found = array_merge($found, $this->getBlocksByType($type, $block['body'])); + $found = array_merge($found, $this->getBlocksByTypes($types, $block['body'])); } } return $found; } public function getSegmentBlocksSegmentIds(): array { - $listSelectionBlocks = $this->getBlocksByType(FormEntity::SEGMENT_SELECTION_BLOCK_TYPE); + $listSelectionBlocks = $this->getBlocksByTypes([FormEntity::SEGMENT_SELECTION_BLOCK_TYPE]); $listSelection = []; foreach ($listSelectionBlocks as $listSelectionBlock) { $listSelection = array_unique( @@ -194,37 +204,4 @@ class FormEntity { public function getSettingsSegmentIds(): array { return $this->settings['segments'] ?? []; } - - public function getFields(array $body = null): array { - $body = $body ?? $this->getBody(); - if (empty($body)) { - return []; - } - - $skippedTypes = ['html', 'divider', 'submit']; - $nestedTypes = ['column', 'columns']; - $fields = []; - - foreach ($body as $field) { - if (!empty($field['type']) && in_array($field['type'], $nestedTypes) && !empty($field['body'])) { - $nestedFields = $this->getFields($field['body']); - if ($nestedFields) { - $fields = array_merge($fields, $nestedFields); - } - continue; - } - - if (empty($field['id']) || empty($field['type']) || in_array($field['type'], $skippedTypes)) { - continue; - } - - if ((int)$field['id'] > 0) { - $fields[] = 'cf_' . $field['id']; - } else { - $fields[] = $field['id']; - } - } - - return $fields; - } } diff --git a/lib/Subscribers/RequiredCustomFieldValidator.php b/lib/Subscribers/RequiredCustomFieldValidator.php index 8e264ba82f..88002fac33 100644 --- a/lib/Subscribers/RequiredCustomFieldValidator.php +++ b/lib/Subscribers/RequiredCustomFieldValidator.php @@ -67,12 +67,15 @@ class RequiredCustomFieldValidator { return $result; } + /** + * @return int[] + */ private function getFormCustomFieldIds(FormEntity $form): array { - $formFields = $form->getFields(); + $formFields = $form->getBlocksByTypes(FormEntity::FORM_FIELD_TYPES); $customFieldIds = []; - foreach ($formFields as $fieldName) { - if (strpos($fieldName, 'cf_') === 0) { - $customFieldIds[] = (int)substr($fieldName, 3); + foreach ($formFields as $formField) { + if (isset($formField['id']) && is_numeric($formField['id'])) { + $customFieldIds[] = (int)$formField['id']; } } return $customFieldIds; diff --git a/lib/Subscribers/SubscriberSubscribeController.php b/lib/Subscribers/SubscriberSubscribeController.php index cf363b8752..0aff2cab0a 100644 --- a/lib/Subscribers/SubscriberSubscribeController.php +++ b/lib/Subscribers/SubscriberSubscribeController.php @@ -100,8 +100,13 @@ class SubscriberSubscribeController { } // only accept fields defined in the form - $formFields = $form->getFields(); - $data = array_intersect_key($data, array_flip($formFields)); + $formFieldIds = array_filter(array_map(function (array $formField): ?string { + if (!isset($formField['id'])) { + return null; + } + return is_numeric($formField['id']) ? "cf_{$formField['id']}" : $formField['id']; + }, $form->getBlocksByTypes(FormEntity::FORM_FIELD_TYPES))); + $data = array_intersect_key($data, array_flip($formFieldIds)); // make sure we don't allow too many subscriptions with the same ip address $timeout = $this->throttling->throttle(); diff --git a/tests/unit/Entities/FormEntityTest.php b/tests/unit/Entities/FormEntityTest.php index 4258fef4c9..98ab3a53f7 100644 --- a/tests/unit/Entities/FormEntityTest.php +++ b/tests/unit/Entities/FormEntityTest.php @@ -51,20 +51,20 @@ class FormEntityTest extends \MailPoetUnitTest { ], ]; - public function testGetBlocksByType() { + public function testGetBlocksByTypes(): void { $formEntity = new FormEntity('Test' ); $formEntity->setBody($this->body); - $paragraphs = $formEntity->getBlocksByType(FormEntity::PARAGRAPH_BLOCK_TYPE); + $paragraphs = $formEntity->getBlocksByTypes([FormEntity::PARAGRAPH_BLOCK_TYPE]); expect($paragraphs)->count(3); expect($paragraphs[0]['params']['content'])->equals('Paragraph 1'); expect($paragraphs[1]['params']['content'])->equals('Paragraph 2'); expect($paragraphs[2]['params']['content'])->equals('Paragraph 3'); - $headings = $formEntity->getBlocksByType(FormEntity::HEADING_BLOCK_TYPE); + $headings = $formEntity->getBlocksByTypes([FormEntity::HEADING_BLOCK_TYPE]); expect($headings)->count(1); expect($headings[0]['params']['content'])->equals('Heading 1'); - $columns = $formEntity->getBlocksByType(FormEntity::COLUMNS_BLOCK_TYPE); + $columns = $formEntity->getBlocksByTypes([FormEntity::COLUMNS_BLOCK_TYPE]); expect($columns)->count(1); expect($columns[0]['body'])->count(2); }