Fix invalid address id on quote address validation

[ihor-sviziev]

Description (*)

Our clients are randomly getting an issue on checkout like Invalid customer address id 12356, there are no clear steps to reproduce, and because of that, they can't place an order.

The error message looks like this:

The shipping information was unable to be saved. Error: "Invalid customer address id 12345"

and happening on POST /rest/default/V1/carts/mine/shipping-information request

During the research, I found quite an old thread where people occasionally complain that issues are reproducing on them starting in 2019 and now (2024) on different Magento versions.

I analyzed what people are noticed and symptoms following: the issue happens when, in DB, for the same quote, we have customer_is_guest set to 1 AND customer_id set to some specific value (not null or 0).

How exactly this is happening is pretty hard to understand, but this is happening in the following flow:

• Quote saving is happening, we have a "customer" object, so we're updating customer_id field ✅, but NOT a is_customer_guest ❌ (so it's becoming out of sync) https://github.com/magento/magento2/blob/b9b23b94be839ff1036b71b306053ceef099ec35/app/code/Magento/Quote/Model/Quote.php#L859-L861
• Quote address validation starts during saving shipping information for placing an order, it checks that customer is_customer_guest flag is set to 0 ❌ (it is guest) and quote address IS linked to customer ✅(it isn't guest) - so validation fails ❌. https://github.com/magento/magento2/blob/f4c3cdc28dcdf95b3693de5f22629cd9f28a40a3/app/code/Magento/Quote/Model/QuoteAddressValidator.php#L154-L160
  https://github.com/magento/magento2/blob/f4c3cdc28dcdf95b3693de5f22629cd9f28a40a3/app/code/Magento/Quote/Model/QuoteAddressValidator.php#L81-L87

To fix this issue, I suggest implementing 2 fixes:

• ~~fix incorrect flag setting for is_customer_guest where the customer id is set~~
  • ~~this change fixes incorrect behavior in that might cause also other issues, fix was suggested by @ssstankiewicz in https://github.com/magento/magento2/issues/23618#issuecomment-929293382~~
  • Looks like tests are failing because of this change, so I reverted it
• improve validation, so that it will work with customer_id field rather than is_customer_guest, so it won't fail to place an order when it's not updated properly
  • this change fixes this specific issue and checkout won't fail, fix suggested by @tonycr7 in https://github.com/magento/magento2/issues/23618#issuecomment-521103233

About test coverage: I don't have time to cover this change with some tests, but I'm sure receiving orders is more important than test coverage, so I would accept it now, and add test coverage later. If someone wants to write test coverage - I can add it.

Related Pull Requests

Fixed Issues (if relevant)

1. Fixes https://github.com/magento/magento2/issues/23618

Manual testing scenarios (*)

1. ...
2. ...

Questions or comments

Contribution checklist (*)

• [ ] Pull request has a meaningful description of its purpose
• [ ] All commits are accompanied by meaningful commit messages
• [ ] All new or changed code is covered with unit/integration tests (if applicable)
• [ ] README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
• [ ] All automated tests passed successfully (all builds are green)

[m2-assistant[bot]]

Hi @ihor-sviziev. Thank you for your contribution! Here are some useful tips on how you can test your changes using Magento test environment.

Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento give me test instance - deploy test instance based on PR changes
• @magento give me 2.4-develop instance - deploy vanilla Magento instance

:exclamation: Automated tests can be triggered manually with an appropriate comment:

• @magento run all tests - run or re-run all required tests against the PR changes
• @magento run <test-build(s)> - run or re-run specific test build(s) For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:

1. Database Compare
2. Functional Tests CE
3. Functional Tests EE
4. Functional Tests B2B
5. Integration Tests
6. Magento Health Index
7. Sample Data Tests CE
8. Sample Data Tests EE
9. Sample Data Tests B2B
10. Static Tests
11. Unit Tests
12. WebAPI Tests
13. Semantic Version Checker

You can find more information about the builds here :information_source: Run only required test builds during development. Run all test builds before sending your pull request for review.

---

For more details, review the Code Contributions documentation. Join Magento Community Engineering Slack and ask your questions in #github channel.

[ihor-sviziev]

@magento run all tests

[ihor-sviziev]

@magento run all tests

[tufahu]

Hi @ihor-sviziev first thank you for the fix!

I am currently working on a production site with these bugs and want to find a quick solution. And thinking about this bug; The change on the app/code/Magento/Quote/Model/Quote.php is not enough?

https://github.com/magento/magento2/pull/38563/files#diff-e0faea9feda5e9238db347acd0149fe4f0b4edb38db8899f8f5c8584235f5313R861

So the first fix you mentioned and shipped by ssstankiewicz seems enough for me. And of course because it's a state bug, the database should be fixed like https://github.com/magento/magento2/issues/23618#issuecomment-2031476354 once

(At first sight the change in app/code/Magento/Quote/Model/QuoteAddressValidator.php seems a bulletproof solution | workaround to handle inconsistent state in database) Or maybe i'm not seeing the whole problem.

What do you think? ssstankiewicz's plugin idea and database fix should be enough? Thanks in advance.

[ihor-sviziev]

@magento run all tests

[ihor-sviziev]

@tufahu, I was actually surprised that so many failed tests appeared by my changes, so now checking which part is causing it

[dphilipps]

For what it's worth, this is the way I reproduced this issue on our system at the end of 2022.

1. Guest checkout off, persistent carts off
2. Set cookie timeout in admin to 120 seconds
3. Sign in to storefront and browse to a product page
4. Add to cart
5. Leave the product page for over 120 seconds without interaction (so the session expires)
6. Click 'Add to cart' again.

This last step attempts to add the product to an expired session, and I believe causes the issue in the quote table.

I've always managed to resolve the issue on a case by case basis with the following SQL. UPDATE quote SET customer_is_guest = 0 WHERE customer_id != 0 AND customer_is_guest = 1 and entity_id = [customer id] limit 1;

I can't reproduce this currently on 2.4.5-p6. I'll try ASAP on the current dev branch and report back.

[ihor-sviziev]

@magento run Functional Tests B2B, Functional Tests EE, WebAPI Tests

[thomas-kl1]

I was wondering why this change was so familiar, it turns out it's something we apply to many of our clients 😅 I thought for a moment it was already fixed in the core. I confirm this issue is really hard to reproduce and happens "randomly" on customers.

[ihor-sviziev]

@magento run Functional Tests B2B, Functional Tests EE, WebAPI Tests

[ihor-sviziev]

@magento run Functional Tests CE, Functional Tests B2B, Functional Tests EE, WebAPI Tests

Failures look not related, let's restart

[engcom-Hotel]

Hello @ihor-sviziev,

It seems the implementation is still pending this PR. Hence we are closing this PR for now, please feel free to reopen it whenever you are ready with it.

Thanks

[ihor-sviziev]

@engcom-Hotel, what's wrong with this PR? As I stated above, test failures look not related

[engcom-Hotel]

Hello @ihor-sviziev,

This PR was in Draft status, if you are done with all the implementation we can reopen it for further process.

Please let us know.

Thanks

[ihor-sviziev]

I see the issue was already solved in https://github.com/magento/magento2/commit/8c14c08bed4e1557a005570f99fc6d51dd75f49c#diff-eea88f8d9d09ee3e619a91aa55a1360085f16d41d98de640edf0371a1e24c43aR159, so I'm closing this PR

[Quazz]

This needs to be checked again. The referenced commit never made it in, instead some other stuff was done, but the needed change appears to not have been applied.

The error can still occur without a patch.

[thomas-kl1]

@Quazz the fix will be released along 2.4.8