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][Checkout] cover skipping payment steps in behat #11705

Merged
merged 3 commits into from
Aug 5, 2020

Conversation

AdamKasp
Copy link
Contributor

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
License MIT

@AdamKasp AdamKasp requested a review from a team as a code owner July 31, 2020 12:12
@probot-autolabeler probot-autolabeler bot added the API APIs related issues and PRs. label Jul 31, 2020
@AdamKasp AdamKasp force-pushed the api-skipping-payment-step branch from d59972b to 4162c6b Compare July 31, 2020 12:35
@AdamKasp AdamKasp force-pushed the api-skipping-payment-step branch 4 times, most recently from 808b5ed to 4d279a3 Compare August 3, 2020 12:16
@AdamKasp AdamKasp force-pushed the api-skipping-payment-step branch 3 times, most recently from 82b97ac to e0d9e47 Compare August 4, 2020 08:30
src/Sylius/Behat/Context/Api/Shop/CheckoutContext.php Outdated Show resolved Hide resolved
src/Sylius/Behat/Context/Api/Shop/CheckoutContext.php Outdated Show resolved Hide resolved
Comment on lines 60 to 63
$shippingMethodName = 'Free';
if ($shippingMethod !== null) {
$shippingMethodName = $shippingMethod->getName();
}
Copy link
Member

Choose a reason for hiding this comment

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

Why did you add it? Shipping method shouldn't be null

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because i reuse function iSelectShippingMethod where ShippingMethod can be null because function: iProceedSelectingBillingCountryAndShippingMethod can have null as a agument

@@ -41,21 +42,25 @@ public function __construct(
}

/**
* @Given I have proceeded selecting :shippingMethodName shipping method
* @When I proceed with :shippingMethodName shipping method
* @Given I have proceeded selecting :shippingMethod shipping method
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 you changed the implementation of steps in ui context?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

at one of the API steps, I needed to get shippingMethod instead shipping method name that implicated changes also in UI steps

Copy link
Member

Choose a reason for hiding this comment

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

I understand, but this shouldn't force changes in UI context, you can have it implemented @Given I have proceeded selecting :shippingMethodName shipping method in UI context and @Given I have proceeded selecting :shippingMethod shipping method in API context

@AdamKasp AdamKasp force-pushed the api-skipping-payment-step branch from badf27b to bebcbc2 Compare August 4, 2020 12:35
@AdamKasp AdamKasp force-pushed the api-skipping-payment-step branch 2 times, most recently from e1d9ea1 to df96304 Compare August 5, 2020 06:49
@AdamKasp AdamKasp force-pushed the api-skipping-payment-step branch from df96304 to d32ff14 Compare August 5, 2020 07:43
/**
* @Then I should not see any information about payment method
*/
public function iShouldNotSeeAnyInformationAboutPaymentMethod()
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
public function iShouldNotSeeAnyInformationAboutPaymentMethod()
public function iShouldNotSeeAnyInformationAboutPaymentMethod(): void

@@ -116,7 +117,7 @@ public function iProceedSelectingPaymentMethod($paymentMethodName)
* @Given I have proceeded order with :shippingMethodName shipping method and :paymentMethodName payment
* @When I proceed with :shippingMethodName shipping method and :paymentMethodName payment
*/
public function iProceedOrderWithShippingMethodAndPayment($shippingMethodName, $paymentMethodName)
public function iProceedOrderWithShippingMethodAndPayment(string $shippingMethodName, $paymentMethodName)
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
public function iProceedOrderWithShippingMethodAndPayment(string $shippingMethodName, $paymentMethodName)
public function iProceedOrderWithShippingMethodAndPayment(string $shippingMethodName, string $paymentMethodName): void

@GSadee GSadee merged commit 54028af into Sylius:master Aug 5, 2020
@GSadee
Copy link
Member

GSadee commented Aug 5, 2020

Thank you, Adam! 🎉

GSadee added a commit that referenced this pull request Aug 7, 2020
This PR was merged into the 1.8-dev branch.

Discussion
----------

| Q               | A
| --------------- | -----
| Branch?         | master
| Bug fix?        | no
| New feature?    | no
| BC breaks?      | no
| Deprecations?   | no
| Related tickets | based on #11705 

| License         | MIT

<!--
 - Bug fixes must be submitted against the 1.7 branch (the lowest possible)
 - Features and deprecations must be submitted against the master 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
-------

fb50dab [Api] Fix failing steps
@AdamKasp AdamKasp deleted the api-skipping-payment-step branch July 19, 2021 06:20
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.

2 participants