silverstripe / silverstripe-admin

Silverstripe Admin Component
BSD 3-Clause "New" or "Revised" License
25 stars 93 forks source link

SecurityAdmin ignores summary_fields and searchable_fields on Group #1302

Open GuySartorelli opened 2 years ago

GuySartorelli commented 2 years ago

There is no search filter in the Groups tab in SecurityAdmin despite having the GridFieldFilterHeader in the groups GridFieldConfig. What's more, if you customise the summary_fields or searchable_fields for Group via an Extension, the SecurityAdmin doesn't respect it.

This seems to be caused by the below lines which override the display fields for GridFieldDataColumns. https://github.com/silverstripe/silverstripe-admin/blob/7f7fc721cb3820487acefa6e1663c252d9d56cc5/code/SecurityAdmin.php#L143-L145 Commenting out those lines provides the search filter and allows customised summary_fields or searchable_fields to be reflected correctly.

It also points out a deeper issue - the GridFieldFilterHeader should probably not be affected by GridFieldDataColumns if searchable_fields has been provided.

forsdahl commented 2 years ago

I think the solution here would be to change GridFieldFilterHeader to not check for filterable columns unless the legacy functionality is used, i.e. change:

https://github.com/silverstripe/silverstripe-framework/blob/724a9007df1d0f5efcdca78ccb022c0959b7ad5b/src/Forms/GridField/GridFieldFilterHeader.php#L497-L506

to:

    public function getHTMLFragments($gridField)
    {
        $forTemplate = new ArrayData([]);

        if ($this->useLegacyFilterHeader) {
            if (!$this->canFilterAnyColumns($gridField)) {
                return null;
            }
            $fieldsList = $this->getLegacyFilterHeader($gridField);
maxime-rainville commented 2 years ago

Not sure, but fixing https://github.com/silverstripe/silverstripe-admin/issues/1350 will probably fix this issue.

GuySartorelli commented 2 years ago

This is no longer a problem for CMS 5 - but it remains a bug for CMS 4.