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

Reusable Blocks: Make the number retrieved from the API unlimited #26486

Merged
merged 2 commits into from
Oct 28, 2020

Conversation

pablinos
Copy link
Member

@pablinos pablinos commented Oct 27, 2020

Description

This is to fix #26352. Having no limit seems like it could be risky, but
there are a number of other places in the codebase where all entities
are returned in one call to the API, including categories, which could
return a similar amount of data to this.

How has this been tested?

  • I created 11 reusable blocks
  • When using the master branch, if I try to insert the first reusable block I created into a new post, it's not available in the block inserter
  • With this branch, all reusable blocks are available in the block inserter

It seems you only get the 10 most recently created reusable blocks currently.

Types of changes

Bug fix (non-breaking change which fixes an issue)

Fixes #26352

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • 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.

This is to fix WordPress#26352. Having no limit seems like it could be risky, but
there are a number of other places in the codebase where all entities
are returned in one call to the API, including categories, which could
return a similar amount of data to this.
@talldan talldan added [Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) [Type] Regression Related to a regression in the latest release Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Oct 27, 2020
@retrofox
Copy link
Contributor

Does it make sense adding at least add a high limit, such as 100 or whatever?

@talldan talldan added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Oct 27, 2020
Copy link
Contributor

@talldan talldan left a comment

Choose a reason for hiding this comment

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

LGTM. Entities do have some in built logic, I think it starts splitting things up into multiple requests when we reach about 50 (though not completely sure of the exact number) items if per_page: -1 is used.

So I think this is probably the best way right now to fix what is quite a bad bug.

@@ -323,7 +323,8 @@ export default compose( [
selectionEnd: getEditorSelectionEnd(),
reusableBlocks: select( 'core' ).getEntityRecords(
'postType',
'wp_block'
'wp_block',
{ 'per_page': -1 }
Copy link
Contributor

@talldan talldan Oct 27, 2020

Choose a reason for hiding this comment

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

There's a linting error—the quotes aren't needed. Fix that and should be good to merge 👍

Thanks for the fix, Paul, and congrats on the first contribution.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, Dan. Sorry, I didn't have my linting setup correctly. It should be fixed now.

@github-actions
Copy link

Congratulations on your first merged pull request, @pablinos! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts:

https://profiles.wordpress.org/me/profile/edit/

And if you don't have a WordPress.org account, you can create one on this page:

https://login.wordpress.org/register

Kudos!

@github-actions github-actions bot added this to the Gutenberg 9.3 milestone Oct 28, 2020
@pablinos pablinos deleted the fix/limited-resuable-blocks branch October 28, 2020 13:40
tellthemachines pushed a commit that referenced this pull request Oct 30, 2020
…6486)

* Make the number of reusable blocks returned from the API unlimited

This is to fix #26352. Having no limit seems like it could be risky, but
there are a number of other places in the codebase where all entities
are returned in one call to the API, including categories, which could
return a similar amount of data to this.

* Remove unnecessary quotes to fix lint error.
@tellthemachines tellthemachines mentioned this pull request Oct 30, 2020
6 tasks
tellthemachines added a commit that referenced this pull request Nov 2, 2020
* nest content styles in container for higher specificity (#26487)

* Reusable Blocks: Make the number retrieved from the API unlimited (#26486)

* Make the number of reusable blocks returned from the API unlimited

This is to fix #26352. Having no limit seems like it could be risky, but
there are a number of other places in the codebase where all entities
are returned in one call to the API, including categories, which could
return a similar amount of data to this.

* Remove unnecessary quotes to fix lint error.

* Fix block inserter WSOD when an empty reusable block exists (#26484)

* Latest Posts: Bring back classname on post list (#26477)

* [Heading Block] Fix double alignment controls in toolbar (#26492)

* fix heading alignments

* add proper supports in deprecation

* update all previous deprecations

* regenerate fixtures for heading

* change migration call

* remove previous className + revert saves

* Revert "regenerate fixtures for heading"

This reverts commit 27af8c3.

* change fixtures

* create new fixtures + fix deprecation save

* address review feedback

* URLInput: Use debounce() instead of throttle() (#26529)

Wait until the user finishes typing before sending an AJAX request. This
ensures that there isn't an AJAX request sent every 200 ms while the
user is typing.

* Fix single column block display for smaller screens. (#26438)

If there is only one column, don't force a 50% flex-basis for small screens.

* Fix incorrectly pluralised strings (#26565)

* Change block mover label i18n

* Update remove block i18n

* Ensure footer remains position fixed when navigating regions (#26533)

* Update package-lock file to ensure static analysis task passes (#26528)

* Removes extra fullstop (#26586)

Co-authored-by: Addison Stavlo <Stavz01@gmail.com>
Co-authored-by: Paul Bunkham <paul@dobit.co.uk>
Co-authored-by: Noah Allen <noahtallen@gmail.com>
Co-authored-by: Kelly Dwan <ryelle@users.noreply.github.com>
Co-authored-by: Nik Tsekouras <ntsekouras@outlook.com>
Co-authored-by: Robert Anderson <robert@noisysocks.com>
Co-authored-by: Aaron D. Campbell <aaron@AaronDCampbell.com>
Co-authored-by: Daniel Richards <daniel.richards@automattic.com>
Co-authored-by: Tammie Lister <tammie@automattic.com>
@tellthemachines tellthemachines removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Synced Patterns Related to synced patterns (formerly reusable blocks) First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Limited to 10 reusable blocks in 9.2.0
4 participants