Introduce filters data type and filter groups to enable saving and/or filter combinations

[MAILPOET-5257]
2023-04-21 13:30:48 +02:00
parent a6fcc60de3
commit 34ca96d007
13 changed files with 195 additions and 69 deletions
--- a/mailpoet/assets/js/src/automation/editor/components/automation/types.ts
+++ b/mailpoet/assets/js/src/automation/editor/components/automation/types.ts
@@ -11,13 +11,23 @@ export type Filter = {
  args: Record<string, unknown>;
 };

+export type FilterGroup = {
+  operator: 'and' | 'or';
+  filters: Filter[];
+};
+
+export type Filters = {
+  operator: 'and' | 'or';
+  groups: FilterGroup[];
+};
+
 export type Step = {
  id: string;
  type: 'root' | 'trigger' | 'action';
  key: string;
  args: Record<string, unknown>;
  next_steps: NextStep[];
-  filters: Filter[];
+  filters?: Filters;
 };

 export type Automation = {
--- a/mailpoet/assets/js/src/automation/editor/components/filters/list.tsx
+++ b/mailpoet/assets/js/src/automation/editor/components/filters/list.tsx
@@ -30,7 +30,8 @@ export function FiltersList(): JSX.Element | null {
    deleteFilterCallback(stepId, filter);
  }, []);

-  if (step.filters.length === 0) {
+  const groups = step.filters?.groups ?? [];
+  if (groups.length === 0) {
    return null;
  }

@@ -51,35 +52,37 @@ export function FiltersList(): JSX.Element | null {
      )}

      <div className="mailpoet-automation-filters-list">
-        {step.filters.map((filter) => (
-          <div
-            key={filter.field_key}
-            className="mailpoet-automation-filters-list-item"
-          >
-            <div className="mailpoet-automation-filters-list-item-content">
-              <span className="mailpoet-automation-filters-list-item-field">
-                {fields[filter.field_key]?.name ??
-                  sprintf(
-                    __('Unknown field "%s"', 'mailpoet'),
-                    filter.field_key,
-                  )}
-              </span>{' '}
-              <span className="mailpoet-automation-filters-list-item-condition">
-                {filters[filter.field_type]?.conditions.find(
-                  ({ key }) => key === filter.condition,
-                )?.label ?? __('unknown condition', 'mailpoet')}
-              </span>{' '}
-              <Value filter={filter} />
-            </div>
-            <Button
-              className="mailpoet-automation-filters-list-item-remove"
-              isSmall
-              onClick={() => onDelete(step.id, filter)}
+        {groups.map((group) =>
+          group.filters.map((filter) => (
+            <div
+              key={filter.field_key}
+              className="mailpoet-automation-filters-list-item"
            >
-              <Icon icon={closeSmall} />
-            </Button>
-          </div>
-        ))}
+              <div className="mailpoet-automation-filters-list-item-content">
+                <span className="mailpoet-automation-filters-list-item-field">
+                  {fields[filter.field_key]?.name ??
+                    sprintf(
+                      __('Unknown field "%s"', 'mailpoet'),
+                      filter.field_key,
+                    )}
+                </span>{' '}
+                <span className="mailpoet-automation-filters-list-item-condition">
+                  {filters[filter.field_type]?.conditions.find(
+                    ({ key }) => key === filter.condition,
+                  )?.label ?? __('unknown condition', 'mailpoet')}
+                </span>{' '}
+                <Value filter={filter} />
+              </div>
+              <Button
+                className="mailpoet-automation-filters-list-item-remove"
+                isSmall
+                onClick={() => onDelete(step.id, filter)}
+              >
+                <Icon icon={closeSmall} />
+              </Button>
+            </div>
+          )),
+        )}
      </div>
    </>
  );
--- a/mailpoet/lib/Automation/Engine/Control/FilterHandler.php
+++ b/mailpoet/lib/Automation/Engine/Control/FilterHandler.php
@@ -19,10 +19,13 @@ class FilterHandler {

  public function matchesFilters(StepRunArgs $args): bool {
    $filters = $args->getStep()->getFilters();
-    foreach ($filters as $filter) {
-      $value = $args->getFieldValue($filter->getFieldKey());
-      if (!$this->matchesFilter($filter, $value)) {
-        return false;
+    $groups = $filters ? $filters->getGroups() : [];
+    foreach ($groups as $group) {
+      foreach ($group->getFilters() as $filter) {
+        $value = $args->getFieldValue($filter->getFieldKey());
+        if (!$this->matchesFilter($filter, $value)) {
+          return false;
+        }
      }
    }
    return true;
--- a/mailpoet/lib/Automation/Engine/Data/FilterGroup.php
+++ b/mailpoet/lib/Automation/Engine/Data/FilterGroup.php
@@ -0,0 +1,48 @@
+<?php declare(strict_types = 1);
+
+namespace MailPoet\Automation\Engine\Data;
+
+class FilterGroup {
+  public const OPERATOR_AND = 'and';
+  public const OPERATOR_OR = 'or';
+
+  /** @var string */
+  private $operator;
+
+  /** @var Filter[] */
+  private $filters;
+
+  public function __construct(
+    string $operator,
+    array $filters
+  ) {
+    $this->operator = $operator;
+    $this->filters = $filters;
+  }
+
+  public function getOperator(): string {
+    return $this->operator;
+  }
+
+  public function getFilters(): array {
+    return $this->filters;
+  }
+
+  public function toArray(): array {
+    return [
+      'operator' => $this->operator,
+      'filters' => array_map(function (Filter $filter): array {
+        return $filter->toArray();
+      }, $this->filters),
+    ];
+  }
+
+  public static function fromArray(array $data): self {
+    return new self(
+      $data['operator'],
+      array_map(function (array $filter) {
+        return Filter::fromArray($filter);
+      }, $data['filters'])
+    );
+  }
+}
--- a/mailpoet/lib/Automation/Engine/Data/Filters.php
+++ b/mailpoet/lib/Automation/Engine/Data/Filters.php
@@ -0,0 +1,48 @@
+<?php declare(strict_types = 1);
+
+namespace MailPoet\Automation\Engine\Data;
+
+class Filters {
+  public const OPERATOR_AND = 'and';
+  public const OPERATOR_OR = 'or';
+
+  /** @var string */
+  private $operator;
+
+  /** @var FilterGroup[] */
+  private $groups;
+
+  public function __construct(
+    string $operator,
+    array $groups
+  ) {
+    $this->operator = $operator;
+    $this->groups = $groups;
+  }
+
+  public function getOperator(): string {
+    return $this->operator;
+  }
+
+  public function getGroups(): array {
+    return $this->groups;
+  }
+
+  public function toArray(): array {
+    return [
+      'operator' => $this->operator,
+      'groups' => array_map(function (FilterGroup $group): array {
+        return $group->toArray();
+      }, $this->groups),
+    ];
+  }
+
+  public static function fromArray(array $data): self {
+    return new self(
+      $data['operator'],
+      array_map(function (array $group) {
+        return FilterGroup::fromArray($group);
+      }, $data['groups'])
+    );
+  }
+}
--- a/mailpoet/lib/Automation/Engine/Data/Step.php
+++ b/mailpoet/lib/Automation/Engine/Data/Step.php
@@ -22,13 +22,12 @@ class Step {
  /** @var NextStep[] */
  protected $nextSteps;

-  /** @var Filter[] */
+  /** @var Filters|null */
  private $filters;

  /**
   * @param array<string, mixed> $args
   * @param NextStep[] $nextSteps
-   * @param Filter[] $filters
   */
  public function __construct(
    string $id,
@@ -36,7 +35,7 @@ class Step {
    string $key,
    array $args,
    array $nextSteps,
-    array $filters = []
+    Filters $filters = null
  ) {
    $this->id = $id;
    $this->type = $type;
@@ -72,8 +71,7 @@ class Step {
    return $this->args;
  }

-  /** @return Filter[] */
-  public function getFilters(): array {
+  public function getFilters(): ?Filters {
    return $this->filters;
  }

@@ -86,9 +84,7 @@ class Step {
      'next_steps' => array_map(function (NextStep $nextStep) {
        return $nextStep->toArray();
      }, $this->nextSteps),
-      'filters' => array_map(function (Filter $filter) {
-        return $filter->toArray();
-      }, $this->filters),
+      'filters' => $this->filters ? $this->filters->toArray() : null,
    ];
  }

@@ -101,9 +97,7 @@ class Step {
      array_map(function (array $nextStep) {
        return NextStep::fromArray($nextStep);
      }, $data['next_steps']),
-      array_map(function (array $filter) {
-        return Filter::fromArray($filter);
-      }, $data['filters'] ?? [])
+      isset($data['filters']) ? Filters::fromArray($data['filters']) : null
    );
  }
 }
--- a/mailpoet/lib/Automation/Engine/Mappers/AutomationMapper.php
+++ b/mailpoet/lib/Automation/Engine/Mappers/AutomationMapper.php
@@ -5,7 +5,6 @@ namespace MailPoet\Automation\Engine\Mappers;
 use DateTimeImmutable;
 use MailPoet\Automation\Engine\Data\Automation;
 use MailPoet\Automation\Engine\Data\AutomationStatistics;
-use MailPoet\Automation\Engine\Data\Filter;
 use MailPoet\Automation\Engine\Data\NextStep;
 use MailPoet\Automation\Engine\Data\Step;
 use MailPoet\Automation\Engine\Storage\AutomationStatisticsStorage;
@@ -43,9 +42,7 @@ class AutomationMapper {
          'next_steps' => array_map(function (NextStep $nextStep) {
            return $nextStep->toArray();
          }, $step->getNextSteps()),
-          'filters' => array_map(function (Filter $filter) {
-            return $filter->toArray();
-          }, $step->getFilters()),
+          'filters' => $step->getFilters() ? $step->getFilters()->toArray() : null,
        ];
      }, $automation->getSteps()),
      'meta' => (object)$automation->getAllMetas(),
--- a/mailpoet/lib/Automation/Engine/Validation/AutomationRules/ValidStepFiltersRule.php
+++ b/mailpoet/lib/Automation/Engine/Validation/AutomationRules/ValidStepFiltersRule.php
@@ -27,13 +27,16 @@ class ValidStepFiltersRule implements AutomationNodeVisitor {
  }

  public function visitNode(Automation $automation, AutomationNode $node): void {
-    $step = $node->getStep();
-    foreach ($step->getFilters() as $filter) {
-      $registryFilter = $this->registry->getFilter($filter->getFieldType());
-      if (!$registryFilter) {
-        continue;
+    $filters = $node->getStep()->getFilters();
+    $groups = $filters ? $filters->getGroups() : [];
+    foreach ($groups as $group) {
+      foreach ($group->getFilters() as $filter) {
+        $registryFilter = $this->registry->getFilter($filter->getFieldType());
+        if (!$registryFilter) {
+          continue;
+        }
+        $this->validator->validate($registryFilter->getArgsSchema(), $filter->getArgs());
      }
-      $this->validator->validate($registryFilter->getArgsSchema(), $filter->getArgs());
    }
  }

--- a/mailpoet/lib/Automation/Engine/Validation/AutomationSchema.php
+++ b/mailpoet/lib/Automation/Engine/Validation/AutomationSchema.php
@@ -29,7 +29,7 @@ class AutomationSchema {
      'key' => Builder::string()->required(),
      'args' => Builder::object()->required(),
      'next_steps' => self::getNextStepsSchema()->required(),
-      'filters' => self::getFiltersSchema()->required(),
+      'filters' => self::getFiltersSchema()->nullable()->required(),
    ]);
  }

@@ -51,14 +51,24 @@ class AutomationSchema {
    )->maxItems(1);
  }

-  public static function getFiltersSchema(): ArraySchema {
-    return Builder::array(
-      Builder::object([
-        'field_type' => Builder::string()->required(),
-        'field_key' => Builder::string()->required(),
-        'condition' => Builder::string()->required(),
-        'args' => Builder::object()->required(),
-      ])
-    );
+  public static function getFiltersSchema(): ObjectSchema {
+    $operatorSchema = Builder::string()->pattern('^and|or$')->required();
+
+    $filterSchema = Builder::object([
+      'field_type' => Builder::string()->required(),
+      'field_key' => Builder::string()->required(),
+      'condition' => Builder::string()->required(),
+      'args' => Builder::object()->required(),
+    ]);
+
+    $filterGroupSchema = Builder::object([
+      'operator' => $operatorSchema,
+      'filters' => Builder::array($filterSchema)->minItems(1)->required(),
+    ]);
+
+    return Builder::object([
+      'operator' => $operatorSchema,
+      'groups' => Builder::array($filterGroupSchema)->minItems(1)->required(),
+    ]);
  }
 }
--- a/mailpoet/tests/integration/Automation/Engine/Control/TriggerHandlerTest.php
+++ b/mailpoet/tests/integration/Automation/Engine/Control/TriggerHandlerTest.php
@@ -5,6 +5,8 @@ namespace MailPoet\Test\Automation\Engine\Control;
 use MailPoet\Automation\Engine\Control\TriggerHandler;
 use MailPoet\Automation\Engine\Data\Automation;
 use MailPoet\Automation\Engine\Data\Filter;
+use MailPoet\Automation\Engine\Data\FilterGroup;
+use MailPoet\Automation\Engine\Data\Filters;
 use MailPoet\Automation\Engine\Data\Step;
 use MailPoet\Automation\Engine\Data\Subject;
 use MailPoet\Automation\Engine\Storage\AutomationRunStorage;
@@ -172,9 +174,10 @@ class TriggerHandlerTest extends \MailPoetTest {
    // automation that doesn't match segments filter
    $unknownId = $segment->getId() + 1;
    $filter = new Filter('enum_array', 'mailpoet:subscriber:segments', 'matches-any-of', ['value' => [$unknownId]]);
+    $filters = new Filters('and', [new FilterGroup('and', [$filter])]);
    $automation = $this->tester->createAutomation(
      'Will not run',
-      new Step('trigger', Step::TYPE_TRIGGER, $trigger->getKey(), [], [], [$filter])
+      new Step('trigger', Step::TYPE_TRIGGER, $trigger->getKey(), [], [], $filters)
    );
    $this->assertInstanceOf(Automation::class, $automation);
    $this->assertCount(0, $this->automationRunStorage->getAutomationRunsForAutomation($automation));
@@ -183,9 +186,10 @@ class TriggerHandlerTest extends \MailPoetTest {

    // matches segments filter
    $filter = new Filter('enum_array', 'mailpoet:subscriber:segments', 'matches-any-of', ['value' => [$segment->getId()]]);
+    $filters = new Filters('and', [new FilterGroup('and', [$filter])]);
    $automation = $this->tester->createAutomation(
      'Will run',
-      new Step('trigger', Step::TYPE_TRIGGER, $trigger->getKey(), [], [], [$filter])
+      new Step('trigger', Step::TYPE_TRIGGER, $trigger->getKey(), [], [], $filters)
    );
    $this->assertInstanceOf(Automation::class, $automation);
    $this->assertCount(0, $this->automationRunStorage->getAutomationRunsForAutomation($automation));
--- a/mailpoet/tests/integration/REST/Automation/Automations/AutomationsDuplicateEndpointTest.php
+++ b/mailpoet/tests/integration/REST/Automation/Automations/AutomationsDuplicateEndpointTest.php
@@ -98,7 +98,7 @@ class AutomationsDuplicateEndpointTest extends AutomationTest {
          'key' => 'core:root',
          'args' => [],
          'next_steps' => [],
-          'filters' => [],
+          'filters' => null,
        ],
      ],
      'meta' => [],
--- a/mailpoet/tests/unit/Automation/Engine/Control/FilterHandlerTest.php
+++ b/mailpoet/tests/unit/Automation/Engine/Control/FilterHandlerTest.php
@@ -8,6 +8,8 @@ use MailPoet\Automation\Engine\Data\Automation;
 use MailPoet\Automation\Engine\Data\AutomationRun;
 use MailPoet\Automation\Engine\Data\Field;
 use MailPoet\Automation\Engine\Data\Filter as FilterData;
+use MailPoet\Automation\Engine\Data\FilterGroup;
+use MailPoet\Automation\Engine\Data\Filters;
 use MailPoet\Automation\Engine\Data\Step;
 use MailPoet\Automation\Engine\Data\StepRunArgs;
 use MailPoet\Automation\Engine\Data\Subject as SubjectData;
@@ -23,7 +25,8 @@ use MailPoetUnitTest;
 class FilterHandlerTest extends MailPoetUnitTest {
  /** @dataProvider dataForTestItFilters */
  public function testItFilters(array $stepFilters, bool $expectation): void {
-    $step = new Step('step', Step::TYPE_TRIGGER, 'test:step', [], [], $stepFilters);
+    $filters = new Filters('and', [new FilterGroup('and', $stepFilters)]);
+    $step = new Step('step', Step::TYPE_TRIGGER, 'test:step', [], [], $filters);
    $subject = $this->createSubject('subject', [
      new Field('test:field-string', Field::TYPE_STRING, 'Test field string', function () {
        return 'abc';
--- a/mailpoet/tests/unit/Automation/Engine/Validation/AutomationRules/ValidStepFiltersRuleTest.php
+++ b/mailpoet/tests/unit/Automation/Engine/Validation/AutomationRules/ValidStepFiltersRuleTest.php
@@ -8,6 +8,8 @@ use Codeception\Stub\Expected;
 use MailPoet\Automation\Engine\Control\RootStep;
 use MailPoet\Automation\Engine\Data\Automation;
 use MailPoet\Automation\Engine\Data\Filter as FilterData;
+use MailPoet\Automation\Engine\Data\FilterGroup;
+use MailPoet\Automation\Engine\Data\Filters;
 use MailPoet\Automation\Engine\Data\Step;
 use MailPoet\Automation\Engine\Integration\Filter;
 use MailPoet\Automation\Engine\Registry;
@@ -48,6 +50,7 @@ class ValidStepFiltersRuleTest extends AutomationRuleTest {
  }

  private function getAutomation(array $filters): Automation {
+    $filters = new Filters('and', [new FilterGroup('and', $filters)]);
    return $this->make(Automation::class, [
      'getSteps' => [
        'root' => new Step('root', 'root', 'core:root', [], [], $filters),