[CRUD:Edit] Add exception message #5190
Conversation
src/Controller/CRUDController.php
Outdated
| @@ -300,6 +300,7 @@ public function deleteAction($id) | |||
| * | |||
| * @param int|string|null $id | |||
| * | |||
| * @throws \RuntimeException If no editable attribut is defined | |||
There was a problem hiding this comment.
attributE, but IMO field would better
src/Controller/CRUDController.php
Outdated
| @@ -330,6 +331,10 @@ public function editAction($id = null) | |||
| $this->admin->setSubject($existingObject); | |||
| $objectId = $this->admin->getNormalizedIdentifier($existingObject); | |||
|
|
|||
| if (!$this->admin->getFormTabs()) { | |||
| throw new \RuntimeException(sprintf('No editable field defined. Did you forget to implement the "configureFormFields" method?')); | |||
There was a problem hiding this comment.
This line is too long, could you please break it.
There was a problem hiding this comment.
Also, why are you using sprintf here?
src/Controller/CRUDController.php
Outdated
| @@ -300,6 +300,7 @@ public function deleteAction($id) | |||
| * | |||
| * @param int|string|null $id | |||
| * | |||
| * @throws \RuntimeException If no editable field is defined | |||
| * @throws NotFoundHttpException If the object does not exist | |||
| * @throws AccessDeniedException If access is not granted | |||
There was a problem hiding this comment.
can you please move the \RuntimeException below the AccessDeniedException?
…ccessDeniedException'
src/Controller/CRUDController.php
Outdated
| @@ -330,6 +331,11 @@ public function editAction($id = null) | |||
| $this->admin->setSubject($existingObject); | |||
| $objectId = $this->admin->getNormalizedIdentifier($existingObject); | |||
|
|
|||
| if (!$this->admin->getFormTabs()) { | |||
| throw new \RuntimeException('No editable field defined. | |||
| Did you forget to implement the "configureFormFields" method?'); | |||
There was a problem hiding this comment.
<?php
throw new \RuntimeException(
'No editable field defined. Did you forget to implement the "configureFormFields" method?'
);
|
Please add a test for this case |
|
Thanks for the test, but can you add one returning false , too? |
|
Hey, @sir-kain this is a good solution, could you maybe apply it to create and show actions also? so we have consistency between actions. Thank you |
|
@OskarStark done? |
|
@kunicmarko20 Yes, I thought about that. |
|
Thank you @sir-kaine, would be great if you could add another PR 👍🏻 |
| @@ -330,6 +331,12 @@ public function editAction($id = null) | |||
| $this->admin->setSubject($existingObject); | |||
| $objectId = $this->admin->getNormalizedIdentifier($existingObject); | |||
|
|
|||
| if (!$this->admin->getFormTabs()) { | |||
There was a problem hiding this comment.
This is BC break cause at this point form can be not initialized yet, but moving call:
$form = $this->admin->getForm();
Before this if fixes that exception.
ps. mine admin controller don't overwrite any method of CRUDController just adds new like in docs.
There was a problem hiding this comment.
This was fixed in #5195
Subject
From the
CRUDController:editAction()add an error message before rendering the html page if no field is defined on theconfigureFormFields()functionChangelog