refactor(psalm): Fix most issues with the workflowengine #55140
Conversation
CarlSchwan
requested review from
a team,
blizzz and
nickvergessen
as code owners
CarlSchwan
requested review from
ArtificialOwl,
come-nc and
salmart-dev
and removed request for
a team
| if (!$subject instanceof Node) { | ||
| throw new \UnexpectedValueException( | ||
| 'Expected Node subject for File entity, got {class}', | ||
| ['app' => Application::APP_ID, 'class' => get_class($subject)] |
There was a problem hiding this comment.
This looks like it should be a log message instead of an exception :D
There was a problem hiding this comment.
I think being an exception here is correct as giving anything else than a Node is a fatal logic error, there was just a mixup on the correct way to creates the error message.
nickvergessen
left a comment
nickvergessen
left a comment
There was a problem hiding this comment.
Just not sure about the server containers, rest looks good
I ran integration tests on access control, and it seems to break the behaviour when creating a flow? |
CarlSchwan
force-pushed
the
carl/cleanup-workflowengine-app
branch
from
3a2a717 to
fbf99bf
Compare
Fixed. The |
This found a real bug where we were catching an Doctrine exception instead of a OCP\DB\Exception. This will be backported separately. Signed-off-by: Carl Schwan <carl.schwan@nextcloud.com>
CarlSchwan
force-pushed
the
carl/cleanup-workflowengine-app
branch
from
fbf99bf to
6154bfa
Compare
CarlSchwan
added
3. to review
blizzz
left a comment
blizzz
left a comment
There was a problem hiding this comment.
Looks good (apart of the commit message… with the right mood everything is a chore. But this is a refactor), have not tested yet. Approving in good trust, but might drive some tests this/next week unless it is merged.
|
Access control CI is passing now, so good to go when Cypress is unrelated |
CarlSchwan
changed the title
CarlSchwan
added
the
♻️ refactor
Summary
This found a real bug where we were catching an Doctrine exception instead of a OCP\DB\Exception. This will be backported separately.
TODO
Checklist