-
Notifications
You must be signed in to change notification settings - Fork 2
Asd 1258 #111
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
Asd 1258 #111
Conversation
…d MFTF, fix frontend error handling
| <actionGroup ref="GoToCheckoutFromMinicartActionGroup" stepKey="goToCheckoutFromMiniCart2"/> | ||
| <waitForPageLoad stepKey="waitForPageLoad"/> | ||
| <!--Change and submit fraudulent shipping address--> | ||
| <actionGroup ref="AmazonShipmentFormActionGroup" stepKey="submitFraudulentShippingAddress" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm possibly missing it, but where does the mismatched address get entered?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's coming from the AmazonShipmentFormActionGroup actionGroup (line 24 here, AmazonShipmentFormActionGroup.xml). Think that was introduced to test Pay Now when it made its debut
Model/CheckoutSessionManagement.php
Outdated
| $result = [ | ||
| 'success' => false, | ||
| 'message' => $this->getTranslationString($message), | ||
| 'responseText' => [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this result used anywhere else that needs updated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a weird thing... in the frontend place-order action, we use Magento's checkout error processor (Magento_Checkout/js/model/error-processor) if something went wrong. That script expects a JSON object that contains a JSON string (which feels convoluted) called responseText, hence the shuffle here. Looks like that script has been that way since at least 2.4.0: https://github.com/magento/magento2/blob/6729b6e01368248abc33300208eb292c95050203/app/code/Magento/Checkout/view/frontend/web/js/model/error-processor.js#L30
We use that error-processor in several scripts, but probably doesn't get tripped very often in testing because 2 of the cases are specific to allowances for a one-step checkout module, a couple more are for failures from address/payment descriptor retrieval, and another on updateCheckoutSession. Presumably, these failures would result in the very generic "Something went wrong with your request. Please try again later." message, since that's what the error-processor falls back to if the responseText couldn't be parsed. I believe you mentioned seeing this in your initial testing.
All that said, it does look like I missed a couple spots where we look for the message key in that result... might be a good idea to keep that in the result object and just add the responseText so error messages display properly on the checkout page(s).
Plugin/ConfirmShippingRegion.php
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jaybeckr This is for a bit of a peculiar scenario Shangamesh and Steffen happened upon...
- The initial address coming from Amazon does not include a region/state, and the store doesn't require a region for addresses in that country (Italy, say)
- Submission of the address succeeds
- Fraudulent address is entered that includes region data
- Submission of the address succeeds
- Order submission fails as expected
- Buyer clicks edit payment/edit address Amazon link, and selects Amazon address with no region data
- Submission succeeds, but the shipping address associated with the quote still has the fraudulent regionId set, though regionCode and region name have been unset
- Order submission fails on region data mismatch
It's unclear why the regionId seems to be "sticking" when the other region data is updated as expected. The failure makes sense when the fraudulent address is in a different country, but the Amazon team seem to have tripped this even when both addresses were in Spain... which is doubly odd, given that state is required for Spanish addresses in our test stores 🤔 at any rate, I don't think there should be any harm in explicitly setting the regionId to match the data in the submitted address (which will be null if state wasn't provided), but please let me know if you see a cleaner way to handle this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the responseText key from CheckoutSessionManagement return values and added this instead, felt a little less repetitive.
Fixes # .
Additional details about this PR