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 title keyword #191

Closed
wants to merge 5 commits into from
Closed

Conversation

gthieleb
Copy link

@gthieleb gthieleb commented Nov 5, 2020

This PR adds the title keyword of a DRF serializer field to properties schema.
The usage is to expose and maintain a more descriptive name of the field in the backend.
This can be used in the frontend e.g. for the label of a formfield or the headerline of a table.

The keyword is derived from the DRF serializer field attribute label. In case of ModelSerializer it is extracted from the model field attribute verbose_name.

References #140

@tfranzel
Copy link
Owner

tfranzel commented Nov 5, 2020

@gthieleb thanks for the PR. generally, i'm in favor of adding a way to include the verbose_name.

the PR clearly shows that the current approach adds a lot of noise without adding information (in the default case). i understand that the title is meaningful when verbose_name is actually set. however, everyone else will just get a capitalized field name with underscores stripped which in itself adds basically no value. i think this has to be approached a bit more defensively, like only adding the title if it is not trivial. generally we try not to bloat the schema where possible.

i also have some stylistic remarks, but above issue takes precedence.

@gthieleb
Copy link
Author

gthieleb commented Nov 6, 2020

Hi @tfranzel, thanks for your comments. I missed to push one commit that should be there now.

I fully agree with you. If someone is not relying on the additional information from the schema this might look like noise and in the worst case looks inconsistence (e.g. by using another inflecting style).

For me I have made some experience with optimistic inflection on the frontend and IMO it is not the cleanest approach.

To be more defensivly. Do you think about adding a configuration switch that enables title keyword in schema?

Btw. Iniitially i mentioned another use case for the title support in i18n. I now realized there exists an extension that uses additional keywords:

x-title-i18n:
    eng: Title on english

So this will likely go part of another feature request.

@tfranzel
Copy link
Owner

tfranzel commented Nov 6, 2020

ideally spectacular could find out if verbose_name is explicitly set and only then put it in the schema. i looked at the django/drf code but i have not found a silver bullet there yet. having a flag would also be a solution, however i would like to exhaust the possiblity of an automatic solution first. once introduced, parameters will never go away 😄

fyi: spectacular does support i18n in the confines of OpenApi 3.0.3. you can retrieve a translated schema with the view (lang as query param) and with the CLI.

there is also a generic issue for extensions #165, but it is not that straight forward how to do that properly.

@krzysieqq
Copy link

Any updates here or in #286 ? Maybe a good idea could be to have a variable in settings that could control showing or hiding titles base on labels/verbose_names. I'm interested in this feature because now I need to custom override an AutoSchema and add this feature manually.

tfranzel added a commit that referenced this pull request Mar 18, 2021
@tfranzel
Copy link
Owner

het @gthieleb, sry for the long wait. i had to go the extra mile but i found a way to get the title without the noise. fyi: parameters do not have a title. i'll close this PR as it is outdated but feel free to comment

@tfranzel tfranzel closed this Mar 18, 2021
@gthieleb
Copy link
Author

hey @tfranzel thanks for your update and the support of this. I will test and let you know something is open.

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

3 participants