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

[API] Pick up cart implementation #11524

Merged
merged 1 commit into from May 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion features/cart/shopping_cart/viewing_cart_summary.feature
Expand Up @@ -7,7 +7,7 @@ Feature: Viewing a cart summary
Background:
Given the store operates on a single channel in "United States"

@ui
@ui @api
Scenario: Viewing information about empty cart
When I see the summary of my cart
Then my cart should be empty
2 changes: 1 addition & 1 deletion src/Sylius/Behat/Client/ApiClientInterface.php
Expand Up @@ -26,7 +26,7 @@ public function subResourceIndex(string $subResource, string $id): Response;

public function show(string $id): Response;

public function create(): Response;
public function create(?RequestInterface $request = null): Response;

public function update(): Response;

Expand Down
7 changes: 4 additions & 3 deletions src/Sylius/Behat/Client/ApiPlatformClient.php
Expand Up @@ -61,9 +61,9 @@ public function show(string $id): Response
return $this->request(Request::show($this->resource, $id, $this->sharedStorage->get('token')));
}

public function create(): Response
public function create(?RequestInterface $request = null): Response
{
return $this->request($this->request);
return $this->request($request ?? $this->request);
}

public function update(): Response
Expand Down Expand Up @@ -118,7 +118,8 @@ public function upload(): Response

public function buildCreateRequest(): void
{
$this->request = Request::create($this->resource, $this->sharedStorage->get('token'));
$this->request = Request::create($this->resource);
$this->request->authorize($this->sharedStorage->get('token'));
}

public function buildUpdateRequest(string $id): void
Expand Down
9 changes: 7 additions & 2 deletions src/Sylius/Behat/Client/Request.php
Expand Up @@ -67,12 +67,12 @@ public static function show(string $resource, string $id, string $token): Reques
);
}

public static function create(string $resource, string $token): RequestInterface
public static function create(string $resource): RequestInterface
{
return new self(
sprintf('/new-api/%s', $resource),
HttpRequest::METHOD_POST,
['CONTENT_TYPE' => 'application/json', 'HTTP_Authorization' => 'Bearer ' . $token]
['CONTENT_TYPE' => 'application/ld+json']
);
}

Expand Down Expand Up @@ -190,6 +190,11 @@ public function removeSubResource(string $subResource, string $id): void
}
}

public function authorize(string $token): void
{
$this->headers['HTTP_Authorization'] = 'Bearer ' . $token;
}

private function mergeArraysUniquely(array $firstArray, array $secondArray): array
{
foreach ($secondArray as $key => $value) {
Expand Down
4 changes: 3 additions & 1 deletion src/Sylius/Behat/Client/RequestInterface.php
Expand Up @@ -21,7 +21,7 @@ public static function subResourceIndex(string $resource, string $id, string $su

public static function show(string $resource, string $id, string $token): self;

public static function create(string $resource, string $token): self;
public static function create(string $resource): self;

public static function update(string $resource, string $id, string $token): self;

Expand Down Expand Up @@ -56,4 +56,6 @@ public function updateFiles(array $newFiles): void;
public function addSubResource(string $key, array $subResource): void;

public function removeSubResource(string $subResource, string $id): void;

public function authorize(string $token): void;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason for extracting this method?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to make a request without authorization, therefore the previous create method was no-go for me. However, the rest of the methods still needs to authorize so IMHO it is better to provide it as a part of Public API. BTW, I would like completely decouple Request creation from APIClient and I see these changes as a first steps in this direction.

}
21 changes: 11 additions & 10 deletions src/Sylius/Behat/Context/Api/Admin/ManagingOrdersContext.php
Expand Up @@ -75,7 +75,7 @@ public function iBrowseOrders(): void
*/
public function iSeeTheOrder(OrderInterface $order): void
{
$this->client->show($order->getNumber());
$this->client->show($order->getTokenValue());
}

/**
Expand All @@ -84,7 +84,7 @@ public function iSeeTheOrder(OrderInterface $order): void
public function iCancelThisOrder(OrderInterface $order): void
{
$this->client->applyTransition(
$this->responseChecker->getValue($this->client->show($order->getNumber()), 'number'),
$this->responseChecker->getValue($this->client->show($order->getTokenValue()), 'tokenValue'),
OrderTransitions::TRANSITION_CANCEL
);
}
Expand Down Expand Up @@ -129,9 +129,10 @@ public function iShouldSeeASingleOrderFromCustomer(CustomerInterface $customer):
*/
public function iShouldBeNotifiedAboutItHasBeenSuccessfullyCanceled(): void
{
$response = $this->client->getLastResponse();
Assert::true(
$this->responseChecker->isUpdateSuccessful($this->client->getLastResponse()),
'Resource could not be completed'
$this->responseChecker->isUpdateSuccessful($response),
'Resource could not be completed. Reason: ' . $response->getContent()
);
}

Expand All @@ -143,7 +144,7 @@ public function itsStateShouldBe(string $state): void
{
/** @var OrderInterface $order */
$order = $this->sharedStorage->get('order');
$orderState = $this->responseChecker->getValue($this->client->show($order->getNumber()), 'state');
$orderState = $this->responseChecker->getValue($this->client->show($order->getTokenValue()), 'state');

Assert::same($orderState, strtolower($state));
}
Expand All @@ -154,7 +155,7 @@ public function itsStateShouldBe(string $state): void
public function itShouldHaveShipmentState(string $state): void
{
$shipmentsIri = $this->responseChecker->getValue(
$this->client->show($this->sharedStorage->get('order')->getNumber()),
$this->client->show($this->sharedStorage->get('order')->getTokenValue()),
'shipments'
);

Expand All @@ -170,7 +171,7 @@ public function itShouldHaveShipmentState(string $state): void
public function itShouldHavePaymentState($state): void
{
$paymentsIri = $this->responseChecker->getValue(
$this->client->show($this->sharedStorage->get('order')->getNumber()),
$this->client->show($this->sharedStorage->get('order')->getTokenValue()),
'payments'
);

Expand All @@ -186,7 +187,7 @@ public function itShouldHavePaymentState($state): void
public function theOrderShouldHaveNumberOfPayments(int $number): void
{
Assert::count(
$this->responseChecker->getValue($this->client->show($this->sharedStorage->get('order')->getNumber()), 'payments'),
$this->responseChecker->getValue($this->client->show($this->sharedStorage->get('order')->getTokenValue()), 'payments'),
$number
);
}
Expand All @@ -197,8 +198,8 @@ public function theOrderShouldHaveNumberOfPayments(int $number): void
public function theOrderShouldHavePaymentState(OrderInterface $order, string $paymentState): void
{
Assert::true(
$this->responseChecker->hasValue($this->client->show($order->getNumber()), 'paymentState', strtolower($paymentState)),
sprintf('Order %s does not have %s payment state', $order->getNumber(), $paymentState)
$this->responseChecker->hasValue($this->client->show($order->getTokenValue()), 'paymentState', strtolower($paymentState)),
sprintf('Order %s does not have %s payment state', $order->getTokenValue(), $paymentState)
);
}

Expand Down
Expand Up @@ -135,12 +135,12 @@ public function thePaymentOfTheOrderShouldBeFor(
}

/**
* @Then /^I should see payment for (the "[^"]+" order) as (\d+)(?:|st|nd|rd|th) in the list$/
* @Then /^I should see payment for the ("[^"]+" order) as (\d+)(?:|st|nd|rd|th) in the list$/
*/
public function iShouldSeePaymentForTheOrderInTheList(string $orderNumber, int $position): void
public function iShouldSeePaymentForTheOrderInTheList(OrderInterface $order, int $position): void
{
Assert::true($this->responseChecker->hasItemOnPositionWithValue(
$this->client->getLastResponse(), $position - 1, 'order', sprintf('/new-api/orders/%s', $orderNumber)
$this->client->getLastResponse(), $position - 1, 'order', sprintf('/new-api/orders/%s', $order->getTokenValue())
));
}

Expand Down
55 changes: 55 additions & 0 deletions src/Sylius/Behat/Context/Api/Shop/CartContext.php
@@ -0,0 +1,55 @@
<?php

/*
* This file is part of the Sylius package.
*
* (c) Paweł Jędrzejewski
*
* 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\Behat\Context\Api\Shop;

use Behat\Behat\Context\Context;
use Sylius\Behat\Client\ApiClientInterface;
use Sylius\Behat\Client\Request;
use Sylius\Behat\Client\ResponseCheckerInterface;
use Webmozart\Assert\Assert;

final class CartContext implements Context
{
/** @var ApiClientInterface */
private $cartsClient;

/** @var ResponseCheckerInterface */
private $responseChecker;

public function __construct(ApiClientInterface $cartsClient, ResponseCheckerInterface $responseChecker)
{
$this->cartsClient = $cartsClient;
$this->responseChecker = $responseChecker;
}

/**
* @When I see the summary of my cart
*/
public function iSeeTheSummaryOfMyCart(): void
{
$this->cartsClient->create(Request::create('orders'));
}

/**
* @Then my cart should be empty
*/
public function myCartShouldBeEmpty(): void
{
$response = $this->cartsClient->getLastResponse();
Assert::true(
$this->responseChecker->isCreationSuccessful($response),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering, cannot we really check is there nothing in the created cart? Of course, it needs to be created first, but the content is crucial for this step imo 🐴

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, I would like to leave it as it is due to time constraints

Copy link
Member

@Zales0123 Zales0123 May 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 but still should be improved 🚀

'Cart has not been created. Reason: ' . $response->getContent()
);
}
}
4 changes: 4 additions & 0 deletions src/Sylius/Behat/Resources/config/services/api.xml
Expand Up @@ -74,6 +74,10 @@
<argument>provinces</argument>
</service>

<service id="sylius.behat.api_platform_client.cart" class="Sylius\Behat\Client\ApiPlatformClient" parent="sylius.behat.api_platform_client">
<argument>carts</argument>
</service>

<service id="sylius.behat.api_platform_client.shipping_category" class="Sylius\Behat\Client\ApiPlatformClient" parent="sylius.behat.api_platform_client">
<argument>shipping-categories</argument>
</service>
Expand Down
Expand Up @@ -18,6 +18,11 @@
<services>
<defaults public="true" />

<service id="sylius.behat.context.api.shop.cart" class="Sylius\Behat\Context\Api\Shop\CartContext">
<argument type="service" id="sylius.behat.api_platform_client.cart" />
<argument type="service" id="Sylius\Behat\Client\ResponseCheckerInterface" />
</service>

<service id="sylius.behat.context.api.shop.channel" class="Sylius\Behat\Context\Api\Shop\ChannelContext">
<argument type="service" id="sylius.behat.shared_storage" />
</service>
Expand Down
1 change: 1 addition & 0 deletions src/Sylius/Behat/Resources/config/suites.yml
Expand Up @@ -7,6 +7,7 @@ imports:
- suites/api/addressing/managing_countries.yml
- suites/api/addressing/managing_zones.yml
- suites/api/admin/login.yml
- suites/api/cart/shopping_cart.yml
- suites/api/channel/managing_channels.yml
- suites/api/currency/managing_currencies.yml
- suites/api/currency/managing_exchange_rates.yml
Expand Down
@@ -0,0 +1,15 @@
# This file is part of the Sylius package.
# (c) Paweł Jędrzejewski

default:
suites:
api_shopping_cart:
contexts:
- sylius.behat.context.hook.doctrine_orm

- sylius.behat.context.setup.channel

- sylius.behat.context.api.shop.cart

filters:
tags: "@shopping_cart && @api"
21 changes: 21 additions & 0 deletions src/Sylius/Bundle/ApiBundle/Command/PickupCart.php
@@ -0,0 +1,21 @@
<?php

/*
* This file is part of the Sylius package.
*
* (c) Paweł Jędrzejewski
*
* 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\Command;

/**
* @psalm-immutable
*/
class PickupCart
{
}
72 changes: 72 additions & 0 deletions src/Sylius/Bundle/ApiBundle/CommandHandler/PickupCartHandler.php
@@ -0,0 +1,72 @@
<?php

/*
* This file is part of the Sylius package.
*
* (c) Paweł Jędrzejewski
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Sylius\Bundle\ApiBundle\CommandHandler;
lchrusciel marked this conversation as resolved.
Show resolved Hide resolved

use Doctrine\Persistence\ObjectManager;
use Sylius\Bundle\ApiBundle\Command\PickupCart;
use Sylius\Component\Channel\Context\ChannelContextInterface;
use Sylius\Component\Core\Model\ChannelInterface;
use Sylius\Component\Core\Model\OrderInterface;
use Sylius\Component\Currency\Model\CurrencyInterface;
use Sylius\Component\Locale\Model\LocaleInterface;
use Sylius\Component\Resource\Factory\FactoryInterface;
use Sylius\Component\Resource\Generator\RandomnessGeneratorInterface;
use Symfony\Component\Messenger\Handler\MessageHandlerInterface;

final class PickupCartHandler implements MessageHandlerInterface
{
/** @var FactoryInterface */
private $cartFactory;

/** @var ChannelContextInterface */
private $channelContext;

/** @var ObjectManager */
private $orderManager;

/** @var RandomnessGeneratorInterface */
private $generator;

public function __construct(
FactoryInterface $cartFactory,
ChannelContextInterface $channelContext,
ObjectManager $orderManager,
RandomnessGeneratorInterface $generator
) {
$this->cartFactory = $cartFactory;
$this->channelContext = $channelContext;
$this->orderManager = $orderManager;
$this->generator = $generator;
}

public function __invoke(PickupCart $pickupCart)
{
/** @var OrderInterface $cart */
$cart = $this->cartFactory->createNew();

/** @var ChannelInterface $channel */
$channel = $this->channelContext->getChannel();
/** @var LocaleInterface $locale */
$locale = $channel->getDefaultLocale();
/** @var CurrencyInterface $currency */
$currency = $channel->getBaseCurrency();

$cart->setChannel($channel);
$cart->setLocaleCode($locale->getCode());
$cart->setCurrencyCode($currency->getCode());
$cart->setTokenValue($this->generator->generateUriSafeString(10));

$this->orderManager->persist($cart);

return $cart;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How orthodox we want to be regarding command and handlers in API? 😄 I theory, the command handler should not return any value

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no other way ATM. At least without a custom data transformer at the end.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So probably we should implement a custom data transformer as fast as possible. The limits our used tool should no be an excuse for the wrong CQS pattern usage 🖖

}
}