search

otto-torino / django-baton

A cool, modern and responsive django admin application based on bootstrap 5 that brings AI to the Django admin - https://baton.sqrt64.it/
MIT License
879 stars 95 forks source link

Incorrect dirty detection #12

Closed Magister closed 5 years ago

Magister commented 5 years ago

This is incorrect with filter_horizontal feature of django-admin. form.serialize() also serializes second form select - and thus form is always "dirty", while in reality nothing has changed. https://github.com/otto-torino/django-baton/blob/031f9d2d164429adc332430e17ab2596a2fedfb7/baton/static/baton/app/src/core/ChangeForm.js#L27-L29

abidibo commented 5 years ago

Thanks, I'll look into it

abidibo commented 5 years ago

Hi @Magister , I've looked into it and I must say I've the opposite behaviour, that is, in my case, the form is never dirty even when adding/removing select choices.

This is due to the fact that jquery serialize method only considers form fields that have a value, while in such widget, the options are not selected, thus do not have a value! Options are simply added to the "current selected" select multiple, but are not selected (you can prove it in your console by selecting the element and inspect its value, it's always an empty array if you don't select/highlight anything). So, such field won't be included in the serialization string.

Now I've overcame this problem parsing the form for such kind of fields, and adding manually the options values to the serialized string.

But a problem still remains: such widgets are rendered in a window onload callback, so no one can assure they are ready when my ChangeForm code inits, thus not being detected in the initData and causing a form to always be dirty if some options are selected. And of course this is the case in my local tests.

There is no clean way (as far as I know, and I won't considering polling the dom) to be sure that django's SelectFilter function has done its job, because it doesn't dispatch events, nor sets some internal variable. I've for now just set a timeout of 500ms before running my initial serialization and it seems to work properly, even if I know, it's not a great solution. If you've a brilliant idea, please share it here.

I'll close this now, when pushing the new release, feel free to re-open the issue if this won't fix your problem.

Thanks

Magister commented 5 years ago

Hi, @abidibo , Yes, looks like you're right, I did not dig deep into details. Your fix does its job for me, thanks a lot.

As for better ways - well, probably it can be possible to somehow patch Django's javascript or (better) monkey-patch widgets to emit some events, but not sure if that will be too fragile/incompatible with different Django admin versions.

So probably your solution is the best as for now...

Thanks.