Reported by @balintbrews at #3503547-2: Display validation errors in dialogs that create sections.
Creating multiple code components in the UI with the same name results in a 500 response. This should've been caught in #3499931: HTTP API for code component config entities.
Match what JSON:API does:
// Return a 409 Conflict response in accordance with the JSON:API spec. See
// http://jsonapi.org/format/#crud-creating-responses-409.
if ($this->entityExists($parsed_entity)) {
throw new ConflictHttpException('Conflict: Entity already exists.');
}
None — but this facilitates #3503547: Display validation errors in dialogs that create sections 👍
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
wim leersComment #3
wim leersLOL
Comment #5
wim leersComment #8
akhil babuUpdated ApiConfigControllers::post() and added the tests. Please review
Comment #9
omkar-pd commentedpipeline failed
Comment #10
wim leersThanks for pushing this forward! 🤩 🙏
This is on the right track, but there are a few things that make this brittle and incomplete. See my review on the MR 😊
Comment #11
akhil babuThanks @wimleers and @omkar, I have updated the PR to include both the js_componen and xb_asset_library entities. The same id error does not occur for pattern entities as
Drupal\experience_builder\Entity\Pattern::generateId()updates the id if an entity with same id exists.Comment #12
akhil babuReady for review
Comment #13
wim leersComment #14
wim leersAfter @akhil babu's (great! 👏 — thanks! 😊🙏) work, 409 error responses started working, but they were using the same format as 500 responses (fatal server-side error), not the same as the much more similar 422 (validation error) responses.
That's bad, and @balintbrews already alluded to a similar problem in #3503547: Display validation errors in dialogs that create sections.
So let's do better. That was harder to explain than do, so I did it: https://git.drupalcode.org/project/experience_builder/-/merge_requests/6... (should pass, but GitLab CI is currently down)
But … that commit makes it painfully clear how inconsistent (and hence convoluted) the current handling is. That change makes the minimal change possible to make 409 responses similar to 422, but look at the crazy logic I had to write for it! 🤪
(This is nobody's fault: it grew organically. First we only had 500 errors, then we added an internal HTTP API that performed validation, so we added 422 responses for those. We decided to match JSON:API's error response structure for validation errors. Hence a mismatch was born.)
So … did that in https://git.drupalcode.org/project/experience_builder/-/merge_requests/6....
Comment #15
wim leersThe sole test failure here is the same random failure we saw on
0.xovernight, as described in detail at #3499550-22: Support server-side massaging and validating of ComponentInputsForm values. The latest0.xtest run fails in a different random way.There's no point explicitly awaiting @longwave's review here IMHO, this is fine as-is. 🤓
Also tightened
/openapi.yml, both to have extra test assurances, and to let XB FE devs know what they should be able to expect.Comment #16
wim leersComment #18
wim leers