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

[WIP] Integrate the promotion coupons #429

Merged
merged 10 commits into from
Oct 24, 2013
Merged

[WIP] Integrate the promotion coupons #429

merged 10 commits into from
Oct 24, 2013

Conversation

jjanvier
Copy link
Contributor

To fix #377

@antonioperic
Copy link
Contributor

go, go, go :)

@pjedrzejewski
Copy link
Member

@jjanvier Nice work, quick tip = https://github.com/Sylius/Sylius/blob/master/features/frontend/checkout_inventory.feature#L38, I added this step definition to simplify adding a product with specified quantity. Should help you making the scenarios a bit shorter. :)

@jjanvier
Copy link
Contributor Author

Thanks ! It will be much better indeed.

@jjanvier
Copy link
Contributor Author

What's done so far :

  • Behat scenarios to handle all types of promotions
  • PromotionBundle : fixing bug on non existent "equal" configuration
  • PromotionBundle : fixing bug on usage limit (usage limit was not checked for promotions)
  • Promotion discount is now applied on the total items value (and not order total)
  • Coupons are integrated through a CouponHandler
  • PromotionProcessor is called every time the cart is saved

Two Behat tests fails because I can not know, with the actual code, if a coupon is eligible to the user's cart. Should I delete those tests or should we find a way to change the method isEligible of PromotionEligibilityChecker ?

ping @pjedrzejewski @stloyd for opinion and review

@pjedrzejewski
Copy link
Member

@jjanvier Dang, I forgot to tell you that there is a sylius_promotion_coupon_to_code form type... It takes the promotion code entered by user, and automatically transforms it into a Coupon instance if found. See the form type here. This allows you to remove the "couponCode" field from the order and leave only the "coupon", using this cool form type. Coupon handler is not needed as well. Also, this "coupon to code" field checks the usage limits, if coupon has been used to many times, it is "invisible". Makes sense?

@pjedrzejewski
Copy link
Member

Wrong button, sorry. :)

*/
public function handle(OrderInterface $order)
{
if (null != $code = $order->getCouponCode()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use strict comparison. Same line below.

@jjanvier
Copy link
Contributor Author

@stloyd @pjedrzejewski OK for review

Everything's green and I used the sylius_promotion_coupon_to_code as described by @pjedrzejewski
Rebased.
First commits related to Behat scenarios have been squashed.

And "Promotion total: (€20.00)" should appear on the page
And "Grand total: €180.00" should appear on the page


Copy link
Member

Choose a reason for hiding this comment

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

2 extra lines here.

*/
public function __construct(ObjectRepository $couponRepository)
public function __construct(ObjectRepository $couponRepository, EventDispatcher $dispatcher)
Copy link
Member

Choose a reason for hiding this comment

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

Same as above, you should typehint the interface. :)

@pjedrzejewski
Copy link
Member

Awesome work Julien. 👍

When I add product "Lenny" to cart, with quantity "2"
Then I should be on the cart summary page
And "Promotion total: (€27.75)" should appear on the page
# 5*25 + 2*15 = 155 - 155*5% - 20 = 127.25
Copy link
Member

Choose a reason for hiding this comment

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

I do this too, but let's remove it as well. 💃

@jjanvier
Copy link
Contributor Author

thanks for reviews @stloyd @pjedrzejewski
waiting for Travis, should be OK

pjedrzejewski pushed a commit that referenced this pull request Oct 24, 2013
[WIP] Integrate the promotion coupons
@pjedrzejewski pjedrzejewski merged commit 76dd8ae into Sylius:master Oct 24, 2013
@pjedrzejewski
Copy link
Member

@jjanvier Awesome work man! Thanks! 👍

@jjanvier jjanvier deleted the feature/coupons-integration branch October 24, 2013 15:49
@antonioperic
Copy link
Contributor

Great work guys 👍 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integrate the promotion coupons
4 participants