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

Support %(app_label)s and %(class)s in related_name #737

Open
thenewguy opened this issue Jun 18, 2021 · 5 comments
Open

Support %(app_label)s and %(class)s in related_name #737

thenewguy opened this issue Jun 18, 2021 · 5 comments

Comments

@thenewguy
Copy link

I am trying to add TaggableManager to an Abstract model. I've added a related name like related_name='%(app_label)s_%(class)s_tags'. This gives an error like The name '%(app_label)s_%(class)s_tags' is invalid related_name for field with HINT: Related name must be a valid Python identifier or end with a '+'

Per the Django docs, this is a valid form of related_name.

Shouldn't TaggableManager be performing the string format operation? At a glance this seems like it would work just fine with generic object tagging

@rtpg
Copy link
Contributor

rtpg commented Jun 19, 2021

OK so I narrowed this down to the fact that TaggableManager is using a totally custom contribute_to_class. In Django's RelatedField.contribute_to_class method we have

            if related_name:
                related_name = related_name % {
                    'class': cls.__name__.lower(),
                    'model_name': cls._meta.model_name.lower(),
                    'app_label': cls._meta.app_label.lower()
                }
                self.remote_field.related_name = related_name

Ideally I would like for us to just find a way to use contribute_to_class directly, but even a patch that would just copy over this behavior could be pulled into the project.

@rtpg rtpg added the bug label Jun 19, 2021
@rtpg
Copy link
Contributor

rtpg commented Jun 19, 2021

Labelling a bug since we are a RelatedField sublcass, so should behave as such when possible.

@thenewguy
Copy link
Author

Haven't used this past an initial investigation but it does seem to be the solution:

class AbstractTaggableManager(TaggableManager):
    def contribute_to_class(self, cls, name):
        self.opts = cls._meta

        if not cls._meta.abstract:
            if self.remote_field.related_name:
                related_name = self.remote_field.related_name
            else:
                related_name = self.opts.default_related_name
            if related_name:
                related_name = related_name % {
                    "class": cls.__name__.lower(),
                    "model_name": cls._meta.model_name.lower(),
                    "app_label": cls._meta.app_label.lower(),
                }
                self.remote_field.related_name = related_name
        
        super().contribute_to_class(cls, name)

@thenewguy
Copy link
Author

As far as testing goes - would it be good enough to simply define an abstract and concrete implementation in the models? System checks seem to throw an error with a related name like related_name="%(app_label)s_%(model_name)s_%(class)s_tags" without this quickfix

@rtpg
Copy link
Contributor

rtpg commented Mar 10, 2022

@thenewguy if you want to take this on, I think the straightforward way would be to just edit one of the existing test models with a related name like what you are discussing, then we can add a quick test/modify an existing one to check that the related name accessor works.

For example one of the models in this file, and editing a test.

And of course for actually getting the bug fixed, just reworking the TaggableManager contribute_to_class function!

Would you be interested in making a PR with your changes? Even just having that as a starting point would be helpful. And if you're not interested, I can look at making a PR instead. But I think you understand the problem well

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

2 participants