Update deprecated jQuery methods #4625
Conversation
From the jQuery upgrade guide: The jqXHR object returned from jQuery.ajax() is a jQuery Deferred and has historically had three extra methods with names matching the arguments object of success, error, and complete. This often confused people who did not realize that the returned object should be treated like a Deferred. As of jQuery 3.0 these methods have been removed. As replacements, use the Deferred standard methods of done, fail, and always, or use the new then and catch methods for Promises/A+ compliance. Note that this does not have any impact at all on the ajax callbacks of the same name passed through the options object, which continue to exist and are not deprecated. This only affects the jqXHR methods.
github-actions
bot
added
the
changelog:solidus_backend
cpfergus1
force-pushed
the
nebulab/fix_jquery_methods
branch
from
42b84dc to
6ae4717
Compare
cpfergus1
changed the title
|
Thanks, @cpfergus1. Do you think it would be challenging to include a regression test? It looks like our test suite was failing to detect that breaking change. |
I think my latest commit might achieve that. It did not appear that we were testing AJAX call failures so I forced it by adding a few lines in a current spec to input bad data. This should cover "fail" and "then" - LMKWYT Edit: It appears that this only works when the driver is not headless or when I add save_and_open_page 🤔 Edit 2: Forcing it to wait for the popup works, but this does not seem like an ideal solution Edit 3: Stopped trying to capture the alert and just utilized accept_alert - test will fail if the alert does not pop up. LMKWYT |
cpfergus1
force-pushed
the
nebulab/fix_jquery_methods
branch
from
42c3a31 to
17cfbc6
Compare
Specs did not catch the deprecated method "error" because there were not any tests that checked for an AJAX error. Added steps to a current current test to ensure an AJAX call fails to cover the method "fail" and "then" incase those methods might be deprecated in the future
cpfergus1
force-pushed
the
nebulab/fix_jquery_methods
branch
from
17cfbc6 to
c8218ba
Compare
It makes sense. |
| @@ -56,7 +56,7 @@ Spree.Models.ImageUpload = Backbone.Model.extend({ | |||
| processData: false, // tell jQuery not to process the data | |||
| contentType: false, // tell jQuery not to set contentType | |||
| xhr: function () { | |||
| var xhr = $.ajaxSettings.xhr(); | |||
| var xhr = $.ajaxSetup.xhr(); // Using default ajax builder but inputting upload settings | |||
There was a problem hiding this comment.
Unsure how this is working for anyone. We're getting errors about ajaxSetup.xhr being undefined, and I cannot find references to it in anywhere else in Solidus, or in any of our external libraries including jQuery itself.
Where is ajaxSetup.xhr being defined?
There was a problem hiding this comment.
It's here, but I think we mistakenly called it without the parenthesis: https://api.jquery.com/jQuery.ajaxSetup/
Opening the console and typing jQuery.ajaxSetup().xhr() seems to be working fine
cc @cpfergus1
Update deprecated jQuery methods (Backport #4625 to v3.2)
Summary
After the inclusion of #4167, it was discovered that some of the extensions (specifically solidus_product_assembly) specs are failing on master with no changes to the code. It was discovered that after moving to jQuery3, some methods no longer existed including one being utilized in ajax calls for splitting shipments and image uploads.
This PR corrects these issues by updating the deprecated methods to utilize those available in jQuery3.
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed (
cross them outif they are not):