-
-
Notifications
You must be signed in to change notification settings - Fork 74
Not compatible with page cache #49
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
Comments
This seems somewhat similar to issue #15 because the timestamp always changes. We could wipe out 2 issues if we improve the way things are done. We should all definitely solve this because the page cache is something tons of websites use. When you say page cache, do you mean browser cache as explained in https://docs.djangoproject.com/en/4.0/topics/cache/#downstream-caches ? Or do you mean per site cache or per view cache? Or all of the above? https://stackoverflow.com/questions/46174733/how-to-clear-browser-cache-with-python-django How are you flushing the page cache? Something like the redis flushall command? If you have code for it please share. Maybe we can make a management command for it? Are you overriding the entire django-cookie-consent files (if so, what files?) in your virtual environment or are you adding your code above to your current project files without overriding django-cookie-consent files? I think it would be great if you could make a PR using your method. I am no javascript expert by any means, so I can't help there. Maybe in the future we could add XHR as an alternative setting and then the django developer could choose which method they prefer. I am fine with either method to fix this problem. Let's see what everyone else thinks. Thanks for sharing this with all of us. I see that this part of the codebase uses XHR: https://github.com/bmihelac/django-cookie-consent/blob/master/cookie_consent/views.py#L50 If you are putting the template tag code inside of https://github.com/bmihelac/django-cookie-consent/blob/master/cookie_consent/templatetags/cookie_consent_tags.py then you will need a bunch of imports in that file based on your code:
Then in your template html insert: That was all I was able to figure out from your code. I will need you to clarify a few things to me so I could understand. Do we even need this code in the template that causes the timestamps to change?
We would need to change the expires here:
https://github.com/bmihelac/django-cookie-consent/blob/master/cookie_consent/util.py#L159 We could simply change the code to be this:
@bmihelac - thoughts? |
Took me a couple re-reads to understand what's happening, but the JS-improvement suggestions make sense to me.
The drawback is that the building and parsing of the cookie_str is now done in both Python and JS, and that code duplication may cause problems in the future if someone forgets to update both if changes are made. I'm not sure how we can defend against this. |
Sorry for the delay, I'm on a final stretch right now. @some1ataplace Currently I'm using Redis as a cache and I use this to delete all the cache: cache.delete_pattern('views.decorators.cache.cache_header.*')
cache.delete_pattern('views.decorators.cache.cache_page.*') I don't touch any django-cookie-consent files, I'm just adding my own code. I don't think I'll have time to work on that before long. The code I posted is more a proof of concept than anything else. @sergei-maertens |
Thanks for getting back - at least there should be sufficient pointers now for people who do have time to pick this up! |
Here are some changes to the above django template tag code based off @sergei-maertens feedback (please check): cookie_consent/templatetags/cookie_consent_tags.py:
cookie_consent/templates/cookie_consent/display.html:
Inclusion tag:
According to the docs, using format_html with mark_safe inside of it should be fine. As for json.dumps, I think removing json.dumps from https://docs.djangoproject.com/en/4.0/ref/utils/#django.utils.html.format_html https://adamj.eu/tech/2020/02/18/safely-including-data-for-javascript-in-a-django-template/ What your django code seems to be doing is practically identical to PR 59 where you are making the cookie_max_age a static non-changing value. The only difference is we are using a customizable inclusion tag template approach here rather than a permanent change approach like in PR 59 to prevent this from ever happening to someone who forgets to use your inclusion template tag. So regarding the cache, I am still a bit confused and just want to clear things up. You mean something like squid3 in linux for the proxy/page cache (forward or reverse proxy)? If you could share your squid configuration to reproduce the problem that would be great. < 8000 a href="https://www.youtube.com/watch?v=EDvx0AaEYQc" rel="nofollow">https://www.youtube.com/watch?v=EDvx0AaEYQc And on the django urls side you are doing this? https://docs.djangoproject.com/en/4.0/topics/cache/#specifying-per-view-cache-in-the-urlconf cache.delete_pattern seems to be part of django-redis: https://github.com/jazzband/django-redis#scan--delete-keys-in-bulk On the linux squid cache side and the redis cache side you are having problems in both places with the cache or just one place? Out of curiousity, how are you doing this: "Every time you change the content of page, you invalidated every caches that cache the page as whole at every levels". As for the javascript code, it seems like we can do this: tests/core/templates/test_page.html:
Then below this add in your javascript code with the following correction: You need an extra parenthesis here:
And for some reason, in display.html we cannot use These things in JS are undesirable so we may have to change a few things: |
I have reworked this, taking in account the comments. Making everything work with cache was a bit more complex than the part we discussed until now. Ideally the cookie groups list needs to be also moved to Javascript, but it's not essential. I currently have a complete solution that will be used on a site. But first, keep in mind that the work I did was with the optics of not modifying django-cookie-consent, so some parts could probably simplify if modifying cookie consent directly. So here's my last version of the code. Template tags from django import template
from django.conf import settings
from django.template.loader import render_to_string
from django.urls import reverse
from cookie_consent.cache import all_cookie_groups
from cookie_consent.models import Cookie, CookieGroup
from cookie_consent.templatetags.cookie_consent_tags import (
cookie_consent_accept_url,
cookie_consent_decline_url,
)
from cookie_consent.util import dict_to_cookie_str
register = template.Library()
@register.inclusion_tag('cookie_consent_script.html')
def add_cookie_consent(template_name):
"""
Used to add the cookie consent base data as a JSON to a page.
template_name takes the name of the template that will be used to create the banner.
Usage example in a template (usually the base template of the site):
{% load cookie_consent_tags %}
{% if request|cookie_consent_enabled %}
{% add_cookie_consent 'cookie_consent_banner.html' %}
{% endif %}
"""
cookie_groups_keys = list(all_cookie_groups().keys())
cookie_groups_values = all_cookie_groups().values()
content = render_to_string(
template_name,
{
'cookie_groups_names': ', '.join(
str(cookie_group) for cookie_group in cookie_groups_values
),
'url_accept': cookie_consent_accept_url(cookie_groups_values),
'url_decline': cookie_consent_decline_url(cookie_groups_values),
'url_cookies': reverse('cookie_consent_cookie_group_list'),
},
)
cookie_value_string = dict_to_cookie_str(
dict(
(cookie_group_key, settings.COOKIE_CONSENT_DECLINE)
for cookie_group_key in cookie_groups_keys
)
)
return {
'options': {
'content': content,
'cookie_groups': cookie_groups_keys,
'cookie_max_age': settings.COOKIE_CONSENT_MAX_AGE,
'cookie_value_string': f'{settings.COOKIE_CONSENT_NAME}={cookie_value_string}',
},
}
@register.inclusion_tag('cookie_consent_groups.html')
def cookie_consent_groups():
"""
Add the cookie consent cookie groups data as JSON to a page.
Example usage in a template:
{% load cookie_consent_tags %}
{% if request|cookie_consent_enabled %}
<div id="reglages-cookies" class="cookie-groups">
<h3>Cookie settings</h3>
{% cookie_consent_groups %}
</div>
"""
groups = list(
CookieGroup.objects.values(
'name', 'is_required', 'varname', 'description'
)
)
keyed_groups = dict((group['varname'], group) for group in groups)
for cookie in Cookie.objects.values(
'name', 'domain', 'description', 'cookiegroup__varname'
):
group = keyed_groups[cookie['cookiegroup__varname']]
if 'cookies' not in group:
group['cookies'] = []
group['version'] = CookieGroup.objects.get(
varname=cookie['cookiegroup__varname']
).get_version()
group['urlAccept'] = reverse(
'cookie_consent_accept', kwargs={'varname': group['varname']}
)
group['urlDecline'] = reverse(
'cookie_consent_decline', kwargs={'varname': group['varname']}
)
del cookie['cookiegroup__varname']
group['cookies'].append(cookie)
return {
'groups_info': {
'groups': groups,
'cookieConsentDecline': settings.COOKIE_CONSENT_DECLINE,
}
} Templates related to the tags They use cookie_consent_script.html (used by {% load static %}
<script type="text/javascript" src="{% static 'cookie_consent/cookiebar.js' %}"></script>
{{ options|json_script:"cookieConsentOptions" }} cookie_consent_groups.html (used by {{ groups_info|json_script:"cookieConsentGroups" }} cookie_consent_banner.html (pass to <div class="cookie-banner">
<p>The <strong>WEBSITE</strong> use cookies of {{ cookie_groups_names }} used to get statistics.</p>
<div class="button-group">
<button type="button" href="{{ url_accept }}" class="cc-cookie-accept">Accept all</button>
<button type="button" href="{{ url_decline }}" class="cc-cookie-decline">Decline all</button>
<a href="{% url 'site:pages' url='privacy' %}#cookie-settings">Cookie settings</a>
</div>
</div> Javascript The functions below could be added to cookiebar.js. The function document.addEventListener('DOMContentLoaded', function () {
const cookieConsentOptionsElement = document.getElementById(
'cookieConsentOptions'
)
if (cookieConsentOptionsElement !== null) {
import('./js/cookiebar.js').then(({ default: cookieConsent }) => {
cookieConsent()
})
}
} function booleanTrim(str, ch) {
return str.split(ch).filter(Boolean).join(ch)
}
function showCookieConsent(cookieConsentOptions) {
/*global showCookieBar*/
const expires = new Date(
Date.now() + cookieConsentOptions.cookie_max_age * 1000
).toUTCString()
const cookieValue =
cookieConsentOptions.cookie_value_string +
'; expires=' +
expires +
'; path=/'
const options = {
content: cookieConsentOptions.content,
cookie_groups: cookieConsentOptions.cookie_groups,
cookie_decline: cookieValue,
beforeDeclined: function () {
document.cookie = cookieValue
}
}
showCookieBar(options)
// Move the banner at the start of the body so it's first when tabbing
// (for accessibility).
document.body.prepend(document.querySelector('.cookie-banner'))
// WARNING: this block below is custom for my site.
// It should probably be a generic call where dev can hook their own logic to activate/deactivate
// their cookies.
//
// Manage Google Analytics and cookie groups checkboxes when choice is made
// from the cookie banner.
document
.querySelector('.cc-cookie-accept')
.addEventListener('click', (event) => {
event.preventDefault()
enable_analytics()
updateCookieCheckboxes(true)
})
document
.querySelector('.cc-cookie-decline')
.addEventListener('click', (event) => {
event.preventDefault()
disable_analytics()
updateCookieCheckboxes(false)
})
}
function cookieConsent() {
// Get the cookie consent options from the page.
const cookieConsentOptionsElement = document.getElementById(
'cookieConsentOptions'
)
if (cookieConsentOptionsElement === null) {
return
}
const cookieConsentOptions = JSON.parse(
cookieConsentOptionsElement.textContent
)
// Get the information from the cookie consent cookie.
const cookieConsentName = cookieConsentOptions.cookie_value_string.split(
'=',
1
)[0]
const consentCookie = document.cookie
.split(';')
.map((cookie) => cookie.trim())
.filter((cookie) => cookie.startsWith(cookieConsentName))[0]
let consentCookieKeysValues = null
// Show cookie consent banner if there's no cookie.
if (consentCookie === undefined) {
showCookieConsent(cookieConsentOptions)
// If there's informations in the cookie, check for a cookie groups
// mismatch between options and cookie informations and show the cookie
// consent banner if there's one.
} else {
consentCookieKeysValues = booleanTrim(
consentCookie.substring(cookieConsentName.length + 1),
'"'
).split('|')
// WARNING: this block below is custom for my site (I'm using Google Consent Mode
// https://developers.google.com/tag-platform/devguides/consent)
// It should probably be a generic call where dev can hook their own logic to activate/deactivate
// their cookies.
//
// On page load, activate Google Analytics if it's enable in the consent's
// cookie.
for (let keyValues of consentCookieKeysValues) {
if (
keyValues.trim().startsWith('analytics=') &&
keyValues !== 'analytics=-1'
) {
enable_analytics()
break
}
}
// Check for a mismatch and show banner if there's one.
for (let cookieGroup of cookieConsentOptions.cookie_groups) {
if (
!consentCookieKeysValues.some((item) =>
item.trim().startsWith(cookieGroup + '=')
)
) {
showCookieConsent(cookieConsentOptions)
break
}
}
}
// WARNING: this block is partly custom for my site. Ideally it should be deal more generically,
// maybe by just removing the body classList check, showCookieConsentGroups is loaded only when the
// cookieConsentGroups JSON data is present anyway.
//
// If we are on the cookie settings page show the settings.
if (document.body.classList.contains('cookie-page')) {
showCookieConsentGroups(consentCookieKeysValues)
}
}
function showCookieConsentGroups(consentCookieKeysValues) {
// Get the cookie consent groups informations from the page.
const groupsInfoElement = document.getElementById('cookieConsentGroups')
if (groupsInfoElement === null) {
return
}
// Create the groups interface.
const groupsContainer = document.createElement('ul')
const groupsInfo = JSON.parse(groupsInfoElement.textContent)
for (let group of groupsInfo.groups) {
const groupContainer = document.createElement('li')
const disabled = group.is_required ? ' disabled' : ''
// Check if group is enable.
let cookieIsEnable = false
if (consentCookieKeysValues) {
const groupCookie = consentCookieKeysValues.filter((cookie) =>
cookie.startsWith(group.varname + '=')
)[0]
if (groupCookie !== undefined) {
let version = groupCookie.split('=')[1]
if (
version &&
version !== groupsInfo.cookieConsentDecline &&
version >= group.version
) {
cookieIsEnable = true
}
}
}
const checked = disabled || cookieIsEnable ? ' checked="checked"' : ''
// Maybe this should be customizable somehow?
groupContainer.innerHTML = `
<h4>
<div class="switch">
<input class="switch-input cookie-consent__checkbox"${disabled}${checked} id="cookie-${group.varname}" type="checkbox" name="cookie-${group.varname}" data-url-accept="${group.urlAccept}" data-url-decline="${group.urlDecline}">
<label class="switch-paddle" for="cookie-${group.varname}">
<span class="show-for-sr">${group
8000
.name}</span>
</label>
</div>
${group.name}
</h4>
<p>${group.description}</p>
`
// Add the group cookies.
const cookiesContainer = document.createElement('dl')
for (let cookieIndex in group.cookies) {
const cookie = group.cookies[cookieIndex]
let name = cookie.name
if (cookie.domain) {
name += ` (${cookie.domain})`
}
cookiesContainer.innerHTML += `
<dt>${name}</dt>
<dd>${cookie.description ? cookie.description : ' '}</dd>
`
}
groupContainer.append(cookiesContainer)
groupsContainer.append(groupContainer)
}
groupsInfoElement.after(groupsContainer)
// Update cookies on checkboxes change.
const checkboxes = document.querySelectorAll(
'.cookie-consent__checkbox:not([disabled])'
)
for (let checkbox of checkboxes) {
checkbox.addEventListener('change', function () {
let url
if (checkbox.checked) {
url = checkbox.dataset.urlAccept
} else {
url = checkbox.dataset.urlDecline
}
// Fetch can be use here instead of axios.
axios
.post(url, {})
.then((response) => {
// WARNING: custom to my site, It should probably be a generic call where dev can hook
// their own logic to activate/deactivate their cookies.
//
// Enable or disable Google Analytics when analytics checkbox
// is changed.
if (response && response.status === 200) {
if (checkbox.name === 'cookie-analytics') {
if (checkbox.checked) {
enable_analytics()
} else {
disable_analytics()
}
}
}
})
.catch((error) => {
console.error(error)
})
})
}
} @some1ataplace For most of your questions regarding the cache, my point of view is that it doesn't matter which cache you use, caches work mostly all on the same principals if correctly set: you cache something so you will not have to build it or request it again to speed things up, but anytime the thing you have cache is changed, the new version needs to replace the previous version in the cache. If a page changes on each call, it can't be cache (or in other words there's no use to cache it). So you also need to be sure that the cache system is correctly set and that means that every time the thing you cache changes, you need to be sure that the cache is flushed or updated. |
Hi @etienned - I've just released You can install it with Can you let me know if this works for you and resolves the issue? If not, please let me know what issues you are still facing so we can tidy this up in a proper non-beta release! |
Hi @sergei-maertens - Thanks for this work! I will try to test it in the next few weeks and report back. |
hi @etienned - have you run into any issues? :) |
Hi @sergei-maertens - sorry for not coming back sooner. A month ago I did test version 0.5.0b0 and for the most part, everything was working as supposed and in a elegant way. Great work! The only part that was problematic for me, but I didn't finished my tests and investigations because other priorities took precedence, was the parameters page. On my site, the parameters are only a part of a more general info page and, in the time I put on this problem, I was not able to integrate the parameters section with working buttons that activate or deactivate the setting on the spot without reloading any page. Not sure that I'm pretty clear here? Sorry for that. I'll try to rework on this this week or the next and I'll let you know. |
Hello @sergei-maertens, I'm mostly there, my parameters page is now working fine. Now I'm working on cleaning up everything and, for this, I wish I could import import {showCookieBar} from '?????/cookie_consent/cookiebar.module.js' I wish to use |
I have the same un-answered problem. I have been considering publishing the JS to NPM and then loading that asset in the Python package, but I'm also not sure about that approach. Other ideas I have is trying to tell the frontend toolchain where additional modules can be found, but that also seems hard to get working reliably without needing constant maintenance, as virtual env paths often differ between environments (or don't exist at all like in docker images) I'm open for ideas/suggestions and general information on how Vite configuration works. Personally I'm only familiar with TypeScript TSC config and webpack. |
I saw some Django projects using the approach of publishing the JS to NPM. I think it's the simplest way. Look at https://github.com/django-webpack/django-webpack-loader for an example. Telling the frontend toolchain where the module can be found seem really fragile to me for many reasons. Unfortunately, I don't have any other ideas. As for Vite, the config is relatively similar to Webpack, nothing really special. |
Just a quick update that I'm working on this in #119, however I'm blocked by #120 for this. Publishing to Github packages is a dead end too. If things don't resolve in the next few days (which they look like they won't), I think I will publish the npm package under my personal account after all (outside of the CI pipeline unfortunately) to hold us over until we have an "official" account to publish the package under. I'm quite satisfied with the technical approach at the repo-level to track the assets. The static assets in the Python package are also not going to be affected by this, so the |
This PR was merged and there is now a django-cookie-consent package on NPM, making it possible to import showCookieBar in your own stack (Vite, webpack, rollup...). This also gives sufficient control to get the list of cookiegroups from and endpoint (you'll have to build this yourself though) instead of relying on the template tag. As long as you then add this to the page before calling I'm open to tweaks in the JS API, since this suggestion requires some unnecessary JSON serialize/deserialize roundtrips, but I think everything is now in place. |
Hi @sergei-maertens, I finally got time to do the integration with the npm package. Mostly everything works as expected but I had to import the npm package like this: import { showCookieBar } from 'django-cookie-consent'; instead as specified in the docs: import {showCookieBar} from '@jazzband/django-cookie-consent'; If I check in the About the |
That sounds right - unfortunately I haven't been able to sort out anything jazzband related on npm so it's published under the package name you mention. I'll make sure to check and fix the documentation! Thank your for the feedback |
I'm interested in using this project to deal with the GDPR, etc. but, unfortunately, it seems to not be compatible with Django page cache that I'm using intensively, as most of the pages of my site don't really change often. The problem I see is that it need to write the expires (that changes all the time) on each request modifying the page every time.
I see two solutions:
showCookieBar
directly in the page, make a XHR to get those values.For my project I implemented the second, because I prefer to avoid the supplemental XHR. Firstly, I made a template tag that add all the necessary JS in the page. I then made a function in a JS file that check the presence of the cookie and if some groups have changed to show or not the cookie bar, and then calculate the expires and call
showCookieBar
when needed.Template tag
Javascript functions
Is good to note that with the second solution, as I implemented it, if I'm adding/modifying/removing a cookie or a cookie group, I need to flush the page cache because the "stable" options values written in the page will changed.
The text was updated successfully, but these errors were encountered: