[Block Library - Site Logo]: Add permissions handling #32919
Merged
Conversation
This file contains 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
ntsekouras
added
[Type] Bug
ntsekouras
requested review from
ockham,
mcsf,
youknowriad,
scruffian and
aristath
ntsekouras
requested review from
ajitbohra,
spacedmonkey and
TimothyBJacobs
as code owners
ntsekouras
commented
Jun 23, 2021
|
Size Change: +563 B (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
aristath
approved these changes
Jun 23, 2021
There was a problem hiding this comment.
Tested the PR and it works as advertised.
The code is clean and makes sense.
Looks good to merge 👍
I saw there's a failing test for e2e-native but it doesn't look related... I restarted the test, let's see if it passes the 2nd time.
EDIT: I see the same tests are failing in trunk so not an issue with this branch 👍
|
I'd rather this expose the |
ntsekouras
force-pushed
the
add/site-logo-permissions-handling
branch
from
8473362 to
bf8fbc4
Compare
I updated the code. I could use another round of review/testing 😄 --edit: it seems I'll need to make some more changes regarding the |
aristath
approved these changes
Jun 25, 2021
ntsekouras
force-pushed
the
add/site-logo-permissions-handling
branch
from
b0cd792 to
82a61bb
Compare
ntsekouras
force-pushed
the
add/site-logo-permissions-handling
branch
from
8d75582 to
ed92e90
Compare
TimothyBJacobs
approved these changes
Jun 25, 2021
youknowriad
reviewed
Jun 25, 2021
youknowriad
pushed a commit
that referenced
this pull request
Jun 25, 2021
* [Block Library - Site Logo]: Add permissions handling * Add only logo id to rest index * add link to logo in rest index * use context 'view' only * fallback to logo id from rest index
youknowriad
removed
the
Backport to WP 6.7 Beta/RC
youknowriad
pushed a commit
that referenced
this pull request
Jun 25, 2021
* [Block Library - Site Logo]: Add permissions handling * Add only logo id to rest index * add link to logo in rest index * use context 'view' only * fallback to logo id from rest index
7 tasks
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Part of: #26573
Currently when a user with no right permissions to edit
Site Logohas a broken experience due to the permissions check happening at the REST API.This PR fixes that by checking the rights and adding a readonly view for those users.
Testing instructions
Site Logoand observe that everything works as before.Spinner.Notes
removeoption in a follow up for this block? Is there any other way to remove it, like in Customizer or something? I couldn't find a way..Checklist:
*.native.jsfiles for terms that need renaming or removal).