Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wagtail admin static assets should be cache busted to prevent outdated cache issues #3700

Closed
thibaudcolas opened this issue Jul 8, 2017 · 9 comments

Comments

@thibaudcolas
Copy link
Member

Issue Summary

When Wagtail is updated, the admin UI should work without the user having to clean their cache. #3693 is an example of this problem, where a release contains a lot of changes to the static JS and CSS of the admin interface. Servers/browsers that have caching enabled will serve the cached files from the previous version, producing a broken UI.

This could be prevented by implementing cache busting by renaming the URLs to the static assets (this is most commonly done with a query string suffix containing a version number, or hash of the files' content).

Steps to Reproduce

  1. Install a project with Wagtail 1.10 and navigate to its admin, open the page explorer menu.
  2. Upgrade the project to Wagtail 1.11 and do the same operation.

Expectation: Updates should "just work" for end users of the product, who may not even be aware of the update.

Technical details

  • Any Wagtail upgrade that includes changes to static files (eg. 1.10 to 1.11)
  • Browser / proxy / server combinations that have caching enabled

Any advice on how best to do this in a Django codebase is very welcome.

@BertrandBordage
Copy link
Member

BertrandBordage commented Jul 8, 2017

As we discussed on the Slack team, a perfect solution is to use ManifestStaticFilesStorage. It automatically adds a hash on all static files to make sure it’s the right version.

I think we should definitely tell people to use it in the docs, as well as including it in the project creation template. Otherwise, we’ll keep getting reports each time we change JS or CSS.

@thibaudcolas
Copy link
Member Author

That's exactly what we need! I'm not sure I understand the details, if this is only an opt-in I'm afraid there will still be a lot of instances where people don't read the docs and get burnt. Can usage be enforced for the Wagtail apps only?

@BertrandBordage
Copy link
Member

We can enforce it, but I don’t think that’s a nice way to do it, as other apps may rely on a different static files storage. We can add a warning, however.

@SalahAdDin
Copy link
Contributor

Hey, interesting approach.

@gasman
Copy link
Collaborator

gasman commented Aug 17, 2017

From experience, making any kind of change from Django's default setup (even something as innocent as splitting settings into base/dev/production) tends to cause confusion or disruption to users one way or another. I'm not fully confident that enabling ManifestStaticFilesStorage won't have unexpected side effects on some setups (e.g. when static assets are hosted remotely), and I'd be happier for this to be something that developers have to consciously enable themselves.

I'd propose having a section in the deployment docs covering this, along with a brief pointer on the upgrading page.

@gasman gasman added this to the some-day milestone Aug 17, 2017
@thibaudcolas
Copy link
Member Author

I'm worried that documentation will be overlooked / and that those specific setups that can't leverage ManifestStaticFilesStorage will still have the issue. Even though cache busting is a known practice, and easily covered in most frameworks, it still is a very common issue for end users to arrive on half-broken UIs because it's been overlooked.

Would it be reasonable to make a custom tag that automatically adds Wagtail's version number in a query string at the end of the URL of static files?

@thibaudcolas
Copy link
Member Author

See also https://stackoverflow.com/questions/49088302/richtextfield-uneditable-with-draftail-in-wagtail-2-0.

I only expect this to become more frequent of a problem as Wagtail updates integrate more client-side changes. It would be great if it "just worked" by default – at the moment any kind of issue of this type is too easily classified as "Wagtail's new version is broken".

@SalahAdDin
Copy link
Contributor

I was thinking about integrate new Draftail plugins in a project: i mean, we would have to install in in the static_src and recompile admin static files, isn't it?

@thibaudcolas
Copy link
Member Author

@SalahAdDin no – please have a look at the official docs. And let's keep this issue about cache busting.

gasman added a commit to gasman/wagtail that referenced this issue Aug 23, 2018
Meteor0id added a commit to Meteor0id/wagtail that referenced this issue Sep 5, 2018
* one letter spelling mistake

Had been merged by the time I noticed my own mistake. Corrected 1 letter.

* Tests negating the search of multiple words (should fail on PostgreSQL).

* Builds a single tsquery to handle complex negations.

* Changelog for PostgreSQL autocomplete.

* Reindex books when adding tags

* Index tag IDs when indexing tags fields

* Find underlying type of related fields

We currently guess that all OneToOneFields are integers and use string
for everything else.

This is usually not an issue as Elasticsearch coerces between strings an
integers automatically. But this causes issues for the new facet feature
as Elasticsearch returns strings for ID fields that are actually
integers.

The field type changes shouldn't cause any trouble for existing indices
as Elasticsearch will continue to automatically coerce the types. Users
who want to use the new facet feature on related fields will have to
rebuild their index.

* Add "missing value" bucket to Elasticsearch facet

https://www.elastic.co/guide/en/elasticsearch/reference/current/search-aggregations-bucket-terms-aggregation.html#_missing_value_12

* Allow indexing arrays of model instances in FilterField

* Support indexing ForeignKeys which point to models with custom PK type

* Always index the specific version of the book

* Make checkbox/radio alignment on Firefox consistent with Webkit

* Implement filter by content type on page search

* Implement custom ordering in page search

* Release note for wagtail#4524

* Fix typo preventing facet example from displaying

* Un-deprecate index methods from Elasticsearch2SearchBackend

As discussed at wagtail#3975 (comment) - the base search backend class now implements generic versions of these methods so that they can be removed from the ES2-specific code without any loss of functionality.

* Block installation of beautifulsoup4==4.6.1

It contains a critical bug that breaks Wagtail: https://bugs.launchpad.net/beautifulsoup/+bug/1784408

* Fetch new translations from Transifex

* Generate new strings for translation

* Version bump to 2.2rc1

* see if requirements_file: null makes readthedocs happy

* Version bump to start work on 2.3

* Rotate deprecation warnings

* Fixes Django 2.1 breaking changes; QUERY_TERMS & auth_views.login()

* Specify postgresql 9.6 on Travis for Django 2.1 support

* Upgrade gulp-sass to 4.0.1 and rebuild package-lock.json with npm 6

The only difference to the generated code appears to be in sourcemap data.

* Run npm audit to fix most vulnerable packages

Generated static files are unchanged by this update.

* Revert "Run npm audit to fix most vulnerable packages"

This reverts commit f283c8b.

* Revert "Upgrade gulp-sass to 4.0.1 and rebuild package-lock.json with npm 6"

This reverts commit 4972077.

* Test against Django 2.1 final

* Migrate to circleci 2.0

* Bundle the l18n library to prevent installation issues

https://bitbucket.org/tkhyn/l18n/issues/4/setuppy-raises-unicodedecodeerror-trying prevents the l18n library from being installed on certain setups with non-Unicode locales:
https://groups.google.com/d/msg/wagtail/X5d8CL3DxUs/httaLso4DAAJ

As a temporary measure until a fixed version of l18n is released, bundle the library into wagtail.utils.l18n.

* Exclude wagtail/utils/l18n from linting

* add indirect dependencies from l18n

* Release notes for 2.1.2

* Fetch new translations from Transifex

* Security audit of npm packages (wagtail#4709)

* Upgrade gulp-sass to 4.0.1 and rebuild package-lock.json with npm 6

The only difference to the generated code appears to be in sourcemap data.

* Run npm audit to fix most vulnerable packages

Generated static files are unchanged by this update.

* Revert package-lock.json to npm 5 format

* Fix WidgetWithScript to accept renderer kwarg

* Perform rich text data conversion in the standard format_value method

This is called as part of Django's widget rendering process, so avoids the need for an explicit conversion within the render method.

* Convert DraftailRichTextArea to template rendering

* Update BlockWidget to accept a renderer kwarg to render and render_with_errors

In this case we just ignore the renderer; blocks have their own separate rendering mechanism, so it's not meaningful for that to be controlled by Django widget renderer classes.

* Define sortable_by on modeladmin.views.IndexView for Django 2.1 compatibility

* Cast values to string when writing to querystring

Mitigation for https://code.djangoproject.com/ticket/29627 in Django 2.1

* Fix passwordless user creation tests for Django 2.1 and clarify WAGTAILUSERS_PASSWORD_REQUIRED docs

The behaviour of `has_usable_password` has changed in Django 2.1, such that `None` is no longer considered a 'non-usable' password: https://docs.djangoproject.com/en/2.1/ref/contrib/auth/#django.contrib.auth.models.User.has_usable_password

As a consequence of the fix applied in Django https://code.djangoproject.com/ticket/28718 , Wagtail users created without a password will now be able to complete the password reset process to gain access to Wagtail. Sites that do not want this behaviour (e.g. because those users should be using an LDAP login instead) should disable password changes via WAGTAIL_PASSWORD_MANAGEMENT_ENABLED and WAGTAIL_PASSWORD_RESET_ENABLED.

* Upgrade to Django-2.1-compatible versions of modelcluster and taggit

* Update documentation to indicate Django 2.1 support

* Upgrade mysqlclient to a supported version

* Stop ignoring failures on Django 2.1 / specify Django 2.1 in project template

* Added a simple scale filter to image_operations.

* Prevent search engines from indexing admin pages

According to a google search I just did, it seems a lot of people have forgotten to add ``Disallow: /admin`` in their robots.txt (or forgot to add robots.txt) at all.

Adding this meta tag into the head of all admin pages should prevent any admin pages being indexed even if this was missed.

* Convert AdminAutoHeightTextInput to template-based rendering

* Convert AdminDateInput, AdminTimeInput, AdminDateTimeInput to template rendering

* Convert AdminTagWidget to template rendering

* Remove spurious uses of unlocalize filter on widget ID

These were added in wagtail#3478 to ensure that numeric object IDs would not be affected by local number formatting; however, within widget templates, attrs.id refers to a string identifier, not a numeric ID.

* Convert HalloRichTextArea to template rendering

* Convert TableBlock widget to Django-1.11-style template rendering

* Fetch new translations from Transifex

* Fill in release date for 2.2

* Specify Elasticsearch <6.3.1

* Respect redirect parameter after login

* Add release notes for 1.12.5

* Add release notes for 1.13.3

Conflicts:
	CHANGELOG.txt
	docs/releases/index.rst

* Add release notes for 1.12.4 - 1.12.6 and 1.13.2 - 1.13.4

Conflicts:
	CHANGELOG.txt
	docs/releases/index.rst

Conflicts:
	CHANGELOG.txt
	docs/releases/index.rst

* Add release notes for 2.0.2

* Pin Beautifulsoup to 4.6.0

* Add release notes for 2.1.3

Conflicts:
	CHANGELOG.txt
	docs/releases/index.rst

* Add release notes for 2.2.1

* Prevent AppRegistryNotReady error when wagtail.contrib.sitemaps is in INSTALLED_APPS

Fixes wagtail#4729

* Release note for wagtail#4730

* Update dependencies to include Django 2.1

* Fix deprecation warning on get_sitemap_urls

* test against Django master

* Trim down Travis test matrix

* fixup! test against Django master

* Fix support for related_query_name in InlinePanel relations

* Eliminate InlinePanel.related, as it just duplicates db_field

* Release note for wagtail#4579

* Add warning about unregistering image formats.

Add warning to documentation to warn that unregistering image formats
that are being used will create an error when viewing or editing pages.

See https://groups.google.com/forum/#!topic/wagtail/X8xTUs-2npA and
wagtail#1471 (comment).

* Validates EmbedBlock URLs against providers.

* Simplify URL validator code on EmbedForm

* Ask issue openers to confirm that bugs are reproducible

* Remove old-skool font-support since woff and woff2 would be sufficient for our browser support

* Wrap deleting page into database transaction

Currently queries executed in the hooks don't run in the transaction
with the page deletion query and it's harder to write hook without
copying the whole view if you want to keep queries running in the hooks
integral with page deletion.

* url() is depriciated. Alias re_path()

Added important note about using url() on older versions of django, but switched the examples to re_path as to comply with Django docs for 2.0 and later.

* depriciate url() for alias re_path()

* Rewind file to the beginning after reading

Fixes wagtail#4738

* Release note for wagtail#4739 in 2.3

* Release note for wagtail#4739 in 2.2.2

* Convert UUID primary key to string before serializing in json. Fixes wagtail#4616.

* Specify file-based sqlite db in tox

* Specify file-based sqlite db for circleci

* Re-establish datetimepicker localisation. Fix wagtail#4584

* Release note for wagtail#4719 in 2.2.2

* Use Django defaults for not caching admin

* Prevent the use of a global variable.

* Fill in release date for 2.2.2

Conflicts:
	CHANGELOG.txt

* Recommend the use of ManifestStaticFilesStorage. Fixes wagtail#3700
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants