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

Check mimum quantity for products in the basket order #15872

Merged
merged 6 commits into from Oct 16, 2019

Conversation

@matthieu-rolland
Copy link
Contributor

matthieu-rolland commented Oct 8, 2019

Questions Answers
Branch? develop
Description? Fixes bug occuring when someone adds a product on a single, someone in the BO changes the minimal quantity, and later the FO user refreshes his basket page.
Type? bug fix
Category? FO
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #14909
How to test? See the referenced issue, but to sum up:

1/ In FO, add a product in your basket (quantity: one)
2/ In BO, increase the minimum quantity for this product (ex: 10)
3/ Refresh the basket page
Expected result:

  • You get an error message warning you about the minimum quantity
  • If you don't refresh but click on "order" (commander) directly, it will redirect you back to the basket page, with the same error message

This change is Reviewable

@matthieu-rolland matthieu-rolland requested a review from PrestaShop/prestashop-core-developers as a code owner Oct 8, 2019
@@ -98,6 +98,10 @@ public function initContent()
Tools::redirect('index.php');
}
// check that minimal quantity conditions are respected for each product in the cart
if (!Tools::getValue('ajax')) {

This comment has been minimized.

Copy link
@PierreRambaud

PierreRambaud Oct 9, 2019

Contributor

Any reason to don't check it in ajax context?

This comment has been minimized.

Copy link
@matthieu-rolland

matthieu-rolland Oct 9, 2019

Author Contributor

Ajax calls on the cart page also go through this method, but does its own checks later and sends back a raw html template to update the page.

Here we want to do this check when the page is loading (or being refreshed), just in case someone comes back to a cart he created earlier, and minimal quantities required for a product changed since then.

(if I don't add this check ajax calls would display two error messages)

This comment has been minimized.

Copy link
@jolelievre

jolelievre Oct 14, 2019

Contributor

Then you should add a comment to explain this behaviour

This comment has been minimized.

Copy link
@matthieu-rolland

matthieu-rolland Oct 15, 2019

Author Contributor

good idea, just did it 👍

@matthieu-rolland matthieu-rolland force-pushed the matthieu-rolland:fix-minimal-quantity branch from cd2fe2a to a561910 Oct 15, 2019
Copy link
Contributor

PierreRambaud left a comment

Reviewed 1 of 4 files at r1, 1 of 2 files at r2.
Reviewable status: 2 of 4 files reviewed, 2 unresolved discussions (waiting on @matthieu-rolland and @PierreRambaud)


controllers/front/CartController.php, line 101 at r2 (raw file):

        }

        /* check that minimal quantity conditions are respected for each product in the cart
       /** 
         * Check that minimal quantity conditions are respected for each product in the cart
         * (this is to be applied only on page load, not for ajax calls) 
         */

Watch out with comments indent, if you use multi lines you need to use this syntax

Copy link
Contributor

PierreRambaud left a comment

Reviewable status: 2 of 4 files reviewed, 3 unresolved discussions (waiting on @matthieu-rolland and @PierreRambaud)


themes/classic/templates/checkout/_partials/cart-detailed-product-line.tpl, line 132 at r2 (raw file):

                value="{$product.quantity}"
                name="product-quantity-spin"
                min="{$product.minimal_quantity}"

Still use by js, isn't a problem?

@matthieu-rolland

This comment has been minimized.

Copy link
Contributor Author

matthieu-rolland commented Oct 15, 2019

Reviewable status: 2 of 4 files reviewed, 3 unresolved discussions (waiting on @matthieu-rolland and @PierreRambaud)

themes/classic/templates/checkout/_partials/cart-detailed-product-line.tpl, line 132 at r2 (raw file):

                value="{$product.quantity}"
                name="product-quantity-spin"
                min="{$product.minimal_quantity}"

Still use by js, isn't a problem?

@PierreRambaud It's used in JS, and the attribute is here, so no I don't think it's a problem ? 🤔
The attribute I removed is in cart-detailed-product-line.tpl, and this is one is not used in JS, as far as I know (do you think I missed something? )

@matthieu-rolland matthieu-rolland force-pushed the matthieu-rolland:fix-minimal-quantity branch from ff8149b to cb2ea2b Oct 16, 2019
Copy link
Contributor Author

matthieu-rolland left a comment

Reviewable status: 1 of 4 files reviewed, 1 unresolved discussion (waiting on @PierreRambaud)


controllers/front/CartController.php, line 101 at r2 (raw file):

Previously, PierreRambaud (GoT) wrote…
       /** 
         * Check that minimal quantity conditions are respected for each product in the cart
         * (this is to be applied only on page load, not for ajax calls) 
         */

Watch out with comments indent, if you use multi lines you need to use this syntax

Done !

@matthieu-rolland

This comment has been minimized.

Copy link
Contributor Author

matthieu-rolland commented Oct 16, 2019

@PierreRambaud PrettyCi seems happy now 👍

@Robin-Fischer-PS Robin-Fischer-PS added this to the 1.7.7.0 milestone Oct 16, 2019
@matks

This comment has been minimized.

Copy link
Contributor

matks commented Oct 16, 2019

Thank you @matthieu-rolland

@matks matks merged commit 277ffaf into PrestaShop:develop Oct 16, 2019
2 of 3 checks passed
2 of 3 checks passed
code-review/reviewable 3 files, 1 discussion left (PierreRambaud, Robin-Fischer-PS)
Details
PrettyCI Code formatting
Details
Travis CI - Pull Request Build Passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.