Skip to content

Commit

Permalink
refactor #15072 [API] Validate endpoints that uses commands (Rafikooo)
Browse files Browse the repository at this point in the history
This PR was merged into the 1.13 branch.

Discussion
----------

| Q               | A                                                            |
|-----------------|--------------------------------------------------------------|
| Branch?         | 1.13 <!-- see the comment below -->                  |
| Bug fix?        | no                                                      |
| New feature?    | no                                                      |
| BC breaks?      | no                                                      |
| Deprecations?   | no<!-- don't forget to update the UPGRADE-*.md file --> |
| License         | MIT                                                          |

Endpoints that dispatches commands, do not have proper validation and expose business logic. This PR solves it with a validator.

Before:
<img width="1727" alt="image" src="https://github.com/Sylius/Sylius/assets/40125720/c61ce5f5-ab45-4e28-a27c-d37bfa45c288">

After:
<img width="1435" alt="image" src="https://github.com/Sylius/Sylius/assets/40125720/6a71fe62-880a-4e0a-84b3-28aea812ffe2">


<!--
 - Bug fixes must be submitted against the 1.12 branch
 - Features and deprecations must be submitted against the 1.13 branch
 - Make sure that the correct base branch is set

 To be sure you are not breaking any Backward Compatibilities, check the documentation:
 https://docs.sylius.com/en/latest/book/organization/backward-compatibility-promise.html
-->


Commits
-------

b644746 [Unit][ProductReviews] Add contract test for POST review payload validation
6f6bf56 [API][ProductReview] Validate product field
09a5b65 [API] Change the constructor signature of the AddProductReview command
84669f4 [Unit][Orders] Add tests for adding items to the cart validation
2876f9b [API] Change the constructor signature of the AddItemToCart command
8b1dce2 [API][Orders] Validate adding items to the cart
a6df3c9 [Maintenance] Use property promotion in commands
aafc207 [Unit][Orders] Add test for changing item quantity of the cart
0ab079e [API] Change ChangeItemQuantityInCart command methods signatures
200ea9e [API][Orders] Validate item quantity change
  • Loading branch information
GSadee committed Jul 4, 2023
2 parents 142e94b + 200ea9e commit b8b5928
Show file tree
Hide file tree
Showing 25 changed files with 293 additions and 148 deletions.
36 changes: 36 additions & 0 deletions UPGRADE-API-1.13.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,41 @@
# UPGRADE FROM `v1.12.x` TO `v1.13.0`

1. The signature of constructor of 'Sylius\Bundle\ApiBundle\Command\Cart\ChangeItemQuantityInCart' command changed:

````diff
public function __construct(
- public int $quantity,
+ public ?int $quantity,
) {
}
````

1. The constructor signature of 'Sylius\Bundle\ApiBundle\Command\Cart\AddItemToCart' changed:

````diff
public function __construct(
- public string $productCode,
+ public ?string $productCode,
- public int $quantity,
+ public ?int $quantity,
) {
}
````

1. The constructor signature of `Sylius\Bundle\ApiBundle\Command\Catalog\AddProductReview` changed:

````diff
public function __construct(
public ?string $title,
public ?int $rating,
public ?string $comment,
- public string $productCode,
+ public ?string $productCode,
public ?string $email = null,
) {
}
````

1. The item operation paths for ProductVariantTranslation resource changed:

- `GET /admin/product-variant-translation/{id}` -> `GET /admin/product-variant-translations/{id}`
Expand Down
4 changes: 2 additions & 2 deletions src/Sylius/Behat/Context/Api/Shop/CheckoutContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -1052,7 +1052,7 @@ public function iShouldBeInformedThatProductVariantDoesNotExist(ProductVariantIn
{
Assert::true($this->isViolationWithMessageInResponse(
$this->client->getLastResponse(),
sprintf('The product variant with %s does not exist.', $productVariant->getCode()),
sprintf('The product variant %s does not exist.', $productVariant->getCode()),
));
}

Expand All @@ -1063,7 +1063,7 @@ public function iShouldBeInformedThatProductVariantWithCodeDoesNotExist(string $
{
Assert::true($this->isViolationWithMessageInResponse(
$this->client->getLastResponse(),
sprintf('The product variant with %s does not exist.', $code),
sprintf('The product variant %s does not exist.', $code),
));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,8 @@ class ChangePaymentMethod implements OrderTokenValueAwareInterface, SubresourceI
*/
public $paymentId;

/**
* @psalm-immutable
*
* @var string
*/
public $paymentMethodCode;

public function __construct(string $paymentMethodCode)
public function __construct(public string $paymentMethodCode)
{
$this->paymentMethodCode = $paymentMethodCode;
}

public function getOrderTokenValue(): ?string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,32 +21,8 @@ class ChangeShopUserPassword implements ShopUserIdAwareInterface
/** @var mixed|null */
public $shopUserId;

/**
* @psalm-immutable
*
* @var string|null
*/
public $newPassword;

/**
* @psalm-immutable
*
* @var string|null
*/
public $confirmNewPassword;

/**
* @psalm-immutable
*
* @var string|null
*/
public $currentPassword;

public function __construct(?string $newPassword, ?string $confirmNewPassword, ?string $currentPassword)
public function __construct(public ?string $newPassword, public ?string $confirmNewPassword, public ?string $currentPassword)
{
$this->newPassword = $newPassword;
$this->confirmNewPassword = $confirmNewPassword;
$this->currentPassword = $currentPassword;
}

public function getShopUserId()
Expand Down
18 changes: 1 addition & 17 deletions src/Sylius/Bundle/ApiBundle/Command/Cart/AddItemToCart.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,24 +22,8 @@ class AddItemToCart implements OrderTokenValueAwareInterface, IriToIdentifierCon
/** @var string|null */
public $orderTokenValue;

/**
* @psalm-immutable
*
* @var string
*/
public $productVariantCode;

/**
* @psalm-immutable
*
* @var int
*/
public $quantity;

public function __construct(string $productVariantCode, int $quantity)
public function __construct(public ?string $productVariantCode, public ?int $quantity)
{
$this->productVariantCode = $productVariantCode;
$this->quantity = $quantity;
}

public static function createFromData(string $tokenValue, string $productVariantCode, int $quantity): self
Expand Down
18 changes: 1 addition & 17 deletions src/Sylius/Bundle/ApiBundle/Command/Cart/BlameCart.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,7 @@
/** @experimental */
class BlameCart
{
/**
* @psalm-immutable
*
* @var string
*/
public $shopUserEmail;

/**
* @psalm-immutable
*
* @var string
*/
public $orderTokenValue;

public function __construct(string $shopUserEmail, string $orderTokenValue)
public function __construct(public string $shopUserEmail, public string $orderTokenValue)
{
$this->shopUserEmail = $shopUserEmail;
$this->orderTokenValue = $orderTokenValue;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,8 @@ class ChangeItemQuantityInCart implements OrderTokenValueAwareInterface, Subreso
/** @var string|null */
public $orderItemId;

/**
* @psalm-immutable
*
* @var int
*/
public $quantity;

public function __construct(int $quantity)
public function __construct(public ?int $quantity)
{
$this->quantity = $quantity;
}

public static function createFromData(string $tokenValue, string $orderItemId, int $quantity): self
Expand Down
10 changes: 1 addition & 9 deletions src/Sylius/Bundle/ApiBundle/Command/Cart/PickupCart.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,6 @@
/** @experimental */
class PickupCart implements ChannelCodeAwareInterface, CustomerEmailAwareInterface, LocaleCodeAwareInterface
{
/**
* @psalm-immutable
*
* @var string|null
*/
public $tokenValue;

/** @var string|null */
public $localeCode;

Expand All @@ -36,9 +29,8 @@ class PickupCart implements ChannelCodeAwareInterface, CustomerEmailAwareInterfa
/** @var string|null */
public $email;

public function __construct(?string $tokenValue = null)
public function __construct(public ?string $tokenValue = null)
{
$this->tokenValue = $tokenValue;
}

public function getChannelCode(): ?string
Expand Down
14 changes: 1 addition & 13 deletions src/Sylius/Bundle/ApiBundle/Command/Cart/RemoveItemFromCart.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,8 @@
/** @experimental */
class RemoveItemFromCart implements OrderTokenValueAwareInterface
{
/** @var string|null */
public $orderTokenValue;

/**
* @psalm-immutable
*
* @var string
*/
public $itemId;

public function __construct(?string $orderTokenValue, string $itemId)
public function __construct(public ?string $orderTokenValue, public string $itemId)
{
$this->orderTokenValue = $orderTokenValue;
$this->itemId = $itemId;
}

public static function removeFromData(string $tokenValue, string $orderItemId): self
Expand Down
50 changes: 5 additions & 45 deletions src/Sylius/Bundle/ApiBundle/Command/Catalog/AddProductReview.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,53 +19,13 @@
/** @experimental */
class AddProductReview implements IriToIdentifierConversionAwareInterface, CustomerEmailAwareInterface
{
/**
* @psalm-immutable
*
* @var string|null
*/
public $title;

/**
* @psalm-immutable
*
* @var int|null
*/
public $rating;

/**
* @psalm-immutable
*
* @var string|null
*/
public $comment;

/**
* @psalm-immutable
*
* @var string
*/
public $productCode;

/**
* @psalm-immutable
*
* @var string|null
*/
public $email;

public function __construct(
?string $title,
?int $rating,
?string $comment,
string $productCode,
?string $email = null,
public ?string $title,
public ?int $rating,
public ?string $comment,
public ?string $productCode,
public ?string $email = null,
) {
$this->title = $title;
$this->rating = $rating;
$this->comment = $comment;
$this->productCode = $productCode;
$this->email = $email;
}

public function getEmail(): ?string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@
namespace Sylius\Bundle\ApiBundle\CommandHandler\Catalog;

use Sylius\Bundle\ApiBundle\Command\Catalog\AddProductReview;
use Sylius\Bundle\ApiBundle\Exception\ProductNotFoundException;
use Sylius\Bundle\CoreBundle\Resolver\CustomerResolverInterface;
use Sylius\Component\Core\Model\CustomerInterface;
use Sylius\Component\Core\Model\ProductInterface;
use Sylius\Component\Core\Repository\ProductRepositoryInterface;
use Sylius\Component\Resource\Factory\FactoryInterface;
use Sylius\Component\Resource\Repository\RepositoryInterface;
use Sylius\Component\Review\Model\ReviewInterface;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Messenger\Handler\MessageHandlerInterface;

/** @experimental */
Expand All @@ -36,9 +38,13 @@ public function __construct(

public function __invoke(AddProductReview $addProductReview): ReviewInterface
{
/** @var ProductInterface $product */
/** @var ProductInterface|null $product */
$product = $this->productRepository->findOneByCode($addProductReview->productCode);

if ($product === null) {
throw new ProductNotFoundException(Response::HTTP_UNPROCESSABLE_ENTITY);
}

/** @var string|null $email */
$email = $addProductReview->getEmail();

Expand Down
26 changes: 26 additions & 0 deletions src/Sylius/Bundle/ApiBundle/Exception/ProductNotFoundException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

/*
* This file is part of the Sylius package.
*
* (c) Sylius Sp. z o.o.
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

declare(strict_types=1);

namespace Sylius\Bundle\ApiBundle\Exception;

use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpKernel\Exception\HttpException;

/** @experimental */
final class ProductNotFoundException extends HttpException
{
public function __construct(int $statusCode = Response::HTTP_NOT_FOUND)
{
parent::__construct($statusCode, 'Product not found.');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,17 @@
<value>sylius</value>
</option>
</constraint>
<property name="productVariantCode">
<constraint name="NotBlank">
<option name="message">sylius.order_item.product_variant.not_blank</option>
<option name="groups">sylius</option>
</constraint>
</property>
<property name="quantity">
<constraint name="NotBlank">
<option name="message">sylius.order_item.quantity.not_blank</option>
<option name="groups">sylius</option>
</constraint>
<constraint name="Range">
<option name="min">1</option>
<option name="minMessage">sylius.order_item.quantity.min</option>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@
</option>
</constraint>
</property>
<property name="productCode">
<constraint name="NotBlank">
<option name="message">sylius.review.product.not_blank</option>
<option name="groups">
<value>sylius</value>
</option>
</constraint>
</property>
<property name="email">
<constraint name="NotBlank">
<option name="message">sylius.review.author.not_blank</option>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@
<option name="minMessage">sylius.order_item.quantity.min</option>
<option name="groups">sylius</option>
</constraint>
<constraint name="NotBlank">
<option name="message">sylius.order_item.quantity.not_blank</option>
<option name="groups">sylius</option>
</constraint>
</property>
</class>
</constraint-mapping>
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ sylius:
product:
not_exist: 'The product %productName% does not exist.'
product_variant:
not_exist: 'The product variant with %productVariantCode% does not exist.'
not_exist: 'The product variant %productVariantCode% does not exist.'
not_longer_available: 'The product variant with name %productVariantName% does not exist.'
not_sufficient: 'The product variant with %productVariantCode% code does not have sufficient stock.'
product_variant_with_name_not_sufficient: 'The product variant with %productVariantName% name does not have sufficient stock.'
Expand Down

0 comments on commit b8b5928

Please sign in to comment.