reviewed data object properties (#787)

shopsys · Feb 13, 2019 · 04155aa · 04155aa
2 parents 92dc2c6 + 06d2760
commit 04155aa
Show file tree

Hide file tree

Showing 46 changed files with 331 additions and 56 deletions.
diff --git a/docs/introduction/entities.md b/docs/introduction/entities.md
@@ -187,6 +187,7 @@ The entity extends `\Shopsys\FrameworkBundle\Model\Localization\AbstractTranslat
 Setting the properties of a domain entity is always done via the main entity itself.
 Basically, that means only the main entity knows about the existence of translation entities.
 The rest of the application uses the main entity as a proxy to the translation-specific properties.
+The extension creates instances of translated entities on-demand and this creation is transparent for user of domain entity.
 *The concept is similar to [domain entities](#domain-entity)* but uses Doctrine extension.
 
 ### Example
@@ -319,7 +320,7 @@ class BrandData
     public $image;
 
     /**
-     * @var string[]
+     * @var string[]|null[]
      */
     public $descriptions;
 
@@ -356,15 +357,63 @@ class BrandData
 }
 ```
 
-### Data for multidomain or multilanguage field
+### Recommendations for data object properties
+
+#### Scalars
+
+Scalars are the most typical properties in data objects.
+You usually don't have a default value for a scalar field, so your PHPDoc annotation will usually look like `string|null`, `int|null`, `float|null`.
+
+If you need to transfer boolean data, we recommend using `bool` with a default value in the constructor (`true`/`false`) because `false` and `null` behaves similarly in php.
+
+#### Arrays
+
+If you need to transfer arrays, use PHPDoc annotation `string[]`, `int[]`, `float[]`, `bool[]` and initialize an empty array in the constructor.
+
+If you care about array keys, use a name of the key in the property name in form `propertyByKey`. Eg. [`TransportData::$pricesByCurrencyId`](/packages/framework/src/Model/Transport/TransportData.php).
+
+#### Unknown types
+
+We don't recommend to use `mixed` or `array` PHPDoc annotation as they aren't expressive.
+
+The only exception in the framework is `$pluginData` in `CategoryData` and `ProductData`.
+The PHPDoc annotation is an `array` in this case because plugins can contain any type.
+
+#### Entities
+
+It is common that you need to transfer an entity to form or other parts of the system.
+
+If you need to transfer one entity, use PHPDoc annotation `entity|null`, eg. `\Shopsys\FrameworkBundle\Model\Pricing\Vat\Vat|null` in [`TransportData::$vat`](/packages/framework/src/Model/Transport/TransportData.php).
+If you need to transfer a collection of entities, use PHPDoc annotation `entity[]` and initialize the array in the constructor, eg. `\Shopsys\FrameworkBundle\Model\Payment\Payment[]` in [`TransportData::$payments`](/packages/framework/src/Model/Transport/TransportData.php).
+
+#### Images
+
+To transfer images via the system, use PHPDoc annotation `\Shopsys\FrameworkBundle\Component\FileUpload\ImageUploadData` and initialize the field in the constructor as you can see in `BrandData` example above.
+
+#### URL addresses
+
+To transfer URL addresses via the system, use PHPDoc annotation `\Shopsys\FrameworkBundle\Component\Router\FriendlyUrl\UrlListData` and initialize the field in the constructor as you can see in `BrandData` example above.
+
+#### Multidomain
+
+[Multidomain property](/docs/introduction/domain-multidomain-multilanguage.md#multidomain-attribute) is an array and has to be indexed by `domainId` - an integer ID of the given domain.
+An example of such property is a `seoH1s` in the `BrandData` example above.
+Data factory has to create an item in this array for each domain ID, otherwise domain entities would not be created correctly (a domain entity should exist for each domain, even with null values).
+
+Therefore the multidomain field has PHPDoc annotation `string[]|null[]` or `int[]|null[]`.
+For boolean multidomain properties, we recommend using default value filled in the factory and PHPDoc annotation `bool[]` only, eg. property [`TransportData::$enabled`](/packages/framework/src/Model/Transport/TransportData.php).
+
+#### Multilanguage
 
-As you can see in example above, multidomain or multilanguage field is an array.
+[Multilanguage property](/docs/introduction/domain-multidomain-multilanguage.md#multilanguage-attribute) is an array and has to be indexed by `locale` - a string identifier of language (you can find them in [`domains.yml`](/project-base/app/config/domains.yml)).
+An example of such property is a `descriptions` in the `BrandData` example above.
+Data factory has to create an item in this array for each locale, otherwise translation entities would not be created correctly (a translation entity should exist for each locale, even with null values).
+Therefore the multidomain field has PHPDoc annotation `string[]|null[]`.
 
-Multidomain field like `seoH1s` have to be indexed by `domainId` - integer ID of the given domain.
-Data factory has to prepare this array to contain keys for all domain IDs, because domain entities are created by these array items (even if their value is null).
+#### Data objects
 
-Multilanguage field like `description` have to be indexed by `locale` - string identifier of language (you can find them in [`domains.yml`](/project-base/app/config/domains.yml)).
-Data factory does not need to prepare this array, because the extension we use for translated entities can handle unprepared translations.
+You can use even data object within your data object when you need composition.
+Eg. [`CustomerData`](/packages/framework/src/Model/Customer/CustomerData.php) or [`OrderData`](/packages/framework/src/Model/Order/OrderData.php) (contains `OrderItemData`).
 
 ## Entity data factory
 

diff --git a/docs/upgrade/UPGRADE-unreleased.md b/docs/upgrade/UPGRADE-unreleased.md
@@ -206,6 +206,12 @@ for instance:
     - <target name="error-pages-generate" description="...">
     + <target name="error-pages-generate" depends="prod-warmup" description="...">
     ```
+- if you have extended any of following factories, provide `Domain` object to parent constructor ([#787](https://github.com/shopsys/shopsys/pull/787))
+    - `AvailabilityDataFactory`
+    - `FlagDataFactory`
+    - `OrderStatusDataFactory`
+    - `ParameterDataFactory`
+    - `UnitDataFactory`
 
 ## [shopsys/product-feed-heureka]
 - if you have extended class HeurekaCategoryDownloader or HeurekaCategoryCronModule ([#788](https://github.com/shopsys/shopsys/pull/788))

diff --git a/packages/framework/src/Component/Domain/Domain.php b/packages/framework/src/Component/Domain/Domain.php
@@ -107,6 +107,20 @@ public function getAllIds()
         return $ids;
     }
 
+    /**
+     * @return string[]
+     */
+    public function getAllLocales(): array
+    {
+        $locales = [];
+        foreach ($this->getAll() as $domainConfig) {
+            $locale = $domainConfig->getLocale();
+            $locales[$locale] = $locale;
+        }
+
+        return $locales;
+    }
+
     /**
      * @return int[]
      */

diff --git a/packages/framework/src/DataFixtures/Demo/UserDataFixtureLoader.php b/packages/framework/src/DataFixtures/Demo/UserDataFixtureLoader.php
@@ -199,7 +199,6 @@ protected function getCustomerDataFromCsvRow(array $row)
         } else {
             $customerData->deliveryAddressData = $this->deliveryAddressDataFactory->create();
         }
-        $userData->domainId = $domainId;
 
         $customerData->userData = $userData;
         $customerData->billingAddressData = $billingAddressData;

diff --git a/packages/framework/src/DataFixtures/Performance/UserDataFixture.php b/packages/framework/src/DataFixtures/Performance/UserDataFixture.php
@@ -180,7 +180,6 @@ private function getRandomCustomerDataByDomainId($domainId, $userNumber)
         $userData->lastName = $this->faker->lastName;
         $userData->email = $userNumber . '.' . $this->faker->safeEmail;
         $userData->password = $this->faker->password;
-        $userData->domainId = $domainId;
         $userData->createdAt = $this->faker->dateTimeBetween('-1 year', 'now');
         $userData->telephone = $this->faker->phoneNumber;
 

diff --git a/packages/framework/src/Model/Category/CategoryData.php b/packages/framework/src/Model/Category/CategoryData.php
@@ -8,7 +8,7 @@
 class CategoryData
 {
     /**
-     * @var string[]
+     * @var string[]|null[]
      */
     public $name;
 
@@ -28,7 +28,7 @@ class CategoryData
     public $seoH1s;
 
     /**
-     * @var string[]
+     * @var string[]|null[]
      */
     public $descriptions;
 

diff --git a/packages/framework/src/Model/Category/CategoryDataFactory.php b/packages/framework/src/Model/Category/CategoryDataFactory.php
@@ -81,6 +81,10 @@ protected function fillNew(CategoryData $categoryData)
             $categoryData->descriptions[$domainId] = null;
             $categoryData->enabled[$domainId] = true;
         }
+
+        foreach ($this->domain->getAllLocales() as $locale) {
+            $categoryData->name[$locale] = null;
+        }
     }
 
     /**

diff --git a/packages/framework/src/Model/Country/CountryData.php b/packages/framework/src/Model/Country/CountryData.php
@@ -5,7 +5,7 @@
 class CountryData
 {
     /**
-     * @var string[]
+     * @var string[]|null[]
      */
     public $names;
 

diff --git a/packages/framework/src/Model/Country/CountryDataFactory.php b/packages/framework/src/Model/Country/CountryDataFactory.php
@@ -73,5 +73,9 @@ protected function fillNew(CountryData $countryData): void
             $countryData->enabled[$domainId] = true;
             $countryData->priority[$domainId] = null;
         }
+
+        foreach ($this->domain->getAllLocales() as $locale) {
+            $countryData->names[$locale] = null;
+        }
     }
 }
diff --git a/packages/framework/src/Model/Customer/UserData.php b/packages/framework/src/Model/Customer/UserData.php
@@ -25,7 +25,7 @@ class UserData
     public $password;
 
     /**
-     * @var int
+     * @var int|null
      */
     public $domainId;
 
@@ -46,6 +46,5 @@ class UserData
 
     public function __construct()
     {
-        $this->domainId = 1;
     }
 }
diff --git a/packages/framework/src/Model/Customer/UserDataFactory.php b/packages/framework/src/Model/Customer/UserDataFactory.php
@@ -46,6 +46,7 @@ public function createForDomainId(int $domainId): UserData
     protected function fillForDomainId(UserData $userData, int $domainId)
     {
         $userData->pricingGroup = $this->pricingGroupSettingFacade->getDefaultPricingGroupByDomainId($domainId);
+        $userData->domainId = $domainId;
     }
 
     /**

diff --git a/packages/framework/src/Model/Localization/Localization.php b/packages/framework/src/Model/Localization/Localization.php
@@ -88,12 +88,7 @@ public function getAdminLocale(): string
     public function getLocalesOfAllDomains(): array
     {
         if ($this->allLocales === null) {
-            $this->allLocales = [];
-            foreach ($this->domain->getAll() as $domainConfig) {
-                $domainLocale = $domainConfig->getLocale();
-
-                $this->allLocales[$domainLocale] = $domainLocale;
-            }
+            $this->allLocales = $this->domain->getAllLocales();
         }
 
         return $this->allLocales;

diff --git a/packages/framework/src/Model/Mail/MailTemplateData.php b/packages/framework/src/Model/Mail/MailTemplateData.php
@@ -35,13 +35,14 @@ class MailTemplateData
     public $attachment;
 
     /**
-     * @var bool|null
+     * @var bool
      */
     public $deleteAttachment;
 
     public function __construct()
     {
         $this->sendMail = false;
         $this->attachment = [];
+        $this->deleteAttachment = false;
     }
 }
diff --git a/packages/framework/src/Model/Mail/MessageData.php b/packages/framework/src/Model/Mail/MessageData.php
@@ -35,7 +35,7 @@ class MessageData
     public $fromName;
 
     /**
-     * @var string
+     * @var string[]
      */
     public $variablesReplacementsForSubject;
 

diff --git a/packages/framework/src/Model/Order/FrontOrderData.php b/packages/framework/src/Model/Order/FrontOrderData.php
@@ -5,17 +5,25 @@
 class FrontOrderData extends OrderData
 {
     /**
-     * @var bool|null
+     * @var bool
      */
     public $companyCustomer;
 
     /**
-     * @var bool|null
+     * @var bool
      */
     public $newsletterSubscription;
 
     /**
-     * @var bool|null
+     * @var bool
      */
     public $disallowHeurekaVerifiedByCustomers;
+
+    public function __construct()
+    {
+        parent::__construct();
+        $this->companyCustomer = false;
+        $this->newsletterSubscription = false;
+        $this->disallowHeurekaVerifiedByCustomers = false;
+    }
 }
diff --git a/packages/framework/src/Model/Order/OrderData.php b/packages/framework/src/Model/Order/OrderData.php
@@ -82,7 +82,7 @@ class OrderData
     public $country;
 
     /**
-     * @var bool|null
+     * @var bool
      */
     public $deliveryAddressSameAsBillingAddress;
 
@@ -174,6 +174,7 @@ class OrderData
     public function __construct()
     {
         $this->itemsWithoutTransportAndPayment = [];
+        $this->deliveryAddressSameAsBillingAddress = false;
     }
 
     /**

diff --git a/packages/framework/src/Model/Order/OrderFacade.php b/packages/framework/src/Model/Order/OrderFacade.php
@@ -307,7 +307,7 @@ public function createOrderFromFront(OrderData $orderData)
 
     /**
      * @param \Shopsys\FrameworkBundle\Model\Order\Order $order
-     * @param bool|null $disallowHeurekaVerifiedByCustomers
+     * @param bool $disallowHeurekaVerifiedByCustomers
      */
     public function sendHeurekaOrderInfo(Order $order, $disallowHeurekaVerifiedByCustomers)
     {

diff --git a/packages/framework/src/Model/Order/Status/OrderStatusData.php b/packages/framework/src/Model/Order/Status/OrderStatusData.php
@@ -5,7 +5,7 @@
 class OrderStatusData
 {
     /**
-     * @var string[]
+     * @var string[]|null[]
      */
     public $name;
 

diff --git a/packages/framework/src/Model/Order/Status/OrderStatusDataFactory.php b/packages/framework/src/Model/Order/Status/OrderStatusDataFactory.php
@@ -2,14 +2,41 @@
 
 namespace Shopsys\FrameworkBundle\Model\Order\Status;
 
+use Shopsys\FrameworkBundle\Component\Domain\Domain;
+
 class OrderStatusDataFactory implements OrderStatusDataFactoryInterface
 {
+    /**
+     * @var \Shopsys\FrameworkBundle\Component\Domain\Domain
+     */
+    protected $domain;
+
+    /**
+     * @param \Shopsys\FrameworkBundle\Component\Domain\Domain $domain
+     */
+    public function __construct(Domain $domain)
+    {
+        $this->domain = $domain;
+    }
+
     /**
      * @return \Shopsys\FrameworkBundle\Model\Order\Status\OrderStatusData
      */
     public function create(): OrderStatusData
     {
-        return new OrderStatusData();
+        $orderStatusData = new OrderStatusData();
+        $this->fillNew($orderStatusData);
+        return $orderStatusData;
+    }
+
+    /**
+     * @param \Shopsys\FrameworkBundle\Model\Order\Status\OrderStatusData $orderStatusData
+     */
+    protected function fillNew(OrderStatusData $orderStatusData): void
+    {
+        foreach ($this->domain->getAllLocales() as $locale) {
+            $orderStatusData->name[$locale] = null;
+        }
     }
 
     /**

diff --git a/packages/framework/src/Model/Payment/PaymentData.php b/packages/framework/src/Model/Payment/PaymentData.php
@@ -7,7 +7,7 @@
 class PaymentData
 {
     /**
-     * @var string[]
+     * @var string[]|null[]
      */
     public $name;
 
@@ -17,12 +17,12 @@ class PaymentData
     public $vat;
 
     /**
-     * @var string[]
+     * @var string[]|null[]
      */
     public $description;
 
     /**
-     * @var string[]
+     * @var string[]|null[]
      */
     public $instructions;
 

diff --git a/packages/framework/src/Model/Payment/PaymentDataFactory.php b/packages/framework/src/Model/Payment/PaymentDataFactory.php
@@ -58,6 +58,12 @@ protected function fillNew(PaymentData $paymentData): void
         foreach ($this->domain->getAllIds() as $domainId) {
             $paymentData->enabled[$domainId] = true;
         }
+
+        foreach ($this->domain->getAllLocales() as $locale) {
+            $paymentData->name[$locale] = null;
+            $paymentData->description[$locale] = null;
+            $paymentData->instructions[$locale] = null;
+        }
     }
 
     /**