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

Catch JSONField error to prevent not being able to show the response data #1666

Closed
loeeess opened this issue Jun 9, 2024 · 5 comments
Closed

Comments

@loeeess
Copy link
Contributor

loeeess commented Jun 9, 2024

Hello,

I use django-filter in combination with DRF and I have created a custom base view for GET requests that is used for various models, and returns a list of all object instances of the selected model. Some of these models have a JSONField, others don't. I use django-filter for each field of the models. Because JSONField is not covered by django-filter by default, it causes a 500 error when I retrieve the list of model instances for a model that includes a JSONField in the response, see the following response:

{
    "detail": "<class 'AssertionError'>: AutoFilterSet resolved field 'model_field_name' with 'exact' lookup to an unrecognized field type JSONField. Try adding an override to 'Meta.filter_overrides'. See: https://django-filter.readthedocs.io/en/main/ref/filterset.html#customise-filter-generation-with-filter-overrides"
}

I am aware of the fact that I can indeed write a custom filter (which I did, works fine). But without this custom implementation, the error prevents me from viewing the GET response in the first place.

My proposed solutions would be to either;

  1. 'Ignore' the JSONField when creating the filters for the model attributes. Then a user can still retrieve their requested data, the only downside being that the JSONField attributes are not included in the filter list.
  2. Implement JSONField in the package, where it is possible to define which key/value pairs can be filtered on.

Thank you.

@carltongibson
Copy link
Owner

Hi @loeeess — thanks for the report.

So I think a FilterSet option to do one of three things when encountering an unknown field type is the way forward here: Raise (current behaviour, should be the default), Warn (pass over, but emit a warning), Ignore (silently pass over). Folks could then set either Warn or Ignore if they hit your use-case.

Would you fancy taking that on?

@loeeess
Copy link
Contributor Author

loeeess commented Jul 1, 2024

Thanks for your reply, @carltongibson! Sounds like a good solution to me. And yes, I can look into it.

@loeeess
Copy link
Contributor Author

loeeess commented Jul 10, 2024

I just added a PR (#1675) for this issue. I realize the documentation for the FilterSet options should be updated as well. If these changes look alright, I don't mind taking on the task of updating these docs.

@carltongibson
Copy link
Owner

@loeeess Great work thanks. I left an initial comment. Let's carry on there. 👍

@carltongibson
Copy link
Owner

Fixed in #1675. Thanks @loeeess

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

No branches or pull requests

2 participants