RFC Separate Form Schema / Controller #19

[tractorcow]

See #6334 for initial story.

Introduction

Currently the SilverStripe\Forms\Form class has a variety of roles; It handles storage of the form schema (fields, layout, etc), handles form and nested field requests, validation, error handling, and rendering.

The basis of this pull request is to separate this into distinct model (i.e. schema) and controller components.

Background

This RFC is being developed in preparation for some future work planned around abstracting form schema and submission handling in GraphQL. This work necessitates that the current "parent controller" dependency on forms is removed, and instead we move to form scaffolding based on the FormBuilder api.

However, the goal of this is to not break or change existing form handling, but to simply segment the current API into separate pieces that can be substituted on a case by case basis.

Note that this RFC explicitly does not dictate how GraphQL based form submission will operate, but instead focuses on any necessary refactor to the current API without significant change in behaviour for traditional forms.

Currently the Form API looks similar to the below.

Old Form.php

class Form extends RequestHandler {
	public function __construct($controller, $name, FieldList $fields, FieldList $actions, Validator $validator = null) {}
	
	public function httpSubmission($request) {}
}

The issue with this model is that it requires both the controller and the name of the form to be known, meaning that it's impossible to create an abstract from unassigned to any particular parent controller.

Proposal

This RFC proposes that Form becomes a direct subclass of ViewableData. Instead the request handler may be requested on a case by case basis for a form via Form::getRequestHandler() method. In order to ensure that quickly scaffolding up any form remains an easy task, the default behaviour of this method will be to return a response handler which behaves as per standard forms.

Note: It is not necessary for a form to have a response handler in all cases, as external API may in fact provide their own response handler objects for any given form.

The new API would look similar to the below:

New Form.php

class Form extends ViewableData implements HasRequestHandler {
	public function __construct(RequestHandler $parent = null, $name = 'Form', FieldList $fields = null, FieldList $actions = null, Validator $validator = null) {}
	
	/**
	 * @return FormRequestHandler
	 */
	public function getRequestHandler() {
		return Injector::inst()->create(FormRequestHandler::class, $this);
	}
}

New FormRequestHandler.php

class FormRequestHandler extends RequestHandler {
	public function __construct(Form $form) {}
	
	public function httpSubmission($request) {}
}

New HasRequestHandler.php

interface HasRequestHandler {
	/** @return RequestHandler */
	public function getRequestHandler();
}

Note the below changes will be necessary:

• All arguments to Form constructor are now optional. It's possible for instance to construct a form with no controller / name. However, form Name is still necessary to generate HTML IDs for itself and child fields, so this will be inferred if omitted.
• The first argument to the form constructor has been relaxed so that a non-controller request handler may be passed.
• Previously, actions could be implemented on either the parent controller or on the form itself. See below for changes to how these are declared.
• Although Forms are no longer RequestHandler subclasses, they implement HasRequestHandler

Note: Although the order of arguments has been retained as a matter of convenience for upgrade, these may not necessarily remain in this order.

Changes to form actions

Previously, form actions could be declared either (in order of precendence):

• On the Form itself
• On the parent controller

In order to minimise the reliance of the form on a parent controller, it will now be possible to declare a closure / callback on form actions to handle successful actions.

class MyObject {
	public function Form() {
		return new Form(null, null, $this->getFields(), $this->getActions());
	}
	protected function getActions() {
		$action = FormAction::create('save', 'Save')
			->setAction(function($data, $form) {
				return json_encode(['status' => 'ok], true);
			});
		return new FieldList($action);
	}
}

Additionally, with the split of form into Model / Controller, it is possible to define actions on either.

Thus the order of action precedence becomes

• action callback
• action on the Form
• action on the FormRequestHandler
• action on any parent controller (if given)

Changes to RequestHandler

RequestHandler::handleRequest will need to be updated to check for actions which return implementors of HasRequestHandler, in order to know that any response object has a delegate object for any request.

This is necessary for traditional Form() methods on parent controllers to continue to handle requests.

public function handleRequest(HTTPRequest $request, DataModel $model) {
	// ...
	if($result instanceof HasRequestHandler) {
		$result = $result->getRequestHandler();
	}
}

The below methods will also be moved from Controller to RequestHandler

• Link()
• redirect()
• redirectBack()
• redirectedTo()

Changes to FormFactory

FormFactory::getForm() will be changed so that it only takes a single parameter, $context.

Form factories will be responsible for choosing a name for any generated form, and the controller is no longer necessary to build any form schema.

interface FormFactory {
	public function getForm($context = []);
}

In addition a new requirement is added to $context is that it must be serialisable, so that a front-end library could include this context in any request. Note: Implementation of GraphQL based form submission is outside of the scope of this PR, but the serialisation of $context will be necessary for this later task.

(Tentative requirement: Non-react controllers can still make use of FormFactory instances to scaffold editable forms for their models, however an additional call to setController() after getForm() will be necessary.)

[wilr]

Looks good @tractorcow. As that is setup will be perfect for the graphQL. For forms without a controller would there need to be an explicit need to define the url for the form action using setFormAction(..). Do we throw any sort of exception if no form action provided?

[tractorcow]

URL can be set explicitly, or it could be inferred from the request handler. This is one reason I wanted to move Link() up to the RequestHandler class.

In the case of GraphQL, this will just be ignored. :)

[tractorcow]

PR at #6692