Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[framework] data object properties review #787

Merged
merged 21 commits into from Feb 13, 2019
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
61290a3
fixed OrderData bool property $deliveryAddressSameAsBillingAddress to…
Feb 1, 2019
a02db2c
fixed CategoryData property $descriptions typehint as it be nullable
Feb 1, 2019
5293598
fixed ProductData phpdocs
Feb 1, 2019
fbf468c
documented ProductData $outOfStockAction property - it is a Product c…
Feb 1, 2019
f7fef71
fixed ProductData $flags property typehint
Feb 1, 2019
e38455f
changed type of Currency::DEFAULT_EXCHANGE_RATE to string as it is ev…
Feb 1, 2019
3e04f81
fixed MailTemplateData $deleteAttachment property to bool with defaul…
Feb 1, 2019
b475bc4
fixed MessageData $variablesReplacementsForSubject property typehint
Feb 4, 2019
c7d657c
refactored responsibility of UserDataFactory::createForDomainId
Feb 4, 2019
06c1fe7
changed UserData property $domainId to int|null and removed default v…
Feb 4, 2019
bbdf595
fixed FrontOrderData bool properties to be always bool
Feb 6, 2019
2638df3
fixed ProductFilterData bool property $inStock to be always bool
Feb 6, 2019
b7c80c7
documented recommendations for *Data object fields
Feb 4, 2019
50d967f
added method Domain:getAllLocales() that returns all locales that are…
Feb 12, 2019
76f646a
added initialization of Data multilanguage properties
Feb 12, 2019
ed0c101
documented recommendations for multilanguage data attributes after th…
Feb 12, 2019
64a1865
added upgrade instructions for multilanguage data factories
Feb 12, 2019
11c44da
explained creation of multidomain and multilanguage data a bit better
PetrHeinz Feb 13, 2019
60c1f77
refactored Localization::getLocalesOfAllDomains() to use Domain::getA…
Feb 13, 2019
06d2760
reworded "phpdoc" to correct "PHPDoc annotation"
Feb 13, 2019
04155aa
reviewed data object properties (#787)
Feb 13, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
62 changes: 55 additions & 7 deletions docs/introduction/entities.md
Expand Up @@ -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
Expand Down Expand Up @@ -319,7 +320,7 @@ class BrandData
public $image;

/**
* @var string[]
* @var string[]|null[]
*/
public $descriptions;

Expand Down Expand Up @@ -356,15 +357,62 @@ class BrandData
}
```

boris-brtan marked this conversation as resolved.
Show resolved Hide resolved
### 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 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 `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 as they aren't expressive.

The only exception in the framework is `$pluginData` in `CategoryData` and `ProductData`. The phpdoc is an `array` in this case because plugins can contain any type.
simara-svatopluk marked this conversation as resolved.
Show resolved Hide resolved

#### 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 `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 `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 `\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 `\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 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).
simara-svatopluk marked this conversation as resolved.
Show resolved Hide resolved

Therefore the multidomain field has phpdoc `string[]|null[]` or `int[]|null[]`.
For boolean multidomain properties, we recommend using default value filled in the factory and phpdoc `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 prepare this array to contain keys for all locales because translation entities are created by these array items (even if their value is null).
simara-svatopluk marked this conversation as resolved.
Show resolved Hide resolved
Therefore the multidomain field has phpdoc `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

Expand Down
6 changes: 6 additions & 0 deletions docs/upgrade/UPGRADE-unreleased.md
Expand Up @@ -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))
Expand Down
13 changes: 13 additions & 0 deletions packages/framework/src/Component/Domain/Domain.php
Expand Up @@ -107,6 +107,19 @@ public function getAllIds()
return $ids;
}

/**
* @return string[]
*/
public function getAllLocales(): array
{
$locales = [];
foreach ($this->getAll() as $domainConfig) {
$locales[] = $domainConfig->getLocale();
}

return array_unique($locales);
}

/**
* @return int[]
*/
Expand Down
Expand Up @@ -199,7 +199,6 @@ protected function getCustomerDataFromCsvRow(array $row)
} else {
$customerData->deliveryAddressData = $this->deliveryAddressDataFactory->create();
}
$userData->domainId = $domainId;
boris-brtan marked this conversation as resolved.
Show resolved Hide resolved

$customerData->userData = $userData;
$customerData->billingAddressData = $billingAddressData;
Expand Down
Expand Up @@ -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;

Expand Down
4 changes: 2 additions & 2 deletions packages/framework/src/Model/Category/CategoryData.php
Expand Up @@ -8,7 +8,7 @@
class CategoryData
{
/**
* @var string[]
* @var string[]|null[]
*/
public $name;

Expand All @@ -28,7 +28,7 @@ class CategoryData
public $seoH1s;

/**
* @var string[]
* @var string[]|null[]
boris-brtan marked this conversation as resolved.
Show resolved Hide resolved
*/
public $descriptions;

Expand Down
4 changes: 4 additions & 0 deletions packages/framework/src/Model/Category/CategoryDataFactory.php
Expand Up @@ -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;
}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/framework/src/Model/Country/CountryData.php
Expand Up @@ -5,7 +5,7 @@
class CountryData
{
/**
* @var string[]
* @var string[]|null[]
*/
public $names;

Expand Down
4 changes: 4 additions & 0 deletions packages/framework/src/Model/Country/CountryDataFactory.php
Expand Up @@ -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;
}
}
}
3 changes: 1 addition & 2 deletions packages/framework/src/Model/Customer/UserData.php
Expand Up @@ -25,7 +25,7 @@ class UserData
public $password;

/**
* @var int
* @var int|null
boris-brtan marked this conversation as resolved.
Show resolved Hide resolved
*/
public $domainId;

Expand All @@ -46,6 +46,5 @@ class UserData

public function __construct()
{
$this->domainId = 1;
}
}
1 change: 1 addition & 0 deletions packages/framework/src/Model/Customer/UserDataFactory.php
Expand Up @@ -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;
}

/**
Expand Down
3 changes: 2 additions & 1 deletion packages/framework/src/Model/Mail/MailTemplateData.php
Expand Up @@ -35,13 +35,14 @@ class MailTemplateData
public $attachment;

/**
* @var bool|null
* @var bool
boris-brtan marked this conversation as resolved.
Show resolved Hide resolved
*/
public $deleteAttachment;

public function __construct()
{
$this->sendMail = false;
$this->attachment = [];
$this->deleteAttachment = false;
}
}
2 changes: 1 addition & 1 deletion packages/framework/src/Model/Mail/MessageData.php
Expand Up @@ -35,7 +35,7 @@ class MessageData
public $fromName;

/**
* @var string
* @var string[]
boris-brtan marked this conversation as resolved.
Show resolved Hide resolved
*/
public $variablesReplacementsForSubject;

Expand Down
14 changes: 11 additions & 3 deletions packages/framework/src/Model/Order/FrontOrderData.php
Expand Up @@ -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;
}
}
3 changes: 2 additions & 1 deletion packages/framework/src/Model/Order/OrderData.php
Expand Up @@ -82,7 +82,7 @@ class OrderData
public $country;

/**
* @var bool|null
boris-brtan marked this conversation as resolved.
Show resolved Hide resolved
* @var bool
*/
public $deliveryAddressSameAsBillingAddress;

Expand Down Expand Up @@ -174,6 +174,7 @@ class OrderData
public function __construct()
{
$this->itemsWithoutTransportAndPayment = [];
$this->deliveryAddressSameAsBillingAddress = false;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/framework/src/Model/Order/OrderFacade.php
Expand Up @@ -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)
{
Expand Down
Expand Up @@ -5,7 +5,7 @@
class OrderStatusData
{
/**
* @var string[]
* @var string[]|null[]
*/
public $name;

Expand Down
Expand Up @@ -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;
}
}

/**
Expand Down
6 changes: 3 additions & 3 deletions packages/framework/src/Model/Payment/PaymentData.php
Expand Up @@ -7,7 +7,7 @@
class PaymentData
{
/**
* @var string[]
* @var string[]|null[]
*/
public $name;

Expand All @@ -17,12 +17,12 @@ class PaymentData
public $vat;

/**
* @var string[]
* @var string[]|null[]
*/
public $description;

/**
* @var string[]
* @var string[]|null[]
*/
public $instructions;

Expand Down
6 changes: 6 additions & 0 deletions packages/framework/src/Model/Payment/PaymentDataFactory.php
Expand Up @@ -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;
}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/framework/src/Model/Pricing/Currency/Currency.php
Expand Up @@ -13,7 +13,7 @@ class Currency
const CODE_CZK = 'CZK';
const CODE_EUR = 'EUR';

const DEFAULT_EXCHANGE_RATE = 1;
const DEFAULT_EXCHANGE_RATE = '1';
boris-brtan marked this conversation as resolved.
Show resolved Hide resolved

/**
* @var int
Expand Down
Expand Up @@ -5,7 +5,7 @@
class AvailabilityData
{
/**
* @var string[]
* @var string[]|null[]
*/
public $name;

Expand Down