Repository: pimcore/pimcore (language: php)

Issue:
[Bug]: Inconsistent Return Type of Workflow/Manager getWorkflowIfExists vs getWorkflowByName
### Pimcore version

11.3.2

### Steps to reproduce

Looking at code https://github.com/pimcore/pimcore/blob/f2a89483da4186ca3d5ada872a714acc94ad4464/lib/Workflow/Manager.php#L194-L211

### Actual Behavior

`getWorkflowByName`  returns `?object` whereas `getWorkflowIfExists` returns as `?WorkflowInterface`.

### Expected Behavior

I would expect both to return `?WorkflowInterface`.


Discussion / hints:
Not sure what label BC-BREAK, means but I don't think this change is backward compatible breaking as it tightens a return type . 
The thing is, that someone could extend the `Workflow/Manager` class and overwrite the `getWorkflowByName` method. 
In that case, changing the signature of `getWorkflowByName` would break that code.
Therefore this change would be a BC break (which doesn't mean that we should not do it, but we need to wait for a major)... 

Fix patch:
diff --git a/doc/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md b/doc/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md
index 6a69fb7b4df..c3589d3ebd2 100644
--- a/doc/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md
+++ b/doc/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md
@@ -31,6 +31,9 @@
 #### [Templates]
 - Removed `key_value_table.html.twig` from `CoreBundle`
 
+#### [Workflow]
+- Method `getWorkflowByName()` now returns `?WorkflowInterface` instead of `?object`. This also affected the `lib/Workflow/Notification/NotificationEmailService.php` and `lib/Workflow/Notification/PimcoreNotificationService.php`.
+
 ## Pimcore 11.6.0
 ### Elements
 #### [Documents]
diff --git a/lib/Workflow/EventSubscriber/NotificationSubscriber.php b/lib/Workflow/EventSubscriber/NotificationSubscriber.php
index abac5ba4c1a..a6c71e110f2 100644
--- a/lib/Workflow/EventSubscriber/NotificationSubscriber.php
+++ b/lib/Workflow/EventSubscriber/NotificationSubscriber.php
@@ -20,10 +20,14 @@
 use Pimcore\Model\Element\ElementInterface;
 use Pimcore\Model\Element\Service;
 use Pimcore\Workflow;
+use Pimcore\Workflow\ExpressionService;
+use Pimcore\Workflow\Manager;
 use Pimcore\Workflow\Notification\NotificationEmailService;
+use Pimcore\Workflow\Notification\PimcoreNotificationService;
 use Pimcore\Workflow\Transition;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\Workflow\Event\Event;
+use Symfony\Component\Workflow\WorkflowInterface;
 use Symfony\Contracts\Translation\TranslatorInterface;
 
 /**
@@ -53,7 +57,13 @@ class NotificationSubscriber implements EventSubscriberInterface
 
     protected Workflow\Manager $workflowManager;
 
-    public function __construct(NotificationEmailService $mailService, Workflow\Notification\PimcoreNotificationService $pimcoreNotificationService, TranslatorInterface $translator, Workflow\ExpressionService $expressionService, Workflow\Manager $workflowManager)
+    public function __construct(
+        NotificationEmailService $mailService,
+        PimcoreNotificationService $pimcoreNotificationService,
+        TranslatorInterface $translator,
+        ExpressionService $expressionService,
+        Manager $workflowManager
+    )
     {
         $this->mailService = $mailService;
         $this->pimcoreNotificationService = $pimcoreNotificationService;
@@ -72,8 +82,14 @@ public function onWorkflowCompleted(Event $event): void
         $subject = $event->getSubject();
         /** @var Transition $transition */
         $transition = $event->getTransition();
+
+
         $workflow = $this->workflowManager->getWorkflowByName($event->getWorkflowName());
 
+        if ($workflow  === null) {
+            return;
+        }
+
         $notificationSettings = $transition->getNotificationSettings();
         foreach ($notificationSettings as $notificationSetting) {
             $condition = $notificationSetting['condition'] ?? null;
@@ -83,17 +99,42 @@ public function onWorkflowCompleted(Event $event): void
                 $notifyRoles = $notificationSetting['notifyRoles'] ?? [];
 
                 if (in_array(self::NOTIFICATION_CHANNEL_MAIL, $notificationSetting['channelType'])) {
-                    $this->handleNotifyPostWorkflowEmail($transition, $workflow, $subject, $notificationSetting['mailType'], $notificationSetting['mailPath'], $notifyUsers, $notifyRoles);
+                    $this->handleNotifyPostWorkflowEmail(
+                        $transition,
+                        $workflow,
+                        $subject,
+                        $notificationSetting['mailType'],
+                        $notificationSetting['mailPath'],
+                        $notifyUsers,
+                        $notifyRoles
+                    );
                 }
 
-                if (in_array(self::NOTIFICATION_CHANNEL_PIMCORE_NOTIFICATION, $notificationSetting['channelType'])) {
-                    $this->handleNotifyPostWorkflowPimcoreNotification($transition, $workflow, $subject, $notifyUsers, $notifyRoles);
+                if (in_array(
+                    self::NOTIFICATION_CHANNEL_PIMCORE_NOTIFICATION,
+                    $notificationSetting['channelType']
+                )) {
+                    $this->handleNotifyPostWorkflowPimcoreNotification(
+                        $transition,
+                        $workflow,
+                        $subject,
+                        $notifyUsers,
+                        $notifyRoles
+                    );
                 }
             }
         }
     }
 
-    private function handleNotifyPostWorkflowEmail(Transition $transition, \Symfony\Component\Workflow\Workflow $workflow, ElementInterface $subject, string $mailType, string $mailPath, array $notifyUsers, array $notifyRoles): void
+    private function handleNotifyPostWorkflowEmail(
+        Transition $transition,
+        WorkflowInterface $workflow,
+        ElementInterface $subject,
+        string $mailType,
+        string $mailPath,
+        array $notifyUsers,
+        array $notifyRoles
+    ): void
     {
         //notify users
         $subjectType = ($subject instanceof Concrete ? $subject->getClassName() : Service::getElementType($subject));
@@ -110,7 +151,13 @@ private function handleNotifyPostWorkflowEmail(Transition $transition, \Symfony\
         );
     }
 
-    private function handleNotifyPostWorkflowPimcoreNotification(Transition $transition, \Symfony\Component\Workflow\Workflow $workflow, ElementInterface $subject, array $notifyUsers, array $notifyRoles): void
+    private function handleNotifyPostWorkflowPimcoreNotification(
+        Transition $transition,
+        WorkflowInterface $workflow,
+        ElementInterface $subject,
+        array $notifyUsers,
+        array $notifyRoles
+    ): void
     {
         $subjectType = ($subject instanceof Concrete ? $subject->getClassName() : Service::getElementType($subject));
         $this->pimcoreNotificationService->sendPimcoreNotification(
diff --git a/lib/Workflow/Manager.php b/lib/Workflow/Manager.php
index 0a73fa6ccf6..edc3dba4047 100644
--- a/lib/Workflow/Manager.php
+++ b/lib/Workflow/Manager.php
@@ -203,20 +203,20 @@ public function getWorkflowIfExists(object $subject, string $workflowName): ?Wor
         return $workflow;
     }
 
-    /**
-     * @deprecated will return ?WorkflowInterface in 12.0.0
-     */
-    public function getWorkflowByName(string $workflowName): ?object
+    public function getWorkflowByName(string $workflowName): ?WorkflowInterface
     {
         $config = $this->getWorkflowConfig($workflowName);
 
-        return Pimcore::getContainer()->get($config->getType() . '.' . $workflowName);
+        $workflow = Pimcore::getContainer()?->get($config->getType() . '.' . $workflowName);
+
+        if (!$workflow instanceof WorkflowInterface) {
+            return null;
+        }
+
+        return $workflow;
     }
 
     /**
-     *
-     *
-     * @throws ValidationException
      * @throws Exception
      */
     public function applyWithAdditionalData(
diff --git a/lib/Workflow/Notification/NotificationEmailService.php b/lib/Workflow/Notification/NotificationEmailService.php
index 3dfcec7c3ec..623a1b2f502 100644
--- a/lib/Workflow/Notification/NotificationEmailService.php
+++ b/lib/Workflow/Notification/NotificationEmailService.php
@@ -17,14 +17,18 @@
 namespace Pimcore\Workflow\Notification;
 
 use Exception;
+use Pimcore\Logger;
+use Pimcore\Mail;
+use Pimcore\Model\Asset;
 use Pimcore\Model\DataObject;
+use Pimcore\Model\Document;
 use Pimcore\Model\Element\ElementInterface;
 use Pimcore\Model\User;
 use Pimcore\Tool;
 use Pimcore\Workflow\EventSubscriber\NotificationSubscriber;
 use Symfony\Component\Routing\RouterInterface;
 use Symfony\Component\Templating\EngineInterface;
-use Symfony\Component\Workflow\Workflow;
+use Symfony\Component\Workflow\WorkflowInterface;
 use Symfony\Contracts\Translation\LocaleAwareInterface;
 use Symfony\Contracts\Translation\TranslatorInterface;
 
@@ -49,7 +53,16 @@ public function __construct(EngineInterface $template, RouterInterface $router,
      * Sends an Mail
      *
      */
-    public function sendWorkflowEmailNotification(array $users, array $roles, Workflow $workflow, string $subjectType, ElementInterface $subject, string $action, string $mailType, string $mailPath): void
+    public function sendWorkflowEmailNotification(
+        array $users,
+        array $roles,
+        WorkflowInterface $workflow,
+        string $subjectType,
+        ElementInterface $subject,
+        string $action,
+        string $mailType,
+        string $mailPath
+    ): void
     {
         try {
             $recipients = $this->getNotificationUsersByName($users, $roles);
@@ -62,16 +75,17 @@ public function sendWorkflowEmailNotification(array $users, array $roles, Workfl
             if ($hostUrl !== '') {
                 // Decide what kind of link to create
                 $objectType = $type = 'object';
-                if ($subject instanceof \Pimcore\Model\Document) {
+                if ($subject instanceof Document) {
                     $objectType = 'document';
                     $type = $subject->getType();
                 }
-                if ($subject instanceof \Pimcore\Model\Asset) {
+                if ($subject instanceof Asset) {
                     $objectType = 'asset';
                     $type = $subject->getType();
                 }
 
-                $deeplink = $hostUrl . $this->router->generate('pimcore_admin_login_deeplink') . '?'.$objectType.'_' . $subject->getId() . '_'. $type;
+                $deeplink = $hostUrl . $this->router->generate('pimcore_admin_login_deeplink');
+                $deeplink .= '?'.$objectType.'_' . $subject->getId() . '_'. $type;
             }
 
             foreach ($recipients as $language => $recipientsPerLanguage) {
@@ -109,17 +123,38 @@ public function sendWorkflowEmailNotification(array $users, array $roles, Workfl
                         break;
                 }
             }
-        } catch (Exception $e) {
-            \Pimcore\Logger::error('Error sending Workflow change notification email.');
+        } catch (Exception) {
+            Logger::error('Error sending Workflow change notification email.');
         }
     }
 
     /**
      * @param User[] $recipients
      */
-    protected function sendPimcoreDocumentMail(array $recipients, string $subjectType, ElementInterface $subject, Workflow $workflow, string $action, string $language, string $mailPath, string $deeplink): void
+    protected function sendPimcoreDocumentMail(
+        array $recipients,
+        string $subjectType,
+        ElementInterface $subject,
+        WorkflowInterface $workflow,
+        string $action,
+        string $language,
+        string $mailPath,
+        string $deeplink
+    ): void
     {
-        $mail = new \Pimcore\Mail(['document' => $mailPath, 'params' => $this->getNotificationEmailParameters($subjectType, $subject, $workflow, $action, $deeplink, $language)]);
+        $mail = new Mail(
+            [
+                'document' => $mailPath,
+                'params' => $this->getNotificationEmailParameters(
+                    $subjectType,
+                    $subject,
+                    $workflow,
+                    $action,
+                    $deeplink,
+                    $language
+                )
+            ]
+        );
 
         foreach ($recipients as $user) {
             $mail->addTo($user->getEmail(), $user->getName());
@@ -131,16 +166,30 @@ protected function sendPimcoreDocumentMail(array $recipients, string $subjectTyp
     /**
      * @param User[] $recipients
      */
-    protected function sendTemplateMail(array $recipients, string $subjectType, ElementInterface $subject, Workflow $workflow, string $action, string $language, string $mailPath, string $deeplink): void
+    protected function sendTemplateMail(
+        array $recipients,
+        string $subjectType,
+        ElementInterface $subject,
+        WorkflowInterface $workflow,
+        string $action,
+        string $language,
+        string $mailPath,
+        string $deeplink
+    ): void
     {
-        $mail = new \Pimcore\Mail();
+        $mail = new Mail();
 
         foreach ($recipients as $user) {
             $mail->addTo($user->getEmail(), $user->getName());
         }
 
         $mail->subject(
-            $this->translator->trans('workflow_change_email_notification_subject', [$subjectType . ' ' . $subject->getFullPath(), $workflow->getName()], 'admin', $language)
+            $this->translator->trans(
+                'workflow_change_email_notification_subject',
+                [$subjectType . ' ' . $subject->getFullPath(), $workflow->getName()],
+                'admin',
+                $language
+            )
         );
 
         $mail->html($this->getHtmlBody($subjectType, $subject, $workflow, $action, $language, $mailPath, $deeplink));
@@ -148,7 +197,15 @@ protected function sendTemplateMail(array $recipients, string $subjectType, Elem
         $mail->send();
     }
 
-    protected function getHtmlBody(string $subjectType, ElementInterface $subject, Workflow $workflow, string $action, string $language, string $mailPath, string $deeplink): string
+    protected function getHtmlBody(
+        string $subjectType,
+        ElementInterface $subject,
+        WorkflowInterface $workflow,
+        string $action,
+        string $language,
+        string $mailPath,
+        string $deeplink
+    ): string
     {
         $translatorLocaleBackup = null;
         if ($this->translator instanceof LocaleAwareInterface) {
@@ -170,7 +227,14 @@ protected function getHtmlBody(string $subjectType, ElementInterface $subject, W
         }
     }
 
-    protected function getNotificationEmailParameters(string $subjectType, ElementInterface $subject, Workflow $workflow, string $action, string $deeplink, string $language): array
+    protected function getNotificationEmailParameters(
+        string $subjectType,
+        ElementInterface $subject,
+        WorkflowInterface $workflow,
+        string $action,
+        string $deeplink,
+        string $language
+    ): array
     {
         $noteDescription = $this->getNoteInfo($subject->getId());
 
diff --git a/lib/Workflow/Notification/PimcoreNotificationService.php b/lib/Workflow/Notification/PimcoreNotificationService.php
index 2f6de37a83a..01b28ccaae3 100644
--- a/lib/Workflow/Notification/PimcoreNotificationService.php
+++ b/lib/Workflow/Notification/PimcoreNotificationService.php
@@ -17,9 +17,10 @@
 namespace Pimcore\Workflow\Notification;
 
 use Exception;
+use Pimcore\Logger;
 use Pimcore\Model\Element\ElementInterface;
 use Pimcore\Model\Notification\Service\NotificationService;
-use Symfony\Component\Workflow\Workflow;
+use Symfony\Component\Workflow\WorkflowInterface;
 use Symfony\Contracts\Translation\TranslatorInterface;
 
 class PimcoreNotificationService extends AbstractNotificationService
@@ -38,7 +39,14 @@ public function __construct(NotificationService $notificationService, Translator
         $this->translator = $translator;
     }
 
-    public function sendPimcoreNotification(array $users, array $roles, Workflow $workflow, string $subjectType, ElementInterface $subject, string $action): void
+    public function sendPimcoreNotification(
+        array $users,
+        array $roles,
+        WorkflowInterface $workflow,
+        string $subjectType,
+        ElementInterface $subject,
+        string $action
+    ): void
     {
         try {
             $recipients = $this->getNotificationUsersByName($users, $roles, true);
@@ -47,7 +55,12 @@ public function sendPimcoreNotification(array $users, array $roles, Workflow $wo
             }
 
             foreach ($recipients as $language => $recipientsPerLanguage) {
-                $title = $this->translator->trans('workflow_change_email_notification_subject', [$subjectType . ' ' . $subject->getFullPath(), $workflow->getName()], 'admin', $language);
+                $title = $this->translator->trans(
+                    'workflow_change_email_notification_subject',
+                    [$subjectType . ' ' . $subject->getFullPath(), $workflow->getName()],
+                    'admin',
+                    $language
+                );
                 $message = $this->translator->trans(
                     'workflow_change_email_notification_text',
                     [
@@ -71,8 +84,8 @@ public function sendPimcoreNotification(array $users, array $roles, Workflow $wo
                     $this->notificationService->sendToUser($recipient->getId(), 0, $title, $message, $subject);
                 }
             }
-        } catch (Exception $e) {
-            \Pimcore\Logger::error('Error sending Workflow change notification.');
+        } catch (Exception) {
+            Logger::error('Error sending Workflow change notification.');
         }
     }
 }


Test patch: