No events thrown for Payment CRUD

Looks like PaymentStorage extends the correct class (CommerceContentEntityStorage), the only thing that we're missing is an "event" in the Payment entity annotation (plus the actual event class and list of events in PaymentEvents, of course).

I've tried the commits mentionned in #5.

Works quite well. I would make few changes :

in PaymentStorage, all the events should be declared

In PaymentEvent, I would change the name of the function getOrder() to getEntity() which is more inline with other events and also because the function does not get the order entity but the payment entity

Finally, the events where not triggered when I wanted them and instead of hacking commerce for my project, I ended up using core hooks. Since payments are entities, using function

hook_commerce_payment_update(Drupal\Core\Entity\EntityInterface $entity)

and

hook_commerce_payment_insert(Drupal\Core\Entity\EntityInterface $entity)

Ok, I just re-rolled this patch, and also added the 'event' annotation to the Payment entity, so now all the events should work. There are tests right now for the create and load event. I also changed the getOrder method to getEntity, and added a test for that (as mentioned in comment 6). This issue is related to https://www.drupal.org/node/2856586 so I'm planning on adding that event too but maybe in another patch that depends on this one.

The PR is here: https://github.com/drupalcommerce/commerce/pull/791

Whew, two years later and this finally needed a re-roll. Updated patch attached for the latest version of Commerce.

I don't know if any other work needs to be done to this - I have just straight up rerolled the latest patch. I haven't attached a diff, since the last patch was a seven-part patch.

You could listen for state transitions in an EventListener to achieve something very similar. You won't get any event when there is no change in the state of the payment though. This is what I implemented:

/**
   * {@inheritdoc}
   */
  public static function getSubscribedEvents() {
    // Logs for payment_default.
    $events['commerce_payment.authorize.pre_transition'] = ['logPayment', -100];
    $events['commerce_payment.void.pre_transition'] = ['onVoid', -100];
    $events['commerce_payment.expire.pre_transition'] = ['logPayment', -100];
    $events['commerce_payment.authorize_capture.pre_transition'] = ['logPayment', -100];
    $events['commerce_payment.capture.pre_transition'] = ['logPayment', -100];
    $events['commerce_payment.partially_refund.pre_transition'] = ['logPayment', -100];
    $events['commerce_payment.refund.pre_transition'] = ['logPayment', -100];

    // Additional logs for payment_manual.
    $events['commerce_payment.create.pre_transition'] = ['logPayment', -100];
    $events['commerce_payment.receive.pre_transition'] = ['logPayment', -100];
    return $events;
  }

  public function logPayment(WorkflowTransitionEvent $event) {...}

While working on a payment logging issue: https://www.drupal.org/project/commerce/issues/2845321, @mglaman pointed me towards this issue. I've separated the payment CRUD events from the payment logging and have combined that code with the latest patch here. Changes to the patch include:
* An update hook for the entity event handler. Without the hook, events weren't firing when the patch was applied to an existing site.
* A few minor updates to the test code.

+++ b/modules/payment/commerce_payment.install
@@ -86,3 +86,13 @@ function commerce_payment_update_8203() {
+  $entity_type->setHandlerClass('event', \Drupal\commerce_payment\Event\PaymentEvent::class);

phpcs says we should move this into a `use` statement. nit on commit

+++ b/modules/payment/src/PaymentStorage.php
@@ -49,7 +51,13 @@ class PaymentStorage extends CommerceContentEntityStorage implements PaymentStor
+    $payment = parent::doCreate($values);
+
+    // Notify other modules.
+    $event = new PaymentEvent($payment);
+    $this->eventDispatcher->dispatch(PaymentEvents::PAYMENT_CREATE, $event);
+
+    return $payment;

Shouldn't the parent still invoke hooks here which CommerceContentEntityStorage fires events on?

Addresses #16. Punting to bojanz for review.

+   *
+   * @var string
+   */

Some of the event constants have var string, some don't. So the file itself is inconsistent. Looking at other files, we usually don't declare it, so I'm going to remove it.

--- a/modules/payment/src/PaymentStorage.php
+++ b/modules/payment/src/PaymentStorage.php
@@ -4,6 +4,8 @@ namespace Drupal\commerce_payment;
 
 use Drupal\commerce\CommerceContentEntityStorage;
 use Drupal\commerce_order\Entity\OrderInterface;
+use Drupal\commerce_payment\Event\PaymentEvent;
+use Drupal\commerce_payment\Event\PaymentEvents;

These imports are now unused and trigger phpcs warnings.

+class PaymentEventsTest extends OrderKernelTestBase {

While I like the tests, they are actually out of place here, because commerce_payment doesn't fire any events, the Commerce base class does. So if we want test coverage for entity events, it needs to live in the base module (let's say tests/src/Kernel/EntityEventsTest).

I'm going to remove the test/subscriber and commit, and you can make the coverage generic in a followup issue (post-2.17). I hope that's okay.

Thanks, everyone!