Make GridFieldComponents work with arbitrary ViewableData where practical

[GuySartorelli]

Currently most GridFieldComponent classes implicitly require some methods which are implemented on DataObject but not on ViewableData.

It is not uncommon to want to use a GridField with other viewable data, such as ArrayData (e.g. pulled from some API).

It should be easy to use most GridField components with arbitrary ViewableData. Any components for which this is not practical should be clearly documented as requiring specific methods (and in CMS 6 probably will be typehinted to DataObject).

Acceptance Criteria

• Where practical, all GridFieldComponent classes work with arbitrary ViewableData
• Tests are put in place to avoid regressions
• For any component where it is not practical to use it with ViewableData (if any), this is clearly documented in a way that developers can identify from within their IDE.

Notes

• There's a wider question about how to consistently and systematically apply this to all GridField components. That can be handled in a follow up CMS 6 card.
• I have explicitly focused on the components provided by silverstripe/framework for this card. See Make GridFieldComponents work with arbitrary ViewableData where practical (gridfieldextensions edition) symbiote/silverstripe-gridfieldextensions#376

PRs

• Make GridField components work with ViewableData where possible #11049
• NEW Add fixtures for arbitrary data gridfield behat tests silverstripe-frameworktest#159
• MNT Add behat tests for gridfield with non-DataObjects silverstripe-admin#1626
• DOC Document using GridField with arbitrary data developer-docs#409

[GuySartorelli]

My proposed rules of thumb for what "where practical" means:

• If making a component "work" with ViewableData means just hiding the component or making the component do nothing, then that is not practical. Hiding a component or disabling a component is not making the component work and those should throw clear exceptions instead. e.g. GridFieldDetailForm_ItemRequest
• Some components already allow using ViewableData by providing setters for information that would otherwise be fetched from DataObjects - e.g. GridFieldDetailForm allows setting a validator, fields, and even a custom item request class in lieu of GridFieldDetailForm_ItemRequest.
  • In these cases, if the setters have not been called and this would result in an exception being thrown, the exception should say something like "Implement $missingMethod on your data class or call $setterMethod on this component".
  • If a component already provides some of these setters but still has a barrier that could be bypassed with another setter, it is practical to add that setter.
  • If a component does not already provide these setters and adding them would be 'hacky' or complex, it may be more appropriate to just throw exceptions - this should be assessed on a case-by-case basis and the pull request should indicate what the reason was for not adding a setter (so the reviewer doesn't have to ask).
• Some components just rely on so much logic from DataObject that it's not practical to find workarounds for it all. One example is GridFieldDetailForm_ItemRequest. In these cases, the PHPDoc for the class should clearly note that it is intended for use with DataObject.
  • Without a new major release, we can't explicitly type-check for DataObject, but we can check up-front for the methods or fields we're expecting and throw an exception if any are missing. That may seem cumbersome but it's probably easier than adding a method_exists() or ->hasMethod() or ClassInfo::hasMethod() check for every method we want to call on the record and provides a much clearer DX as to why this component isn't working for them.
• Ultimately this issue is aimed at removing barriers that shouldn't be there, and adding clear DX about the barriers that should be there. At the end of the day "practical" is a judgment call as to whether the barrier seems reasonable or not to the person doing the work.

[maxime-rainville]

The following modules have GridField components:

• bringyourownideas/silverstripe-maintenance
• colymba/gridfield-bulk-editing
• silverstripe/blog
• silverstripe/ckan-registry
• silverstripe/gridfieldqueuedexport
• silverstripe/lumberjack
• silverstripe/securityreport
• fluent
• subsite
• userforms
• versioned-admin
• versioned
• symbiote/silverstripe-advanced-workflow
• symbiote/silverstripe-gridfieldextensions

Maybe updating those is beyond the scope of this card.

[GuySartorelli]

@maxime-rainville I already made a card for symbiote/silverstripe-gridfieldextensions since that has generic components (see link in issue description)

Advanced workflow is explicitly out of scope - as are a few others there which are only intended to be used in the way the module uses it, rather than in developer-defined gridfields. ckan-registry isn't even supported anymore....

Maybe open a card to cover any there which:

• are actually still supported
• are intended to be used in developer-defined gridfields beyond those already declared in the module

[maxime-rainville]

It is done.