Dynamic notification / action data

[shadyvb]

This adds two enhancements:

• Enhancement: Allow action data attribute to be a callback receiving action arguments and returning data to be stored, to allow dynamic on-execution data storage.

• Enhancement: Add a new filter to allow manipulating notification object before output, allows for further highly-dynamic data gathering on-output, or data that shouldn't be stored in DB.

• Enhancement: Allow overriding the action data sanitisation callback to permit more flexible schemas.

• Updated changelog

• Updated version number in package.json and plugin.php file with appropriate MAJOR.MINOR.PATCH change

[roborourke]

This is all solid and makes sense but I think we can do a little better on the notification data sanitisation - I've made a suggestion for a sanitize_notification_data() function that takes the whole array rather than something applied to each item.

[mattheu]

@shadyvb @roborourke I wonder if we should be able to add arbitrary data to the notification itself, instead of passing it on the action (or maybe in addition to the action?)

As an example - we have a 'like article' feature, a notification is fired when this action happens. We want to group the 'like' notifications for a single article together so need some data to use for this. We don't need any actions for this functionality.

@shadyvb Do you think if we could pass the data on the notification itself in a very similar way to what you are doing here for actions, then you could achieve your goals with that alone or do you also need to pass the data on the action too?

[roborourke]

Based on what you're saying @mattheu I agree that there's a use case for arbitrary meta data to be associated with a notification such as 'type' etc... I'm thinking there's a few things that are currently missing that would make this possible:

1. Passing the initiating Event object or Event id to the destination handler here would enable you to add some arbitrary data based on the event at the point of creating the notification here
2. Dashboard Notification API schema updated with a generic object type here
3. Update the notification sanitisation function to sanitise the new arbitrary data field in the same way as the action level data

I was originally thinking you could wither use the UI object and set_data() to pass some custom data back to the destination handler if you're creating them in the UI but that doesn't really help if you're wanting to keep things purely in code or modify the destination handler based on the initiating event. Eg you could create a version of the existing destination handler but it's suboptimal:

Destination::register( 'dashboard_like', function ( $recipients, $messages ) {
  $messages = array_map( function ( $message ) {
       $message['type'] = 'like';
       return $messages;
  }, $messages );
  return HM\Workflows\Dashboard\dashboard_handler( $recipients, $messages );
} );

[shadyvb]

@mattheu We do need an action for the like or follow event, and I was working with a type and a data attribute on the action object to group notifs. Having them on the notification level makes sense ( specially the type one ), but my motive was that notifications’ consumers would not need data unless they’re parsing for actionables, in which case it might make a bit more sense to attach them to an action instead ? too thin a line here, since this might cause data dup as well 🤔

TBH it also complicates the grouping logic a bit! having to match at the action level instead of notification level. It made sense when I did it, but I think I’d have gone for notification-level data if the method was there.

Happy for #90 to be merged and then our client PR can work off that.

@roborourke Thanks for the insight, I guess @mattheu ‘s approach in #90 would bridge the gap we hit, along side changes in this PR.

I’ll work on the sanitisation enhancements you proposed and push back for review.

[shadyvb]

For the type param, I think we can just forward the event id which should work for our use case.