search

sezzle / sezzle-magento2

Apache License 2.0
5 stars 3 forks source link

Observer implementation not compatible with the Interface #27

Open lbajsarowicz opened 1 month ago

lbajsarowicz commented 1 month ago

In the Observers implemented for Sezzle extension, none of them is compatible with the Interface:

https://github.com/sezzle/sezzle-magento2/blob/c7825f3bf438eccbf4a084bf823f07d22b10e789/Observer/SendOrderConfirmationMailObserver.php#L48

https://github.com/sezzle/sezzle-magento2/blob/c7825f3bf438eccbf4a084bf823f07d22b10e789/Observer/PaymentMethodAvailabilityObserver.php#L45

https://github.com/sezzle/sezzle-magento2/blob/c7825f3bf438eccbf4a084bf823f07d22b10e789/Observer/OrderStatusChangeOnVoidObserver.php#L23

Please see the declaration of ObserverInterface

https://github.com/magento/magento2/blob/5a2037c35de89ce2c50e230164ff3ea7b67ac127/lib/internal/Magento/Framework/Event/ObserverInterface.php#L16-L23

arijit-sezzle commented 1 month ago

Sorry, I could not find any reason why you think its not compatible unless you think its because of the return type?

lbajsarowicz commented 1 month ago

@arijit-sezzle Yes, the return type of Observer classes should be void.

arijit-sezzle commented 1 month ago

Makes sense. Just curious, did you find out its erroring in 2.4.7 because of this?

lbajsarowicz commented 1 month ago

Before letting our Developers integrate new extensions, we perform thorough Code Review to make sure that 3rd party extension won't cause any issues. In this specific case, PHPStan reported that your code is flaky, so I fixed the issue in a Composer Patch and informed you about the problem.