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

Allow storing additional metadata #34

Open
9mido opened this issue Oct 28, 2020 · 5 comments
Open

Allow storing additional metadata #34

9mido opened this issue Oct 28, 2020 · 5 comments

Comments

@9mido
Copy link

9mido commented Oct 28, 2020

While navigating other websites and how they setup their cookie pages I see the following things:

  1. Last updated all cookies on X date (maybe this could be tied to each group or all groups) - since third parties could add/remove cookies that they use over time.
  2. Expiration/Lifespan/Duration for each cookie
  3. Type for each cookie (Essential / Strictly Necessary, Targeting/Advertising, Analytics / Performance, Unclassified, etc)
  4. First party or third party for each cookie
  5. A <details> tag to save page space (optional - nice to have, but not sure if lawyers would like it you are "hiding" them unless clicked to be revealed)

Maybe adding these things to the database as non-required fields would help maintenance via admin and give users the option to include them or not. Otherwise, not a big deal if no PR is made since this stuff could be added to the cookie description as their own sentence or a programmer could make changes by overriding the templates.

There may be laws that require these small pieces of information to be stated which is why I bring it up but I am not a lawyer so not entirely sure. Erroring on the side of caution here. Plus hopefully helping others out by giving them more info that they might want to include.

Example:
https://stripe.com/cookie-settings
https://stripe.com/cookies-policy/legal

@sergei-maertens
Copy link
Collaborator

I think I'd opt for a swappable model here so users can override the models/templates as needed for their specific legislation.

Tentatively putting this for the 1.0 release, as that would solidify the public API.

@sergei-maertens sergei-maertens added this to the Release 1.0 milestone Sep 4, 2022
@some1ataplace
Copy link
Contributor

some1ataplace commented Mar 23, 2023

https://github.com/openwisp/django-swappable-models

To create swappable models for the Django Cookie Consent app, you can follow these steps:

  1. Create a new Django app in your project called cookie_consent_custom or something similar.
  2. Create a models.py file in the cookie_consent_custom app with the following code:
from django.db import models
from cookie_consent.models import CookieGroup, Cookie

class CustomCookieGroup(CookieGroup):
    pass

class CustomCookie(Cookie):
    pass
  1. In your project's settings.py file, add cookie_consent_custom to the INSTALLED_APPS list.

  2. Add the following lines to your settings.py file to make the CookieGroup, Cookie, and CookieConsent models swappable:

COOKIE_CONSENT_CUSTOM_COOKIE_GROUP_MODEL = 'cookie_consent_custom.CustomCookieGroup'
COOKIE_CONSENT_CUSTOM_COOKIE_MODEL = 'cookie_consent_custom.CustomCookie'
COOKIE_CONSENT_CUSTOM_COOKIE_CONSENT_MODEL = 'cookie_consent_custom.CustomCookieConsent'
  1. Run python manage.py makemigrations and python manage.py migrate to create the new database tables for the swappable models.

You might run into some errors when doing this yourself:

django.core.exceptions.FieldError: Local field 'name' in class 'CookieGroup' clashes with field of the same name from base class 'CookieGroup'.

The error message you are seeing is because you are trying to define a field in your custom model with the same name as a field in the base model. This is not allowed in Django because it would cause a conflict when trying to access the field.

To avoid this error, you should give your custom field a different name. For example, if you want to create a custom CookieGroup model with a field for the group's name, you could define it like this:

from django.db import models
from cookie_consent.models import CookieGroup, Cookie, CookieConsent

class CustomCookieGroup(CookieGroup):
    custom_name = models.CharField(max_length=255)

In this example, the custom field is named custom_name instead of name, which avoids the conflict with the base CookieGroup model's name field.

You can do the same for any other fields you want to customize in your swappable models. Just make sure to give them unique names to avoid conflicts with the base models.

To keep some of the model fields from the original model with the same name in the new model when creating swappable models in Django, you can use the db_column attribute in your custom model's field definition.

For example, let's say you want to create a custom CookieGroup model that adds a new field called description, but you want to keep the original name field with the same name. You could define the custom model like this:

from django.db import models
from cookie_consent.models import CookieGroup, Cookie, CookieConsent

class CustomCookieGroup(CookieGroup):
    description = models.CharField(max_length=255)
    name = models.CharField(max_length=255, db_column='name')

In this example, the name field in the custom model has the same name as the name field in the base CookieGroup model, but it uses the db_column attribute to specify a different database column name. This allows you to keep the same field name in your code while avoiding conflicts with the base model's field.

You can use the db_column attribute in the same way for any other fields you want to keep with the same name in your custom models.

cookie_consent_custom.Cookie.cookiegroup2: (fields.E304) Reverse accessor 'CookieGroup.cookie_set' for 'cookie_consent_custom.Cookie.cookiegroup2' clashes with reverse accessor for 'cookie_consent.Cookie.cookiegroup'.
    HINT: Add or change a related_name argument to the definition for 'cookie_consent_custom.Cookie.cookiegroup2' or 'cookie_consent.Cookie.cookiegroup'.

The error message you are seeing is because you have two models, Cookie and cookie_consent_custom.Cookie, that both have a foreign key to the CookieGroup model. Django is trying to create a reverse accessor for each of these foreign keys, but the default name for the reverse accessor clashes between the two models.

To fix this error, you can add a related_name argument to one or both of the foreign key fields. For example, you could add a related_name to the cookiegroup field in the Cookie model like this:

from django.db import models
from cookie_consent.models import CookieGroup

class Cookie(models.Model):
    cookiegroup = models.ForeignKey(
        CookieGroup,
        on_delete=models.CASCADE,
        related_name='custom_cookies',
    )

In this example, the related_name argument is set to 'custom_cookies', which creates a new reverse accessor for the CookieGroup model that won't clash with the default reverse accessor created by the cookie_consent app.

You can do the same for the cookiegroup2 field in the cookie_consent_custom.Cookie model, or choose a different related_name value for each field to avoid conflicts.

Example Swappable Model:

cookie_consent_custom models.py:

from django.db import models
from cookie_consent.models import CookieGroup, Cookie

class CookieType(models.Model):
    name = models.CharField(max_length=20)
 
    def __str__(self):
        return self.name

    class Meta:
        verbose_name = 'Cookie Type'
        verbose_name_plural = 'Cookie Types'
        ordering = ('name',)

class CookieGroup(CookieGroup):
    name2 = models.CharField('Name', max_length=100, blank=True, db_column='name')
    description2 = models.TextField('Description', blank=True, db_column='description')
    is_required2 = models.BooleanField('Is required',help_text='Are cookies in this group required.',default=False, db_column='is_required')
    is_deletable2 = models.BooleanField('Is deletable',help_text='Are cookies in this group deletable.', default=True, db_column='is_deletable')
    ordering2 = models.IntegerField('Ordering', default=0, db_column='ordering')
    created2 = models.DateTimeField('Created', auto_now_add=True, blank=True, db_column='created')
    updated = models.DateTimeField('Updated', blank=True) 

    class Meta:
        verbose_name = 'Cookie Group'
        verbose_name_plural = 'Cookie Groups'
        ordering = ('ordering',)

    def __str__(self):
        return self.name


class Cookie(Cookie):
    cookiegroup2 = models.ForeignKey(CookieGroup,on_delete=models.CASCADE, db_column='cookiegroup', related_name='cookiegroup')
    cookietype2 = models.ForeignKey(CookieType, default=1, on_delete=models.SET_NULL, null=True, db_column='cookietype')
    name2 = models.CharField('Name', max_length=256, db_column='name')
    description2 = models.TextField('Description', blank=True, db_column='description')
    path2 = models.TextField('Path', blank=True, default="/", db_column='path')
    domain2 = models.CharField('Domain', max_length=256, blank=True, db_column='domain')
    duration = models.CharField('Duration', max_length=256)
    created2 = models.DateTimeField('Created', auto_now_add=True, blank=True, db_column='created')

    class Meta:
        verbose_name = 'Cookie'
        verbose_name_plural = 'Cookies'

    def __str__(self):
        return "%s %s%s" % (self.name, self.domain, self.path)

cookie_consent_custom admin.py:

from django.contrib import admin

from .models import CookieGroup, Cookie, CookieType

class CookieTypeAdmin(admin.ModelAdmin):
    readonly_fields = ('id',)
    list_display_links = ('name',)
    list_display = ('id', 'name',) 
    search_fields = ('id', 'name',)
    ordering = ('id',)

class CookieGroupAdmin(admin.ModelAdmin):
    readonly_fields = ('id',)
    list_display_links = ('name',)
    list_display = ('id', 'name', 'description', 'is_required', 'is_deletable', 'ordering', 'created',) 
    search_fields = ('id', 'name',)
    ordering = ('id',)

class CookieAdmin(admin.ModelAdmin):
    readonly_fields = ('id',)
    list_display_links = ('cookiegroup',)
    list_display = ('id', 'cookiegroup', 'cookietype2', 'name', 'description', 'path', 'domain', 'duration', 'created',) 
    search_fields = ('id', 'name',)
    ordering = ('id',)

admin.site.register(CookieType, CookieTypeAdmin)
admin.site.register(CookieGroup, CookieGroupAdmin)
admin.site.register(Cookie, CookieAdmin)

@sergei-maertens sergei-maertens mentioned this issue Jun 11, 2023
4 tasks
@sergei-maertens
Copy link
Collaborator

@9mido while looking at libraries for swappable models, I'm not entirely confident that's a responsible decision since I'm not seeing much activity. Given that Django now supports JSONField on all major databases, would that be an escape hatch that can work?

  • You can override the admin form and register your own custom form that writes to keys of the JSON Field
  • You can provide your own templates to display the additional data from the JSON Field
  • If necessary, you can do proper queries on the data inside

What do you think?

@some1ataplace
Copy link
Contributor

@sergei-maertens What if the user needed more than 1 JSON Field? It may take a beginner a lot longer trying to figure out what you said to do in your bullet points compared to simply adding a few fields in the django model like most beginners know how to do. I do think using a JSON Field is a creative way of doing this that I would have never thought was possible. But a swappable model and a JSON Field each have pros and cons. Maybe we can have both?

Pros of JSONField:

  1. Flexibility: Store flexible and variable data structures.
  2. Simplified Schema Changes: Add or remove fields dynamically.
  3. Querying and Manipulation: Convenient querying and manipulation methods.
  4. Easy Integration: Seamless integration with Django's database infrastructure.
  5. Atomic Updates: Support for efficient in-place modifications.
  6. Space Efficient: More space efficient than a related table if data doesn't repeat.

Cons of JSONField:

  1. Limited Database Compatibility: Not available in all database backends.
  2. Performance Considerations: Performance implications with complex queries or large objects.
  3. Lack of Schema Validation: No enforcement of data integrity and validation.
  4. Lack of Referential Integrity: Cannot enforce foreign key relationships.

Pros of Swappable Model:

  1. Extensibility: Highly customizable and extendable architectures without making huge changes to your codebase.
  2. Ease of customization: Easy customization of Django User model.
  3. Modular Architecture: Fits well into modular designs and pluggable apps.

Cons of Swappable Model:

  1. Increased Complexity: Adds complexity to the project (relationships, maintenance, etc.)
  2. Potential for Errors: Problems if swapping is not handled correctly.
  3. Database Migration: Challenging migration process, potential for data loss.

@sergei-maertens
Copy link
Collaborator

What if the user needed more than 1 JSON Field?

That's a bit weird imo - the point is that you can decide what your data model is and drop it in there as you see fit. The data for two fields would easily fit into a single JSON field if you use a dict to structure/serialize that data with a key that's recognizable for yourself.

... too simply adding a few fields in the django model

There are essentially two ways to be able to do that:

  1. the fields are added to the models shipped in the library. I'm worried this is a slippery slope - there'll be always be yet another field that's relevant for someone's use case and we can't keep adding all fields everyone requests.
  2. we make the model swappable, so that users can add their own fields. This comes with a serious technical cost as this now needs to be maintained, migrations need to be managed, there is a risk of us adding future fields that will break other people's models... and it's using a private internal Django API. While it has been stable, I try to avoid using internal Django API because of the potential maintenance burden.

So I think I'd need some solid examples of requirements that wouldn't work with a JSONField to justify the drawbacks of swappable models. The original examples in this post give pure metadata examples that do not require integrity guarantees (like foreign keys provide) and would easily be achieved with JSONField.

Combined with the admin, you can (with a little bit of work) define your own form for the admin that makes it seem like they are actual separate fields which handles saving the data into the proper "slots" in the JSONField (that would be a good documentation page/recipe!), which handles your input validation for you.

@sergei-maertens sergei-maertens changed the title Small Tweaks Allow storing additional metadata Sep 24, 2023
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

3 participants