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

API Fetch: Improve isMediaUploadRequest check #34417

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

@Mamaduka
Copy link
Member

@Mamaduka Mamaduka commented Aug 31, 2021

Description

Updates isMediaUploadRequest check and to allow per_page=-1 get requests for media items.

Fixes #34410.

How has this been tested?

Unit tests are running locally:

npm run test-unit -- packages/api-fetch/src/middlewares/test/media-upload.js

Running the following code in the browser console doesn't result in the error - 400: Invalid parameter(s): per_page

wp.data.select('core').getMediaItems({ per_page: -1 });

Check that mediaUploadMiddleware is applied to upload requests. This can be done by checking the "Request call stack" in the Network tab.

Screenshots

CleanShot 2021-08-31 at 15 39 39

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).
@glendaviesnz
Copy link
Contributor

@glendaviesnz glendaviesnz commented Aug 31, 2021

This worked for me as advertised, and when applied to #34389 it correctly paged the result set for galleries over 100 when per_page set to -1.

I don't think I have enough background around this part of the code though to sign off on it. @youknowriad, can you see any gotchas with this since you initially added this middleware? The background is that this change is needed to switch the gallery getMedia call to use getMediaItems instead in order reduce the number of calls needed to get all the image data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants