Warning: Undefined array key "id" in Drupal\jsonapi\Controller\EntityResource->patchIndividual()

MR should be updated for 11.x

Also will need to research why that value is not set. Could be masking a larger issue.

Thanks for the contribution, i'd prefer we use 'isset' instead of empty since empty has a few issues.

Hi smustgrave (#4)
This problem can be reproduced only if the id attribute wasn't added to the request body. So we can be calm about the module functionality.

Hi bbrala (#5)
Sure I can do this, but could you a bit clarify what issues we can get with using empty() in this part of the code?
I used it because, in this way, we will skip all cases when data['id'] contains an empty string or any other value that can't be for the uuid
Thanks!

If you use isset then you know the key exists. Although array_key_exists is technically even cleaner, isset is fine. After that the code would check if the value is an uuid. Does that make sense?

If you use isset then you know the key exists. Although array_key_exists is technically even cleaner, isset is fine. After that the code would check if the value is an uuid. Does that make sense?

bbrala
Thanks for your response!
I got your point and updated the pull request now it uses !isset() instead empty().
Also was created a new branch for the 11.x version of Drupal core.

Still needs a test

This problem can be reproduced only if the id attribute wasn't added to the request body. So we can be calm about the module functionality.

How is this reproducible from core? This may be a bug in core that putting isset() is covering up.

Added test for the mentioned issue.
Also, there was the same issue just under a few lines that I fixed as well.
I am uploading the test-only patch as well.

This may be a bug in core that putting isset() is covering up.

@smustgrave There is nothing above, Drupal\jsonapi\Controller\EntityResource->patchIndividual() it is Controller. Also, JSON:API correctly responds to the request without the "id" key in the body. The only issue is PHP warnings.

@ReINFaTe No need to upload a separate test only patch or created a new branch if tests are included in the bugfix patch. We can run a test-only Gitlab pipeline now, which will automatically run the tests from the MR, see: https://www.drupal.org/about/core/blog/drupal-cores-gitlab-ci-testing-is... → (Test-only runs are now automated in GitLab CI).

The tests-only patch does show the issue, so this may be good to go. Lets see.

I read the IS and comments. I did not find any unanswered questions.

Added a question to the MR.

Appears the question was answered, doesn't appear a change is required.

Thanks for answering my question.

This is eligible for backport to 10.2.x as a bug fix.

Committed and pushed b930619b05 to 11.x and bb6903e8e6 to 10.2.x. Thanks!

longwave → closed merge request !4484

longwave → closed merge request !4531

Automatically closed - issue fixed for 2 weeks with no activity.