Skip to content

Add payment logging #610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

maciejzgadzaj
Copy link
Contributor

@maciejzgadzaj
Copy link
Contributor Author

maciejzgadzaj commented Jan 20, 2017

Result:

  • with full captures/refunds:

order-activity-log-full

  • with partial captures/refunds:

order-activity-log

Copy link

@heddn heddn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the events that were added are a duplicate of the already added ones provided by state_machine. No need to add the Event and Events classes. I'll do a quick review of the rest of the patch as well.

* The transition event.
*/
public function onAuthorizeTransition(WorkflowTransitionEvent $event) {
$this->dispatch(PaymentEvents::PAYMENT_AUTHORIZED, $event->getEntity());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this log?

* The transition event.
*/
public function onVoidTransition(WorkflowTransitionEvent $event) {
$this->dispatch(PaymentEvents::PAYMENT_VOIDED, $event->getEntity());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why no logging?

* The transition event.
*/
public function onExpireTransition(WorkflowTransitionEvent $event) {
$this->dispatch(PaymentEvents::PAYMENT_EXPIRED, $event->getEntity());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And on through the rest of this subscriber...

@@ -24,6 +24,7 @@
* bundle_plugin_type = "commerce_payment_type",
* handlers = {
* "access" = "Drupal\commerce_payment\PaymentAccessControlHandler",
* "event" = "Drupal\commerce_payment\Event\PaymentEvent",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is necessary if we state_machine

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is, because payments do not use transitions.

@mglaman
Copy link
Contributor

mglaman commented Sep 16, 2017

Closing for #797. Will make sure issues credits correct on D.o issue.

@mglaman mglaman closed this Sep 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants