jet-admin / jet-django

Jet Bridge (Django) for Jet Admin – Admin panel framework for your application
https://app.jetadmin.io/demo
Other
194 stars 33 forks source link

[Bug] No support for non-number ID fields #11

Open amitassaraf opened 5 years ago

amitassaraf commented 5 years ago

I use django-hashid for my "id" field in my models.

When running jet-django along with these models I get the following error:

  File "/Users/amit/workspace/project/project-platform/src/project_platform/web/urls.py", line 18, in <module>
    from jet_django.urls import jet_urls
  File "/usr/local/lib/python3.7/site-packages/jet_django-0.5.8-py3.7.egg/jet_django/urls.py", line 40, in <module>
    jet_urls = init_urls()
  File "/usr/local/lib/python3.7/site-packages/jet_django-0.5.8-py3.7.egg/jet_django/urls.py", line 21, in init_urls
    jet.register_related_models()
  File "/usr/local/lib/python3.7/site-packages/jet_django-0.5.8-py3.7.egg/jet_django/admin/jet.py", line 23, in register_related_models
    self.register(item['model'], hidden=True)
  File "/usr/local/lib/python3.7/site-packages/jet_django-0.5.8-py3.7.egg/jet_django/admin/jet.py", line 9, in register
    self.models.append(JetAdminModelDescription(Model, fields, actions, hidden))
  File "/usr/local/lib/python3.7/site-packages/jet_django-0.5.8-py3.7.egg/jet_django/admin/model_description.py", line 25, in __init__
    self.filter_class = model_filter_class_factory(Model, self.get_display_model_fields(), self.get_model_relations())
  File "/usr/local/lib/python3.7/site-packages/jet_django-0.5.8-py3.7.egg/jet_django/filters/model.py", line 164, in model_filter_class_factory
    class FilterSet(django_filters.FilterSet):
  File "/usr/local/lib/python3.7/site-packages/jet_django-0.5.8-py3.7.egg/jet_django/deps/django_filters/filterset.py", line 71, in __new__
    new_class.base_filters = new_class.get_filters()
  File "/usr/local/lib/python3.7/site-packages/jet_django-0.5.8-py3.7.egg/jet_django/filters/model.py", line 207, in get_filters
    filters[filter_name] = cls.filter_for_field(field, field_name, lookup_expr)
  File "/usr/local/lib/python3.7/site-packages/jet_django-0.5.8-py3.7.egg/jet_django/filters/model.py", line 225, in filter_for_field
    field, lookup_type = resolve_field(field, lookup_expr)
  File "/usr/local/lib/python3.7/site-packages/jet_django-0.5.8-py3.7.egg/jet_django/deps/django_filters/utils.py", line 218, in resolve_field
    raise FieldLookupError(model_field, lookup_expr) from e
jet_django.deps.django_filters.exceptions.FieldLookupError: Unsupported lookup 'gt' for field 'app.MyModel.id'.

Seems to be a problem with the id field being a non-integer.

I upgraded my entire project to python 3 just for this dashboard :) Hope we can get this to work !

Thanks!

amitassaraf commented 5 years ago

I believe this needs a fix, I solved part of it by forking django-hashid-field and adding gt and some more lookups to the allowed lookups list. (This is a bit weird, you shouldn't require all lookups on every field)

Still, I had a problem with the need to override your internal json serializer in order to support serialising of hashids (It is fine, I can live with this).

But after all of this I still couldn't get this to work because when the dashboard makes the following request:

http://localhost:8080/jet_api/models/app;mymodel/?id__in=x813juuvKdYwprAgwKmMgW8Laz091Z5q&_per_page=10000

The API returns an error 400 (Bad Request) and says the following: Enter a number.

Which adds to the theory of no support for non integer id fields.

Sadly due to the nature of the Jet-Django project of using an API instead of the default Django Admin functions to provide the connection between the DB and the dashboard, I had to abandon using this in my project. There are too many custom fields which don't support serialisation, small bugs and hiccups, and too much work to transfer the project from Django admin to your API. It will be the same amount of work as developing our own dashboard based on a "React Admin Template". Good luck though! Will come back to check it out in a few months when this project will mature a bit.

f1nality commented 5 years ago

@amitassaraf hello! that's strange, because jet works OK with non numeric PK, at least when they are not called id (you can change primary key field in Jet Admin interface).

Can you tell me your project name so I can investigate it? Which custom fields you need to support? I'm sure building you own admin interface with the same functinality will take you much more time, while we constantly expand Jet Admin functionality to cover more and more user cases.

amitassaraf commented 5 years ago

@f1nality Hi, I was using it on a project in my startup. Yeah it's strange that it didn't work, but once I changed it from using a django-hashid-field to a normal integer it worked. Though I still had some issues with display artifacts, not all fields & some model relationships not showing (probably stemming from the fact that I added this to such an already complex project), and some other issues that combined with the django-hashid-field problem (a deal breaker for my project sadly) caused me to move on and try later down the road.

The logic for me was to use this was based on the fact that I could get it working easily. I knew that it would be only a matter of time before I would need to develop my own custom dashboard to fit the needs of my startup perfectly, but because of the issues I decided to just start working on the custom version right away.

Amazing project though! Would definitely use it for other projects.

In terms of the custom fields, I had to implement my own serialisation for two fields which I was using, the django-hashid-field (which had other problems besides serialisation, mentioned above), django-countries and django-money.

Both these field didn't work out of the box and I had to fork your repo to override your serialiser in order to provide my own serialiser for these fields.

When I get time I'll setup a project and check if there really is a problem using a non-integer PK field when it is named id but that is the impression I got after serialising the django-hashid-field.

amitassaraf commented 5 years ago

@f1nality I recreated this issue in an empty project here jet-django-pk-test. There seems to be an issue. When playing around I get code 500 errors AttributeError: 'tuple' object has no attribute 'name' also note that in the test you cannot edit any models because when trying to save them the server returns an error 500.

I played around with my startup project a bit more and managed to solve the issue with the bad request by allowing integer lookups in my HashID field and when serialising it, I serialise it to it's integer value.

Sadly, as is life, I came across an issue that is solvable but I think it is a bit much.

django.db.utils.ProgrammingError: can't adapt type 'Hashid' in jet_django/utils/siblings.py @ get_row_number

I've taken a peek at the code and I see this:

args = join_args + where_args + [instance.pk] (instance.pk evaluates to a 'Hashid' instance in my project)

and then this:

cursor.execute(query, args)

Obviously this wouldn't work as django-db has no idea what Hashid is 😢 and I don't think masking it as an integer could go on much longer, I could serialise it to be like an integer here too but I think I have already done too much in terms of jet-django editing for this to work with my hashid fields (fyi I tried it just to be sure and it worked).

btw even after fixing all this seems to be issues like reverse related fields are not showing in the editing panel and thus I cannot create models because they are non-nullable, and some tables are showing empty even though they should have several records.