diff --git a/src/Forms/Form.php b/src/Forms/Form.php index 7f9b2251ff8..db7044ac9fb 100644 --- a/src/Forms/Form.php +++ b/src/Forms/Form.php @@ -12,7 +12,6 @@ use SilverStripe\Control\Session; use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Injector\Injector; -use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DataObjectInterface; use SilverStripe\ORM\FieldType\DBHTMLText; use SilverStripe\ORM\ValidationResult; @@ -136,7 +135,7 @@ class Form extends ViewableData implements HasRequestHandler /** * Populated by {@link loadDataFrom()}. * - * @var DataObject|null + * @var ViewableData|null */ protected $record; @@ -1223,10 +1222,10 @@ public function sessionFieldError($message, $fieldName, $type = ValidationResult } /** - * Returns the DataObject that has given this form its data + * Returns the record that has given this form its data * through {@link loadDataFrom()}. * - * @return DataObject + * @return ViewableData */ public function getRecord() { @@ -1285,7 +1284,7 @@ public function validationResult() const MERGE_AS_SUBMITTED_VALUE = 0b1000; /** - * Load data from the given DataObject or array. + * Load data from the given record or array. * * It will call $object->MyField to get the value of MyField. * If you passed an array, it will call $object[MyField]. @@ -1306,7 +1305,7 @@ public function validationResult() * @uses FormField::setSubmittedValue() * @uses FormField::setValue() * - * @param array|DataObject $data + * @param array|ViewableData $data * @param int $mergeStrategy * For every field, {@link $data} is interrogated whether it contains a relevant property/key, and * what that property/key's value is. @@ -1351,7 +1350,7 @@ public function loadDataFrom($data, $mergeStrategy = 0, $fieldList = null) // If an object is passed, save it for historical reference through {@link getRecord()} // Also use this to determine if we are loading a submitted form, or loading - // from a dataobject + // from a record $submitted = true; if (is_object($data)) { $this->record = $data; @@ -1480,7 +1479,7 @@ public function loadDataFrom($data, $mergeStrategy = 0, $fieldList = null) * Save the contents of this form into the given data object. * It will make use of setCastedField() to do this. * - * @param DataObjectInterface $dataObject The object to save data into + * @param ViewableData&DataObjectInterface $dataObject The object to save data into * @param FieldList $fieldList An optional list of fields to process. This can be useful when you have a * form that has some fields that save to one object, and some that save to another. */ @@ -1523,7 +1522,7 @@ public function saveInto(DataObjectInterface $dataObject, $fieldList = null) * {@link FieldList->dataFields()}, which filters out * any form-specific data like form-actions. * Calls {@link FormField->dataValue()} on each field, - * which returns a value suitable for insertion into a DataObject + * which returns a value suitable for insertion into a record * property. * * @return array diff --git a/src/Forms/FormField.php b/src/Forms/FormField.php index dc14e477b16..e8c821f65d3 100644 --- a/src/Forms/FormField.php +++ b/src/Forms/FormField.php @@ -8,13 +8,13 @@ use SilverStripe\Control\RequestHandler; use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Convert; -use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DataObjectInterface; use SilverStripe\ORM\FieldType\DBField; use SilverStripe\ORM\FieldType\DBHTMLText; use SilverStripe\ORM\ValidationResult; use SilverStripe\View\AttributesHTML; use SilverStripe\View\SSViewer; +use SilverStripe\View\ViewableData; /** * Represents a field in a form. @@ -454,11 +454,11 @@ public function Value() } /** - * Method to save this form field into the given {@link DataObject}. + * Method to save this form field into the given record. * * By default, makes use of $this->dataValue() * - * @param DataObject|DataObjectInterface $record DataObject to save data into + * @param ViewableData|DataObjectInterface $record Record to save data into */ public function saveInto(DataObjectInterface $record) { @@ -697,7 +697,7 @@ public function attrValue() * or a submitted form value they should override setSubmittedValue() instead. * * @param mixed $value Either the parent object, or array of source data being loaded - * @param array|DataObject $data {@see Form::loadDataFrom} + * @param array|ViewableData $data {@see Form::loadDataFrom} * @return $this */ public function setValue($value, $data = null) @@ -712,7 +712,7 @@ public function setValue($value, $data = null) * data formats. * * @param mixed $value - * @param array|DataObject $data + * @param array|ViewableData $data * @return $this */ public function setSubmittedValue($value, $data = null) diff --git a/src/Forms/GridField/GridField.php b/src/Forms/GridField/GridField.php index 19210651ccc..2590aa7585e 100644 --- a/src/Forms/GridField/GridField.php +++ b/src/Forms/GridField/GridField.php @@ -20,11 +20,14 @@ use SilverStripe\Forms\GridField\FormAction\StateStore; use SilverStripe\ORM\ArrayList; use SilverStripe\ORM\DataList; -use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DataObjectInterface; use SilverStripe\ORM\FieldType\DBField; +use SilverStripe\ORM\Filterable; +use SilverStripe\ORM\Limitable; +use SilverStripe\ORM\Sortable; use SilverStripe\ORM\SS_List; use SilverStripe\View\HTML; +use SilverStripe\View\ViewableData; /** * Displays a {@link SS_List} in a grid format. @@ -83,12 +86,12 @@ class GridField extends FormField /** * Data source. * - * @var SS_List + * @var SS_List&Filterable&Sortable&Limitable */ protected $list = null; /** - * Class name of the DataObject that the GridField will display. + * Class name of the records that the GridField will display. * * Defaults to the value of $this->list->dataClass. * @@ -205,7 +208,7 @@ public function setModelClass($modelClassName) } /** - * Returns a data class that is a DataObject type that this GridField should look like. + * Returns the class name of the record type that this GridField should contain. * * @return string * @@ -374,7 +377,7 @@ public function getCastedValue($value, $castingDefinition) /** * Set the data source. * - * @param SS_List $list + * @param SS_List&Filterable&Sortable&Limitable $list * * @return $this */ @@ -388,7 +391,7 @@ public function setList(SS_List $list) /** * Get the data source. * - * @return SS_List + * @return SS_List&Filterable&Sortable&Limitable */ public function getList() { @@ -398,7 +401,7 @@ public function getList() /** * Get the data source after applying every {@link GridField_DataManipulator} to it. * - * @return SS_List + * @return SS_List&Filterable&Sortable&Limitable */ public function getManipulatedList() { @@ -461,7 +464,7 @@ private function addStateFromRequest(): void if (($request instanceof NullHTTPRequest) && Controller::has_curr()) { $request = Controller::curr()->getRequest(); } - + $stateStr = $this->getStateManager()->getStateFromRequest($this, $request); if ($stateStr) { $oldState = $this->getState(false); @@ -744,7 +747,7 @@ public function FieldHolder($properties = []) /** * @param int $total * @param int $index - * @param DataObject $record + * @param ViewableData $record * @param array $attributes * @param string $content * @@ -762,7 +765,7 @@ protected function newCell($total, $index, $record, $attributes, $content) /** * @param int $total * @param int $index - * @param DataObject $record + * @param ViewableData $record * @param array $attributes * @param string $content * @@ -780,7 +783,7 @@ protected function newRow($total, $index, $record, $attributes, $content) /** * @param int $total * @param int $index - * @param DataObject $record + * @param ViewableData $record * * @return array */ @@ -798,7 +801,7 @@ protected function getRowAttributes($total, $index, $record) /** * @param int $total * @param int $index - * @param DataObject $record + * @param ViewableData $record * * @return array */ @@ -869,7 +872,7 @@ public function getColumns() /** * Get the value from a column. * - * @param DataObject $record + * @param ViewableData $record * @param string $column * * @return string @@ -922,7 +925,7 @@ public function addDataFields($fields) * Use of this method ensures that any special rules around the data for this gridfield are * followed. * - * @param DataObject $record + * @param ViewableData $record * @param string $fieldName * * @return mixed @@ -949,7 +952,7 @@ public function getDataFieldValue($record, $fieldName) /** * Get extra columns attributes used as HTML attributes. * - * @param DataObject $record + * @param ViewableData $record * @param string $column * * @return array diff --git a/src/Forms/GridField/GridFieldAddExistingAutocompleter.php b/src/Forms/GridField/GridFieldAddExistingAutocompleter.php index ff6176d9276..d8f83678c69 100644 --- a/src/Forms/GridField/GridFieldAddExistingAutocompleter.php +++ b/src/Forms/GridField/GridFieldAddExistingAutocompleter.php @@ -32,6 +32,8 @@ * * For easier setup, have a look at a sample configuration in * {@link GridFieldConfig_RelationEditor}. + * + * The modelClass of the GridField this component is in must be a DataObject subclass. */ class GridFieldAddExistingAutocompleter extends AbstractGridFieldComponent implements GridField_HTMLProvider, GridField_ActionProvider, GridField_DataManipulator, GridField_URLHandler { @@ -106,6 +108,10 @@ public function getHTMLFragments($gridField) { $dataClass = $gridField->getModelClass(); + if (!is_a($dataClass, DataObject::class, true)) { + throw new LogicException(__CLASS__ . " must be used with DataObject subclasses. Found '$dataClass'"); + } + $forTemplate = new ArrayData([]); $forTemplate->Fields = new FieldList(); @@ -191,11 +197,17 @@ public function handleAction(GridField $gridField, $actionName, $arguments, $dat */ public function getManipulatedData(GridField $gridField, SS_List $dataList) { + $dataClass = $gridField->getModelClass(); + + if (!is_a($dataClass, DataObject::class, true)) { + throw new LogicException(__CLASS__ . " must be used with DataObject subclasses. Found '$dataClass'"); + } + $objectID = $gridField->State->GridFieldAddRelation(null); if (empty($objectID)) { return $dataList; } - $object = DataObject::get_by_id($gridField->getModelClass(), $objectID); + $object = DataObject::get_by_id($dataClass, $objectID); if ($object) { $dataList->add($object); } @@ -227,6 +239,10 @@ public function doSearch($gridField, $request) $searchStr = $request->getVar('gridfield_relationsearch'); $dataClass = $gridField->getModelClass(); + if (!is_a($dataClass, DataObject::class, true)) { + throw new LogicException(__CLASS__ . " must be used with DataObject subclasses. Found '$dataClass'"); + } + $searchFields = ($this->getSearchFields()) ? $this->getSearchFields() : $this->scaffoldSearchFields($dataClass); @@ -337,6 +353,10 @@ public function getSearchFields() */ public function scaffoldSearchFields($dataClass) { + if (!is_a($dataClass, DataObject::class, true)) { + throw new LogicException(__CLASS__ . " must be used with DataObject subclasses. Found '$dataClass'"); + } + $obj = DataObject::singleton($dataClass); $fields = null; if ($fieldSpecs = $obj->searchableFields()) { @@ -387,6 +407,10 @@ public function scaffoldSearchFields($dataClass) */ public function getPlaceholderText($dataClass) { + if (!is_a($dataClass, DataObject::class, true)) { + throw new LogicException(__CLASS__ . " must be used with DataObject subclasses. Found '$dataClass'"); + } + $searchFields = ($this->getSearchFields()) ? $this->getSearchFields() : $this->scaffoldSearchFields($dataClass); diff --git a/src/Forms/GridField/GridFieldAddNewButton.php b/src/Forms/GridField/GridFieldAddNewButton.php index dffcabdcd08..67d9436574f 100644 --- a/src/Forms/GridField/GridFieldAddNewButton.php +++ b/src/Forms/GridField/GridFieldAddNewButton.php @@ -2,7 +2,9 @@ namespace SilverStripe\Forms\GridField; +use LogicException; use SilverStripe\Control\Controller; +use SilverStripe\Core\ClassInfo; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\RelationList; use SilverStripe\View\ArrayData; @@ -12,8 +14,7 @@ * This component provides a button for opening the add new form provided by * {@link GridFieldDetailForm}. * - * Only returns a button if {@link DataObject->canCreate()} for this record - * returns true. + * Only returns a button if canCreate() for this record returns true. */ class GridFieldAddNewButton extends AbstractGridFieldComponent implements GridField_HTMLProvider { @@ -36,7 +37,16 @@ public function __construct($targetFragment = 'before') public function getHTMLFragments($gridField) { - $singleton = singleton($gridField->getModelClass()); + $modelClass = $gridField->getModelClass(); + $singleton = singleton($modelClass); + + if (!$singleton->hasMethod('canCreate')) { + throw new LogicException( + __CLASS__ . ' cannot be used with models that do not implement canCreate().' + . " Remove this component from your GridField or implement canCreate() on $modelClass" + ); + } + $context = []; if ($gridField->getList() instanceof RelationList) { $record = $gridField->getForm()->getRecord(); @@ -51,7 +61,7 @@ public function getHTMLFragments($gridField) if (!$this->buttonName) { // provide a default button name, can be changed by calling {@link setButtonName()} on this component - $objectName = $singleton->i18n_singular_name(); + $objectName = $singleton->hasMethod('i18n_singular_name') ? $singleton->i18n_singular_name() : ClassInfo::shortName($singleton); $this->buttonName = _t('SilverStripe\\Forms\\GridField\\GridField.Add', 'Add {name}', ['name' => $objectName]); } diff --git a/src/Forms/GridField/GridFieldConfig_Base.php b/src/Forms/GridField/GridFieldConfig_Base.php index ea080390fc9..3db245fe872 100644 --- a/src/Forms/GridField/GridFieldConfig_Base.php +++ b/src/Forms/GridField/GridFieldConfig_Base.php @@ -2,6 +2,8 @@ namespace SilverStripe\Forms\GridField; +use SilverStripe\Dev\Deprecation; + /** * A simple readonly, paginated view of records, with sortable and searchable * headers. @@ -23,9 +25,11 @@ public function __construct($itemsPerPage = null) $this->addComponent(GridFieldPageCount::create('toolbar-header-right')); $this->addComponent($pagination = GridFieldPaginator::create($itemsPerPage)); - $sort->setThrowExceptionOnBadDataType(false); - $filter->setThrowExceptionOnBadDataType(false); - $pagination->setThrowExceptionOnBadDataType(false); + Deprecation::withNoReplacement(function () use ($sort, $filter, $pagination) { + $sort->setThrowExceptionOnBadDataType(false); + $filter->setThrowExceptionOnBadDataType(false); + $pagination->setThrowExceptionOnBadDataType(false); + }); $this->extend('updateConfig'); } diff --git a/src/Forms/GridField/GridFieldConfig_RecordEditor.php b/src/Forms/GridField/GridFieldConfig_RecordEditor.php index bcb3079d95f..82a273dc585 100644 --- a/src/Forms/GridField/GridFieldConfig_RecordEditor.php +++ b/src/Forms/GridField/GridFieldConfig_RecordEditor.php @@ -1,6 +1,8 @@ addComponent($pagination = GridFieldPaginator::create($itemsPerPage)); $this->addComponent(GridFieldDetailForm::create(null, $showPagination, $showAdd)); - $sort->setThrowExceptionOnBadDataType(false); - $filter->setThrowExceptionOnBadDataType(false); - $pagination->setThrowExceptionOnBadDataType(false); + Deprecation::withNoReplacement(function () use ($sort, $filter, $pagination) { + $sort->setThrowExceptionOnBadDataType(false); + $filter->setThrowExceptionOnBadDataType(false); + $pagination->setThrowExceptionOnBadDataType(false); + }); $this->extend('updateConfig'); } diff --git a/src/Forms/GridField/GridFieldConfig_RelationEditor.php b/src/Forms/GridField/GridFieldConfig_RelationEditor.php index 2cb243f51b5..20a495ec5e4 100644 --- a/src/Forms/GridField/GridFieldConfig_RelationEditor.php +++ b/src/Forms/GridField/GridFieldConfig_RelationEditor.php @@ -2,6 +2,8 @@ namespace SilverStripe\Forms\GridField; +use SilverStripe\Dev\Deprecation; + /** * Similar to {@link GridFieldConfig_RecordEditor}, but adds features to work * on has-many or many-many relationships. @@ -43,9 +45,11 @@ public function __construct($itemsPerPage = null) $this->addComponent($pagination = GridFieldPaginator::create($itemsPerPage)); $this->addComponent(GridFieldDetailForm::create()); - $sort->setThrowExceptionOnBadDataType(false); - $filter->setThrowExceptionOnBadDataType(false); - $pagination->setThrowExceptionOnBadDataType(false); + Deprecation::withNoReplacement(function () use ($sort, $filter, $pagination) { + $sort->setThrowExceptionOnBadDataType(false); + $filter->setThrowExceptionOnBadDataType(false); + $pagination->setThrowExceptionOnBadDataType(false); + }); $this->extend('updateConfig'); } diff --git a/src/Forms/GridField/GridFieldDataColumns.php b/src/Forms/GridField/GridFieldDataColumns.php index a852e3107f1..41b0713d9c9 100644 --- a/src/Forms/GridField/GridFieldDataColumns.php +++ b/src/Forms/GridField/GridFieldDataColumns.php @@ -4,7 +4,8 @@ use SilverStripe\Core\Convert; use InvalidArgumentException; -use SilverStripe\ORM\DataObject; +use LogicException; +use SilverStripe\View\ViewableData; /** * @see GridField @@ -87,7 +88,15 @@ public function setDisplayFields($fields) public function getDisplayFields($gridField) { if (!$this->displayFields) { - return singleton($gridField->getModelClass())->summaryFields(); + $modelClass = $gridField->getModelClass(); + $singleton = singleton($modelClass); + if (!$singleton->hasMethod('summaryFields')) { + throw new LogicException( + 'Cannot dynamically determine columns. Pass the column names to setDisplayFields()' + . " or implement a summaryFields() method on $modelClass" + ); + } + return $singleton->summaryFields(); } return $this->displayFields; } @@ -146,7 +155,7 @@ public function getFieldFormatting() * HTML for the column, content of the element. * * @param GridField $gridField - * @param DataObject $record Record displayed in this row + * @param ViewableData $record Record displayed in this row * @param string $columnName * @return string HTML for the column. Return NULL to skip. */ @@ -180,7 +189,7 @@ public function getColumnContent($gridField, $record, $columnName) * Attributes for the element containing the content returned by {@link getColumnContent()}. * * @param GridField $gridField - * @param DataObject $record displayed in this row + * @param ViewableData $record displayed in this row * @param string $columnName * @return array */ @@ -216,7 +225,7 @@ public function getColumnMetadata($gridField, $column) /** * Translate a Object.RelationName.ColumnName $columnName into the value that ColumnName returns * - * @param DataObject $record + * @param ViewableData $record * @param string $columnName * @return string|null - returns null if it could not found a value */ @@ -269,7 +278,7 @@ protected function castValue($gridField, $fieldName, $value) /** * * @param GridField $gridField - * @param DataObject $item + * @param ViewableData $item * @param string $fieldName * @param string $value * @return string diff --git a/src/Forms/GridField/GridFieldDeleteAction.php b/src/Forms/GridField/GridFieldDeleteAction.php index 35c3c1e694f..a0d81c322bf 100644 --- a/src/Forms/GridField/GridFieldDeleteAction.php +++ b/src/Forms/GridField/GridFieldDeleteAction.php @@ -2,9 +2,12 @@ namespace SilverStripe\Forms\GridField; +use LogicException; use SilverStripe\Control\Controller; -use SilverStripe\ORM\DataObject; +use SilverStripe\ORM\DataList; +use SilverStripe\ORM\DataObjectInterface; use SilverStripe\ORM\ValidationException; +use SilverStripe\View\ViewableData; /** * This class is a {@link GridField} component that adds a delete action for @@ -72,13 +75,12 @@ public function getGroup($gridField, $record, $columnName) /** * * @param GridField $gridField - * @param DataObject $record + * @param DataObjectInterface&ViewableData $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) { @@ -105,7 +107,7 @@ public function augmentColumns($gridField, &$columns) * Return any special attributes that will be used for FormField::create_tag() * * @param GridField $gridField - * @param DataObject $record + * @param DataObjectInterface&ViewableData $record * @param string $columnName * @return array */ @@ -153,7 +155,7 @@ public function getActions($gridField) /** * * @param GridField $gridField - * @param DataObject $record + * @param DataObjectInterface&ViewableData $record * @param string $columnName * @return string|null the HTML for the column */ @@ -179,29 +181,39 @@ public function getColumnContent($gridField, $record, $columnName) */ public function handleAction(GridField $gridField, $actionName, $arguments, $data) { + $list = $gridField->getList(); if ($actionName == 'deleterecord' || $actionName == 'unlinkrelation') { - /** @var DataObject $item */ - $item = $gridField->getList()->byID($arguments['RecordID']); + /** @var DataObjectInterface&ViewableData $item */ + $item = $list->byID($arguments['RecordID']); if (!$item) { return; } if ($actionName == 'deleterecord') { + $this->checkForRequiredMethod($item, 'canDelete'); + if (!$item->canDelete()) { throw new ValidationException( _t(__CLASS__ . '.DeletePermissionsFailure', "No delete permissions") ); } + if (!($list instanceof DataList)) { + // We need to make sure to exclude the item since the list items have already been determined. + // This must happen before deletion while the item still has its ID set. + $gridField->setList($list->exclude(['ID' => $item->ID])); + } $item->delete(); } else { + $this->checkForRequiredMethod($item, 'canEdit'); + if (!$item->canEdit()) { throw new ValidationException( _t(__CLASS__ . '.EditPermissionsFailure', "No permission to unlink record") ); } - $gridField->getList()->remove($item); + $list->remove($item); } } } @@ -209,16 +221,19 @@ public function handleAction(GridField $gridField, $actionName, $arguments, $dat /** * * @param GridField $gridField - * @param DataObject $record + * @param DataObjectInterface&ViewableData $record * @param string $columnName * @return GridField_FormAction|null */ private function getRemoveAction($gridField, $record, $columnName) { if ($this->getRemoveRelation()) { + $this->checkForRequiredMethod($record, 'canEdit'); + if (!$record->canEdit()) { return null; } + $title = _t(__CLASS__ . '.UnlinkRelation', "Unlink"); $field = GridField_FormAction::create( @@ -233,9 +248,12 @@ private function getRemoveAction($gridField, $record, $columnName) ->setDescription($title) ->setAttribute('aria-label', $title); } else { + $this->checkForRequiredMethod($record, 'canDelete'); + if (!$record->canDelete()) { return null; } + $title = _t(__CLASS__ . '.Delete', "Delete"); $field = GridField_FormAction::create( @@ -274,4 +292,20 @@ public function setRemoveRelation($removeRelation) $this->removeRelation = (bool) $removeRelation; return $this; } + + /** + * Checks if a required method exists - and if not, throws an exception. + * + * @throws LogicException if the required method doesn't exist + */ + private function checkForRequiredMethod($record, string $method): void + { + if (!$record->hasMethod($method)) { + $modelClass = get_class($record); + throw new LogicException( + __CLASS__ . " cannot be used with models that don't implement {$method}()." + . " Remove this component from your GridField or implement {$method}() on $modelClass" + ); + } + } } diff --git a/src/Forms/GridField/GridFieldDetailForm.php b/src/Forms/GridField/GridFieldDetailForm.php index 0497dc564f6..07299cc93fd 100644 --- a/src/Forms/GridField/GridFieldDetailForm.php +++ b/src/Forms/GridField/GridFieldDetailForm.php @@ -3,6 +3,7 @@ namespace SilverStripe\Forms\GridField; use Closure; +use LogicException; use SilverStripe\Control\Director; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; @@ -13,10 +14,11 @@ use SilverStripe\Core\Extensible; use SilverStripe\Core\Injector\Injector; use SilverStripe\Forms\FieldList; +use SilverStripe\Forms\FieldsValidator; use SilverStripe\Forms\Validator; use SilverStripe\ORM\DataList; use SilverStripe\ORM\DataObject; -use SilverStripe\ORM\Filterable; +use SilverStripe\View\ViewableData; /** * Provides view and edit forms at GridField-specific URLs. @@ -62,7 +64,7 @@ class GridFieldDetailForm extends AbstractGridFieldComponent implements GridFiel protected $validator; /** - * @var FieldList Falls back to {@link DataObject->getCMSFields()} if not defined. + * @var FieldList Falls back to {@link $record->getCMSFields()} if not defined. */ protected $fields; @@ -143,28 +145,31 @@ public function handleItem($gridField, $request) // if no validator has been set on the GridField then use the Validators from the record. if (!$this->getValidator()) { - $this->setValidator($record->getCMSCompositeValidator()); + if ($record->hasMethod('getCMSCompositeValidator')) { + $validator = $record->getCMSCompositeValidator(); + } else { + $validator = FieldsValidator::create(); + } + $this->setValidator($validator); } return $handler->handleRequest($request); } - /** - * @param GridField $gridField - * @param HTTPRequest $request - * @return DataObject|null - */ - protected function getRecordFromRequest(GridField $gridField, HTTPRequest $request): ?DataObject + protected function getRecordFromRequest(GridField $gridField, HTTPRequest $request): ?ViewableData { - /** @var DataObject $record */ + /** @var ViewableData $record */ if (is_numeric($request->param('ID'))) { - /** @var Filterable $dataList */ $dataList = $gridField->getList(); $record = $dataList->byID($request->param('ID')); } else { $record = Injector::inst()->create($gridField->getModelClass()); } + if ($record && !$record->hasField('ID')) { + throw new LogicException(get_class($record) . ' must have an ID field.'); + } + return $record; } @@ -174,7 +179,7 @@ protected function getRecordFromRequest(GridField $gridField, HTTPRequest $reque * This only works when the list passed to the GridField is a {@link DataList}. * * @param $gridField The current GridField - * @param $id The ID of the DataObject to open + * @param $id The ID of the record to open */ public function getLostRecordRedirection(GridField $gridField, HTTPRequest $request, ?int $id = null): ?string { @@ -216,7 +221,7 @@ public function getLostRecordRedirection(GridField $gridField, HTTPRequest $requ * Build a request handler for the given record * * @param GridField $gridField - * @param DataObject $record + * @param ViewableData $record * @param RequestHandler $requestHandler * @return GridFieldDetailForm_ItemRequest */ @@ -248,7 +253,7 @@ public function setTemplate($template) } /** - * @return String + * @return string */ public function getTemplate() { @@ -266,7 +271,7 @@ public function setName($name) } /** - * @return String + * @return string */ public function getName() { @@ -276,8 +281,8 @@ public function getName() /** * Enable redirection to missing records. * - * If a GridField shows a filtered list, and the DataObject is not in the list but exists in the - * database, and the DataObject has a CMSEditLink method, then the system will redirect to the + * If a GridField shows a filtered list, and the record is not in the list but exists in the + * database, and the record has a CMSEditLink method, then the system will redirect to the * URL returned by that method. */ public function setRedirectMissingRecords(bool $redirectMissingRecords): self diff --git a/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php b/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php index 8804c69eea5..252b270bd60 100644 --- a/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php +++ b/src/Forms/GridField/GridFieldDetailForm_ItemRequest.php @@ -9,6 +9,7 @@ use SilverStripe\Control\HTTPResponse; use SilverStripe\Control\RequestHandler; use SilverStripe\Core\Convert; +use SilverStripe\Core\ClassInfo; use SilverStripe\Forms\CompositeField; use SilverStripe\Forms\FieldList; use SilverStripe\Forms\Form; @@ -17,6 +18,7 @@ use SilverStripe\Forms\LiteralField; use SilverStripe\ORM\ArrayList; use SilverStripe\ORM\DataObject; +use SilverStripe\ORM\DataObjectInterface; use SilverStripe\ORM\FieldType\DBHTMLText; use SilverStripe\ORM\HasManyList; use SilverStripe\ORM\ManyManyList; @@ -28,6 +30,7 @@ use SilverStripe\View\ArrayData; use SilverStripe\View\HTML; use SilverStripe\View\SSViewer; +use SilverStripe\View\ViewableData; class GridFieldDetailForm_ItemRequest extends RequestHandler { @@ -64,7 +67,7 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler protected $component; /** - * @var DataObject + * @var ViewableData */ protected $record; @@ -96,7 +99,7 @@ class GridFieldDetailForm_ItemRequest extends RequestHandler * * @param GridField $gridField * @param GridFieldDetailForm $component - * @param DataObject $record + * @param ViewableData&DataObjectInterface $record * @param RequestHandler $requestHandler * @param string $popupFormName */ @@ -125,11 +128,12 @@ public function Link($action = null) */ public function view($request) { - if (!$this->record->canView()) { + // Assume item can be viewed if canView() isn't implemented + if ($this->record->hasMethod('canView') && !$this->record->canView()) { $this->httpError(403, _t( __CLASS__ . '.ViewPermissionsFailure', 'It seems you don\'t have the necessary permissions to view "{ObjectTitle}"', - ['ObjectTitle' => $this->record->singular_name()] + ['ObjectTitle' => $this->getModelName()] )); } @@ -215,17 +219,25 @@ public function ItemEditForm() } } - if (!$this->record->canView()) { + // Assume item can be viewed if canView() isn't implemented + if ($this->record->hasMethod('canView') && !$this->record->canView()) { $controller = $this->getToplevelController(); return $controller->httpError(403, _t( __CLASS__ . '.ViewPermissionsFailure', 'It seems you don\'t have the necessary permissions to view "{ObjectTitle}"', - ['ObjectTitle' => $this->record->singular_name()] + ['ObjectTitle' => $this->getModelName()] )); } $fields = $this->component->getFields(); if (!$fields) { + if (!$this->record->hasMethod('getCMSFields')) { + $modelClass = get_class($this->record); + throw new LogicException( + 'Cannot dynamically determine form fields. Pass the fields to GridFieldDetailForm::setFields()' + . " or implement a getCMSFields() method on {$modelClass}" + ); + } $fields = $this->record->getCMSFields(); } @@ -249,15 +261,15 @@ public function ItemEditForm() $form->loadDataFrom($this->record, $this->record->ID == 0 ? Form::MERGE_IGNORE_FALSEISH : Form::MERGE_DEFAULT); - if ($this->record->ID && !$this->record->canEdit()) { + if ($this->record->ID && (!$this->record->hasMethod('canEdit') || !$this->record->canEdit())) { // Restrict editing of existing records $form->makeReadonly(); // Hack to re-enable delete button if user can delete - if ($this->record->canDelete()) { + if ($this->record->hasMethod('canDelete') && $this->record->canDelete()) { $form->Actions()->fieldByName('action_doDelete')->setReadonly(false); } } elseif (!$this->record->ID - && !$this->record->canCreate(null, $this->getCreateContext()) + && (!$this->record->hasMethod('canCreate') || !$this->record->canCreate(null, $this->getCreateContext())) ) { // Restrict creation of new records $form->makeReadonly(); @@ -367,7 +379,7 @@ protected function getRightGroupField() $rightGroup->push($previousAndNextGroup); - if ($component && $component->getShowAdd() && $this->record->canCreate()) { + if ($component && $component->getShowAdd() && $this->record->hasMethod('canCreate') && $this->record->canCreate()) { $rightGroup->push( LiteralField::create( 'new-record', @@ -386,7 +398,7 @@ protected function getRightGroupField() } /** - * Build the set of form field actions for this DataObject + * Build the set of form field actions for the record being handled * * @return FieldList */ @@ -399,8 +411,12 @@ protected function getFormActions() $majorActions->setFieldHolderTemplate(get_class($majorActions) . '_holder_buttongroup'); $actions->push($majorActions); - if ($this->record->ID !== 0) { // existing record - if ($this->record->canEdit()) { + if ($this->record->ID !== null && $this->record->ID !== 0) { // existing record + if ($this->record->hasMethod('canEdit') && $this->record->canEdit()) { + if (!($this->record instanceof DataObjectInterface)) { + throw new LogicException(get_class($this->record) . ' must implement ' . DataObjectInterface::class); + } + $noChangesClasses = 'btn-outline-primary font-icon-tick'; $majorActions->push(FormAction::create('doSave', _t('SilverStripe\\Forms\\GridField\\GridFieldDetailForm.Save', 'Save')) ->addExtraClass($noChangesClasses) @@ -410,7 +426,10 @@ protected function getFormActions() ->setAttribute('data-text-alternate', _t('SilverStripe\\CMS\\Controllers\\CMSMain.SAVEDRAFT', 'Save'))); } - if ($this->record->canDelete()) { + if ($this->record->hasMethod('canDelete') && $this->record->canDelete()) { + if (!($this->record instanceof DataObjectInterface)) { + throw new LogicException(get_class($this->record) . ' must implement ' . DataObjectInterface::class); + } $actions->insertAfter('MajorActions', FormAction::create('doDelete', _t('SilverStripe\\Forms\\GridField\\GridFieldDetailForm.Delete', 'Delete')) ->setUseButtonTag(true) ->addExtraClass('btn-outline-danger btn-hide-outline font-icon-trash-bin action--delete')); @@ -490,9 +509,9 @@ protected function getBackLink() * {@see Form::saveInto()} * * Handles detection of falsey values explicitly saved into the - * DataObject by formfields + * record by formfields * - * @param DataObject $record + * @param ViewableData $record * @param SS_List $list * @return array List of data to write to the relation */ @@ -518,11 +537,11 @@ public function doSave($data, $form) $isNewRecord = $this->record->ID == 0; // Check permission - if (!$this->record->canEdit()) { + if (!$this->record->hasMethod('canEdit') || !$this->record->canEdit()) { $this->httpError(403, _t( __CLASS__ . '.EditPermissionsFailure', 'It seems you don\'t have the necessary permissions to edit "{ObjectTitle}"', - ['ObjectTitle' => $this->record->singular_name()] + ['ObjectTitle' => $this->getModelName()] )); return null; } @@ -537,7 +556,7 @@ public function doSave($data, $form) 'SilverStripe\\Forms\\GridField\\GridFieldDetailForm.Saved', 'Saved {name} {link}', [ - 'name' => $this->record->i18n_singular_name(), + 'name' => $this->getModelName(), 'link' => $link ] ); @@ -744,12 +763,12 @@ public function httpError($errorCode, $errorMessage = null) } /** - * Loads the given form data into the underlying dataobject and relation + * Loads the given form data into the underlying record and relation * * @param array $data * @param Form $form * @throws ValidationException On error - * @return DataObject Saved record + * @return ViewableData&DataObjectInterface Saved record */ protected function saveFormIntoRecord($data, $form) { @@ -766,7 +785,7 @@ protected function saveFormIntoRecord($data, $form) $this->record = $this->record->newClassInstance($newClassName); } - // Save form and any extra saved data into this dataobject. + // Save form and any extra saved data into this record. // Set writeComponents = true to write has-one relations / join records $form->saveInto($this->record); // https://github.com/silverstripe/silverstripe-assets/issues/365 @@ -788,7 +807,7 @@ protected function saveFormIntoRecord($data, $form) public function doDelete($data, $form) { $title = $this->record->Title; - if (!$this->record->canDelete()) { + if (!$this->record->hasMethod('canDelete') || !$this->record->canDelete()) { throw new ValidationException( _t('SilverStripe\\Forms\\GridField\\GridFieldDetailForm.DeletePermissionsFailure', "No delete permissions") ); @@ -799,7 +818,7 @@ public function doDelete($data, $form) 'SilverStripe\\Forms\\GridField\\GridFieldDetailForm.Deleted', 'Deleted {type} "{name}"', [ - 'type' => $this->record->i18n_singular_name(), + 'type' => $this->getModelName(), 'name' => htmlspecialchars($title ?? '', ENT_QUOTES) ] ); @@ -870,7 +889,7 @@ public function getGridField() } /** - * @return DataObject + * @return ViewableData */ public function getRecord() { @@ -906,7 +925,7 @@ public function Breadcrumbs($unlinked = false) ])); } else { $items->push(ArrayData::create([ - 'Title' => _t('SilverStripe\\Forms\\GridField\\GridField.NewRecord', 'New {type}', ['type' => $this->record->i18n_singular_name()]), + 'Title' => _t('SilverStripe\\Forms\\GridField\\GridField.NewRecord', 'New {type}', ['type' => $this->getModelName()]), 'Link' => false ])); } @@ -920,4 +939,12 @@ public function Breadcrumbs($unlinked = false) $this->extend('updateBreadcrumbs', $items); return $items; } + + private function getModelName(): string + { + if ($this->record->hasMethod('i18n_singular_name')) { + return $this->record->i18n_singular_name(); + } + return ClassInfo::shortName($this->record); + } } diff --git a/src/Forms/GridField/GridFieldEditButton.php b/src/Forms/GridField/GridFieldEditButton.php index 5c52c45d80c..e3119fa6242 100644 --- a/src/Forms/GridField/GridFieldEditButton.php +++ b/src/Forms/GridField/GridFieldEditButton.php @@ -3,9 +3,9 @@ namespace SilverStripe\Forms\GridField; use SilverStripe\Control\Controller; -use SilverStripe\ORM\DataObject; use SilverStripe\View\ArrayData; use SilverStripe\View\SSViewer; +use SilverStripe\View\ViewableData; /** * Provides the entry point to editing a single record presented by the @@ -91,7 +91,7 @@ public function augmentColumns($gridField, &$columns) * Return any special attributes that will be used for FormField::create_tag() * * @param GridField $gridField - * @param DataObject $record + * @param ViewableData $record * @param string $columnName * @return array */ @@ -139,7 +139,7 @@ public function getActions($gridField) /** * @param GridField $gridField - * @param DataObject $record + * @param ViewableData $record * @param string $columnName * @return string The HTML for the column */ diff --git a/src/Forms/GridField/GridFieldExportButton.php b/src/Forms/GridField/GridFieldExportButton.php index d52d9c8219e..7a1b63e4826 100644 --- a/src/Forms/GridField/GridFieldExportButton.php +++ b/src/Forms/GridField/GridFieldExportButton.php @@ -3,12 +3,13 @@ namespace SilverStripe\Forms\GridField; use League\Csv\Writer; +use LogicException; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse; use SilverStripe\Core\Config\Config; use SilverStripe\ORM\DataList; -use SilverStripe\ORM\DataObject; use SilverStripe\ORM\ArrayList; +use SilverStripe\View\ViewableData; /** * Adds an "Export list" button to the bottom of a {@link GridField}. @@ -155,7 +156,15 @@ protected function getExportColumnsForGridField(GridField $gridField) return $dataCols->getDisplayFields($gridField); } - return DataObject::singleton($gridField->getModelClass())->summaryFields(); + $modelClass = $gridField->getModelClass(); + $singleton = singleton($modelClass); + if (!$singleton->hasMethod('summaryFields')) { + throw new LogicException( + 'Cannot dynamically determine columns. Add a GridFieldDataColumns component to your GridField' + . " or implement a summaryFields() method on $modelClass" + ); + } + return $singleton->summaryFields(); } /** @@ -225,8 +234,9 @@ public function generateExportFileData($gridField) // Remove limit as the list may be paginated, we want the full list for the export $items = $items->limit(null); - /** @var DataObject $item */ + /** @var ViewableData $item */ foreach ($items as $item) { + // Assume item can be viewed if canView() isn't implemented if (!$item->hasMethod('canView') || $item->canView()) { $columnData = []; diff --git a/src/Forms/GridField/GridFieldFilterHeader.php b/src/Forms/GridField/GridFieldFilterHeader.php index 187b9bd4fea..edeb9f69337 100755 --- a/src/Forms/GridField/GridFieldFilterHeader.php +++ b/src/Forms/GridField/GridFieldFilterHeader.php @@ -7,10 +7,12 @@ use SilverStripe\Control\Controller; use SilverStripe\Control\HTTPResponse; use SilverStripe\Core\ClassInfo; +use SilverStripe\Dev\Deprecation; use SilverStripe\Forms\FieldList; use SilverStripe\Forms\Form; use SilverStripe\Forms\Schema\FormSchema; use SilverStripe\ORM\Filterable; +use SilverStripe\ORM\Search\SearchContext; use SilverStripe\ORM\SS_List; use SilverStripe\View\ArrayData; use SilverStripe\View\SSViewer; @@ -27,11 +29,12 @@ class GridFieldFilterHeader extends AbstractGridFieldComponent implements GridFi * See {@link setThrowExceptionOnBadDataType()} * * @var bool + * @deprecated 5.2.0 Will be removed without equivalent functionality */ protected $throwExceptionOnBadDataType = true; /** - * @var \SilverStripe\ORM\Search\SearchContext + * @var SearchContext */ protected $searchContext = null; @@ -66,17 +69,21 @@ public function getURLHandlers($gridField) * {@link GridFieldConfig} subclasses set this to false for flexibility. * * @param bool $throwExceptionOnBadDataType + * @deprecated 5.2.0 Will be removed without equivalent functionality */ public function setThrowExceptionOnBadDataType($throwExceptionOnBadDataType) { + Deprecation::notice('5.2.0', 'Will be removed without equivalent functionality'); $this->throwExceptionOnBadDataType = $throwExceptionOnBadDataType; } /** * See {@link setThrowExceptionOnBadDataType()} + * @deprecated 5.2.0 Will be removed without equivalent functionality */ public function getThrowExceptionOnBadDataType() { + Deprecation::notice('5.2.0', 'Will be removed without equivalent functionality'); return $this->throwExceptionOnBadDataType; } @@ -103,6 +110,7 @@ protected function checkDataType($dataList) if ($dataList instanceof Filterable) { return true; } else { + // This will be changed to always throw an exception in a future major release. if ($this->throwExceptionOnBadDataType) { throw new LogicException( static::class . " expects an SS_Filterable list to be passed to the GridField." @@ -243,17 +251,35 @@ public function canFilterAnyColumns($gridField) * Generate a search context based on the model class of the of the GridField * * @param GridField $gridfield - * @return \SilverStripe\ORM\Search\SearchContext + * @return SearchContext */ public function getSearchContext(GridField $gridField) { if (!$this->searchContext) { - $this->searchContext = singleton($gridField->getModelClass())->getDefaultSearchContext(); + $modelClass = $gridField->getModelClass(); + $singleton = singleton($modelClass); + if (!$singleton->hasMethod('getDefaultSearchContext')) { + throw new LogicException( + 'Cannot dynamically instantiate SearchContext. Pass the SearchContext to setSearchContext()' + . " or implement a getDefaultSearchContext() method on $modelClass" + ); + } + $this->searchContext = $singleton->getDefaultSearchContext(); } return $this->searchContext; } + /** + * Sets a specific SearchContext instance for this component to use, instead of the default + * context provided by the ModelClass. + */ + public function setSearchContext(SearchContext $context): static + { + $this->searchContext = $context; + return $this; + } + /** * Returns the search field schema for the component * @@ -280,8 +306,6 @@ public function getSearchFieldSchema(GridField $gridField) $searchField = $searchField && property_exists($searchField, 'name') ? $searchField->name : null; } - $name = $gridField->Title ?: $inst->i18n_plural_name(); - // Prefix "Search__" onto the filters for the React component $filters = $context->getSearchParams(); if (!empty($filters)) { @@ -295,7 +319,7 @@ public function getSearchFieldSchema(GridField $gridField) $schema = [ 'formSchemaUrl' => $schemaUrl, 'name' => $searchField, - 'placeholder' => _t(__CLASS__ . '.Search', 'Search "{name}"', ['name' => $name]), + 'placeholder' => _t(__CLASS__ . '.Search', 'Search "{name}"', ['name' => $this->getTitle($gridField, $inst)]), 'filters' => $filters ?: new \stdClass, // stdClass maps to empty json object '{}' 'gridfield' => $gridField->getName(), 'searchAction' => $searchAction->getAttribute('name'), @@ -307,6 +331,19 @@ public function getSearchFieldSchema(GridField $gridField) return json_encode($schema); } + private function getTitle(GridField $gridField, object $inst): string + { + if ($gridField->Title) { + return $gridField->Title; + } + + if (ClassInfo::hasMethod($inst, 'i18n_plural_name')) { + return $inst->i18n_plural_name(); + } + + return ClassInfo::shortName($inst); + } + /** * Returns the search form for the component * @@ -350,7 +387,7 @@ public function getSearchForm(GridField $gridField) $field->addExtraClass('stacked no-change-track'); } - $name = $gridField->Title ?: singleton($gridField->getModelClass())->i18n_plural_name(); + $name = $this->getTitle($gridField, singleton($gridField->getModelClass())); $this->searchForm = $form = new Form( $gridField, diff --git a/src/Forms/GridField/GridFieldLevelup.php b/src/Forms/GridField/GridFieldLevelup.php index 8986d323c3c..32d22d30d61 100644 --- a/src/Forms/GridField/GridFieldLevelup.php +++ b/src/Forms/GridField/GridFieldLevelup.php @@ -2,6 +2,7 @@ namespace SilverStripe\Forms\GridField; +use LogicException; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\FieldType\DBField; use SilverStripe\ORM\Hierarchy\Hierarchy; @@ -13,6 +14,8 @@ * Adds a "level up" link to a GridField table, which is useful when viewing * hierarchical data. Requires the managed record to have a "getParent()" * method or has_one relationship called "Parent". + * + * The modelClass of the GridField this component is in must be a DataObject subclass. */ class GridFieldLevelup extends AbstractGridFieldComponent implements GridField_HTMLProvider { @@ -53,6 +56,10 @@ public function getHTMLFragments($gridField) $modelClass = $gridField->getModelClass(); $parentID = 0; + if (!is_a($modelClass, DataObject::class, true)) { + throw new LogicException(__CLASS__ . " must be used with DataObject subclasses. Found '$modelClass'"); + } + if (!$this->currentID) { return null; } diff --git a/src/Forms/GridField/GridFieldPaginator.php b/src/Forms/GridField/GridFieldPaginator.php index 970c1da2b78..5e2c4f01a2c 100755 --- a/src/Forms/GridField/GridFieldPaginator.php +++ b/src/Forms/GridField/GridFieldPaginator.php @@ -9,6 +9,7 @@ use SilverStripe\View\ArrayData; use SilverStripe\View\SSViewer; use LogicException; +use SilverStripe\Dev\Deprecation; /** * GridFieldPaginator paginates the {@link GridField} list and adds controls @@ -33,6 +34,7 @@ class GridFieldPaginator extends AbstractGridFieldComponent implements GridField /** * See {@link setThrowExceptionOnBadDataType()} + * @deprecated 5.2.0 Will be removed without equivalent functionality */ protected $throwExceptionOnBadDataType = true; @@ -57,9 +59,11 @@ public function __construct($itemsPerPage = null) * * @param bool $throwExceptionOnBadDataType * @return $this + * @deprecated 5.2.0 Will be removed without equivalent functionality */ public function setThrowExceptionOnBadDataType($throwExceptionOnBadDataType) { + Deprecation::notice('5.2.0', 'Will be removed without equivalent functionality'); $this->throwExceptionOnBadDataType = $throwExceptionOnBadDataType; return $this; } @@ -68,9 +72,11 @@ public function setThrowExceptionOnBadDataType($throwExceptionOnBadDataType) * See {@link setThrowExceptionOnBadDataType()} * * @return bool + * @deprecated 5.2.0 Will be removed without equivalent functionality */ public function getThrowExceptionOnBadDataType() { + Deprecation::notice('5.2.0', 'Will be removed without equivalent functionality'); return $this->throwExceptionOnBadDataType; } @@ -86,6 +92,7 @@ protected function checkDataType($dataList) if ($dataList instanceof Limitable) { return true; } else { + // This will be changed to always throw an exception in a future major release. if ($this->throwExceptionOnBadDataType) { throw new LogicException( static::class . " expects an SS_Limitable list to be passed to the GridField." diff --git a/src/Forms/GridField/GridFieldPrintButton.php b/src/Forms/GridField/GridFieldPrintButton.php index 1fc965808e1..0cf56bf6b12 100644 --- a/src/Forms/GridField/GridFieldPrintButton.php +++ b/src/Forms/GridField/GridFieldPrintButton.php @@ -2,16 +2,17 @@ namespace SilverStripe\Forms\GridField; +use LogicException; use SilverStripe\Control\HTTPRequest; use SilverStripe\Core\Convert; use SilverStripe\Core\Extensible; use SilverStripe\ORM\ArrayList; -use SilverStripe\ORM\DataObject; use SilverStripe\ORM\FieldType\DBDatetime; use SilverStripe\ORM\FieldType\DBHTMLText; use SilverStripe\Security\Security; use SilverStripe\View\ArrayData; use SilverStripe\View\Requirements; +use SilverStripe\View\ViewableData; /** * Adds an "Print" button to the bottom or top of a GridField. @@ -161,7 +162,15 @@ protected function getPrintColumnsForGridField(GridField $gridField) return $dataCols->getDisplayFields($gridField); } - return DataObject::singleton($gridField->getModelClass())->summaryFields(); + $modelClass = $gridField->getModelClass(); + $singleton = singleton($modelClass); + if (!$singleton->hasMethod('summaryFields')) { + throw new LogicException( + 'Cannot dynamically determine columns. Add a GridFieldDataColumns component to your GridField' + . " or implement a summaryFields() method on $modelClass" + ); + } + return $singleton->summaryFields(); } /** @@ -226,8 +235,9 @@ public function generatePrintData(GridField $gridField) /** @var GridFieldDataColumns $gridFieldColumnsComponent */ $gridFieldColumnsComponent = $gridField->getConfig()->getComponentByType(GridFieldDataColumns::class); - /** @var DataObject $item */ + /** @var ViewableData $item */ foreach ($items->limit(null) as $item) { + // Assume item can be viewed if canView() isn't implemented if (!$item->hasMethod('canView') || $item->canView()) { $itemRow = new ArrayList(); diff --git a/src/Forms/GridField/GridFieldSortableHeader.php b/src/Forms/GridField/GridFieldSortableHeader.php index e43f1aa9e98..1320bbc4c94 100644 --- a/src/Forms/GridField/GridFieldSortableHeader.php +++ b/src/Forms/GridField/GridFieldSortableHeader.php @@ -11,7 +11,9 @@ use SilverStripe\View\ArrayData; use SilverStripe\View\SSViewer; use LogicException; +use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Injector\Injector; +use SilverStripe\Dev\Deprecation; /** * GridFieldSortableHeader adds column headers to a {@link GridField} that can @@ -26,6 +28,7 @@ class GridFieldSortableHeader extends AbstractGridFieldComponent implements Grid * See {@link setThrowExceptionOnBadDataType()} * * @var bool + * @deprecated 5.2.0 Will be removed without equivalent functionality */ protected $throwExceptionOnBadDataType = true; @@ -45,9 +48,11 @@ class GridFieldSortableHeader extends AbstractGridFieldComponent implements Grid * * @param bool $throwExceptionOnBadDataType * @return $this + * @deprecated 5.2.0 Will be removed without equivalent functionality */ public function setThrowExceptionOnBadDataType($throwExceptionOnBadDataType) { + Deprecation::notice('5.2.0', 'Will be removed without equivalent functionality'); $this->throwExceptionOnBadDataType = $throwExceptionOnBadDataType; return $this; } @@ -56,9 +61,11 @@ public function setThrowExceptionOnBadDataType($throwExceptionOnBadDataType) * See {@link setThrowExceptionOnBadDataType()} * * @return bool + * @deprecated 5.2.0 Will be removed without equivalent functionality */ public function getThrowExceptionOnBadDataType() { + Deprecation::notice('5.2.0', 'Will be removed without equivalent functionality'); return $this->throwExceptionOnBadDataType; } @@ -74,6 +81,7 @@ protected function checkDataType($dataList) if ($dataList instanceof Sortable) { return true; } else { + // This will be changed to always throw an exception in a future major release. if ($this->throwExceptionOnBadDataType) { throw new LogicException( static::class . " expects an SS_Sortable list to be passed to the GridField." @@ -147,7 +155,7 @@ public function getHTMLFragments($gridField) if ($tmpItem instanceof SS_List) { // It's impossible to sort on a HasManyList/ManyManyList break; - } elseif ($tmpItem && method_exists($tmpItem, 'hasMethod') && $tmpItem->hasMethod($methodName)) { + } elseif ($tmpItem && ClassInfo::hasMethod($tmpItem, $methodName)) { // The part is a relation name, so get the object/list from it $tmpItem = $tmpItem->$methodName(); } elseif ($tmpItem instanceof DataObject diff --git a/src/Forms/GridField/GridFieldViewButton.php b/src/Forms/GridField/GridFieldViewButton.php index ea4129eb223..a0efccd6a67 100644 --- a/src/Forms/GridField/GridFieldViewButton.php +++ b/src/Forms/GridField/GridFieldViewButton.php @@ -62,7 +62,8 @@ public function getColumnsHandled($field) public function getColumnContent($field, $record, $col) { - if (!$record->canView()) { + // Assume item can be viewed if canView() isn't implemented + if ($record->hasMethod('canView') && !$record->canView()) { return null; } $data = new ArrayData([ diff --git a/src/Forms/GridField/GridField_ActionMenuItem.php b/src/Forms/GridField/GridField_ActionMenuItem.php index 7f22cc15c28..d07b23b8867 100644 --- a/src/Forms/GridField/GridField_ActionMenuItem.php +++ b/src/Forms/GridField/GridField_ActionMenuItem.php @@ -2,7 +2,7 @@ namespace SilverStripe\Forms\GridField; -use SilverStripe\ORM\DataObject; +use SilverStripe\View\ViewableData; /** * GridField action menu item interface, this provides data so the action @@ -21,7 +21,7 @@ interface GridField_ActionMenuItem extends GridFieldComponent * @see {@link GridField_ActionMenu->getColumnContent()} * * @param GridField $gridField - * @param DataObject $record + * @param ViewableData $record * * @return string $title */ @@ -33,7 +33,7 @@ public function getTitle($gridField, $record, $columnName); * @see {@link GridField_ActionMenu->getColumnContent()} * * @param GridField $gridField - * @param DataObject $record + * @param ViewableData $record * * @return array $data */ @@ -46,7 +46,7 @@ public function getExtraData($gridField, $record, $columnName); * @see {@link GridField_ActionMenu->getColumnContent()} * * @param GridField $gridField - * @param DataObject $record + * @param ViewableData $record * * @return string|null $group */ diff --git a/src/Forms/GridField/GridField_ActionMenuLink.php b/src/Forms/GridField/GridField_ActionMenuLink.php index d8727c84067..77d0fa3f0f7 100644 --- a/src/Forms/GridField/GridField_ActionMenuLink.php +++ b/src/Forms/GridField/GridField_ActionMenuLink.php @@ -2,7 +2,7 @@ namespace SilverStripe\Forms\GridField; -use SilverStripe\ORM\DataObject; +use SilverStripe\View\ViewableData; /** * Allows GridField_ActionMenuItem to act as a link @@ -15,7 +15,7 @@ interface GridField_ActionMenuLink extends GridField_ActionMenuItem * @see {@link GridField_ActionMenu->getColumnContent()} * * @param GridField $gridField - * @param DataObject $record + * @param ViewableData $record * * @return string $url */ diff --git a/src/Forms/GridField/GridField_ColumnProvider.php b/src/Forms/GridField/GridField_ColumnProvider.php index 5863560f785..ee900ea400b 100644 --- a/src/Forms/GridField/GridField_ColumnProvider.php +++ b/src/Forms/GridField/GridField_ColumnProvider.php @@ -34,7 +34,7 @@ public function getColumnsHandled($gridField); * HTML for the column, content of the element. * * @param GridField $gridField - * @param DataObject $record - Record displayed in this row + * @param ViewableData $record - Record displayed in this row * @param string $columnName * @return string - HTML for the column. Return NULL to skip. */ @@ -44,7 +44,7 @@ public function getColumnContent($gridField, $record, $columnName); * Attributes for the element containing the content returned by {@link getColumnContent()}. * * @param GridField $gridField - * @param DataObject $record displayed in this row + * @param ViewableData $record displayed in this row * @param string $columnName * @return array */ diff --git a/src/Forms/GridField/GridField_SaveHandler.php b/src/Forms/GridField/GridField_SaveHandler.php index ece1a509159..f562426156e 100644 --- a/src/Forms/GridField/GridField_SaveHandler.php +++ b/src/Forms/GridField/GridField_SaveHandler.php @@ -3,6 +3,7 @@ namespace SilverStripe\Forms\GridField; use SilverStripe\ORM\DataObjectInterface; +use SilverStripe\View\ViewableData; /** * A component which is used to handle when a {@link GridField} is saved into @@ -15,7 +16,7 @@ interface GridField_SaveHandler extends GridFieldComponent * Called when a grid field is saved - i.e. the form is submitted. * * @param GridField $grid - * @param DataObjectInterface $record + * @param DataObjectInterface&ViewableData $record */ public function handleSave(GridField $grid, DataObjectInterface $record); } diff --git a/src/ORM/Search/BasicSearchContext.php b/src/ORM/Search/BasicSearchContext.php new file mode 100644 index 00000000000..9bdd960a187 --- /dev/null +++ b/src/ORM/Search/BasicSearchContext.php @@ -0,0 +1,188 @@ += 3) && (!in_array(gettype($limit), ['array', 'NULL', 'string']))) { + Deprecation::notice( + '5.1.0', + '$limit should be type of array|string|null' + ); + $limit = null; + } + + $searchParams = $this->applySearchFilters($this->normaliseSearchParams($searchParams)); + $result = $this->applyGeneralSearchField($searchParams, $existingQuery); + + // Filter the list by the requested filters. + if (!empty($searchParams)) { + $result = $result->filter($searchParams); + } + + // Only sort if a sort value is provided - sort by "false" just means use the existing sort. + if ($sort) { + $result = $result->sort($sort); + } + + // Limit must be last so that ArrayList results don't have an applied limit before they can be filtered/sorted. + $result = $result->limit($limit); + + return $result; + } + + private function normaliseSearchParams(array $searchParams): array + { + $normalised = []; + foreach ($searchParams as $field => $searchTerm) { + if ($this->clearEmptySearchFields($searchTerm)) { + $normalised[str_replace('__', '.', $field)] = $searchTerm; + } + } + return $normalised; + } + + private function applySearchFilters(array $searchParams): array + { + $applied = []; + foreach ($searchParams as $fieldName => $searchTerm) { + // Ignore the general search field - we'll deal with that in a special way. + if ($fieldName === static::config()->get('general_search_field_name')) { + $applied[$fieldName] = $searchTerm; + continue; + } + $filterTerm = $this->getFilterTerm($fieldName); + $applied["{$fieldName}:{$filterTerm}"] = $searchTerm; + } + return $applied; + } + + private function applyGeneralSearchField(array &$searchParams, Filterable $existingQuery): Filterable + { + $generalFieldName = static::config()->get('general_search_field_name'); + if (array_key_exists($generalFieldName, $searchParams)) { + $searchTerm = $searchParams[$generalFieldName]; + if (Config::inst()->get($this->modelClass, 'general_search_split_terms') !== false) { + $searchTerm = explode(' ', $searchTerm); + } + $generalFilter = []; + foreach ($this->getSearchFields()->dataFieldNames() as $fieldName) { + if ($fieldName === $generalFieldName) { + continue; + } + if (!$this->getCanGeneralSearch($fieldName)) { + continue; + } + $filterTerm = $this->getGeneralSearchFilterTerm($fieldName); + $generalFilter["{$fieldName}:{$filterTerm}"] = $searchTerm; + } + $result = $existingQuery->filterAny($generalFilter); + unset($searchParams[$generalFieldName]); + } + + return $result ?? $existingQuery; + } + + private function getCanGeneralSearch(string $fieldName): bool + { + $singleton = singleton($this->modelClass); + + // Allowed if we're dealing with arbitrary data. + if (!ClassInfo::hasMethod($singleton, 'searchableFields')) { + return true; + } + + $fields = $singleton->searchableFields(); + + // Not allowed if the field isn't searchable. + if (!isset($fields[$fieldName])) { + return false; + } + + // Allowed if 'general' isn't part of the spec, or is explicitly truthy. + return !isset($fields[$fieldName]['general']) || $fields[$fieldName]['general']; + } + + /** + * Get the search filter for the given fieldname when searched from the general search field. + */ + private function getGeneralSearchFilterTerm(string $fieldName): string + { + $filterClass = Config::inst()->get($this->modelClass, 'general_search_field_filter'); + if ($filterClass) { + return $this->getTermFromFilter(Injector::inst()->create($filterClass, $fieldName)); + } + + if ($filterClass === '') { + return $this->getFilterTerm($fieldName); + } + + return 'PartialMatch:nocase'; + } + + private function getFilterTerm(string $fieldName): string + { + $filter = $this->getFilter($fieldName) ?? PartialMatchFilter::create($fieldName); + return $this->getTermFromFilter($filter); + } + + private function getTermFromFilter(SearchFilter $filter): string + { + $modifiers = $filter->getModifiers() ?? []; + + // Get the string used to refer to the filter, e.g. "PartialMatch" + // Ask the injector for it first - but for any not defined there, fall back to string manipulation. + $filterTerm = Injector::inst()->getServiceName(get_class($filter)); + if (!$filterTerm) { + $filterTerm = preg_replace('/Filter$/', '', ClassInfo::shortName($filter)); + } + + // Add modifiers to filter + foreach ($modifiers as $modifier) { + $filterTerm .= ":{$modifier}"; + } + + return $filterTerm; + } +} diff --git a/src/ORM/Search/SearchContext.php b/src/ORM/Search/SearchContext.php index 70e97d9e3ae..24984236ca0 100644 --- a/src/ORM/Search/SearchContext.php +++ b/src/ORM/Search/SearchContext.php @@ -17,6 +17,7 @@ use SilverStripe\Forms\CheckboxField; use InvalidArgumentException; use Exception; +use LogicException; use SilverStripe\Core\Config\Config; use SilverStripe\Dev\Deprecation; use SilverStripe\ORM\DataQuery; @@ -104,7 +105,18 @@ public function __construct($modelClass, $fields = null, $filters = null) */ public function getSearchFields() { - return ($this->fields) ? $this->fields : singleton($this->modelClass)->scaffoldSearchFields(); + if ($this->fields?->exists()) { + return $this->fields; + } + + $singleton = singleton($this->modelClass); + if (!$singleton->hasMethod('scaffoldSearchFields')) { + throw new LogicException( + 'Cannot dynamically determine search fields. Pass the fields to setFields()' + . " or implement a scaffoldSearchFields() method on {$this->modelClass}" + ); + } + return $singleton->scaffoldSearchFields(); } protected function applyBaseTableFields() @@ -431,7 +443,7 @@ public function setFields($fields) */ public function addField($field) { - $this->fields->push($field); + $this->fields?->push($field); } /** @@ -441,7 +453,7 @@ public function addField($field) */ public function removeFieldByName($fieldName) { - $this->fields->removeByName($fieldName); + $this->fields?->removeByName($fieldName); } /** @@ -488,7 +500,7 @@ public function getSummary() continue; } - $field = $this->fields->fieldByName($filter->getFullName()); + $field = $this->fields?->fieldByName($filter->getFullName()); if (!$field) { continue; } diff --git a/tests/behat/src/CmsFormsContext.php b/tests/behat/src/CmsFormsContext.php index 3c471fcbfa1..46ddb5ca7a8 100644 --- a/tests/behat/src/CmsFormsContext.php +++ b/tests/behat/src/CmsFormsContext.php @@ -12,6 +12,7 @@ use SilverStripe\BehatExtension\Utility\StepHelper; use Symfony\Component\DomCrawler\Crawler; use Behat\Mink\Element\NodeElement; +use Facebook\WebDriver\WebDriverExpectedCondition; use SilverStripe\SiteConfig\SiteConfig; /** @@ -445,6 +446,23 @@ public function stepIClickTheGridFieldButtonForRow($buttonLabel, $gridFieldName, $button->click(); } + /** + * @When /^I click the "([^"]*)" button in the "([^"]*)" gridfield for the "([^"]*)" row, confirming the dialog$/ + * @param string $buttonLabel + * @param string $gridFieldName + * @param string $rowName + */ + public function stepIClickTheGridFieldButtonForRowConfirmingDialog($buttonLabel, $gridFieldName, $rowName) + { + $this->stepIClickTheGridFieldButtonForRow($buttonLabel, $gridFieldName, $rowName); + $session = $this->getSession()->getDriver()->getWebDriver(); + $session->wait()->until( + WebDriverExpectedCondition::alertIsPresent(), + "Alert is expected" + ); + $session->switchTo()->alert()->accept(); + } + /** * Finds a button in the gridfield row * diff --git a/tests/php/Forms/GridField/GridFieldAddExistingAutocompleterTest.php b/tests/php/Forms/GridField/GridFieldAddExistingAutocompleterTest.php index c8b16e2ee43..e58d30837a5 100644 --- a/tests/php/Forms/GridField/GridFieldAddExistingAutocompleterTest.php +++ b/tests/php/Forms/GridField/GridFieldAddExistingAutocompleterTest.php @@ -2,6 +2,7 @@ namespace SilverStripe\Forms\Tests\GridField; +use LogicException; use SilverStripe\Control\HTTPRequest; use SilverStripe\Core\Convert; use SilverStripe\Dev\CSSContentParser; @@ -17,6 +18,7 @@ use SilverStripe\Forms\Tests\GridField\GridFieldTest\Stadium; use SilverStripe\Forms\Tests\GridField\GridFieldTest\Team; use SilverStripe\ORM\ArrayList; +use SilverStripe\View\ArrayData; class GridFieldAddExistingAutocompleterTest extends FunctionalTest { @@ -167,6 +169,118 @@ function ($item) { ); } + public function testGetHTMLFragmentsNeedsDataObject() + { + $component = new GridFieldAddExistingAutocompleter(); + $gridField = $this->getGridFieldForComponent($component); + $list = new ArrayList(); + $dataClass = ArrayData::class; + $list->setDataClass($dataClass); + $gridField->setList($list); + + $this->expectException(LogicException::class); + $this->expectExceptionMessage( + GridFieldAddExistingAutocompleter::class + . " must be used with DataObject subclasses. Found '$dataClass'" + ); + // Calling the method will throw an exception. + $component->getHTMLFragments($gridField); + } + + public function testGetManipulatedDataNeedsDataObject() + { + $component = new GridFieldAddExistingAutocompleter(); + $gridField = $this->getGridFieldForComponent($component); + $list = new ArrayList(); + $dataClass = ArrayData::class; + $list->setDataClass($dataClass); + $gridField->setList($list); + + $this->expectException(LogicException::class); + $this->expectExceptionMessage( + GridFieldAddExistingAutocompleter::class + . " must be used with DataObject subclasses. Found '$dataClass'" + ); + + // Calling the method will throw an exception. + $component->getManipulatedData($gridField, $list); + } + + public function testDoSearchNeedsDataObject() + { + $component = new GridFieldAddExistingAutocompleter(); + $gridField = $this->getGridFieldForComponent($component); + $list = new ArrayList(); + $dataClass = ArrayData::class; + $list->setDataClass($dataClass); + $gridField->setList($list); + + $this->expectException(LogicException::class); + $this->expectExceptionMessage( + GridFieldAddExistingAutocompleter::class + . " must be used with DataObject subclasses. Found '$dataClass'" + ); + + // Calling the method will throw an exception. + $component->doSearch($gridField, new HTTPRequest('GET', '')); + } + + public function testScaffoldSearchFieldsNeedsDataObject() + { + $component = new GridFieldAddExistingAutocompleter(); + $gridField = $this->getGridFieldForComponent($component); + $list = new ArrayList(); + $dataClass = ArrayData::class; + $list->setDataClass($dataClass); + $gridField->setList($list); + + $this->expectException(LogicException::class); + $this->expectExceptionMessage( + GridFieldAddExistingAutocompleter::class + . " must be used with DataObject subclasses. Found '$dataClass'" + ); + + // Calling the method will either throw an exception or not. + // The test pass/failure is explicitly about whether an exception is thrown. + $component->scaffoldSearchFields($dataClass); + } + + public function testGetPlaceholderTextNeedsDataObject() + { + $component = new GridFieldAddExistingAutocompleter(); + $gridField = $this->getGridFieldForComponent($component); + $list = new ArrayList(); + $dataClass = ArrayData::class; + $list->setDataClass($dataClass); + $gridField->setList($list); + + $this->expectException(LogicException::class); + $this->expectExceptionMessage( + GridFieldAddExistingAutocompleter::class + . " must be used with DataObject subclasses. Found '$dataClass'" + ); + + // Calling the method will either throw an exception or not. + // The test pass/failure is explicitly about whether an exception is thrown. + $component->getPlaceholderText($dataClass); + } + + public function testSetPlaceholderTextDoesntNeedDataObject() + { + $component = new GridFieldAddExistingAutocompleter(); + $gridField = $this->getGridFieldForComponent($component); + $list = new ArrayList(); + $dataClass = ArrayData::class; + $list->setDataClass($dataClass); + $gridField->setList($list); + + // Prevent from being marked risky. + // This test passes if there's no exception thrown. + $this->expectNotToPerformAssertions(); + + $component->setPlaceholderText(''); + } + protected function getGridFieldForComponent($component) { $config = GridFieldConfig::create()->addComponents( diff --git a/tests/php/Forms/GridField/GridFieldAddNewButtonTest.php b/tests/php/Forms/GridField/GridFieldAddNewButtonTest.php index c0dd6a40c61..900c490e27e 100644 --- a/tests/php/Forms/GridField/GridFieldAddNewButtonTest.php +++ b/tests/php/Forms/GridField/GridFieldAddNewButtonTest.php @@ -2,6 +2,7 @@ namespace SilverStripe\Forms\Tests\GridField; +use LogicException; use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\SapphireTest; use SilverStripe\Forms\FieldList; @@ -9,11 +10,14 @@ use SilverStripe\Forms\GridField\GridField; use SilverStripe\Forms\GridField\GridFieldAddNewButton; use SilverStripe\Forms\GridField\GridFieldConfig; +use SilverStripe\Forms\GridField\GridFieldConfig_Base; use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\Person; use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\PeopleGroup; use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\Category; use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\TestController; +use SilverStripe\ORM\ArrayList; use SilverStripe\ORM\SS_List; +use SilverStripe\View\ArrayData; class GridFieldAddNewButtonTest extends SapphireTest { @@ -76,6 +80,24 @@ public function testButtonPassesNoParentContextToSingletonWhenNoParentRecordExis $this->mockButtonFragments($list, null); } + public function testGetHTMLFragmentsThrowsException() + { + $component = new GridFieldAddNewButton(); + $config = new GridFieldConfig_Base(); + $config->addComponent($component); + $gridField = new GridField('dummy', 'dummy', new ArrayList(), $config); + $modelClass = ArrayData::class; + $gridField->setModelClass($modelClass); + + $this->expectException(LogicException::class); + $this->expectExceptionMessage( + GridFieldAddNewButton::class . ' cannot be used with models that do not implement canCreate().' + . " Remove this component from your GridField or implement canCreate() on $modelClass" + ); + + $component->getHTMLFragments($gridField); + } + protected function mockButtonFragments(SS_List $list, $parent = null) { $form = Form::create( diff --git a/tests/php/Forms/GridField/GridFieldDataColumnsTest.php b/tests/php/Forms/GridField/GridFieldDataColumnsTest.php index 5588dcf3585..b9e0c039796 100644 --- a/tests/php/Forms/GridField/GridFieldDataColumnsTest.php +++ b/tests/php/Forms/GridField/GridFieldDataColumnsTest.php @@ -3,15 +3,18 @@ namespace SilverStripe\Forms\Tests\GridField; use InvalidArgumentException; +use LogicException; use SilverStripe\Forms\GridField\GridFieldDataColumns; use SilverStripe\Security\Member; use SilverStripe\Dev\SapphireTest; use SilverStripe\Forms\GridField\GridField; +use SilverStripe\Forms\GridField\GridFieldConfig_Base; +use SilverStripe\ORM\ArrayList; +use SilverStripe\View\ArrayData; use stdClass; class GridFieldDataColumnsTest extends SapphireTest { - /** * @covers \SilverStripe\Forms\GridField\GridFieldDataColumns::getDisplayFields */ @@ -23,6 +26,19 @@ public function testGridFieldGetDefaultDisplayFields() $this->assertEquals($expected, $columns->getDisplayFields($obj)); } + /** + * @covers \SilverStripe\Forms\GridField\GridFieldDataColumns::getDisplayFields + */ + public function testGridFieldGetDisplayFieldsWithArrayList() + { + $list = new ArrayList([new ArrayData(['Title' => 'My Item'])]); + $obj = new GridField('testfield', 'testfield', $list); + $expected = ['Title' => 'Title']; + $columns = $obj->getConfig()->getComponentByType(GridFieldDataColumns::class); + $columns->setDisplayFields($expected); + $this->assertEquals($expected, $columns->getDisplayFields($obj)); + } + /** * @covers \SilverStripe\Forms\GridField\GridFieldDataColumns::setDisplayFields * @covers \SilverStripe\Forms\GridField\GridFieldDataColumns::getDisplayFields @@ -76,4 +92,22 @@ public function testFieldFormatting() $columns->getFieldFormatting() ); } + + public function testGetDisplayFieldsThrowsException() + { + $component = new GridFieldDataColumns(); + $config = new GridFieldConfig_Base(); + $config->addComponent($component); + $gridField = new GridField('dummy', 'dummy', new ArrayList(), $config); + $modelClass = ArrayData::class; + $gridField->setModelClass($modelClass); + + $this->expectException(LogicException::class); + $this->expectExceptionMessage( + 'Cannot dynamically determine columns. Pass the column names to setDisplayFields()' + . " or implement a summaryFields() method on $modelClass" + ); + + $component->getDisplayFields($gridField); + } } diff --git a/tests/php/Forms/GridField/GridFieldDeleteActionTest.php b/tests/php/Forms/GridField/GridFieldDeleteActionTest.php index 32538092ff5..8786d20bdad 100644 --- a/tests/php/Forms/GridField/GridFieldDeleteActionTest.php +++ b/tests/php/Forms/GridField/GridFieldDeleteActionTest.php @@ -2,6 +2,8 @@ namespace SilverStripe\Forms\Tests\GridField; +use LogicException; +use ReflectionMethod; use SilverStripe\Control\Controller; use SilverStripe\Control\HTTPRequest; use SilverStripe\Control\HTTPResponse_Exception; @@ -12,6 +14,7 @@ use SilverStripe\Forms\Form; use SilverStripe\Forms\GridField\GridField; use SilverStripe\Forms\GridField\GridFieldConfig; +use SilverStripe\Forms\GridField\GridFieldConfig_Base; use SilverStripe\Forms\GridField\GridFieldDeleteAction; use SilverStripe\Forms\Tests\GridField\GridFieldTest\Cheerleader; use SilverStripe\Forms\Tests\GridField\GridFieldTest\Permissions; @@ -22,6 +25,7 @@ use SilverStripe\ORM\ValidationException; use SilverStripe\Security\Security; use SilverStripe\Security\SecurityToken; +use SilverStripe\View\ArrayData; class GridFieldDeleteActionTest extends SapphireTest { @@ -230,4 +234,70 @@ public function testMenuGroup() $group = $action->getGroup($gridField, $this->list->first(), 'dummy'); $this->assertNull($group, 'A menu group does not exist when the user cannot delete'); } + + public function provideHandleActionThrowsException() + { + return [ + 'unlinks relation' => [true], + 'deletes related record' => [false], + ]; + } + + /** + * @dataProvider provideHandleActionThrowsException + */ + public function testHandleActionThrowsException(bool $unlinkRelation) + { + $component = new GridFieldDeleteAction(); + $config = new GridFieldConfig_Base(); + $config->addComponent($component); + $gridField = new GridField('dummy', 'dummy', new ArrayList([new ArrayData(['ID' => 1])]), $config); + $modelClass = ArrayData::class; + $gridField->setModelClass($modelClass); + + $this->expectException(LogicException::class); + $permissionMethod = $unlinkRelation ? 'canEdit' : 'canDelete'; + $this->expectExceptionMessage( + GridFieldDeleteAction::class . " cannot be used with models that don't implement {$permissionMethod}()." + . " Remove this component from your GridField or implement {$permissionMethod}() on $modelClass" + ); + + // Calling the method will throw an exception. + $secondArg = $unlinkRelation ? 'unlinkrelation' : 'deleterecord'; + $component->handleAction($gridField, $secondArg, ['RecordID' => 1], []); + } + + public function provideGetRemoveActionThrowsException() + { + return [ + 'removes relation' => [true], + 'deletes related record' => [false], + ]; + } + + /** + * @dataProvider provideGetRemoveActionThrowsException + */ + public function testGetRemoveActionThrowsException(bool $removeRelation) + { + $component = new GridFieldDeleteAction(); + $component->setRemoveRelation($removeRelation); + $config = new GridFieldConfig_Base(); + $config->addComponent($component); + $gridField = new GridField('dummy', 'dummy', new ArrayList([new ArrayData(['ID' => 1])]), $config); + $modelClass = ArrayData::class; + $gridField->setModelClass($modelClass); + + $this->expectException(LogicException::class); + $permissionMethod = $removeRelation ? 'canEdit' : 'canDelete'; + $this->expectExceptionMessage( + GridFieldDeleteAction::class . " cannot be used with models that don't implement {$permissionMethod}()." + . " Remove this component from your GridField or implement {$permissionMethod}() on $modelClass" + ); + + // Calling the method will throw an exception. + $reflectionMethod = new ReflectionMethod($component, 'getRemoveAction'); + $reflectionMethod->setAccessible(true); + $reflectionMethod->invokeArgs($component, [$gridField, new ArrayData(), '']); + } } diff --git a/tests/php/Forms/GridField/GridFieldDetailFormTest.php b/tests/php/Forms/GridField/GridFieldDetailFormTest.php index 894965403f0..34c1848c1d9 100644 --- a/tests/php/Forms/GridField/GridFieldDetailFormTest.php +++ b/tests/php/Forms/GridField/GridFieldDetailFormTest.php @@ -2,14 +2,20 @@ namespace SilverStripe\Forms\Tests\GridField; +use LogicException; +use ReflectionMethod; use SilverStripe\Control\Controller; +use SilverStripe\Control\HTTPRequest; use SilverStripe\Dev\CSSContentParser; use SilverStripe\Dev\FunctionalTest; +use SilverStripe\Forms\FieldList; use SilverStripe\Forms\Form; use SilverStripe\Forms\GridField\GridField; +use SilverStripe\Forms\GridField\GridFieldConfig_RecordEditor; use SilverStripe\Forms\GridField\GridFieldDetailForm; use SilverStripe\Forms\GridField\GridFieldDetailForm_ItemRequest; use SilverStripe\Forms\HiddenField; +use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\ArrayDataWithID; use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\Category; use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\CategoryController; use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\GroupController; @@ -18,6 +24,8 @@ use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\Person; use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\PolymorphicPeopleGroup; use SilverStripe\Forms\Tests\GridField\GridFieldDetailFormTest\TestController; +use SilverStripe\ORM\ArrayList; +use SilverStripe\View\ArrayData; class GridFieldDetailFormTest extends FunctionalTest { @@ -487,4 +495,121 @@ public function testRedirectMissingRecords() $this->autoFollowRedirection = $origAutoFollow; } + + public function provideGetRecordFromRequestFindExisting() + { + return [ + 'No records' => [ + 'data' => [], + 'hasRecord' => false, + ], + 'Records exist but without ID field' => [ + 'data' => [new ArrayDataWithID()], + 'hasRecord' => false, + ], + 'Record exists with matching ID' => [ + 'data' => [new ArrayDataWithID(['ID' => 32])], + 'hasRecord' => true, + ], + 'Record exists, no matching ID' => [ + 'data' => [new ArrayDataWithID(['ID' => 1])], + 'hasRecord' => false, + ], + ]; + } + + /** + * @dataProvider provideGetRecordFromRequestFindExisting + */ + public function testGetRecordFromRequestFindExisting(array $data, bool $hasRecord) + { + $controller = new TestController(); + $form = $controller->Form(null, new ArrayList($data)); + $gridField = $form->Fields()->dataFieldByName('testfield'); + if (empty($data)) { + $gridField->setModelClass(ArrayDataWithID::class); + } + $component = $gridField->getConfig()->getComponentByType(GridFieldDetailForm::class); + $request = new HTTPRequest('GET', $gridField->Link('item/32')); + $request->match(Controller::join_links($gridField->Link(), 'item/$ID')); + + $reflectionMethod = new ReflectionMethod($component, 'getRecordFromRequest'); + $reflectionMethod->setAccessible(true); + $this->assertSame($hasRecord, (bool) $reflectionMethod->invoke($component, $gridField, $request)); + } + + public function provideGetRecordFromRequestCreateNew() + { + // Note that in all of these scenarios a new record gets created, so it *shouldn't* matter what's already in there. + return [ + 'No records' => [ + 'data' => [], + ], + 'Records exist but without ID field' => [ + 'data' => [new ArrayDataWithID()], + ], + 'Record exists with ID field' => [ + 'data' => [new ArrayDataWithID(['ID' => 32])], + ], + ]; + } + + /** + * @dataProvider provideGetRecordFromRequestCreateNew + */ + public function testGetRecordFromRequestCreateNew(array $data) + { + $controller = new TestController(); + $form = $controller->Form(null, new ArrayList($data)); + $gridField = $form->Fields()->dataFieldByName('testfield'); + if (empty($data)) { + $gridField->setModelClass(ArrayDataWithID::class); + } + $component = $gridField->getConfig()->getComponentByType(GridFieldDetailForm::class); + $request = new HTTPRequest('GET', $gridField->Link('item/new')); + $request->match(Controller::join_links($gridField->Link(), 'item/$ID')); + + $reflectionMethod = new ReflectionMethod($component, 'getRecordFromRequest'); + $reflectionMethod->setAccessible(true); + $this->assertEquals(new ArrayDataWithID(['ID' => 0]), $reflectionMethod->invoke($component, $gridField, $request)); + } + + public function provideGetRecordFromRequestWithoutData() + { + // Note that in all of these scenarios a new record gets created, so it *shouldn't* matter what's already in there. + return [ + 'No records' => [ + 'data' => [], + ], + 'Records exist but without ID field' => [ + 'data' => [new ArrayData()], + ], + 'Record exists with ID field' => [ + 'data' => [new ArrayData(['ID' => 32])], + ], + ]; + } + + /** + * @dataProvider provideGetRecordFromRequestWithoutData + */ + public function testGetRecordFromRequestWithoutData(array $data) + { + $controller = new TestController(); + $form = $controller->Form(null, new ArrayList($data)); + $gridField = $form->Fields()->dataFieldByName('testfield'); + if (empty($data)) { + $gridField->setModelClass(ArrayData::class); + } + $component = $gridField->getConfig()->getComponentByType(GridFieldDetailForm::class); + $request = new HTTPRequest('GET', $gridField->Link('item/new')); + $request->match(Controller::join_links($gridField->Link(), 'item/$ID')); + + $this->expectException(LogicException::class); + $this->expectExceptionMessage(ArrayData::class . ' must have an ID field.'); + + $reflectionMethod = new ReflectionMethod($component, 'getRecordFromRequest'); + $reflectionMethod->setAccessible(true); + $reflectionMethod->invoke($component, $gridField, $request); + } } diff --git a/tests/php/Forms/GridField/GridFieldDetailFormTest/ArrayDataWithID.php b/tests/php/Forms/GridField/GridFieldDetailFormTest/ArrayDataWithID.php new file mode 100644 index 00000000000..00fed3896c1 --- /dev/null +++ b/tests/php/Forms/GridField/GridFieldDetailFormTest/ArrayDataWithID.php @@ -0,0 +1,15 @@ +filter('Name', 'My Group') - ->sort('Name') - ->First(); + if (!$list) { + $group = PeopleGroup::get() + ->filter('Name', 'My Group') + ->sort('Name') + ->First(); + $list = $group->People(); + } - $field = new GridField('testfield', 'testfield', $group->People()); + $field = new GridField('testfield', 'testfield', $list); $field->getConfig()->addComponent(new GridFieldToolbarHeader()); $field->getConfig()->addComponent(new GridFieldAddNewButton('toolbar-header-right')); $field->getConfig()->addComponent(new GridFieldViewButton()); diff --git a/tests/php/Forms/GridField/GridFieldDetailForm_ItemRequestTest.php b/tests/php/Forms/GridField/GridFieldDetailForm_ItemRequestTest.php new file mode 100644 index 00000000000..0a13a074970 --- /dev/null +++ b/tests/php/Forms/GridField/GridFieldDetailForm_ItemRequestTest.php @@ -0,0 +1,34 @@ +setModelClass($modelClass); + $itemRequest = new GridFieldDetailForm_ItemRequest($gridField, new GridFieldDetailForm(), new ArrayData(), new Controller(), ''); + + $this->expectException(LogicException::class); + $this->expectExceptionMessage( + 'Cannot dynamically determine form fields. Pass the fields to GridFieldDetailForm::setFields()' + . " or implement a getCMSFields() method on $modelClass" + ); + + $itemRequest->ItemEditForm(); + } +} diff --git a/tests/php/Forms/GridField/GridFieldExportButtonTest.php b/tests/php/Forms/GridField/GridFieldExportButtonTest.php index 3496332b4cd..2f4c9078c59 100644 --- a/tests/php/Forms/GridField/GridFieldExportButtonTest.php +++ b/tests/php/Forms/GridField/GridFieldExportButtonTest.php @@ -3,6 +3,8 @@ namespace SilverStripe\Forms\Tests\GridField; use League\Csv\Reader; +use LogicException; +use ReflectionMethod; use SilverStripe\Forms\Tests\GridField\GridFieldExportButtonTest\NoView; use SilverStripe\Forms\Tests\GridField\GridFieldExportButtonTest\Team; use SilverStripe\ORM\DataList; @@ -12,8 +14,10 @@ use SilverStripe\Forms\GridField\GridFieldConfig; use SilverStripe\Forms\GridField\GridFieldExportButton; use SilverStripe\Forms\GridField\GridField; +use SilverStripe\Forms\GridField\GridFieldDataColumns; use SilverStripe\Forms\GridField\GridFieldPaginator; use SilverStripe\ORM\FieldType\DBField; +use SilverStripe\View\ArrayData; class GridFieldExportButtonTest extends SapphireTest { @@ -155,13 +159,16 @@ public function testNoCsvHeaders() public function testArrayListInput() { $button = new GridFieldExportButton(); + $columns = new GridFieldDataColumns(); + $columns->setDisplayFields(['ID' => 'ID']); + $this->gridField->getConfig()->addComponent($columns); $this->gridField->getConfig()->addComponent(new GridFieldPaginator()); //Create an ArrayList 1 greater the Paginator's default 15 rows $arrayList = new ArrayList(); for ($i = 1; $i <= 16; $i++) { - $dataobject = new DataObject(['ID' => $i]); - $arrayList->add($dataobject); + $datum = new ArrayData(['ID' => $i]); + $arrayList->add($datum); } $this->gridField->setList($arrayList); @@ -192,6 +199,25 @@ public function testZeroValue() ); } + public function testGetExportColumnsForGridFieldThrowsException() + { + $component = new GridFieldExportButton(); + $gridField = new GridField('dummy', 'dummy', new ArrayList()); + $gridField->getConfig()->removeComponentsByType(GridFieldDataColumns::class); + $modelClass = ArrayData::class; + $gridField->setModelClass($modelClass); + + $this->expectException(LogicException::class); + $this->expectExceptionMessage( + 'Cannot dynamically determine columns. Add a GridFieldDataColumns component to your GridField' + . " or implement a summaryFields() method on $modelClass" + ); + + $reflectionMethod = new ReflectionMethod($component, 'getExportColumnsForGridField'); + $reflectionMethod->setAccessible(true); + $reflectionMethod->invoke($component, $gridField); + } + protected function createReader($string) { $reader = Reader::createFromString($string); diff --git a/tests/php/Forms/GridField/GridFieldFilterHeaderTest.php b/tests/php/Forms/GridField/GridFieldFilterHeaderTest.php index 145a87441a8..2a760c25ade 100644 --- a/tests/php/Forms/GridField/GridFieldFilterHeaderTest.php +++ b/tests/php/Forms/GridField/GridFieldFilterHeaderTest.php @@ -2,6 +2,7 @@ namespace SilverStripe\Forms\Tests\GridField; +use LogicException; use SilverStripe\Control\HTTPRequest; use SilverStripe\Core\Config\Config; use SilverStripe\Dev\SapphireTest; @@ -20,6 +21,7 @@ use SilverStripe\ORM\ArrayList; use SilverStripe\ORM\DataList; use SilverStripe\ORM\DataObject; +use SilverStripe\View\ArrayData; class GridFieldFilterHeaderTest extends SapphireTest { @@ -226,4 +228,20 @@ public function testRenderHeadersNonDataObject() $this->assertNull($htmlFragment); } + + public function testGetDisplayFieldsThrowsException() + { + $component = new GridFieldFilterHeader(); + $gridField = new GridField('dummy', 'dummy', new ArrayList()); + $modelClass = ArrayData::class; + $gridField->setModelClass($modelClass); + + $this->expectException(LogicException::class); + $this->expectExceptionMessage( + 'Cannot dynamically instantiate SearchContext. Pass the SearchContext to setSearchContext()' + . " or implement a getDefaultSearchContext() method on $modelClass" + ); + + $component->getSearchContext($gridField); + } } diff --git a/tests/php/Forms/GridField/GridFieldLevelupTest.php b/tests/php/Forms/GridField/GridFieldLevelupTest.php new file mode 100644 index 00000000000..ffa7eeccbac --- /dev/null +++ b/tests/php/Forms/GridField/GridFieldLevelupTest.php @@ -0,0 +1,29 @@ +setModelClass($modelClass); + + $this->expectException(LogicException::class); + $this->expectExceptionMessage( + GridFieldLevelup::class . " must be used with DataObject subclasses. Found '$modelClass'" + ); + + $component->getHTMLFragments($gridField); + } +} diff --git a/tests/php/Forms/GridField/GridFieldPrintButtonTest.php b/tests/php/Forms/GridField/GridFieldPrintButtonTest.php index 5a8b0df4ab3..8b9b2f61d2b 100644 --- a/tests/php/Forms/GridField/GridFieldPrintButtonTest.php +++ b/tests/php/Forms/GridField/GridFieldPrintButtonTest.php @@ -2,6 +2,8 @@ namespace SilverStripe\Forms\Tests\GridField; +use LogicException; +use ReflectionMethod; use SilverStripe\Dev\SapphireTest; use SilverStripe\Control\Controller; use SilverStripe\Forms\FieldList; @@ -10,7 +12,10 @@ use SilverStripe\Forms\GridField\GridFieldConfig; use SilverStripe\Forms\GridField\GridFieldPaginator; use SilverStripe\Forms\GridField\GridField; +use SilverStripe\Forms\GridField\GridFieldDataColumns; use SilverStripe\Forms\Tests\GridField\GridFieldPrintButtonTest\TestObject; +use SilverStripe\ORM\ArrayList; +use SilverStripe\View\ArrayData; class GridFieldPrintButtonTest extends SapphireTest { @@ -33,21 +38,19 @@ protected function setUp(): void public function testLimit() { - $this->assertEquals(42, $this->getTestableRows()->count()); + $this->assertEquals(42, $this->getTestableRows(TestObject::get())->count()); } public function testCanViewIsRespected() { $orig = TestObject::$canView; TestObject::$canView = false; - $this->assertEquals(0, $this->getTestableRows()->count()); + $this->assertEquals(0, $this->getTestableRows(TestObject::get())->count()); TestObject::$canView = $orig; } - private function getTestableRows() + private function getTestableRows($list) { - $list = TestObject::get(); - $button = new GridFieldPrintButton(); $button->setPrintColumns(['Name' => 'My Name']); @@ -62,4 +65,50 @@ private function getTestableRows() $printData = $button->generatePrintData($gridField); return $printData->ItemRows; } + + public function testGeneratePrintData() + { + $names = [ + 'Bob', + 'Alice', + 'John', + 'Jane', + 'Sam', + ]; + + $list = new ArrayList(); + foreach ($names as $name) { + $list->add(new ArrayData(['Name' => $name])); + } + + $rows = $this->getTestableRows($list); + + $foundNames = []; + foreach ($rows as $row) { + foreach ($row->ItemRow as $column) { + $foundNames[] = $column->CellString; + } + } + + $this->assertSame($names, $foundNames); + } + + public function testGetPrintColumnsForGridFieldThrowsException() + { + $component = new GridFieldPrintButton(); + $gridField = new GridField('dummy', 'dummy', new ArrayList()); + $gridField->getConfig()->removeComponentsByType(GridFieldDataColumns::class); + $modelClass = ArrayData::class; + $gridField->setModelClass($modelClass); + + $this->expectException(LogicException::class); + $this->expectExceptionMessage( + 'Cannot dynamically determine columns. Add a GridFieldDataColumns component to your GridField' + . " or implement a summaryFields() method on $modelClass" + ); + + $reflectionMethod = new ReflectionMethod($component, 'getPrintColumnsForGridField'); + $reflectionMethod->setAccessible(true); + $reflectionMethod->invoke($component, $gridField); + } } diff --git a/tests/php/ORM/Search/BasicSearchContextTest.php b/tests/php/ORM/Search/BasicSearchContextTest.php new file mode 100644 index 00000000000..58c97aacd03 --- /dev/null +++ b/tests/php/ORM/Search/BasicSearchContextTest.php @@ -0,0 +1,304 @@ + 'James', + 'Email' => 'james@example.com', + 'HairColor' => 'brown', + 'EyeColor' => 'brown', + ], + [ + 'Name' => 'John', + 'Email' => 'john@example.com', + 'HairColor' => 'blond', + 'EyeColor' => 'blue', + ], + [ + 'Name' => 'Jane', + 'Email' => 'jane@example.com', + 'HairColor' => 'brown', + 'EyeColor' => 'green', + ], + [ + 'Name' => 'Hemi', + 'Email' => 'hemi@example.com', + 'HairColor' => 'black', + 'EyeColor' => 'brown eyes', + ], + [ + 'Name' => 'Sara', + 'Email' => 'sara@example.com', + 'HairColor' => 'black', + 'EyeColor' => 'green', + ], + [ + 'Name' => 'MatchNothing', + 'Email' => 'MatchNothing', + 'HairColor' => 'MatchNothing', + 'EyeColor' => 'MatchNothing', + ], + ]; + + $list = new ArrayList(); + foreach ($data as $datum) { + $list->add(new ArrayData($datum)); + } + return $list; + } + + private function getSearchableFields(string $generalField): FieldList + { + return new FieldList([ + new HiddenField($generalField), + new TextField('Name'), + new TextField('Email'), + new TextField('HairColor'), + new TextField('EyeColor'), + ]); + } + + public function testResultSetFilterReturnsExpectedCount() + { + $context = new BasicSearchContext(ArrayData::class); + $results = $context->getQuery(['Name' => ''], existingQuery: $this->getList()); + + $this->assertEquals(6, $results->Count()); + + $results = $context->getQuery(['EyeColor' => 'green'], existingQuery: $this->getList()); + $this->assertEquals(2, $results->Count()); + + $results = $context->getQuery(['EyeColor' => 'green', 'HairColor' => 'black'], existingQuery: $this->getList()); + $this->assertEquals(1, $results->Count()); + } + + public function provideApplySearchFilters() + { + $idFilter = new ExactMatchFilter('ID'); + $idFilter->setModifiers(['nocase']); + return [ + 'defaults to PartialMatch' => [ + 'searchParams' => [ + 'q' => 'This one gets ignored', + 'ID' => 47, + 'Name' => 'some search term', + ], + 'filters' => null, + 'expected' => [ + 'q' => 'This one gets ignored', + 'ID:PartialMatch' => 47, + 'Name:PartialMatch' => 'some search term', + ], + ], + 'respects custom filters and modifiers' => [ + 'searchParams' => [ + 'q' => 'This one gets ignored', + 'ID' => 47, + 'Name' => 'some search term', + ], + 'filters' => ['ID' => $idFilter], + 'expected' => [ + 'q' => 'This one gets ignored', + 'ID:ExactMatch:nocase' => 47, + 'Name:PartialMatch' => 'some search term', + ], + ], + ]; + } + + /** + * @dataProvider provideApplySearchFilters + */ + public function testApplySearchFilters(array $searchParams, ?array $filters, array $expected) + { + $context = new BasicSearchContext(ArrayData::class); + $reflectionApplySearchFilters = new ReflectionMethod($context, 'applySearchFilters'); + $reflectionApplySearchFilters->setAccessible(true); + + if ($filters) { + $context->setFilters($filters); + } + + $this->assertSame($expected, $reflectionApplySearchFilters->invoke($context, $searchParams)); + } + + public function provideGetGeneralSearchFilterTerm() + { + return [ + 'defaults to case-insensitive partial match' => [ + 'filterType' => null, + 'fieldFilter' => null, + 'expected' => 'PartialMatch:nocase', + ], + 'uses default even when config is explicitly "null"' => [ + 'filterType' => null, + 'fieldFilter' => new StartsWithFilter('MyField'), + 'expected' => 'PartialMatch:nocase', + ], + 'uses configuration filter over field-specific filter' => [ + 'filterType' => ExactMatchFilter::class, + 'fieldFilter' => new StartsWithFilter(), + 'expected' => 'ExactMatch', + ], + 'uses field-specific filter if provided and config is empty string' => [ + 'filterType' => '', + 'fieldFilter' => new StartsWithFilter('MyField'), + 'expected' => 'StartsWith', + ], + ]; + } + + /** + * @dataProvider provideGetGeneralSearchFilterTerm + */ + public function testGetGeneralSearchFilterTerm(?string $filterType, ?SearchFilter $fieldFilter, string $expected) + { + $context = new BasicSearchContext(ArrayData::class); + $reflectionGetGeneralSearchFilterTerm = new ReflectionMethod($context, 'getGeneralSearchFilterTerm'); + $reflectionGetGeneralSearchFilterTerm->setAccessible(true); + + if ($fieldFilter) { + $context->setFilters(['MyField' => $fieldFilter]); + } + + Config::modify()->set(ArrayData::class, 'general_search_field_filter', $filterType); + + $this->assertSame($expected, $reflectionGetGeneralSearchFilterTerm->invoke($context, 'MyField')); + } + + public function provideGetQuery() + { + // Note that the search TERM is the same for both scenarios, + // but because the search FIELD is different, we get different results. + return [ + 'search against hair' => [ + 'searchParams' => [ + 'HairColor' => 'brown', + ], + 'expected' => [ + 'James', + 'Jane', + ], + ], + 'search against eyes' => [ + 'searchParams' => [ + 'EyeColor' => 'brown', + ], + 'expected' => [ + 'James', + 'Hemi', + ], + ], + 'search against all' => [ + 'searchParams' => [ + 'q' => 'brown', + ], + 'expected' => [ + 'James', + 'Jane', + 'Hemi', + ], + ], + ]; + } + + /** + * @dataProvider provideGetQuery + */ + public function testGetQuery(array $searchParams, array $expected) + { + $list = $this->getList(); + $context = new BasicSearchContext(ArrayData::class); + $context->setFields($this->getSearchableFields(BasicSearchContext::config()->get('general_search_field_name'))); + + $results = $context->getQuery($searchParams, existingQuery: $list); + $this->assertSame($expected, $results->column('Name')); + } + + public function testGeneralSearch() + { + $list = $this->getList(); + $generalField = BasicSearchContext::config()->get('general_search_field_name'); + $context = new BasicSearchContext(ArrayData::class); + $context->setFields($this->getSearchableFields($generalField)); + + $results = $context->getQuery([$generalField => 'brown'], existingQuery: $list); + $this->assertSame(['James', 'Jane', 'Hemi'], $results->column('Name')); + $results = $context->getQuery([$generalField => 'b'], existingQuery: $list); + $this->assertSame(['James', 'John', 'Jane', 'Hemi', 'Sara'], $results->column('Name')); + } + + public function testGeneralSearchSplitTerms() + { + $list = $this->getList(); + $generalField = BasicSearchContext::config()->get('general_search_field_name'); + $context = new BasicSearchContext(ArrayData::class); + $context->setFields($this->getSearchableFields($generalField)); + + // These terms don't exist in a single field in this order on any object, but they do exist in separate fields. + $results = $context->getQuery([$generalField => 'john blue'], existingQuery: $list); + $this->assertSame(['John'], $results->column('Name')); + $results = $context->getQuery([$generalField => 'eyes sara'], existingQuery: $list); + $this->assertSame(['Hemi', 'Sara'], $results->column('Name')); + } + + public function testGeneralSearchNoSplitTerms() + { + Config::modify()->set(ArrayData::class, 'general_search_split_terms', false); + $list = $this->getList(); + $generalField = BasicSearchContext::config()->get('general_search_field_name'); + $context = new BasicSearchContext(ArrayData::class); + $context->setFields($this->getSearchableFields($generalField)); + + // These terms don't exist in a single field in this order on any object + $results = $context->getQuery([$generalField => 'john blue'], existingQuery: $list); + $this->assertCount(0, $results); + + // These terms exist in a single field, but not in this order. + $results = $context->getQuery([$generalField => 'eyes brown'], existingQuery: $list); + $this->assertCount(0, $results); + + // These terms exist in a single field in this order. + $results = $context->getQuery([$generalField => 'brown eyes'], existingQuery: $list); + $this->assertSame(['Hemi'], $results->column('Name')); + } + + public function testSpecificFieldsCanBeSkipped() + { + $general1 = $this->objFromFixture(SearchContextTest\GeneralSearch::class, 'general1'); + $list = new ArrayList(); + $list->merge(SearchContextTest\GeneralSearch::get()); + $generalField = BasicSearchContext::config()->get('general_search_field_name'); + $context = new BasicSearchContext(SearchContextTest\GeneralSearch::class); + + // We're searching for a value that DOES exist in a searchable field, + // but that field is set to be skipped by general search. + $results = $context->getQuery([$generalField => $general1->ExcludeThisField], existingQuery: $list); + $this->assertNotEmpty($general1->ExcludeThisField); + $this->assertCount(0, $results); + } +} diff --git a/tests/php/ORM/Search/BasicSearchContextTest.yml b/tests/php/ORM/Search/BasicSearchContextTest.yml new file mode 100644 index 00000000000..dfdd30f344f --- /dev/null +++ b/tests/php/ORM/Search/BasicSearchContextTest.yml @@ -0,0 +1,28 @@ +SilverStripe\ORM\Tests\Search\SearchContextTest\GeneralSearch: + general0: + Name: General Zero + DoNotUseThisField: omitted + HairColor: blue + ExcludeThisField: excluded + ExactMatchField: Some specific value here + PartialMatchField: A partial match is allowed for this field + MatchAny1: Some match any field + MatchAny2: Another match any field + general1: + Name: General One + DoNotUseThisField: omitted + HairColor: brown + ExcludeThisField: excluded + ExactMatchField: This requires an exact match + PartialMatchField: This explicitly allows partial matches + MatchAny1: first match + MatchAny2: second match + general2: + Name: MatchNothing + DoNotUseThisField: MatchNothing + HairColor: MatchNothing + ExcludeThisField: MatchNothing + ExactMatchField: MatchNothing + PartialMatchField: MatchNothing + MatchAny1: MatchNothing + MatchAny2: MatchNothing diff --git a/tests/php/ORM/Search/SearchContextTest.php b/tests/php/ORM/Search/SearchContextTest.php index 7236709cbd4..839de5bc96c 100644 --- a/tests/php/ORM/Search/SearchContextTest.php +++ b/tests/php/ORM/Search/SearchContextTest.php @@ -18,6 +18,7 @@ use SilverStripe\ORM\Filters\ExactMatchFilter; use SilverStripe\ORM\Filters\PartialMatchFilter; use SilverStripe\ORM\Search\SearchContext; +use SilverStripe\View\ArrayData; class SearchContextTest extends SapphireTest { @@ -527,4 +528,18 @@ public function testMatchAnySearchWithFilters() $results = $context->getResults(['PartialMatchField' => 'an']); $this->assertCount(1, $results); } + + public function testGetSearchFieldsThrowsException() + { + $modelClass = ArrayData::class; + $context = new SearchContext($modelClass); + + $this->expectException(LogicException::class); + $this->expectExceptionMessage( + 'Cannot dynamically determine search fields. Pass the fields to setFields()' + . " or implement a scaffoldSearchFields() method on {$modelClass}" + ); + + $context->getSearchFields(); + } }