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

overrides on django filters only work the first request #950

Closed
wgiddens opened this issue Feb 28, 2023 · 3 comments · Fixed by #951
Closed

overrides on django filters only work the first request #950

wgiddens opened this issue Feb 28, 2023 · 3 comments · Fixed by #951

Comments

@wgiddens
Copy link
Contributor

Describe the bug
When using extend_schema_field to override the description on a django_filter filter, the overridden description only works for the first request.

To Reproduce
Create a custom filter and override description.

For example:

@extend_schema_field({"description": "overridden description"})
class CustomBooleanFilter(BooleanFilter):
    pass

View the schema multiple times. It will show overridden description only on the 1st request.

Expected behavior
It should always use the overridden schema.

I think this is because the description gets popped from the schema object in resolve_filter_field. Unless there is a use case I'm not thinking of, that should probably be a schema.get().

It looks like enum and readOnly get popped as well. They probably have the same issue.

@tfranzel
Copy link
Owner

Hi @wgiddens good catch, but this all happens for a reason.

  • description is popped because it gets reinserted one level up. the schema itself is nested in the parameter object. the description is on the top level
  • enum is written back into the schema 2 lines down. so basically irrelevant for your argument.
  • readOnly gets popped because its makes little sense for parameters. this is a remnant of model synthesis.

Sure, this could all be written differently but we usually start from a scratch schema that gets refined along the way. I think the actual issue is that we break isolation by not making a copy here. All other branches start from a fresh (and isolated) schema object.

Note however that by decorating a raw schema, you essentially throw away the inferred type information. Raw schema means: "Get out of the way, I know what I am doing." In that case we just do some trimming, nothing more.

@wgiddens
Copy link
Contributor Author

wgiddens commented Mar 1, 2023

That makes sense. Copying the annotation here fixes it. I opened a PR here: #951

@tfranzel
Copy link
Owner

tfranzel commented Mar 1, 2023

added a test case that checks for the modification. thanks @wgiddens for the PR

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 a pull request may close this issue.

2 participants