Tags longer than 100 chars raise 500 error

[halitcelik]

In wagtail admin where django-taggit is being used for documents this error happens. It is mentioned in this wagtail issue. Locally I could fix the issue changing these lines to:

def clean(self, value):
        value = super().clean(value)
        try:
            for tag in value.split(","):
                if len(tag) > 100:
                    raise forms.ValidationError(
                _("One or more tag(s) exceed max length (100 chars).")
            )
        except ValueError:
            forms.ValidationError(
                _("Please provide a comma-separated list of tags.")
            )
        try:
            return parse_tags(value)
        except ValueError:
            raise forms.ValidationError(
                _("Please provide a comma-separated list of tags.")
            )

I am not sure if this is the correct way to fix it. Please let me know if it is I can create a PR.

[Temidayo32]

@halitcelik are you also getting this error working off your development server?

[halitcelik]

@Temidayo32 I saw the discussions in wagtail issues. So, maybe we can discuss possible solutions. I'm happy to help if I can.

[Temidayo32]

@halitcelik I would be interested in that

[halitcelik]

@Temidayo32 I'm already halfway through for a PR. It would be good to get the opinion of the maintainer.

[Temidayo32]

@halitcelik Great!

[Temidayo32]

@halitcelik I made a pull request that resolves this in Wagtail. Hopefully, it get merged!

https://github.com/wagtail/wagtail/pull/11119

[rtpg]

@halitcelik this is a bit tricky, because the max length in TagBase of 100 is not really accessible way over in TagField.

What I think would be good is the following:

• Change TagField to take an optional parameter in the constructor, max_tag_length, defaulting to 100
• Use that in the validator like in the snippet you have (perhaps making it so that max_tag_length of None opts out of the validation)
• Noting that there's an extra validator on TagField in the changelog (and setting to None is good)

This would make the field work "as expected" out of the box, and offer a straightforward way forward for people upgrading in existing setups.

Tell me if you're up for the PR! If you have a WIP set of changes, just pushing that up is also fine (I can make any final changes to get it to work if you want)

[halitcelik]

@rtpg Thanks a lot. I'll create a PR. I thought this would not break the existing setups because we already have a database validation for 100 max length.

[halitcelik]

@rtpg I created a PR: https://github.com/jazzband/django-taggit/pull/874 I am a bit confused about passing None in the Tagfield max_tag_length. If somebody sets it to None and we skip validation, it will still raise 500 in a later step. What do you think?

[BertrandBordage]

@rtpg You mentioned:

this is a bit tricky, because the max length in TagBase of 100 is not really accessible way over in TagField.

What do you mean, it's not accessible? Can't we do this?

Then no need to worry about None. The max_length should always be set, unless in the future we have some way of defining TagBase.name as a TextField using a swappable model :thinking:

[rtpg]

@BertrandBordage well, you can customize the through model in TaggableManager and just show up with another tag model entirely (that just follows a similar API, but might back the name with a text field or something else).

This is one of those things where I wouldn't be surprised if nobody is actually in this scenario, but I would also not be surprised if someone was in this scenario, so I like a PR that handles both cases. Having said that.... I dunno, maybe that's a bit too wild of a concept

[BertrandBordage]

@rtpg I see, if you override name in a custom TagBase descendant and rewrite tagged items to use it. It's a shame, I thought only tagged items were generally customized 😕

[BertrandBordage]

TaggableManager has a method for generating a form field, and it has access to the relation, meaning it can introspect the tag model. Can't we use that, combined with a new max_tag_length constructor parameter for TagField? Of course, we would pass max_tag_length only if issubclass(form_class, TagField).

And in the doc, we would mention the need to fill max_tag_length when TagField is used manually, outside of the context of TaggableManager.

[hamidrabedi]

Same issue here with drf:

class BlogSerializer(TaggitSerializer, serializers.ModelSerializer):
    tags = TagListSerializerField()
    class Meta:
        model = Blog
        fields = (
            "tags",
        )

and when i try to add one tag with more that 100 character:

django.db.utils.DataError: value too long for type character varying(100)

[29deepanshutyagi]

i want to solve this issue , if this issue is still open @Temidayo32 and if possible kindly gave me required information to solve this issue