adamchainz / django-upgrade

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

Make `@admin.register` fixer skip duplicately named admin classes #471

Closed adamchainz closed 3 months ago

adamchainz commented 3 months ago

Currently the fixer does this:

 class BookAdmin(admin.ModelAdmin):
     ...

 class BookAdmin(admin.ModelAdmin):
     ...

- admin.site.register(Book, BookAdmin)

The registration is unsafely removed.

UnknownPlatypus commented 3 months ago

Hum that's weird, I tried to add the following test and it passed :thinking:

def test_duplicated_class_definition():
    check_transformed(
        """\
        from django.contrib import admin

        class BookAdmin(admin.ModelAdmin):
            pass

        class BookAdmin(admin.ModelAdmin):
            pass

        admin.site.register(Book, BookAdmin)
        """,
        """\
        from django.contrib import admin

        @admin.register(Book)
        class BookAdmin(admin.ModelAdmin):
            pass

        @admin.register(Book)
        class BookAdmin(admin.ModelAdmin):
            pass

        """,
    )
adamchainz commented 3 months ago

Oh, thank you for trying. I simplified it out of a project I was just helping out on. I will check to see if there are any other relevant details I missed.

adamchainz commented 3 months ago

Right, the failure was actually:

 class BookAdmin(admin.ModelAdmin):
     ...

- admin.site.register(Book, BookAdmin)

 class BookAdmin(admin.ModelAdmin):
     ...  

I’m going to adjust the fixer to bail when a duplicate definition is found.