Skip to content
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

Notifications: streamline notification payloads by introducing a params DTO #2216

Open
dkarlovi opened this issue Feb 14, 2023 · 3 comments
Open

Comments

@dkarlovi
Copy link
Contributor

dkarlovi commented Feb 14, 2023

Q A
Bug report? no
Feature request? yes
BC Break report? no
RFC? no

Currently, notification payloads are ad-hoc defined in CoreShop\Bundle\CoreBundle\EventListener\NotificationRules listeners and CoreShop\Bundle\NotificationBundle\Processor\EventedRuleProcessor.

The idea here is to streamline these payloads into a common structure which is easier to rely on if, for example, we dispatch these notifications to outside systems (think webhooks). One external example we can inspire oureselves from is Azure Alerts Common Schema or Cloud events.

Subject

The $subject is the object the notification is dealing with and it needs to be instance of \CoreShop\Component\Resource\Model\ResourceInterface. Here's a list of current subject types across listeners:

Listener Subject
CustomerListener CoreShop\Component\Core\Model\CustomerInterface
OrderCommentsListener CoreShop\Component\Core\Model\OrderInterface
OrderDocumentWorkflowListener CoreShop\Component\Order\Model\OrderDocumentInterface
OrderWorkflowListener CoreShop\Component\Order\Model\OrderInterface
PaymentWorkflowListener CoreShop\Component\Core\Model\PaymentInterface

It's obvious the subject is meant to be directly the resource we're talking about, that part is fine.

Rule Type

Rule type is the type of rules we want to apply, it's defined by the conditions and actions available for the type.

Currently available are: user, order, shipment, invoice, payment, quote.

Params type

This is where we're getting to the crux of this issue, the inconsistencies between different rule appliers becomes visible here. Since the rule type defined above represents the rule type, not the event type, events are optionally named in the $params payload. This allows the conditions / actions to differentiate between different notifications triggered by the same rule, for example.

Some of these are defined as consts (example in CustomerListener), some are hardcoded (example in OrderCommentsListener), in some it's not set (example in OrderDocumentWorkflowListener).

This property should be be normalized and always defined.

DTO

My proposal is to introduce a DTO the purpose of which is to solve these common scenarios and streamline the params payload.

The proposed DTO schema is as follows:

timestamp: <autopopulated on object creation>
type: <namespaced previous params type> # example: user.password.reset_request
user: <reference to the user which performed the action which triggered the event>, can be null
customer: <reference to the customer which performed the action which triggered the event>, can be null
context:
    store: <reference to the store which triggered the event>
    locale: <locale used when performing the action which triggered the event>
    parent: <reference to a "parent" of the subject, if applicable> # example, order for order comment or document
reference: # optional
    link: <link provided for the action> # example, password reset, newsletter subscribe
    token: <token used in the link>
workflow: # optional
    name: <workflow name used>
    from: <from state>
    to: <to state>
    transition: <transtion name>
attributes: # free form, what currently $params is
    foo: 123

The "reference" in the description would mean either an object or a reasonable (serializeable) representation of the object in question, most probably always looking like

id: 123
name: <foo> # optional

but used consistently.

Example usage: own password reset

type: user.password.reset_request
customer: {id: 123}
context:
    store: {id: 1, name: Store CH}
    locale: de
reference:
    link: http://example.ch/reset?token=123
    token: 123

Example usage: password reset on behalf of customer

type: user.password.reset_request
user: {id: 9} # user triggering the password reset
customer: {id: 123} # on behalf of the customer
context:
    store: {id: 1, name: Store CH}
    locale: de
reference:
    link: http://example.ch/reset?token=123
    token: 123
attributes:

Example usage: order comment

type: order.comment
user: {id: 9} # user adding the comment
customer: {id: 123} # for the customer
context:
    store: {id: 1, name: Store CH}
    locale: de
    parent: {id: 1234} # order which this comment belongs to
attributes: 
    comment: sehr gut # mandatory for type: order.comment
    submitAsEmail: true

Example usage: order document

type: order.document
customer: {id: 123}
context:
    store: {id: 1, name: Store CH}
    locale: de
    parent: {id: 1234} # order which this document belongs to
workflow: 
    name: Some name here
    from: preparing
    to: ready
    transition: print

Example usage: order workflow

type: order.workflow
customer: {id: 123}
context:
    store: {id: 1, name: Store CH}
    locale: de
workflow: 
    name: Some name here
    from: completing
    to: completed
    transition: finalize

Example usage: payment workflow

type: payment.workflow
customer: {id: 123}
context:
    store: {id: 1, name: Store CH}
    locale: de
    parent: {id: 1234} # order which this payment belongs to
workflow: 
    name: Some name here
    from: completing
    to: completed
    transition: finalize
@dkarlovi
Copy link
Contributor Author

FYI we're prototyping this in our own repo, will PR if any of it makes sense.

@dpfaffenbauer
Copy link
Member

@dkarlovi any update on this from your side?

@dkarlovi
Copy link
Contributor Author

I've implemented this on my project, would need to clean it up substantially to PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants