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

Add Data Producer plugins for creating, updating and deleting entities. #1231

Closed
wants to merge 11 commits into from

Conversation

Cryt1c
Copy link
Contributor

@Cryt1c Cryt1c commented Aug 10, 2021

Continuation of #1212

@klausi klausi added the 4.x label Aug 16, 2021
Copy link
Contributor

@larowlan larowlan left a 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()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!$create_access->isALlowed()) {
if (!$create_access->isAllowed()) {

Copy link
Contributor

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,
Copy link
Contributor

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'?

Copy link
Contributor

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.");
Copy link
Contributor

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?

Copy link
Contributor

Choose a reason for hiding this comment

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

@Sam152
Copy link

Sam152 commented Nov 12, 2021

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.

@Kingdutch
Copy link
Contributor

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.

@acbramley
Copy link
Contributor

@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?

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants