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

Add @admin.display decorators in models file too #389

Open
UnknownPlatypus opened this issue Sep 24, 2023 · 5 comments
Open

Add @admin.display decorators in models file too #389

UnknownPlatypus opened this issue Sep 24, 2023 · 5 comments

Comments

@UnknownPlatypus
Copy link
Contributor

UnknownPlatypus commented Sep 24, 2023

Description

I think the display fixer doesn't support this use case from the docs:

  • A string representing a model attribute or method (without any required arguments).

Would be nice to have it work in that case too:

+from django.contrib import admin
from django.db import models

class Person(models.Model):
    name = models.CharField(max_length=50)
    birthday = models.DateField()

+   @admin.display(description='Birth decade')
    def decade_born_in(self):
        return '%d’s' % (self.birthday.year // 10 * 10)
-   decade_born_in.short_description = 'Birth decade'
@adamchainz
Copy link
Owner

Yeah good point, would be nice to fix that.

@UnknownPlatypus
Copy link
Contributor Author

I'm not quite sure how to gracefully handle adding the from django.contrib import admin import if missing. Maybe you have an idea how to do it ?

@adamchainz
Copy link
Owner

I have ideas but haven’t implemented anything. I gave up on adding imports in the timezone.utc fixer.

Something like “add after the last import statement at the top of the file” could work. We’d probably want to check that there’s nothing called admin in the module...

@UnknownPlatypus
Copy link
Contributor Author

UnknownPlatypus commented Nov 1, 2023

add after the last import statement at the top of the file

This seems manageable, finding the last import in the module is not too hard. Doing that we can also check for possible import with the same name

We’d probably want to check that there’s nothing called admin in the module...

I feel like this cost a lot, we would have to check every import, every variable assignment, every function/class definition and if there is any wildcard import, this is unsafe (but I'd say we can ignore this case). I'm not sure what do do here

@adamchainz
Copy link
Owner

I agree it would be a bit costly, but we’d only need to do it in the case where we’d like to add the import. Anyway, this particularly fixer feels somewhat niche, at least in my experience I haven’t seen admin methods on models.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants