NEW: Event-driven snapshot actions

[unclecheese]

Summary

This pull request makes a number of improvements to the public API for action-driven snapshot creation. While most of the core functionality around how snapshots are created in certain contexts is preserved, the execution path to get there, and the way a developer can customise and/or extend that pattern is improved using a more event-driven paradigm.

Problems addressed

• "Action identifiers" were opaque, not grouped or namespaced, and difficult to debug
• Reacting to a CMS user action was opted into by assigning a message to an identifier. This felt like a really roundabout approach for what is fundamentally the more common pattern of subscribing to an event.
• The subscriber API was extension hooks, which is too low level and warrants a layer of abstraction.
• Snapshot object was overloaded with CMS concerns (e.g. forms, graphql, controllers).
• Messages were not translatable (or at best, difficult to translate)
• Encouraged to imperative programming, e.g. if ($action === 'save')

Key solutions

• Listeners are mostly the same, but hand off control to a central Dispatcher instance that fires events.
• Snapshot creators are just event handlers (HandlerInterface)
• Event handlers are given EventContext objects that provide relevant event metadata.
• All EventContext objects must declare an action identifier (getAction(): string)
• Complex computations around extracting action identifiers (e.g. from GridField state or GraphQL queries) is now the concern of EventContext classes.
• Messages are created computationally by the event handlers using i18n, and can be overridden with config
• Customising/adding/removing snapshot creators can be done declaratively in config/Injector
• Imperative checks for things like $action === 'save' are done reactively with a custom event handler, e.g. formSubmitted.save
• Remove "model" snapshots
• More code reuse, tidying up

For discussion

The entire event API (everything in Dispatch and Listener) has no concerns about snapshots. Only the event handlers that create the snapshots do (everything in Handler). I think it would make good sense to spin off the events into a separate module to maintain a nice separation of concerns and afford devs a chance to plug into reactive programming in the CMS without installing snapshots.

[mfendeksilverstripe]

Awesome work! I really like this event based approach :), the code looks more DRY as well. I found some issues and I have some suggestions.

[mfendeksilverstripe]

Suggested change

	Dispatcher::singleton()->trigger('formSubmitted', new FormContext($funcName, $form, $request, $vars));
	$this->triggerAction($request, $funcName, $vars, $form);

[unclecheese]

Where is that method defined?

[mfendeksilverstripe]

Suggested change

	}
	}
	/**
	* Extension point in @see FormRequestHandler::httpSubmission
	* form handler action via form submission action
	*
	* @param HTTPRequest $request
	* @param $funcName
	* @param $vars
	* @param Form $form
	*/
	public function afterCallFormHandlerMethod(HTTPRequest $request, $funcName, $vars, $form): void
	{
	$this->triggerAction($request, $funcName, $vars, $form);
	}
	/**
	* Extension point in @see FormRequestHandler::httpSubmission
	* form method action via form submission action
	*
	* @param HTTPRequest $request
	* @param $funcName
	* @param $vars
	* @param Form $form
	*/
	public function afterCallFormHandlerFormMethod(HTTPRequest $request, $funcName, $vars, $form): void
	{
	$this->triggerAction($request, $funcName, $vars, $form);
	}
	/**
	* Extension point in @see FormRequestHandler::httpSubmission
	* form field method action via form submission action
	*
	* @param HTTPRequest $request
	* @param $funcName
	* @param $vars
	* @param Form $form
	*/
	public function afterCallFormHandlerFieldMethod(HTTPRequest $request, $funcName, $vars, $form): void
	{
	$this->triggerAction($request, $funcName, $vars, $form);
	}
	/**
	* @param HTTPRequest $request
	* @param $funcName
	* @param $vars
	* @param Form $form
	*/
	private function triggerAction(HTTPRequest $request, $funcName, $vars, $form): void
	{
	Dispatcher::singleton()->trigger('formSubmitted', new FormContext($funcName, $form, $request, $vars));
	}

This needs to cover all 4 extension points for Form submissions

[unclecheese]

See the revision I made to that PR. I didn't think we needed four hooks. silverstripe/silverstripe-framework#9340

[mfendeksilverstripe]

@unclecheese I have a test integration here which is helpful to cover all the test cases:

https://github.com/silverstripeltd/tourism1/pull/2797

[mfendeksilverstripe]

@unclecheese Thanks for the new fixes :). it seems to be working reasonably well now. I am now confident that all requirements from previous version were retained.

I'm moving the development of this feature to the terraformers repo as this will allow more people to collaborate more easily. Feel free to continue providing awesome feedback / fixes.

Here is the new PR:

#4

I added one more commit with some fixes:

30a887f

I also updated the TNZ integration branch which now supports almost all actions that were available previously. I recommend using this branch for testing out Snapshots features on a real project, it makes catching bugs much easier:

https://github.com/silverstripeltd/tourism1/pull/2797

[unclecheese]

Excellent. I was just working on testing it with TNZ, so this is great.

[unclecheese]

I'll get working on the test suite to get this green, and cover the new API surface.