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 custom admin classes in @admin.register fixer #228

Merged

Conversation

UnknownPlatypus
Copy link
Contributor

@UnknownPlatypus UnknownPlatypus commented Sep 18, 2022

For #190

Hey, so I started working on the custom admin support and I had a few interrogations to finish it.

  1. Is a stacked syntax ok when the same admin model is used to register a model against different sites ?
class MyAdminSite(admin.AdminSite):
    site_header = "My very custom administration"

custom_site = MyAdminSite(name="myadmin")

+@admin.register(Comment)
+@admin.register(Comment, site=custom_site)
class CommentAdmin(admin.ModelAdmin):
    pass


-admin.site.register(Comment, CommentAdmin)
-custom_site.register(Comment, CommentAdmin)
  1. What should I do if from django.contrib import admin is missing (I suppose it's quitte common if you work with custom sites). I'm not sure where to add the import

  2. Should we extend the looks_like_admin_file check heuristic to the regular fixers too ?

@adamchainz
Copy link
Owner

  1. Is a stacked syntax ok when the same admin model is used to register a model against different sites ?
class MyAdminSite(admin.AdminSite):
    site_header = "My very custom administration"

custom_site = MyAdminSite(name="myadmin")

+@admin.register(Comment)
+@admin.register(Comment, site=custom_site)
class CommentAdmin(admin.ModelAdmin):
    pass
-admin.site.register(Comment, CommentAdmin)
-custom_site.register(Comment, CommentAdmin)

This seems fine to me - the decorator returns the admin class, unchanged, so that should work.

2. What should I do if `from django.contrib import admin` is missing (I suppose it's quitte common if you work with custom sites). I'm not sure where to add the import

I don't think we need to handle this case. It's idiomatic to import admin in order to subclass admin.ModelAdmin.

3. Should we extend the `looks_like_admin_file` check heuristic to the regular fixers too ?

I think it's fine to rely on the admin import in those cases.

@UnknownPlatypus
Copy link
Contributor Author

Great, thanks for the quick response.

Then I suppose you can check the code.
Your answers are matching the assumptions I made 😄

If was thinking the changelog and docs are still relevant but let me know if you want me to add a mention somewhere of the different extensions to the initial fixer.

Cheers.

@UnknownPlatypus UnknownPlatypus marked this pull request as ready for review September 18, 2022 17:38
@adamchainz
Copy link
Owner

I don't think the changelog needs updating but the readme could do with another example for cusotm sites, and maybe a sentence describing how custom sites are detected.

@adamchainz
Copy link
Owner

Alright, looks good to merge. I've pushed a few small fixes, including support files with names like custom_admin.py. This is the most common pattern I've seen, where a projcet might have a main admin with classes in admin.py files, then a second admin site with its classes in <name>_admin.py files.

@adamchainz adamchainz merged commit 23b9504 into adamchainz:main Sep 20, 2022
@UnknownPlatypus UnknownPlatypus deleted the admin_register_custom_admin_site branch September 21, 2022 21:00
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

Successfully merging this pull request may close these issues.

None yet

2 participants