-
Notifications
You must be signed in to change notification settings - Fork 30.1k
[IMP] website_sale*: move stock check at checkout #192183
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
base: master
Are you sure you want to change the base?
[IMP] website_sale*: move stock check at checkout #192183
Conversation
09f80cc
to
ce3d2ec
Compare
f5afd28
to
6634cb2
Compare
11c7a04
to
a46c5ab
Compare
36b5b0e
to
0028ce6
Compare
This comment was marked as outdated.
This comment was marked as outdated.
0028ce6
to
a83a0d8
Compare
ab2ff81
to
85020da
Compare
c76d884
to
68b8824
Compare
Hello @vchu-odoo , sorry for all the late changes 🫣 I noticed yesterday that I had committed a formatted version of the website_sale/**/templates.xml file which added a bunch of diff for no reason. While fixing this, I also took a bit of time to rework this file, it should be the same but cleaner IMO 😄. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the great work!
here are some questions/suggestions.
Btw, really appreciate good test coverage for website_sale!
af44c27
to
1329062
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small comments left.
Could you please rebase since there are conflicts and squash as there are too many commits? :)
Thank you for your work!
if clear: | ||
self.shop_warning = '' | ||
return warn | ||
def _pop_shop_warnings(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you always call it only for one order, why do you need to support multiple?
"""Whether all the order lines are available on the current website.""" | ||
self.ensure_one() | ||
# Uses list comprehension to ensure all the shop warnings are updated. | ||
return all([sol._check_availability() for sol in self.order_line]) # noqa: C419 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why?
return all([sol._check_availability() for sol in self.order_line]) # noqa: C419 | |
return all([sol._check_availability() for sol in self.order_line]) |
1329062
to
0a18827
Compare
Seems like a big rebase is needed here 👀 |
e40e986
to
d9cc402
Compare
d9cc402
to
71bcfa1
Compare
*: payment_stripe, website_event_sale, website_sale_collect, website_sale_loyalty, website_sale_mondialrelay, website_sale_stock Before this commit, it was challenging to add logic to the checkout flow's checking process as all major checks were performed at the payment step using the same method. This commit lays the foundation for future improvements by revamping the checking process during the checkout flow. Consequently: - The checks are now split at each step: cart overview, delivery, payment. - To proceed to the next step, the previous steps must be valid. - The warnings have been reworked. The primary motivation and initial benefit of this refactor is to improve stock validation. Consequently: - The stock check is now performed earlier in the checkout process: - On the cart overview step, the customer can only proceed if at least one warehouse on the website can fulfill the order. - When click and collect is installed, the stock is checked again on the delivery step using the selected delivery method, as they may not share the same stock source. - A new badge is added on the delivery selection step to indicate whether the delivery method is available for the order, helping customers understand the differences between in-store and standard delivery methods. See also: - odoo/enterprise#77155 task-4328322
71bcfa1
to
dccc001
Compare
@Feyensv its time 😱 (When you can don't worry😅) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not going too deep for this first review, mostly wanted to highlight some things in the current approach that seem potentially wrong or could be improved 👀
- sometimes the super call comes first, sometimes it comes last (did you have a specific logic/order for that?)
- the current logic is kinda flawed if there are multiple kind of warnings on the cart, you'll only see the last one returned by the method, not the others.
- You consider shop warnings as 'blocking', but warnings weren't blocking before. Some warnings put on the order/lines are only informative (and added by the add to cart call) but now if you get those, you won't be able to continue your checkout if you have any 👀

values.update(self._get_cart_update_values(order_sudo)) | ||
return values | ||
|
||
def _get_cart_update_values(self, order_sudo): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method name seems kinda confusing to me, sounds like the values you provide to a method call, not the values you get as feedback from the request.
The stock check during the checkout process is currently performed at the payment step once the origin of the goods is known. However, it would be more beneficial to execute this check further down the process if we are certain that the products are not available in any warehouse. This way, we can prevent the user from proceeding if the setup is not valid.
Before introducing new changes, this PR first introduces a refactor of the checking process during the checkout flow to allow for the improvement that follows. Consequently:
Following the revamp of the checking process:
task-4328322
See also:
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr