modlinltd / django-advanced-filters

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

Django Compress causes errors #188

Open RumitAP opened 10 months ago

RumitAP commented 10 months ago

When I try to compress my project down, the static files within the django-advanced-filters causes errors.

To Reproduce Steps to reproduce the behavior:

  1. Go to the root of external project in terminal
  2. python manage.py collectstatic
  3. python manage.py compress --force
  4. See error
    Error parsing template /home/rap/.local/lib/python3.8/site-packages/advanced_filters/templates/admin/advanced_filters.html: Invalid template name in 'extends' tag: ''. Got this from the 'original_change_list_template' variable.
    done

Expected behavior It should be able to compress these files down.

Details (please complete the following information):

Additional context Add any other context about the problem here, including traceback and additional log files.

RumitAP commented 10 months ago

erielias@gmail.com

vitormhenrique commented 10 months ago

Just made a PR to fix the issue: https://github.com/modlinltd/django-advanced-filters/pull/189

asfaltboy commented 5 months ago

I haven't used django-compress in a while, but as far as I can tell it will not do much with Django's admin templates, unless you use the {% compress %} tag into your templates.

I can't see a setting to skip apps, might be a question for django-compress maintainers. Though, if you are providing a custom change list template, or otherwise wish to compress the resources in the admin site, set your COMPRESS_OFFLINE_CONTEXT to the template name. E.g:

COMPRESS_OFFLINE_CONTEXT = {
    "original_change_list_template": "admin/change_list.html"
}

@vitormhenrique thanks for your contribution. I can see your PR changes the template to default to the default django admin template, but the existence of the variable original_change_list_template is there in the first place to allow users to set ModelAdmin.change_list_template to their own custom value, i.e a different template, as per django-admin API.

For more details see:

  1. Django's docs for reference.
  2. Django compress: usage
  3. Django compress: offlien compression caveats
vitormhenrique commented 5 months ago

Hello @asfaltboy The idea of the PR was indeed to give a default value in the case of original_change_list_template is null.

So it still allow users to use a different template but "forces" a default value. I noticed other apps had the same issue with compressor and they all fixed like this, that is why I went this route.

I will try to use the COMPRESS_OFFLINE_CONTEXT but for someone using the app without digging into the source code would be difficult to know all the custom templates that would need to default to get the app working with compressor.

Thank you for your reply, I'll do more digging and let you know!

asfaltboy commented 5 months ago

That's fair @vitormhenrique, I've merged your fix PR, please let me know if this fixes the issue for you and @RumitAP