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

[4.3]: OrderQuery::withCustomer is useless #3211

Closed
Anubarak opened this issue Jul 6, 2023 · 2 comments
Closed

[4.3]: OrderQuery::withCustomer is useless #3211

Anubarak opened this issue Jul 6, 2023 · 2 comments
Labels
bug commerce4 Issues related to Commerce v4

Comments

@Anubarak
Copy link

Anubarak commented Jul 6, 2023

What happened?

Description

OrderQuery::withCustomer is pretty much useless since Order::setEmail already fetches the customer

/**
 * Sets the orders user based on the email address provided.
 *
 * @param string|null $email
 * @throws Exception
 */
public function setEmail(?string $email): void
{
    if (!$email) {
        $this->_customer = null;
        $this->_customerId = null;
        $this->_email = null;
        return;
    }

    if ($this->_email === $email) {
        return;
    }

    $user = Craft::$app->getUsers()->ensureUserByEmail($email);
    $this->_email = $email;
    $this->setCustomer($user);
}

Because of that I have a bunch of duplicated queries and I'm not sure how to get rid of it

Steps to reproduce

  1. execute Order::find()->customerId(1)->withCustomer()->all()
  2. think there should be none duplicate queries
  3. see there are N duplicate queries for each order

Expected behavior

  1. no duplicates

Solution to solve this:

  1. do not execute Craft::$app->getUsers()->ensureUserByEmail($email); in case the customer is going to be eager loaded (maybe pass a parameter to the order)

Craft CMS version

4.x

Craft Commerce version

4.x

PHP version

8.2

Operating system and version

No response

Database type and version

No response

Image driver and version

No response

Installed plugins and versions

No response

@Anubarak Anubarak added commerce4 Issues related to Commerce v4 bug labels Jul 6, 2023
@lukeholder
Copy link
Member

Hi @Anubarak

This is true at the moment, you are correct, but with upcoming deprecation of setEmail this will no longer be the case. Please see the PR to follow along: #3201

Thanks.

@Anubarak
Copy link
Author

Anubarak commented Jul 10, 2023

@lukeholder Thank you very much
Do you know when this will be included?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug commerce4 Issues related to Commerce v4
Projects
None yet
Development

No branches or pull requests

2 participants