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

Cover "Order details" scenarios in API #15553

Merged
merged 26 commits into from Dec 6, 2023
Merged

Conversation

jakubtobiasz
Copy link
Member

Q A
Branch? 1.13
Bug fix? no
New feature?
BC breaks? no
Deprecations?
Related tickets
License MIT

@jakubtobiasz jakubtobiasz added the API APIs related issues and PRs. label Nov 21, 2023
@jakubtobiasz jakubtobiasz requested a review from a team as a code owner November 21, 2023 17:04
@jakubtobiasz jakubtobiasz changed the title Cover seeing_order.feature scenario 🚧 Cover "Order details" scenarios in API Nov 21, 2023
Copy link

github-actions bot commented Nov 21, 2023

Bunnyshell Preview Environment deployed

It will be automatically stopped in 4 hours.

Use the command /bns:start to start it if it's stopped.

Component Endpoints
mailhog https://mailhog-w7roq4.bunnyenv.com/
php https://store-w7roq4.bunnyenv.com/

Available commands:

  • /bns:stop to stop the environment
  • /bns:deploy to redeploy the environment
  • /bns:delete to remove the environment

jakubtobiasz and others added 21 commits December 4, 2023 16:02
@Wojdylak Wojdylak changed the title 🚧 Cover "Order details" scenarios in API Cover "Order details" scenarios in API Dec 4, 2023
NoResponseMate
NoResponseMate previously approved these changes Dec 5, 2023
Comment on lines 233 to 242
* "@id": string,
* "@type": string,
* "variant": string,
* "productName": string,
* "id": int,
* "quantity": int,
* "unitPrice": int,
* "originalUnitPrice": int,
* "total": int,
* "subtotal": int
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* "@id": string,
* "@type": string,
* "variant": string,
* "productName": string,
* "id": int,
* "quantity": int,
* "unitPrice": int,
* "originalUnitPrice": int,
* "total": int,
* "subtotal": int
* "productName": string

Adding everything seems like an overkill

Comment on lines 317 to 320
* @Then it should have shipment in state :state
* @Then /^order "[^"]+" should have shipment state "([^"]+)"$/
*/
public function itShouldHaveShipmentState(string $state): void
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* @Then it should have shipment in state :state
* @Then /^order "[^"]+" should have shipment state "([^"]+)"$/
*/
public function itShouldHaveShipmentState(string $state): void
* @Then /^(it) should have shipment in state "([^"]+)"$/
* @Then /^(order "[^"]+") should have shipment state "([^"]+)"$/
*/
public function itShouldHaveShipmentState(OrderInterface $order, string $state): void

Should work virtually the same since we're getting an order from shared storage either way

TheMilek
TheMilek previously approved these changes Dec 5, 2023
Comment on lines 500 to 508
/** @var OrderInterface $order */
$order = $this->sharedStorage->get('order');

$adjustments = $this->client->subResourceIndex(
Resources::ORDERS,
Resources::ADJUSTMENTS,
$order->getTokenValue(),
forgetResponse: true,
);
Copy link
Member

Choose a reason for hiding this comment

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

I would be after moving it to some private method, as we are using it several times 🤔

Suggested change
/** @var OrderInterface $order */
$order = $this->sharedStorage->get('order');
$adjustments = $this->client->subResourceIndex(
Resources::ORDERS,
Resources::ADJUSTMENTS,
$order->getTokenValue(),
forgetResponse: true,
);
$adjustments = $this->getAdjustmentsForOrder();

GSadee
GSadee previously approved these changes Dec 6, 2023
Comment on lines 475 to 476
/** @var OrderInterface $order */
$order = $this->sharedStorage->get('order');
Copy link
Member

Choose a reason for hiding this comment

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

It can be provided by the transformer

}

/**
* @Then /^its subtotal should be ([^"]+)$/
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @Then /^its subtotal should be ([^"]+)$/
* @Then its subtotal should be :subtotal

Comment on lines +926 to +929
Assert::same(
$this->responseChecker->getValue($this->client->getLastResponse(), 'payments'),
[],
);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Assert::same(
$this->responseChecker->getValue($this->client->getLastResponse(), 'payments'),
[],
);
Assert::isEmpty($this->responseChecker->getValue($this->client->getLastResponse(), 'payments'));

}

/**
* @Then /^the order "[^"]+" should have order shipping state "([^"]+)"$/
Copy link
Member

Choose a reason for hiding this comment

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

This step definition does not fit the arguments, as the order number will be taken as the state

Copy link
Member Author

Choose a reason for hiding this comment

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

It won't be as the order number is not between parentheses.

Comment on lines +951 to +954
Assert::same(
$this->responseChecker->getValue($this->client->getLastResponse(), 'shipments'),
[],
);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Assert::same(
$this->responseChecker->getValue($this->client->getLastResponse(), 'shipments'),
[],
);
Assert::isEmpty($this->responseChecker->getValue($this->client->getLastResponse(), 'shipments'));

foreach ($adjustments as $adjustment) {
if (in_array($adjustment['order_item_unit']['@id'], $orderItem['units'])) {
Assert::same($this->getTotalAsInt(trim($price," ~")), $adjustment['amount']);
return;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return;
return;

if ($isMinus) {
return $amount * -1;
}
return $amount;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return $amount;
return $amount;

Comment on lines +77 to +81
<service id="Sylius\Bundle\ApiBundle\Controller\GetOrderAdjustmentsAction">
<argument type="service" id="sylius.repository.order" />
<tag name="controller.service_arguments" />
</service>

Copy link
Member

Choose a reason for hiding this comment

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

It should be added to services/controllers.xml

@GSadee GSadee merged commit 1b255b4 into Sylius:1.13 Dec 6, 2023
7 checks passed
@GSadee
Copy link
Member

GSadee commented Dec 6, 2023

Thanks, Jacob! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API APIs related issues and PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants