-
Notifications
You must be signed in to change notification settings - Fork 201
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
Add Data Producer plugins for creating, updating and deleting entities. #1231
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps send your changes as a PR against @Sam152's original PR instead of creating a new one?
$field_access_errors = []; | ||
foreach ($values as $field_name => $value) { | ||
$create_access = $entity->get($field_name)->access('edit', NULL, TRUE); | ||
if (!$create_access->isALlowed()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (!$create_access->isALlowed()) { | |
if (!$create_access->isAllowed()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$context->addCacheableDependency($access); | ||
if (!$access->isAllowed()) { | ||
return [ | ||
'was_successful' => FALSE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would 'result' be a better name for this than 'was_successful'?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// things such as the owner of the entity or the ID of the entity. | ||
$update_fields = array_filter($values, function (string $field_name) use ($entity, $context) { | ||
if (!$entity->hasField($field_name)) { | ||
throw new \Exception("Could not update '$field_name' field, since it does not exist on the given entity."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of throwing an exception, should this keep track of errors and return ['errors'=>]
like other code paths in this producer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The author of this PR asked to fork it and I said it was a good idea. I don't have the time to push it forward any further. |
From building my own mutations for creation, updating and deletion I'm wondering if these producers should be in the module or whether they're better suited in a contrib module or in the documentation as examples. I would expect most GraphQL APIs to be more complex and more specifically tailored then simply exposing an entire entity for creation/update/deletion. |
@Kingdutch after fleshing our API out a bit more I tend to agree with you here. Maybe we just take some of the documentation from Sam on #1212 and commit that instead? |
Continuation of #1212