Remove getFields method from FormEntity

[MAILPOET-3032]

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'),

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;
-  }
 }

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) {
+    foreach ($formFields as $formField) {
-      if (strpos($fieldName, 'cf_') === 0) {
+      if (isset($formField['id']) && is_numeric($formField['id'])) {
-        $customFieldIds[] = (int)substr($fieldName, 3);
+        $customFieldIds[] = (int)$formField['id'];
       }
     }
     return $customFieldIds;

lib/Subscribers/SubscriberSubscribeController.php

@@ -100,8 +100,13 @@ class SubscriberSubscribeController {
     }
 
     // only accept fields defined in the form
-    $formFields = $form->getFields();
+    $formFieldIds = array_filter(array_map(function (array $formField): ?string {
-    $data = array_intersect_key($data, array_flip($formFields));
+      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();

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);
   }