Moving array access for CAPTCHA _.isEmpty check

[MellenIO]

When upgrading to 2.3.6 from 2.3.5-p2, a new CAPTCHA zone "payment_processing_request" is added.

If a customer has an existing session (from 2.3.5-p2 or prior), and tries to access checkout with their existing session data, checkout will fail to load because the check to see if captchaData[formId] exists is not a valid function call.
_.isEmpty() only returns a boolean value, so it cannot be enumerated like an array. Array access syntax has been moved in to the check, rather than outside it.

Description (*)

This change allows a _.isEmpty() check in the defaultCaptcha.js file to complete successfully, resolving issues with the checkout failing to load or infinitely loading after upgrading.

Related Pull Requests

N/A

Fixed Issues (if relevant)

N/A - no reported issue for this yet?

Manual testing scenarios (*)

1. Create an instance prior to 2.3.6 (eg: 2.3.5-p2)
2. Load the /checkout route in a valid way (eg: add to basket, create an account, click "go to checkout")
3. Observe the captcha key of the mage-cache-storage local storage key. There should not be an entry for payment_processing_request
4. Upgrade to 2.3.6 or another version that does include the fix introduced via MC-36200
5. Load the /checkout route with the same session as before
6. Observe an infinite load on the checkout page and errors in the console along the lines of:

defaultCaptcha.js:60 Uncaught TypeError: Cannot read property 'timestamp' of undefined
    at UiClass.checkCustomerData (defaultCaptcha.js:60)
    at UiClass.<anonymous> (defaultCaptcha.js:43)
    at Function.each (jquery.js:382)
    at UiClass.initialize (defaultCaptcha.js:38)
    at UiClass.initialize (wrapper.js:109)
    at UiClass._super (wrapper.js:106)
    at UiClass.initialize (loginCaptcha.js:21)
    at UiClass.initialize (wrapper.js:109)
    at new UiClass (class.js:49)
    at Object.initComponent (layout.js:137)

7. Apply patch
8. Attempt to load /checkout again
9. Observe checkout loads as expected

Questions or comments

I'm not sure if an issue is needed for this PR, but happy to make one if one is needed. This will also need to be backported to 2.3.6.

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)
• All automated tests passed successfully (all builds are green)

Resolved issues:

1. resolves [Issue] Moving array access for CAPTCHA _.isEmpty check #31641: Moving array access for CAPTCHA _.isEmpty check

[m2-assistant]

Hi @MellenIO. Thank you for your contribution
Here is some useful tips 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

❗ 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

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

For more details, please, review the Magento Contributor Guide documentation.

⚠️ According to the Magento Contribution requirements, all Pull Requests must go through the Community Contributions Triage process. Community Contributions Triage is a public meeting.

🕙 You can find the schedule on the Magento Community Calendar page.

📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket.

🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel

✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel

[sidolov]

@magento create issue

[ihor-sviziev]

@MellenIO looks good but feels like a duplicate of #30537 #30602 that were closed due to inability to reproduce the issue. I hope this time it's reproducible.
@magento run all tests

[ihor-sviziev]

@magento run Functional Tests CE, Functional Tests EE

[ihor-sviziev]

@magento run Functional Tests CE, Functional Tests EE

[MellenIO]

@MellenIO looks good but feels like a duplicate of #30537 #30602 that were closed due to inability to reproduce the issue. I hope this time it's reproducible.

@ihor-sviziev It should be reproducible as I ran in to this error when upgrading a client. #30537 sounds similar but had a different bug compared to myself, but I wonder if this is just a change between 2.4.x and 2.3.x?

[magento-engcom-team]

Hi @ihor-sviziev, thank you for the review.
ENGCOM-8630 has been created to process this Pull Request

[ihor-sviziev]

@magento run Functional Tests EE

[engcom-Delta]

Hi @MellenIO Could you clarify what issue should be fixed by PR?
When customer registers and login on frontend, payment_processing_request is present in mage-cache-storage local storage key on 2.4-develop

And there is not clear what issue is caused by typo in captchaData)[formId] as JS not operator turn undefined value of captchaData)[formId] into true

Also issue #30537 is reproducible on PR branch

[engcom-Hotel]

@magento run Functional Tests EE

[MellenIO]

@engcom-Delta did you create the customer & visit checkout on a version without the fix introduced in MC-36200?

If you create the customer & then try to checkout in a prior version, you should have the key missing in your cache storage. What version did you initially upgrade from/to?

And there is not clear what issue is caused by typo in captchaData)[formId] as JS not operator turn undefined value of captchaData)[formId] into true

If you don't have a key in your cache storage that Magento expects, the code will crash because _.isEmpty(var) will not return an array, but rather a boolean. From my testing in Firefox & Chrome's web consoles, doing any form of booleanValue[arrayKey] will give you undefined, which is a falsey value.

[engcom-Hotel]

@magento run Functional Tests EE

[engcom-Hotel]

@magento run Functional Tests EE

[magento-engcom-team]

Hi @ihor-sviziev, thank you for the review.
ENGCOM-8630 has been created to process this Pull Request

[engcom-Delta]

✔️ QA passed
Was checked case described in Manual testing scenarios
Result:
Before:
❌ An infinite load on the checkout page and errors in the console

After:
✔️ Checkout page is available, no errors in console

[engcom-Delta]

Note: Automation tests are passed

[m2-assistant]

Hi @MellenIO, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.