modlinltd / django-advanced-filters

Add advanced filtering abilities to Django admin
https://pypi.org/project/django-advanced-filters/
MIT License
771 stars 173 forks source link

Bug fixes #97

Open Robert-Lebedeu opened 5 years ago

Robert-Lebedeu commented 5 years ago

CHANGES:

  1. Templates:
    • Removed the title inside the change_form template.
    • Commented the _af_handler.destroy() call inside common_js_init because I didn't want to clean up every time the add rule button was pressed.
  2. Forms:
    • Changed isnull operator from None to 'None' (Before it wasn't working and with 'None' worked perfectly).
    • The value field now is not required anymore if you select one of these operators: 'isnull', 'istrue, 'isfalse' (Also the value field becomes a readonly field, in these cases).
    • I also removed the part that auto-selects the range operator with datetime fields. This was creating some issues and I thought it was a useless feature to keep in so I get rid of that part.
  3. Static:
    • Added try-catch when using Django-Grappelli and JQuery-UI
    • Changed the modify_widget
    • I found some functions that were called twice with out any valid reason, I commented that parts and this corrected some bugs (for example: now range operator works properly).
    • I also commented the field_selected(...).first() call because I thought that it didn't make any sense to re-initiate the first rule (this problem was forcing you to use always an equal operator as your first rule) .
    • I also changed the initialize_select2 function to set the autocomplete only with foreign keys and 'iexact' operator.
lorenzomorandini commented 5 years ago

This PR actually fixes a lot of JavaScript issues, can it be merged please?

asfaltboy commented 5 years ago

There's a failing test and a few dubious changes, but I'm sure it can be brought to shape. @lorenzomorandini would you care to review & test the code?

lorenzomorandini commented 5 years ago

Ok, I will review it

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.05%) to 95.969% when pulling b902218547bbbd95b6706b127b234f9e96e2b375 on Robert-Lebedeu:develop into 4f1575a67e751330606a9a3ee4f33eda5214b44e on modlinltd:develop.

lorenzomorandini commented 5 years ago

I've been trying this version for a few days and all javascript problems seem to have gone:

asfaltboy commented 4 years ago

Update: I'm working on a test suite for JS using selenium, once I am able to anchor all feature and reproduce the issues mentioned here, I this it will reduce the risk when merging this PR.