Skip to content
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

Revert jQuery changes to xhr var in image upload #4707

Conversation

cpfergus1
Copy link
Contributor

@cpfergus1 cpfergus1 commented Nov 8, 2022

Summary

This PR reverts 6ae4717 from #4625. A set of parenthesis was missing in the call and should have been $.ajaxSetup().xhr(). An additional test was created covering the drop window for images and the change to $.ajaxSetup().xhr() was fully reverted because the reason cited for the change could not be verified (I cannot find a source that states the method is deprecated).

Resolves #4706

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

  • I have written a thorough PR description.
  • I have kept my commits small and atomic.
  • I have used clear, explanatory commit messages.

The following are not always needed (cross them out if they are not):

  • I have added automated tests to cover my changes.
  • I have attached screenshots to demo visual changes.
  • I have opened a PR to update the guides.
  • I have updated the readme to account for my changes.

@github-actions github-actions bot added the changelog:solidus_backend Changes to the solidus_backend gem label Nov 8, 2022
@cpfergus1 cpfergus1 force-pushed the cpfergus1/fix-broken-image-upload-js branch 2 times, most recently from 1166cdf to b2c5ba7 Compare November 8, 2022 13:46
@cpfergus1 cpfergus1 changed the title Revert jQuery changes to xhr field in image upload Revert jQuery changes to xhr var in image upload Nov 8, 2022
The was changed by mistake to `$.ajaxSetup.xhr()` and broke the image
upload drop field. The error was missed because there were not any
existing tests for the image drop field so the script was not being
properly tested.

Although `$.ajaxSetup().xhr()` could work in this situation, it will
be fully reverted to original due to inability to find source of
deprecation cited in commit that changed this function.
@cpfergus1 cpfergus1 force-pushed the cpfergus1/fix-broken-image-upload-js branch from b2c5ba7 to 58fcaae Compare November 8, 2022 14:16
Copy link
Contributor

@waiting-for-dev waiting-for-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @cpfergus1!

@waiting-for-dev waiting-for-dev added the type:bug Error, flaw or fault label Nov 8, 2022
@codecov
Copy link

codecov bot commented Nov 8, 2022

Codecov Report

Merging #4707 (58fcaae) into master (1fe96cc) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #4707   +/-   ##
=======================================
  Coverage   86.12%   86.12%           
=======================================
  Files         577      577           
  Lines       14640    14640           
=======================================
  Hits        12608    12608           
  Misses       2032     2032           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@pelargir
Copy link
Contributor

pelargir commented Nov 8, 2022

Thank you!

@waiting-for-dev waiting-for-dev merged commit 7609502 into solidusio:master Nov 9, 2022
@kennyadsl
Copy link
Member

@waiting-for-dev we need a backport here. what about using our brand-new label feature? 😎

@waiting-for-dev
Copy link
Contributor

@kennyadsl, I also thought that. But, as far as I can see, the reverted commit is only in master. It was merged on September 13th, while Solidus v3.2 was released on August 18th 🤔

@kennyadsl
Copy link
Member

@waiting-for-dev please look at the history here. It seems that the same commit has been done prior to bumping to 3.2.3. I think it doesn't show in the list of versions where the commit exists because it has been backported.

@waiting-for-dev
Copy link
Contributor

Oh, all right, it makes sense 🙂

@waiting-for-dev
Copy link
Contributor

Bot 🤖 working for v3.2

@github-actions
Copy link

github-actions bot commented Nov 9, 2022

💚 All backports created successfully

Status Branch Result
v3.2

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

waiting-for-dev added a commit that referenced this pull request Nov 9, 2022
[v3.2] Merge pull request #4707 from cpfergus1/cpfergus1/fix-broken-image-upload-js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:solidus_backend Changes to the solidus_backend gem type:bug Error, flaw or fault
Projects
None yet
Development

Successfully merging this pull request may close these issues.

$.ajaxSetup.xhr is not a function
5 participants