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

RestFulModelAdmin incompatibility with from django.contrib.admin.ModelAdmin #9

Open
jayvdb opened this issue Aug 9, 2020 · 4 comments
Labels

Comments

@jayvdb
Copy link
Contributor

jayvdb commented Aug 9, 2020

I realise that RestFulModelAdmin isnt currently supposed to be used like a normal admin.ModelAdmin, however,

a) it is confusing/misleading to explain that people should create these beasts in "admin.py" and have the class ending with ..ModelAdmin
b) it is misleading to even say that django-restful-admin (current) is "Expose Django's admin as a RESTFUL service", or use "admin" in the name, as it doesnt use any part of django.contrib.admin afaics, and were it is asserted was wrong #8
c) there is a richness within the Django ModelAdmin that django-restful-admin can use to great effect to provide a great REST api, especially all of the Django admin apps which enhance Django Admin, some of which should enhance django-restful-admin also. Some of the "Todo" in the README indicate that Django Admin features would be integrated.

So with that in mind, the first step would be to decide whether the restful admin is going to become part of the typical admin class, or be separate with a link to the typical admin class.

This issue is about the former of those options, as I feel that is neater. (There is one significant reason against it, that DRF will instantiate "duplicate" instances because it thinks they are viewsets, but that can be prevented with python magic) The latter implies a need to create a lot of additional classes which are actually just specialised DRF viewsets, in which case I should just use DRF viewsets and the extremely large collection of helpers that exist for normal DRF viewsets.

So if I define

class MymodelAdmin(admin.ModelAdmin, RestFulModelAdmin):
    ...


admin.site.register(Mymodel, MymodelAdmin)
api_admin.site.register(Mymodel, MymodelAdmin)

And I run drf-spectacular to generate a schema, I get the following error

Traceback (most recent call last):
  File "manage.py", line 52, in <module>
    main()
  File "manage.py", line 48, in main
    execute_from_command_line(sys.argv)
  File "/usr/lib/python3.8/site-packages/django/core/management/__init__.py", line 401, in execute_from_command_line
    utility.execute()
  File "/usr/lib/python3.8/site-packages/django/core/management/__init__.py", line 395, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "adhoc_deps/before/django_compat_patcher/fixers/django1_10.py", line 411, in run_from_argv
    original_run_from_argv(self, argv)
  File "/usr/lib/python3.8/site-packages/django/core/management/base.py", line 328, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/usr/lib/python3.8/site-packages/django/core/management/base.py", line 369, in execute
    output = self.handle(*args, **options)
  File "/usr/lib/python3.8/site-packages/drf_spectacular/management/commands/spectacular.py", line 50, in handle
    schema = generator.get_schema(request=None, public=True)
  File "/usr/lib/python3.8/site-packages/drf_spectacular/generators.py", line 175, in get_schema
    paths=self.parse(request, public),
  File "/usr/lib/python3.8/site-packages/drf_spectacular/generators.py", line 135, in parse
    for path, path_regex, method, view in self._get_paths_and_endpoints(None if public else request):
  File "/usr/lib/python3.8/site-packages/drf_spectacular/generators.py", line 117, in _get_paths_and_endpoints
    view = self.create_view(callback, method, request)
  File "/usr/lib/python3.8/site-packages/drf_spectacular/generators.py", line 84, in create_view
    view = super().create_view(callback, method, request)
  File "/usr/lib/python3.8/site-packages/rest_framework/schemas/generators.py", line 192, in create_view
    view = callback.cls(**getattr(callback, 'initkwargs', {}))
TypeError: __init__() got an unexpected keyword argument 'suffix'

getattr(callback, 'initkwargs', {}) is returning {'suffix': 'List', 'basename': 'mymodel', 'detail': False}

I can work around this by changing the admin class a bit

class MymodelAdmin(RestFulModelAdmin, admin.ModelAdmin):
    def __init__(self, model=Mymodel, site=admin.site, **kwargs):
        RestFulModelAdmin.__init__(self, **kwargs)
        admin.ModelAdmin.__init__(self, model, site)

The could be avoided if RestFulModelAdmin.__init__ understood model and site args, and also understood that it should not pass viewset specific args shouldnt be used when invoking `the super.__init__1

@amirasaran
Copy link
Owner

I have been using ViewSet as ModelAdmin because I can do anything that I want with ApiView.
I can customize serializing, add pagination, add filters, and support any things that I want.

You have a Good Idea and if you can do it I'm happy to hear from you. and this repository is open to merging your pull requests.

With the next version, we can implement support Django default admin customization.

@jayvdb
Copy link
Contributor Author

jayvdb commented Sep 23, 2020

If you can get a test suite started (c.f. #11), I'll make RestFulModelAdmin a normal django.contrib.admin.ModelAdmin subclass, and then you can integrate features from django.contrib.admin.ModelAdmin.

@amirasaran
Copy link
Owner

If you can get a test suite started (c.f. #11), I'll make RestFulModelAdmin a normal django.contrib.admin.ModelAdmin subclass, and then you can integrate features from django.contrib.admin.ModelAdmin.

@jayvdb
I suggest you create a mixin class instead of a subclass for more flexibility.

@jayvdb
Copy link
Contributor Author

jayvdb commented Sep 26, 2020

On mixin vs subclass, that is irrelevant. Re-read my issue. If django-restful-admin is going to use ModelAdmin features, it needs to be able to be part of the MRO of a ModelAdmin subclass. A mixin is just a lazy and monkey-ier way of doing that, avoiding the brittle nature of subclassing. But in this case "brittle" is another way of describing type-safety.

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

No branches or pull requests

2 participants