search

silverstripe / silverstripe-framework

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

FIX Don't load data up front for lazy-loaded searchable dropdown #11278

Closed GuySartorelli closed 1 week ago

GuySartorelli commented 2 weeks ago

We replaced the auto-scaffolding of has_one form fields with this new thing, and we said it would be better because you don't need the NumericField fallback anymore - but the workaround in the linked issue is to manually replace the field with NumericField so we've obviously messed up there.

Ultimately getSource() should just return the DataList directly, but we can't do that in a minor much less a patch because the return type is strongly typed for that method.

In the meantime, the new logic avoids calling getSource() altogether as much as we can avoid it, since calling that method is usually not required.

I couldn't remove it being called from castedCopy() without copying a bunch of boilerplate from FormField - and it is a pretty messy method. Because of that, I also can't sensibly stop performReadonlyTransformation() from calling it, so readonly versions of the field will still suck when you have large datasets. If that specific scenario becomes a problem for someone we can look at dealing with it, but until someone complains about it I'm inclined to leave it alone for CMS 5.

Issue

johannesx75 commented 1 week ago

... Because of that, I also can't sensibly stop performReadonlyTransformation() from calling it, so readonly versions of the field will still suck when you have large datasets. If that specific scenario becomes a problem for someone we can look at dealing with it ...

@GuySartorelli Unfortunately this scenario is indeed a problem. The reason we noticed the lazy loading problem in the first place was the scaffolding for DataObjects in Model admin. With this fix it is now possible to open Detailforms for existing DataObjects. But you can't create nested DataObjects anymore.

If you have ModelA that has_many ModelB and you open the Detailform for ModelA, than a GridField for ModelB will be added to the form. If you click on "Add new ModelB" the NewItemForm will try to make the Field for the has_one ModelA readonly ...

The workaround for now is to remove the field in the NewItemForm.

GuySartorelli commented 3 days ago

@johannesx75 I've opened https://github.com/silverstripe/silverstripe-framework/issues/11293 as a follow-up issue to handle specifically readonly fields. If you'd like to try tackling it with a new pull request that will greatly improve the chances that this is resolved.

I didn't fully understand what you were saying in your comment but given you quoted the bit about readonly fields, I assume that's what the problem is.