GridFieldDeleteAction does not allow you to unlink relations with many_many associations if you don't have "edit" permission

[warslett]

Affected Version

Probably all versions. Observed in 4.3.0

Description

The GridFieldDeleteAction does not allow you to unlink a record if you don't have the canEdit permission on the related record. This is probably the correct behaviour in some situations but not others. There are plenty of situations where a user might not have permission to edit a record but should be able to link other records to it.

This behaviour originates from a check that takes place on line 219 of vendor/silverstripe/framework/src/Forms/GridField/GridFieldDeleteAction.php and a check when the action is handled on line 198 of the same file.

The specific example we keep banging into is where some data in the CMS is managed via an API integration (for example Job Adverts from a third party job posting service) so we don't give any user permission to edit it directly in the CMS. However, if we want to use some of that data in an Element on the site (for instance an element that displays a list of selected of Job Adverts) then we still want the CMS user to be able to link those records and unlink them from our element via a gridfield.

There are also contradictions in this behaviour because although we do not allow the user to unlink when they don't have edit permission, we do allow them to "Link Existing". So they can add but not remove.

Steps to Reproduce

#SelectedJobs.php
<?php

namespace BiffBangPow\Elements;

use DNADesign\Elemental\Models\BaseElement;
use JobPosting;
use SilverStripe\Forms\FieldList;
use SilverStripe\Forms\GridField\GridField;
use SilverStripe\Forms\GridField\GridField_ActionMenu;
use SilverStripe\Forms\GridField\GridFieldAddExistingAutocompleter;
use SilverStripe\Forms\GridField\GridFieldConfig_Base;
use SilverStripe\Forms\GridField\GridFieldDataColumns;

class SelectedJobs extends BaseElement
{

    private static $many_many = [
        'JobPostings' => JobPosting::class
    ];

    /**
     * @return FieldList
     */
    public function getCMSFields()
    {
        $fields = parent::getCMSFields();

        $config = GridFieldConfig_Base::create();
        $config->addComponent(new GridFieldAddExistingAutocompleter('before', 'Title'));
        $config->addComponent(new GridField_ActionMenu());
        $config->addComponent(new GridfieldDeleteAction(true));
        $config->getComponentByType(GridFieldDataColumns::class)->setDisplayFields([
            'Title' => 'Title'
        ]);
        $fields->addFieldToTab('Root.JobPostings', new GridField('JobPostings', 'Jobs', $this->JobPostings(), $config));

        return $fields;
    }
}

#JobPosting.php
<?php

use SilverStripe\ORM\DataObject;

class JobPosting extends DataObject
{

    ...

    /**
     * @param null $member
     * @return bool|int
     */
    public function canView($member = null)
    {
        return true;
    }

    /**
     * @param null $member
     * @return bool|int
     */
    public function canEdit($member = null)
    {
        return false;
    }

    /**
     * @param null $member
     * @return bool|int
     */
    public function canDelete($member = null)
    {
        return false;
    }

    /**
     * @param null $member
     * @param array $context
     * @return bool|int
     */
    public function canCreate($member = null, $context = [])
    {
        return false;
    }

    /**
     * @param null $member
     * @return bool|int
     */
    public function canArchive($member = null)
    {
        return false;
    }

    /**
     * @param null $member
     * @return bool
     */
    public function canRestoreToDraft($member = null)
    {
        return false;
    }
}

Expected Behaviour

If I can add existing records to a many_many relationship using Gridfield I should also be able to remove them regardless of whether I have permission to edit or delete. I should be able to see an unlink button alongside the record and when I click it the record should be removed from the gridfield.

Current Behaviour

I cannot unlink a record when I do not have edit permission on that record. Regardless of the fact that I can legitimately "add" those relationships. The unlink button is not displayed on the gridfield.

[warslett]

the temporary work around is to create your own alternative delete action. Here is mine (wouldn't be so much code if getRemoveAction was protected!):

<?php

use SilverStripe\Forms\GridField\GridField;
use SilverStripe\Forms\GridField\GridField_ActionMenuItem;
use SilverStripe\Forms\GridField\GridField_FormAction;
use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\ValidationException;

class GridfieldDeleteAction extends \SilverStripe\Forms\GridField\GridFieldDeleteAction
{

    /**
     * @inheritdoc
     */
    public function getTitle($gridField, $record, $columnName)
    {
        $field = $this->getRemoveAction($gridField, $record, $columnName);

        if ($field) {
            return $field->getAttribute('title');
        }

        return _t(__CLASS__ . '.Delete', "Delete");
    }

    /**
     * @inheritdoc
     */
    public function getGroup($gridField, $record, $columnName)
    {
        $field = $this->getRemoveAction($gridField, $record, $columnName);

        return $field ? GridField_ActionMenuItem::DEFAULT_GROUP: null;
    }

    /**
     *
     * @param GridField $gridField
     * @param DataObject $record
     * @param string $columnName
     * @return string|null the attribles for the action
     */
    public function getExtraData($gridField, $record, $columnName)
    {

        $field = $this->getRemoveAction($gridField, $record, $columnName);

        if ($field) {
            return $field->getAttributes();
        }

        return null;
    }

    /**
     *
     * @param GridField $gridField
     * @param DataObject $record
     * @param string $columnName
     * @return string|null the HTML for the column
     */
    public function getColumnContent($gridField, $record, $columnName)
    {
        $field = $this->getRemoveAction($gridField, $record, $columnName);

        if ($field) {
            return $field->Field();
        }

        return null;
    }

    /**
     *
     * @param GridField $gridField
     * @param DataObject $record
     * @param string $columnName
     * @return GridField_FormAction|null
     */
    private function getRemoveAction($gridField, $record, $columnName)
    {
        if ($this->getRemoveRelation()) {
            $title = _t(__CLASS__ . '.UnlinkRelation', "Unlink");

            $field = GridField_FormAction::create(
                $gridField,
                'UnlinkRelation' . $record->ID,
                false,
                "unlinkrelation",
                ['RecordID' => $record->ID]
            )
                ->addExtraClass('btn btn--no-text btn--icon-md font-icon-link-broken grid-field__icon-action gridfield-button-unlink action-menu--handled')
                ->setAttribute('classNames', 'gridfield-button-unlink font-icon-link-broken')
                ->setDescription($title)
                ->setAttribute('aria-label', $title);
        } else {
            if (!$record->canDelete()) {
                return null;
            }
            $title = _t(__CLASS__ . '.Delete', "Delete");

            $field = GridField_FormAction::create(
                $gridField,
                'DeleteRecord' . $record->ID,
                false,
                "deleterecord",
                ['RecordID' => $record->ID]
            )
                ->addExtraClass('action--delete btn--icon-md font-icon-trash-bin btn--no-text grid-field__icon-action action-menu--handled')
                ->setAttribute('classNames', 'action--delete font-icon-trash')
                ->setDescription($title)
                ->setAttribute('aria-label', $title);
        }

        return $field;
    }

    /**
     * Handle the actions and apply any changes to the GridField
     *
     * @param GridField $gridField
     * @param string $actionName
     * @param array $arguments
     * @param array $data Form data
     * @throws ValidationException
     */
    public function handleAction(GridField $gridField, $actionName, $arguments, $data)
    {
        if ($actionName == 'deleterecord' || $actionName == 'unlinkrelation') {
            /** @var DataObject $item */
            $item = $gridField->getList()->byID($arguments['RecordID']);
            if (!$item) {
                return;
            }

            if ($actionName == 'deleterecord') {
                if (!$item->canDelete()) {
                    throw new ValidationException(
                        _t(__CLASS__ . '.DeletePermissionsFailure', "No delete permissions")
                    );
                }

                $item->delete();
            } else {

                $gridField->getList()->remove($item);
            }
        }
    }
}

[chillu]

Thanks for the comprehensive bug report. My suggestion would be that we inspect the canEdit method on a many-many-through "link" object by default. That's an accurate reflection of "can modify relationship", as opposed to "can modify record". The problem is that we're not already doing this, so it would be a breaking change: Anyone already using many-many-through on 4.x would need to their "link" object, since by default DataObject->canEdit() requires ADMIN permissions.

Do you want to have a crack at writing this logic for GridFieldDeleteAction, and making this behaviour opt-in? Something like GridField::$enforce_many_many_through_permissions. We'd likely need to implement this in other places on GridField as well to make it consistent.

[phptek]

I have spotted a similar behaviour, which results in the "unlink" action not displaying (for an admin user, so prob not a canXX() issue). Instead, it seems to be down to VersionedGridFieldArchiveExtension. If I remove/comment that extension from versionedgridfield.yml, I see the "Unlink" action.

I'll raise a separate issue for this.