sulu / SuluAutomationBundle

MIT License
16 stars 18 forks source link

Truncating ta_tasks and ta_task_executions leads to an error in the admin area #49

Open PatrickJuergens opened 3 years ago

PatrickJuergens commented 3 years ago
Q A
Bug? yes
New Feature? no
Bundle Version b91b7311242eb758c252ec71f074963c379eb1e0
Sulu Version 2.1.8
Browser Version Browser name and version

Actual Behavior

If the two tables ta_tasks and ta_task_executions are truncated via the database, there are still references left in the content that lead to a 500:

The identifier uuid is missing for a query of Task\TaskBundle\Entity\Task in vendor/doctrine/orm/lib/Doctrine/ORM/ORMException.php (line 315) in vendor/doctrine/orm/lib/Doctrine/ORM/EntityManager.php :: missingIdentifierField (line 422) in vendor/doctrine/orm/lib/Doctrine/ORM/EntityRepository.php -> find (line 151) in vendor/php-task/task-bundle/src/Entity/TaskRepository.php -> find (line 37) in vendor/sulu/automation-bundle/Controller/TaskController.php -> findByUuid (line 179) in vendor/sulu/automation-bundle/Controller/TaskController.php -> extendResponseItem (line 143) in vendor/symfony/http-kernel/HttpKernel.php -> cgetAction (line 158) in vendor/symfony/http-kernel/HttpKernel.php -> handleRaw (line 80) in vendor/symfony/http-kernel/Kernel.php -> handle (line 201)

Expected Behavior

If the tables are empty and there are references in the content, this should not lead to an error.

Steps to Reproduce

Create a task for a specific content Execute the task Empty the two tables ta_tasks, ta_task_executions Call the automation overview via the admin area for the specific content

Possible Solutions

        try {
            $task = $this->taskRepository->findByUuid($item['taskId']);
            $executions = $this->taskExecutionRepository->findByTask($task);
            if (0 < count($executions)) {
                $item['status'] = $executions[0]->getStatus();
            }
        } catch (\Exception $exception) {
        }
alexander-schranz commented 3 years ago

@PatrickJuergens Thanks for reporting!

Something like the ORMException is unexpected and is not something we should use catch on it. If I understand correctly the $item['taskId'] returns a uuid which is not longer in the database. Then I think the TaskRepository should throw a TaskNotFoundException we could catch here as we should not run into a ORMException in any case. /cc @wachterjohannes what do you think?

PatrickJuergens commented 3 years ago

Yes, $item['taskId'] returns an ID that no longer exists in the database, a TaskNotFoundException caught in this case should solve the problem.

alexander-schranz commented 3 years ago

I think this need then be implemented here: https://github.com/php-task/TaskBundle/blob/dec507fa2143cf6205267f382cb053bf1b42a4c8/src/Entity/TaskRepository.php#L35-L38 that it throws a exception when the entity does not exist. So I think instead of using find which just returns a proxy reference object. It actually should query and then throw the new Exception. Which we then can catch in the automationbundle. Do you wan to create a PR for this?

PatrickJuergens commented 3 years ago

Yes, I'll try.