Added hooks to Media for staticfiles app

[djw]

When constructing the URLs for form media, django simply prepends the asset path with MEDIA_URL or STATIC_URL. Instead it should use staticfiles_storage.url(path).

[djw]

I've created a patch here:

​https://github.com/djw/django/tree/ticket_18455

All the tests in 'forms' pass with this patch applied. However, they fail when running as part of the full test suite, and I need some advice on how to fix this.

The issue is that the tests in forms/tests/media.py run with overridden STATIC_URL and MEDIA_URL. When run in isolation, the static files storage backend is initialised based on these settings. However if the backend has been initialised before these tests run, it doesn't respect the new values of STATIC_URL and MEDIA_URL, so the tests fail. What's the best way to go about fixing this?

Also the absolute_path() method has an optional prefix argument — I can't see where that's used, so I dont know how to support it properly.

[Claude Paroz]

Please try also to write a test which doesn't pass with the current code, to demonstrate this ticket's rationale.

About tests not passing in full run, try to see which global variable is the culprit and reset this variable in the test setUp method (or use the settings_change signal).

[djw]

I've added a couple of commits to that branch which

1. Reset the static files storage backend in setUp and tearDown, so that the full test suite passes
2. Adds a test which fails on current master, but passes on this branch

[Aymeric Augustin]

By importing django.contrib.staticfiles in django.forms, you're adding a dependency of core on contrib. This is at best frowed upon, at worst prohibited.

---

if the backend has been initialised before these tests run, it doesn't respect the new values of STATIC_URL and MEDIA_URL, so the tests fail. What's the best way to go about fixing this?

We should add a listener on the setting_changed signal that resets caches that depend on STATIC_URL or MEDIA_URL.

[Jannis Leidel]

Using the staticfiles_storage in the Django core is a no-go. The staticfiles app is a contrib app and should be considered optional.

You can however easily override that by providing an own Media class, as documented. I suggest to add an own class to the staticfiles app so users can use that in their form class.

Of course this implies that we first of all clean up the Media class API in the forms library in the first place. E.g. renaming it to Static for example.

So to summarize I think this should be fixed by:

1. add a new Static class to the form library that has hooks to implement different way to get the path of a file
2. add a Static subclass to staticfiles that users can import in their forms modules to use the staticfiles app instead of just joining with STATIC_URL

While you're there you should also think about extending the Static class to be more flexible with regard to multiple files etc.

This ticket obviously can only handle a few of those issues, so please open new tickets for the other parts.

[James Aylett]

How should this work with 3rd party apps? contrib.admin implements its own switcher between staticfiles and the simpler static built-in template tag; perhaps staticfiles should provide such a switcher itself that anyone could use (allowing it to be used in ModelAdmin.Media and Widget.Media inner classes, for instance), and then building the staticfiles subclass of (new, putative) Static around it, meaning that re-usable apps could use that without either tying people into using staticfiles or stopping people from using it. (And preventing everyone having to re-implement the same thing independently.)

[Aymeric Augustin]

There's an issue with providing the switcher in staticfiles: for Django to pick it, you have to add django.contrib.staticfiles to INSTALLED_APPS, which means the switcher loses its purprose.

[James Aylett]

I was thinking as a function, not as a templatetag (which is sometimes how it's used inside contrib.admin).

[Maxime Lorant]

Duplicate of #21221, which has a PR pending.