-
Notifications
You must be signed in to change notification settings - Fork 9.4k
JS error in console on checkout when recaptcha for checkout/placing order is not enabled #33741
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
Comments
Hi @basvanpoppel. Thank you for your report.
Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:
For more details, please, review the Magento Contributor Assistant documentation. Please, add a comment to assign the issue:
🕙 You can find the schedule on the Magento Community Calendar page. 📞 The triage of issues 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 |
Can you proceed checkout with this js error? |
Not actually blocker but turn out even you not enable settings (Set NO) but code base still check it by default |
bug reported too fast :) |
✅ Confirmed by @mrtuvn Issue Available: @mrtuvn, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself. |
Hi @basvanpoppel Is P3 priority correct for such case ? |
Component: Recaptcha |
Hi @mrtuvn I would give it P2: Checkout doesn't break (the recaptcha component isn't used, so the JS error doesn't actually hurt) it's not critical and no work-around is needed, which follows P3 description. But this is awful for developers working on checkout: I have several custom ui components, layout processors and mixins on any checkout i work on. If there's an issue after upgrading, I will first look at my customisations, then at third party modules, then Magento. Imagine every dev going through this process after seeing this issue, that's a lot of wasted time - and a lot of unhappy devs. |
I'm working on a PR for this. |
New version new bug. I would sugest to set it P1 or better P -100 !?!?!?!?! My opinion |
In addition to this:
|
@basvanpoppel ? install a clean instance of m2 you can't checkout cause of those issues without reCaptcha configuration being enabled and valid means Magento2 does not work out of the box. By default if extension is in system and is not configured to be used should mean no assets are included in storefront at all. Aim for this in your PR, thanks. |
Fix for the impatient (Magento composer installations). @basvanpoppel -this implements your suggested PR (magento/security-package#302) changes :)
|
@basvanpoppel @dudzio12 although good effort it does not free you from all the loaded reCaptcha mixins defined in following files, those will still wrap what they wrap enabled/disabled layout nodes included or not
it mostly means that even if recaptchas are not used your default behaviour is wrapped and no other module tests for those wrapped conditions and mostly assume that defaults are used |
@basvanpoppel That's understandable of course, I needed patch to kickstart my project's upgrades, so it is what it is. I'm still looking forward for you to complete the PR :) Then I will prepare another patch to fix the issue. @mrtuvn What is the politics on P2 bug fixes? It will be released as separate patch, next patch release |
p2 label is lower priority than p1/p0. It's depend on specific case of ticket. It's not related with fix order deliver to next release. i'm not sure magento team will deliver this fixes as patch (seperate) or regular fix as normal. |
@mrtuvn this issue still has 'ready for dev' status, while this pr should be in the review process, I think: magento/security-package#302 @speedupmate This PR just fixes this specific issue. Preventing other modules from adding wrappers isn't my goal here. If you know of a better way to fix this issue, please share it. |
Part of this is the layoutprocessor style config you are trying to add but with a plugin to requirejs-config collection class to just exclude non-configured extension files on matching conditions (like enabled/disabled , auth failing etc). Even better would be stopping popularising mixins as a main go to weapon in devdocs for editing/wrapping component functionality as this is really a sledgehammer that affects wider scope than usual extension tries to cover. |
@chittima: could you leave some more detailed info please? Like providing commits that fixed this. Just closing an issue with so little explanation is a bit rude ... |
Hello @hostep @mrtuvn @glevhen Let me try to shade more lite on the issue based on communication and collaboration in the internal Jira ticket.
Based on such verification, the ticket was closed by QA with the resolution "cannot reproduce" - according to workflow. If you still experience a similar issue on the latest version of code I would recommend opening a new issue with updated versions/steps and actual/expected results |
Unbelievable !!!! |
It's actually pretty close, the release of 2.4.3-p1 😉 Thanks @sdzhepa for the extra info, however it would still be nice if somebody could figure out how it got fixed. |
Just updated from 2.4.3 to 2.4.3-p1 and still experiencing this error in checkout unless reCAPTCHA is configured. Errors being thrown due to optional configuration not being configured wastes a lot of time. As covered above when updating between versions going through a smashing any log exceptions or console errors thrown post upgrade is standard process and this error in checkout is just another distraction and another pointless issue to investigate. I don't know why errors are seen as acceptable, when the goal should be 0 errors and warnings. Is there actually going to be a fix put in place for this? |
@engcom-November |
Do we expect a patch to be released soon? |
Just a link for those looking for the actual SOLUTION magento/security-package@d25432d |
For anyone looking for a patch on Magento v2.4.3-p1
|
I would have expected this to be released as a Magento Quality Patch, or is this issue too small for that? |
|
Preconditions (*)
Steps to reproduce (*)
Expected result (*)
Actual result (*)
Please provide Severity assessment for the Issue as Reporter. This information will help during Confirmation and Issue triage processes.
ps. I think severity should be higher, because any dev working om checkout will spot this, and will be looking for a fix. (I first checked all of my modules, themes and 3rd party modules: it's time consuming.)
The text was updated successfully, but these errors were encountered: