adamchainz / django-upgrade

Automatically upgrade your Django projects.
MIT License
1.02k stars 64 forks source link

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

Open UnknownPlatypus opened 1 year ago

UnknownPlatypus commented 1 year ago

Description

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

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 commented 1 year ago

Yeah good point, would be nice to fix that.

UnknownPlatypus commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 9 months ago

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.