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

Why is 'through' in TagField.forbidden_fields? #33

Open
fdemmer opened this issue Sep 18, 2017 · 5 comments
Open

Why is 'through' in TagField.forbidden_fields? #33

fdemmer opened this issue Sep 18, 2017 · 5 comments

Comments

@fdemmer
Copy link

fdemmer commented Sep 18, 2017

Thanks for another tagging library and i am not being sarcastic!

"Based on ForeignKey and ManyToManyField, so it's easy to query" is a big selling point for me, so I was surprised to see, that giving a custom through model is not possible.

When I remove the code "forbidding" that, all I need still seems to work. What is the reason behind forbidding custom through models? What side-effects should I be aware of when circumventing the check?

@radiac
Copy link
Owner

radiac commented Sep 19, 2017

Hi, and thanks - hope you find it useful!

I originally set through as a forbidden field because when it's updating tag relationships, Tagulous used to merrily go around clearing the through relationships at will, so it would have been hard to guarantee the integrity of any additional data in the through table.

The save action has changed in recent releases though, so it looks likes it should be safe now - but I'd still be wary of storing anything important in a through model at the moment until there are some comprehensive tests around it. See where it updates the through model: https://github.com/radiac/django-tagulous/blob/develop/tagulous/models/managers.py#L490

This isn't something I've looked at testing or supporting, because I couldn't see any situation where you'd be able to attach any meaningful information to the through table, given the TagField goes to great lengths to abstract the actual mechanics of the M2M to allow for syntax like mymodel.tags = 'foo, bar' etc.

Can you give an example of what sort of thing we might want to do with a through model? If it's something people are going to want, it may just be a case of writing some tests and allowing the param through.

@fdemmer
Copy link
Author

fdemmer commented Sep 19, 2017

thank you for the fast and detailed response :)

my requirement is, ordering of tags per assignment. the tags are used as "genres" and some things are "more" one genre, than another, so they need to be ordered. i am not using the TagField directly in the admin, but inlines for the through model to assign them and set the ordering.

@radiac radiac closed this as completed Apr 30, 2018
@mjpieters
Copy link

Can you tell me more about the status of this? I have exactly this requirement; the tags attached to a given instance of a model prescribe additional information that you can add to that instance, so

object tag description image
foo spam foo uses spam to ... [image]
bar spam bar just loves spam, because ... [image]

I was hoping to use a through model for this, rather than having to manage an extra relationship that's nothing more than a (object_id, tag_id, tag_extras) tuple.

@mjpieters
Copy link

Just to document this for others, here is a sample model based on what I'm working with at the moment, and how I removed the restriction on setting a through table:

from django.db import models
from tagulous.models import TagField, TagTreeModel

# allow setting a through table on tag fields
TagField.forbidden_fields = tuple(
    v for v in TagField.forbidden_fields if v != "through"
)

class PageTag(TagTreeModel):
    class TagMeta:
        initial = [
            t.strip()
            for t in """\
            Lorum Ipsum/Dolor Sit Amet
            Lorum Ipsum/Consectetur Adipiscing Elit
            Lorum Ipsum/Sed Do Eiusmod
            At Vero Eos/At Accusamus
            At Vero Eos/Et Iusto Odio
        """.splitlines()
            if t.strip()
        ]
        autocomplete_initial = True
        max_count = 8

    icon = models.fields.URLField()

class Page(models.Model):
    title = models.CharField(max_length=100)
    slug = models.SlugField()
    page_descriptions = TagField(PageTag, through="PageTagDescription")

class PageTagDescription(models.Model):
    class Meta:
        # a page can only pick any tag once
        unique_together = (("page", "page_tag"),)

    page = models.ForeignKey(Page, on_delete=models.CASCADE)
    page_tags = models.ForeignKey(PageTag, on_delete=models.CASCADE)

    description = models.TextField()
    image = models.ImageField(null=True)

with admin.py registrations:

import tagulous
from django.contrib import admin
from .models import Page, PageTag, PageTagDescription

class PageTagAdmin(tagulous.admin.TagTreeModelAdmin):
    list_display = ['name', 'count', 'protected', 'icon']

class PageTagDescriptionInline(admin.StackedInline):
    model = PageTagDescription
    extra = 1

class PageAdmin(admin.ModelAdmin):
    inlines = (PageTagDescriptionInline,)
    exclude = ('page_tags',)
    prepopulated_fields = {"slug": ("name",)}

admin.site.register(PageTag, PageTagAdmin)
tagulous.admin.register(Page, PageAdmin)

@radiac
Copy link
Owner

radiac commented May 19, 2019

Thanks for letting me know. It looks like I closed this task prematurely and I'd forgotten this was an issue. I'll reopen so I remember to take a look at this before the next release.

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