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

optimize-contructor: #74

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hieuhidro
Copy link

  • Remove $session->getQuote from constructor
  • Load the quote when it is needed.

name: PR Template

Description

  • Enhancement: Remove $session->getQuote() from constructor, load the quote when it is needed. Load the quote from constructor is a bad practice, if the session hasn't loaded the quote from database, this line will load the quote from database, it increase the create object time.
  • This line bypass the function loadCustomerQuote (\Magento\Checkout\Observer\LoadCustomerQuoteObserver)

How To Reproduce

Steps to reproduce the behavior:

  1. Go to 'domain.com/checkout/'
  2. Login as a customer
  3. Affirm constructor by passing the function loadCustomerQuote

Expected behavior

  • Affirm loads the quote after $checkoutSession->loadCustomerQuote()

Screenshots

Benefits

  • Improve performance and fix the issue load wrong customer quote.

Additional information

Hidro Le added 2 commits April 7, 2022 12:22
- Remove $session->getQuote from constructor
- Load the quote when it is needed.
@sunilit42
Copy link

  • I m getting speed issue from popup close to redirect on success page
  • there is no loader, when it is redirect to page, so customer is confuse is doing something or not

can we do something?

@hieuhidro
Copy link
Author

  • I m getting speed issue from popup close to redirect on success page
  • there is no loader, when it is redirect to page, so customer is confuse is doing something or not

can we do something?

You means my update affects those features?

@sunilit42
Copy link

I did not test with your changes
I did not see any link to create issue, so i added comment here
sorry about that

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.

2 participants