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

checkout: Allow fully discounted carts to be placed. #198

Merged
merged 13 commits into from
Mar 24, 2020

Conversation

C4rter
Copy link
Contributor

@C4rter C4rter commented Mar 10, 2020

Before, payment was always required even if the grand total of a cart was zero.
Now, when no payment is required the order is being placed without payment processing.
In the checkoutform, no payment selection is set in this case.

  • Add changelog
  • Fix float comparison issues (currently tests are failing)
  • Move some code in tests to helper functions
  • Move coupon logic from inmemory cart to integration test layer
  • Check new standard place order

Before, payment was always required even if the grand total of a cart was zero.
Now, when no payment is required the order is being placed without payment processing.
In the checkoutform, no payment selection is set in this case.
@zohmi
Copy link
Contributor

zohmi commented Mar 10, 2020

Would be nice to start with some tests for the checkoutcontroller.

@danielpoe
Copy link
Member

Would be nice to start with some tests for the checkoutcontroller.

If we start this we should do black box functional test and avoid mocking all dependencies of the controller imho.

@carstendietrich
Copy link
Member

@danielpoe is the in memory cart intended to be used in a production shop? I added a 100% discount voucher to be able to test our changes here.. would duplicate the in memory adapter to the integration test project if needed..

@danielpoe
Copy link
Member

@danielpoe is the in memory cart intended to be used in a production shop? I added a 100% discount voucher to be able to test our changes here.. would duplicate the in memory adapter to the integration test project if needed..

Its not ready yet but I think we should have this in mind:

  • probably the cartstorage should be exchangeable in future (to work in cluster - e.g. with redis)
  • the supported coupons could also be configured then - maybe we add an optional cartVoucherCodeEvaluation interface that you can inject in the integrationtests with your testdata?

…g payment. Add additional check for zero cart to review step.
@carstendietrich
Copy link
Member

carstendietrich commented Mar 12, 2020

@danielpoe is the in memory cart intended to be used in a production shop? I added a 100% discount voucher to be able to test our changes here.. would duplicate the in memory adapter to the integration test project if needed..

Its not ready yet but I think we should have this in mind:

  • probably the cartstorage should be exchangeable in future (to work in cluster - e.g. with redis)
  • the supported coupons could also be configured then - maybe we add an optional cartVoucherCodeEvaluation interface that you can inject in the integrationtests with your testdata?

Okay for the first step we'll add the possibility of the cartVoucherCodeEvaluation to the in memory behaviour. For supporting redis cart storage I'll add an issue (see #199).

Currently we are also facing float rounding issues so cart.GrandTotal().IsZero() doesn't work always. See randomly failing tests.

@carstendietrich
Copy link
Member

@danielpoe we added a GiftCardHandler and VoucherHandler interface so that we are able to use the InMemory Cart with project / test specific voucher / gift card handling. You may have a look :)

@danielpoe
Copy link
Member

Awesome :-)

@zohmi
Copy link
Contributor

zohmi commented Mar 24, 2020

Thanks for contributing :) Only left some minor feedback

@zohmi zohmi merged commit 988ab52 into master Mar 24, 2020
@carstendietrich carstendietrich deleted the support-fully-discounted-carts branch April 6, 2020 11:28
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.

None yet

5 participants