-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Allow multiple events subscription with regexp #3512
Allow multiple events subscription with regexp #3512
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, just a few thoughts on difference in behavior depending on type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a comment about the discussion happened, let me know!
While working on this PR I noticed the illogic situation exposed in #3519. I think that PR needs attention and to be merged before this one. |
052fcbe
to
cadf034
Compare
core/spec/lib/spree/event/adapters/active_support_notifications_spec.rb
Outdated
Show resolved
Hide resolved
`Spree::Event` events can now be subscribed also using regexp event names, when using the default ActiveSupportNotification adapter. As ActiveSupportNotification can manage regexp when subscribing to events, it makes sense to allow `Spree::Event` to leverage this feature. Every specific adapter can decide independently their supported event name types.
The documentation now includes explanations on using Regexp for subscribing to multiple events and its gotcha.
cadf034
to
3c3475e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @spaghetticode!
Ref #3481
Spree::Event
events can now be subscribed also using regexp event names, when using the default ActiveSupportNotification adapter.As ActiveSupportNotification can manage regexp when subscribing to events, it makes sense to allow
Spree::Event
to leverage this feature.Every specific adapter can decide independently their supported event name types.
Checklist: