Merged
Conversation
Mamaduka
commented
Oct 25, 2022
Member
Author
There was a problem hiding this comment.
We should avoid returning new object references here. It will fail shallow equality check and cause unnecessary rerenders.
The postContentBlock calculation has guard clauses, so I think it's safe to return undefined. We can also use the EMPTY_OBJECT reference if that's preferred.
Contributor
There was a problem hiding this comment.
It's also being given a default on line 119 where we destructure the return value of useSelect so I suppose we should remove that one too.
Member
Author
There was a problem hiding this comment.
Assigning default values during destructuring is okay; that doesn't change values returned inside the selector function.
|
Size Change: +20 B (0%) Total Size: 1.28 MB
ℹ️ View Unchanged
|
5b1c75b to
458f248
Compare
tellthemachines
approved these changes
Oct 26, 2022
Contributor
tellthemachines
left a comment
There was a problem hiding this comment.
Thanks for uncovering this bug!
Tested with author user and admin user, on both block and classic themes, and also checked the template editor. Everything is working fine ✅
I guess an outstanding issue here is that an author user isn't able to get the correct layout styles in the post editor, but that's something to be looked at separately.
Contributor
There was a problem hiding this comment.
It's also being given a default on line 119 where we destructure the return value of useSelect so I suppose we should remove that one too.
Member
Author
|
Thanks for testing, @tellthemachines! Unfortunately, the incorrect layout issue remains for non-admin users. Maybe we can add the |
Contributor
I just tried out an alternative approach to fetching the Post Content in #45299 which works for all user types. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
This is a follow-up for #44258.
PR fixes permission errors triggered by
getEditedPostTemplatefor users with lower capabilities.Why?
The editor should check permission before making requests when data isn't available for everyone.
How?
Add check for user permissions before requesting
getEditedPostTemplate.Testing Instructions