FIX allow extensions to modify canXYZ methods

[robbieaverill]

• Resolves BaseElement's canXYZ() methods can't be extended #29

[wilr]

This fix will call the extensions but it won't actually modify the response to it right? Think the pattern for this should be something like

function canX(...) {
$extended = $this->extendedCan(FUNCTION, $member);
if($extended !== null) return $extended;

.. $existing code

[robbieaverill]

@wilr yeah, it will call the parent method but won't use its results. It's DataObject::canDelete() for example that provides the extension hook.

[wilr]

See https://github.com/silverstripe-australia/silverstripe-advancedworkflow/blob/master/code/dataobjects/WorkflowInstance.php#L412-L416

[robbieaverill]

@wilr no worries, let's go that way!

[robbieaverill]

FYI I've gone with PSR-2 since it's coming - if you want me to use standard SS style I'm happy to change it

[wilr]

Nah PSR1/2 FTW

[robbieaverill]

Cool. Hold this PR - I'm not sure it's working correctly with canView().

[robbieaverill]

Looks like versioned-data-objects is returning 0 somewhere in the canView permissions check and preventing them from being visible. Looking into it.

Even with "Access to all CMS sections" enabled, Versioned::canView is still returning 0 for a non-admin role.

[robbieaverill]

@wilr - having had a quick read of the Versioned docs about permissions it sounds sensible to me that this module should implement its own permission check for canView which it does already, so perhaps we shouldn't try to allow the extension hook for it?

I'm confused about two things here:

1. Is silverstripe-versioned-dataobjects required over the framework's included Versioned functionality which can apply to DataObjects as well?
2. Is there a way to still provide the extension hooks from BaseElement, while not letting the permission errors from Versioned prevent the actions? I'm by no means a master in this, but would moving the canXYZ methods into a BaseElementExtension which would extend VersionedDataObject be a better option? This way the result you return from the canXYZ method would be isolated to that extension, and the base permission check would treat it as an extension's two cents rather than the base DataObjects two cents...? It appears to be working for me. I'm gonna push that change up and let you decide if you like it :)

[robbieaverill]

Hey @wilr - what do you think about this change?

[robbieaverill]

@wilr - rebased onto latest master changes

[wilr]

Yeah I think that is possibly a better way to go and allows users to sub in anything else for permissions.