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

[BugFix][Core] Added shipping method eligibility checker to core methods resolver #5951

Merged

Conversation

koemeet
Copy link
Contributor

@koemeet koemeet commented Sep 1, 2016

Q A
Bug fix? yes
New feature? no
BC breaks? no
Related tickets fixes #5861
License MIT

Check for shipping method eligibility when getting shipping methods based on zone and channel.

@@ -49,6 +52,7 @@ function it_implements_shipping_methods_by_zones_and_channel_resolver_interface(
}

function it_returns_shipping_methods_matched_for_shipment_order_shipping_address_and_order_channel(
$eligibilityChecker,
Copy link
Member

Choose a reason for hiding this comment

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

Could you please use type hints also for constructor dependencies mocks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I didn't know that was possible, nice!

@koemeet
Copy link
Contributor Author

koemeet commented Sep 9, 2016

Ready.

@pjedrzejewski pjedrzejewski merged commit cd9e2ca into Sylius:master Sep 10, 2016
@pjedrzejewski
Copy link
Member

Thank you Steffen! We should add scenarios for this functionality as it has not been migrated from the "legacy" properly.

@koemeet koemeet deleted the bugfix/core-shipping-methods-resolver branch September 10, 2016 10:53
@koemeet koemeet restored the bugfix/core-shipping-methods-resolver branch September 16, 2016 13:54
pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019
…methods-resolver

[BugFix][Core] Added shipping method eligibility checker to core methods resolver
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shipping categories ?
3 participants