silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
719 stars 823 forks source link

[FORMS] GroupedList fails with an ArrayList of plain PHP Objects #6581

Open wernerkrauss opened 7 years ago

wernerkrauss commented 7 years ago

Fatal error: Call to undefined method MyPlainObject::hasMethod() in /vagrant/www/framework/model/GroupedList.php on line 21

https://github.com/silverstripe/silverstripe-framework/blob/3.5/model/GroupedList.php#L21

robbieaverill commented 7 years ago

method_exists($item, 'hasMethod')? 😄

wernerkrauss commented 7 years ago

@robbieaverill sure... but hasMethod() also checks methods on DataExtensions, so it's not just a replacement.

I wrote a local solution:

protected function getKeyOfItem($item, $index) {

    if (is_object($item)) {
        if (method_exists($item, 'hasMethod') && $item->hasMethod($index)) {
            return $item->$index();
        }

        //fallback for plain PHP objects
        if (method_exists($item, $index)) {
            return $item->$index();
        }

        return $item->$index;
    }

    return $item[$index];
}

If you think it's a good enhancement I can make a PR against 3.5

sminnee commented 7 years ago

Seems like a reasonable patch.

dhensby commented 6 years ago

@wernerkrauss did you ever submit the patch to 3.5?

tractorcow commented 6 years ago

@wernerkrauss It'd be better to group the condition if you can help it, rather than duplicate the action. Do you want to PR? :P