Core Data: Support entities in the 'canUser' selector #63322
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
Mamaduka
added
[Type] Enhancement
|
Size Change: +199 B (+0.01%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
Mamaduka
force-pushed
the
update/can-user-support-entities
branch
from
15462d4 to
a74fd72
Compare
Mamaduka
requested review from
ajitbohra,
nerrad,
ryanwelcher,
juanmaguitar,
fabiankaegy and
ndiego
as code owners
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Mamaduka
requested review from
TimothyBJacobs,
a team,
mcsf and
aaronrobertshaw
and removed request for
juanmaguitar,
ajitbohra,
ryanwelcher,
nerrad,
ndiego,
fabiankaegy and
a team
Mamaduka
requested review from
youknowriad,
jsnajdr and
tyxla
and removed request for
mcsf
|
I've tested editing post meta bindings and post title and everything seems to be working as expected 🙂 Regarding the code, it looks good after a quick look, but I'll defer the review to other folks because I am not fully familiar with the use cases, and I don't have the full context. |
|
Flaky tests detected in 19e01ea. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9874496323
|
Mamaduka
commented
Jul 10, 2024
tyxla
reviewed
Jul 10, 2024
| config.name === resource.name && | ||
| config.kind === resource.kind | ||
| ); | ||
| if ( ! entityConfig ) { |
There was a problem hiding this comment.
Should we log an error in case kind or name don't correspond to an entity type?
There was a problem hiding this comment.
No other entity resolver does this; maybe we should re-evaluate missing config cases together?
| const key = ( | ||
| typeof resource === 'object' | ||
| ? [ action, resource.kind, resource.name, resource.id ] | ||
| : [ action, resource, id ] |
There was a problem hiding this comment.
It could also be useful to log an error so the developer knows about unexpected usage.
@Mamaduka I see you mentioned those are rare in stores, but at the same time, when debugging store errors one often has to go deep, exactly because errors will often be silent or swallowed.
|
I have resolved most of the feedback and to-do items. It should be ready for another review. |
Mamaduka
force-pushed
the
update/can-user-support-entities
branch
from
f87e919 to
df1dbd2
Compare
|
Thanks everyone for the reviews and feedback. I rebased the branch just in case and will merge after the CI checks are green. |
This was referenced Jul 11, 2024
Mamaduka
added
the
Needs Dev Note
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Jul 18, 2024
* Core Data: Support entities in the 'canUser' selector * Cleanup 'canUser' unit tests * Add unit tests * Replace 'canUserEditEntityRecord' selector usages * Make 'canUserEditEntityRecord' forwarded selector/resolver * Add checks when the resource is an entity * Return false when entity arg is malformed * Update types and JSDoc * Throw an error when an entity resource object is malformed * Deprecate canUserEditEntityRecord Co-authored-by: Mamaduka <[email protected]> Co-authored-by: ellatrix <[email protected]> Co-authored-by: jsnajdr <[email protected]> Co-authored-by: tyxla <[email protected]> Co-authored-by: SantosGuillamot <[email protected]> Co-authored-by: adamziel <[email protected]> Co-authored-by: TimothyBJacobs <[email protected]>
97 tasks
|
Hey @Mamaduka 👋 Would you be able to help write a dev note for this for the 6.7 release? We are planning to have this as part of a larger Miscellaneous Editor Updates note. We are hoping to get all drafts in by October 13th to leave some time for reviews before the RC1. All Dev Notes get tracked in #65784 so feel free to leave a note there or ping me directly :) Please let us know if you can assist with that. Thanks in advance :) |
Mamaduka
removed
the
Needs Dev Note
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.
What?
Resolves #43751.
Alternative to #63292.
PR updates the
canUserselector to support core data entities. The new supported and recommended syntax:Why?
canUser- currently only supports resources in thewp/v2namespace. Additionally, you must know the final REST API path. Typically, however, only an entity's kind and name are known.canUserEntityRecord- is limited to only the edit action check. It was a wrapper forcanUserand only supported thewp/v2namespace, which hasn't been required since WP 5.9.The update also opens up the possibility of leveraging "target hints" and bulk fetch permissions data for DataViews.
Todo
canUserEditEntityRecordselector.Testing Instructions
Basic Testing
Testing canUserEntityRecord changes
Testing Instructions for Keyboard
Same.
Screenshot