Refactor attribute type system to use enums and manage dynamic list attributes table names

.github/PULL_REQUEST_TEMPLATE.md

@@ -1,3 +1,3 @@
-"@coderabbitai summary"
+@coderabbitai summary
 
 Thanks for contributing to phpList!

composer.json

@@ -77,7 +77,8 @@
         "symfony/lock": "^6.4",
         "webklex/php-imap": "^6.2",
         "ext-imap": "*",
-        "tatevikgr/rss-feed": "dev-main"
+        "tatevikgr/rss-feed": "dev-main",
+        "ext-pdo": "*"
     },
     "require-dev": {
         "phpunit/phpunit": "^9.5",

config/parameters.yml.dist

@@ -23,6 +23,8 @@ parameters:
     env(PHPLIST_DATABASE_PASSWORD): 'phplist'
     database_prefix: '%%env(DATABASE_PREFIX)%%'
     env(DATABASE_PREFIX): 'phplist_'
+    list_table_prefix: '%%env(LIST_TABLE_PREFIX)%%'
+    env(LIST_TABLE_PREFIX): 'listattr_'
 
     # Email configuration
     app.mailer_from: '%%env(MAILER_FROM)%%'

config/services/managers.yml

@@ -48,6 +48,21 @@ services:
     autowire: true
     autoconfigure: true
 
+  PhpList\Core\Domain\Subscription\Service\Manager\DynamicListAttrManager:
+    autowire: true
+    autoconfigure: true
+
+  Doctrine\DBAL\Schema\AbstractSchemaManager:
+    factory: ['@doctrine.dbal.default_connection', 'createSchemaManager']
+
+  PhpList\Core\Domain\Subscription\Service\Manager\DynamicListAttrTablesManager:
+    autowire: true
+    autoconfigure: true
+    public: true
+    arguments:
+      $dbPrefix: '%database_prefix%'
+      $dynamicListTablePrefix: '%list_table_prefix%'
+
   PhpList\Core\Domain\Subscription\Service\Manager\SubscriberHistoryManager:
     autowire: true
     autoconfigure: true

config/services/mappers.yml

@@ -4,10 +4,10 @@ services:
     autoconfigure: true
     public: false
 
-  PhpList\Core\Domain\Subscription\Service\CsvRowToDtoMapper:
+  PhpList\Core\Domain\Subscription\Service\ArrayToDtoMapper:
     autowire: true
     autoconfigure: true
 
-  PhpList\Core\Domain\Subscription\Service\CsvImporter:
+  PhpList\Core\Domain\Subscription\Service\CsvToDtoImporter:
     autowire: true
     autoconfigure: true

config/services/repositories.yml

@@ -70,7 +70,8 @@ services:
   PhpList\Core\Domain\Subscription\Repository\DynamicListAttrRepository:
     autowire: true
     arguments:
-      $prefix: '%database_prefix%'
+      $dbPrefix: '%database_prefix%'
+      $dynamicListTablePrefix: '%list_table_prefix%'
   PhpList\Core\Domain\Subscription\Repository\SubscriberHistoryRepository:
     parent: PhpList\Core\Domain\Common\Repository\AbstractRepository
     arguments:

resources/translations/messages.en.xlf

@@ -734,6 +734,10 @@ Thank you.</target>
         <source>Conflict: email and foreign key refer to different subscribers.</source>
         <target>__Conflict: email and foreign key refer to different subscribers.</target>
       </trans-unit>
+      <trans-unit id="HHtgwG9" resname="Invalid email: %email%">
+        <source>Invalid email: %email%</source>
+        <target>__Invalid email: %email%</target>
+      </trans-unit>
     </body>
   </file>
 </xliff>

src/Domain/Subscription/Exception/AttributeTypeChangeNotAllowed.php

@@ -0,0 +1,19 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpList\Core\Domain\Subscription\Exception;
+
+use RuntimeException;
+
+class AttributeTypeChangeNotAllowed extends RuntimeException
+{
+    public function __construct(string $oldType, string $newType)
+    {
+        parent::__construct(sprintf(
+            'attribute_definition.type_change_not_allowed:%s->%s',
+            $oldType,
+            $newType
+        ));
+    }
+}

src/Domain/Subscription/Model/AttributeTypeEnum.php

@@ -0,0 +1,56 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpList\Core\Domain\Subscription\Model;
+
+use PhpList\Core\Domain\Subscription\Exception\AttributeTypeChangeNotAllowed;
+
+enum AttributeTypeEnum: string
+{
+    case Text = 'text';
+    case Number = 'number';
+    case Date = 'date';
+    case Select = 'select';
+    case Checkbox = 'checkbox';
+    case MultiSelect = 'multiselect';
+    case CheckboxGroup = 'checkboxgroup';
+    case Radio = 'radio';
+
+    public function equals(self $other): bool
+    {
+        return $this === $other;
+    }
+
+    public function isMultiValued(): bool
+    {
+        return match ($this) {
+            self::Select,
+            self::Checkbox,
+            self::MultiSelect,
+            self::Radio,
+            self::CheckboxGroup => true,
+            default => false,
+        };
+    }
+
+    public function canTransitionTo(self $toType): bool
+    {
+        if ($this === $toType) {
+            return true;
+        }
+
+        if ($this->isMultiValued() !== $toType->isMultiValued()) {
+            return false;
+        }
+
+        return true;
+    }
+
+    public function assertTransitionAllowed(self $toType): void
+    {
+        if (!$this->canTransitionTo($toType)) {
+            throw new AttributeTypeChangeNotAllowed($this->value, $toType->value);
+        }
+    }
+}

src/Domain/Subscription/Model/Dto/AttributeDefinitionDto.php

@@ -4,18 +4,26 @@
 
 namespace PhpList\Core\Domain\Subscription\Model\Dto;
 
+use InvalidArgumentException;
+use PhpList\Core\Domain\Subscription\Model\AttributeTypeEnum;
+
 class AttributeDefinitionDto
 {
     /**
      * @SuppressWarnings("BooleanArgumentFlag")
+     * @param DynamicListAttrDto[] $options
+     * @throws InvalidArgumentException
      */
     public function __construct(
         public readonly string $name,
-        public readonly ?string $type = null,
+        public readonly ?AttributeTypeEnum $type = null,
         public readonly ?int $listOrder = null,
         public readonly ?string $defaultValue = null,
         public readonly ?bool $required = false,
-        public readonly ?string $tableName = null,
+        public readonly array $options = [],
     ) {
+        if (trim($this->name) === '') {
+            throw new InvalidArgumentException('Name cannot be empty');
+        }
     }
 }

src/Domain/Subscription/Model/Dto/DynamicListAttrDto.php

@@ -0,0 +1,33 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpList\Core\Domain\Subscription\Model\Dto;
+
+use InvalidArgumentException;
+use Symfony\Component\Serializer\Annotation\SerializedName;
+
+class DynamicListAttrDto
+{
+    public readonly ?int $id;
+
+    public readonly string $name;
+
+    #[SerializedName('listorder')]
+    public readonly ?int $listOrder;
+
+    public function __construct(
+        ?int $id,
+        string $name,
+        ?int $listOrder = null
+    ) {
+        $trimmed = trim($name);
+        if ($trimmed === '') {
+            throw new InvalidArgumentException('Option name cannot be empty');
+        }
+
+        $this->id = $id;
+        $this->name = $trimmed;
+        $this->listOrder = $listOrder;
+    }
+}

src/Domain/Subscription/Model/SubscriberAttributeDefinition.php

@@ -48,9 +48,10 @@ public function getName(): string
         return $this->name;
     }
 
-    public function getType(): ?string
+    /** @SuppressWarnings(PHPMD.StaticAccess) */
+    public function getType(): ?AttributeTypeEnum
     {
-        return $this->type;
+        return $this->type === null ? null : AttributeTypeEnum::from($this->type);
     }
 
     public function getListOrder(): ?int
@@ -80,9 +81,20 @@ public function setName(string $name): self
         return $this;
     }
 
-    public function setType(?string $type): self
+    /** @SuppressWarnings(PHPMD.StaticAccess) */
+    public function setType(?AttributeTypeEnum $type): self
     {
-        $this->type = $type;
+        if ($type === null) {
+            $this->type = null;
+            return $this;
+        }
+
+        if ($this->type !== null) {
+            $currentType = AttributeTypeEnum::from($this->type);
+            $currentType->assertTransitionAllowed($type);
+        }
+        $this->type = $type->value;
+
         return $this;
     }