feat: Use Django textual metadata in GraphQL

[noelleleigh]

Description

Automatically add Django model docstring and field help_text to the Strawberry type description and field descriptions, respectively.

These descriptions can be overridden by passing the description kwarg to strawberry.django.type() or strawberry.django.field().

Added tests:

• tests/test_types.py::test_field_metadata_preserved
• tests/test_types.py::test_field_metadata_overridden

Types of Changes

• [ ] Core
• [ ] Bugfix
• [ ] New feature
• [x] Enhancement/optimization
• [x] Documentation

Checklist

• [x] My code follows the code style of this project.
• [x] My change requires a change to the documentation.
• [x] I have updated the documentation accordingly.
• [x] I have read the CONTRIBUTING document.
• [x] I have added tests to cover my changes.
• [x] I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[bellini666]

@patrick91 any suggestions on where to add configurations for @noelleleigh to implement? Or maybe we should do that first in strawberry so that we can use it here?

[patrick91]

@patrick91 any suggestions on where to add configurations for @noelleleigh to implement? Or maybe we should do that first in strawberry so that we can use it here?

in Strawberry core we have a config parameter on the schema, we could extend that maybe?

or use the common django way of defining settings for Strawberry django?

in core we use config on the schema so we can have multiple schemas with different configurations

[noelleleigh]

in Strawberry core we have a config parameter on the schema, we could extend that maybe?

or use the common django way of defining settings for Strawberry django?

in core we use config on the schema so we can have multiple schemas with different configurations

Could you point me to the Strawberry core code for this? Also I don't have a preference for how to configure this, so give me your desired strategy and I'll implement it.

[patrick91]

in Strawberry core we have a config parameter on the schema, we could extend that maybe? or use the common django way of defining settings for Strawberry django? in core we use config on the schema so we can have multiple schemas with different configurations

Could you point me to the Strawberry core code for this? Also I don't have a preference for how to configure this, so give me your desired strategy and I'll implement it.

This is where we set the schema config: https://github.com/strawberry-graphql/strawberry/blob/9b3be99a765ae2fdb95c9802507e5600351c7006/strawberry/schema/schema.py#L68

And it is used in the schema converter, it might be more difficult to use in the context of strawberry django though 🤔

There's an in-flight PR for this feature in core: https://github.com/strawberry-graphql/strawberry/pull/1645 I'm mentioning it because I don't know if it would conflict with what you're doing, I'd love to merge the PR once it is slimmed down a bit :)

[noelleleigh]

This is where we set the schema config: https://github.com/strawberry-graphql/strawberry/blob/9b3be99a765ae2fdb95c9802507e5600351c7006/strawberry/schema/schema.py#L68

And it is used in the schema converter, it might be more difficult to use in the context of strawberry django though 🤔

I've poked around a bit and it doesn't appear that any existing code in strawberry-graphql-django has either the schema or its config in scope. It's not clear to me how my code in the process_type function could even reference the schema config at all. Am I missing something?

Looking at https://github.com/strawberry-graphql/strawberry/pull/1645, that seems to be looking at docstrings on the decorated dataclasses, whereas this PR tries to use the docstrings from the model classes referred to in the @strawberry_django.type decorator, so I don't think they would conflict?

[patrick91]

This is where we set the schema config: strawberry-graphql/strawberry@9b3be99/strawberry/schema/schema.py#L68 And it is used in the schema converter, it might be more difficult to use in the context of strawberry django though 🤔

I've poked around a bit and it doesn't appear that any existing code in strawberry-graphql-django has either the schema or its config in scope. It's not clear to me how my code in the process_type function could even reference the schema config at all. Am I missing something?

No, you're exactly right, this is where the complexity is going to be, I don't have a good suggestion at the moment for handling this. Unfortunately having the configuration at the schema level made some implementations quite complex and it will make this one complex too.

We could try to figure out something for this, maybe like we did for the name conversion part: https://github.com/strawberry-graphql/strawberry/blob/main/strawberry/schema/name_converter.py

or we could decide that for django we can use a "global" configuration, like other framework do. What do you think? /cc @bellini666

[noelleleigh]

A Django setting would also be a simpler option

[patrick91]

A Django setting would also be a simpler option

yeah, let's go with that, we can update it later if needed 😊

[noelleleigh]

A Django setting would also be a simpler option

yeah, let's go with that, we can update it later if needed 😊

Should it be a single setting that controls both type and field description inclusion? Or two settings, one for each?

STRAWBERRY_DJANGO_INHERIT_DESCRIPTIONS = True

vs.

STRAWBERRY_DJANGO_INHERIT_MODEL_DESCRIPTIONS = True
STRAWBERRY_DJANGO_INHERIT_FIELD_DESCRIPTIONS = True

And what should their default value(s) be?

[patrick91]

A Django setting would also be a simpler option

yeah, let's go with that, we can update it later if needed 😊

Should it be a single setting that controls both type and field description inclusion? Or two settings, one for each?

STRAWBERRY_DJANGO_INHERIT_DESCRIPTIONS = True

vs.

STRAWBERRY_DJANGO_INHERIT_MODEL_DESCRIPTIONS = True
STRAWBERRY_DJANGO_INHERIT_FIELD_DESCRIPTIONS = True

And what should their default value(s) be?

I'd do this personally:

STRAWBERRY_DJANGO = {
    "FIELD_DESCRIPTION_FROM_HELP_TEXT": False,
    "TYPE_DESCRIPTION_FROM_MODEL_DOCSTRING": False,
}

and disabled by default for backwards compatibility and to also prevent potential information leaking 😊

[bellini666]

or we could decide that for django we can use a "global" configuration, like other framework do. What do you think? /cc @bellini666

I like the idea! :)

We still need to think how to do that configuration for strawberry in the future though, but for django I think this is perfect.

[noelleleigh]

Alright, I've added code for new Django settings, added tests, and added documentation.

[noelleleigh]

How do those settings changes look to y'all?