crud icon indicating copy to clipboard operation
crud copied to clipboard

Move beforePaginate() of RelatedModelsListener in a separate listener.

Open ADmad opened this issue 3 years ago • 2 comments

Containing related model in the query is a very different operation than setting view vars for select options for related models.

Also CrudView's ViewListener contains associated models in it's beforePaginate() and beforeFind(), so having a separate listener in Crud for containing would allow better control.

ADmad avatar Dec 19 '22 19:12 ADmad

@ADmad

Containing related model in the query is a very different operation than setting view vars for select options for related models.

Agreed.

Unfortunatley, the documentation could be more clear about this and the sad fact that the two behaviors can't be controlled separately, at the moment.

Also, the current behavior can lead the application to load the full data of potentially large database tables. Because of this, we recently ran into a performance/memory limit issue.

Also CrudView's ViewListener contains associated models in it's beforePaginate() and beforeFind(), so having a separate listener in Crud for containing would allow better control.

While a separate listener would certainly be much cleaner, a simple flag for controlling the view variable population would probably be much easier to implement. Also, less complicated to keep backwards compatibility.

If you are open to the idea of such a flag, me or my team mate @T0nyDamage would be interested to contribute.

ravage84 avatar Nov 27 '24 17:11 ravage84

I still want this to be a separate listener but I guess we can have a config as a stop gap solution. Please make a draft PR and we can see.

ADmad avatar Nov 28 '24 16:11 ADmad