feat: add exception handling for the FlashBagException from araise/core #64
Conversation
Ruesa18
force-pushed
the
feature/113-add-custom-flash-messages
branch
from
a910a09
to
7356a7c
Compare
src/Controller/CrudController.php
Outdated
| $response->setCallback( | ||
| function () use ($writer) { | ||
| $writer->save('php://output'); | ||
| assert($entity !== null); |
There was a problem hiding this comment.
On a scale from 1-10, how evil is this?
Because if this doesn't exist the code above should throw a NotFoundHttpException, I think... So this shouldn't be an issue. But please take a look at the whole code @tuxes3 - just to make sure 😅
There was a problem hiding this comment.
It wouldn't be too evil, if it would make sense. The Entity is only needed in the RelationContent Export case which @ArnoEgli developed. In the standard case there is no need for an entity. Please compare the implementation on develop with yours. There is no need to have the entity if acronym or entityId is not set.
There was a problem hiding this comment.
The surrounding lines just changed anyways. So I'm gonna take the develop version of this function and try it again. Thanks for the feedback 👍
Ruesa18
force-pushed
the
feature/113-add-custom-flash-messages
branch
2 times, most recently
from
a53bb91
to
754e60d
Compare
src/Controller/CrudController.php
Outdated
| $response->setCallback( | ||
| function () use ($writer) { | ||
| $writer->save('php://output'); | ||
| assert($entity !== null); |
There was a problem hiding this comment.
It wouldn't be too evil, if it would make sense. The Entity is only needed in the RelationContent Export case which @ArnoEgli developed. In the standard case there is no need for an entity. Please compare the implementation on develop with yours. There is no need to have the entity if acronym or entityId is not set.
Ruesa18
force-pushed
the
feature/113-add-custom-flash-messages
branch
from
754e60d
to
3caffb6
Compare
Ruesa18
force-pushed
the
feature/113-add-custom-flash-messages
branch
from
3caffb6
to
f7f4bb4
Compare
|
@tuxes3 Is https://github.com/araise-dev/CrudBundle/pull/64/files#diff-c14e169d433ac4ee49286b8916edc55b04f023a389093d8e68a7555646fcfed0R479 still required since we removed every Logger call now? |
Yes, we will keep that. |
tuxes3
force-pushed
the
feature/113-add-custom-flash-messages
branch
from
f7f4bb4
to
148fa25
Compare
tuxes3
force-pushed
the
feature/113-add-custom-flash-messages
branch
from
148fa25
to
c77399f
Compare
References: https://dev.whatwedo.ch/araise/araise-meta/-/issues/113
Depends on: araise-dev/CoreBundle#30