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

Merge fields used in meta attribute passed on DjangoObjectType #1132

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sebsasto
Copy link
Contributor

Currently I'm working on an integration with Django Parler that require to subclass DjangoObjectType to pass the translated fields into the original model. However I cannot do this because the _meta.fields gets overwritten in the __init_subclass_with_meta__.

This PR allows to pass fields in the _meta attribute and merge them with the other fields. This is the same behaviour of the
__init_subclass_with_meta__ inside ObjectType

@sebsasto
Copy link
Contributor Author

@zbyte64 I added the test cases. Please let me know if it is ok

@sebsasto
Copy link
Contributor Author

If anyone is curious to see an example on real case scenario, let me exemplify with Parler. Django Parler have a functionality that adds all the translated fields into the main Model. In this case we can access directly the translated fields from the main model. However, those fields are not part of the main Meta class and therefore DjangoObjectType does not recognise them.

In the example below I'm injecting Django Parler fields in a custom DjangoObjectType using the feature introduced in this PR.

class DjangoParlerObjectType(DjangoObjectType):

    class Meta:
            abstract = True

    @classmethod
    def __init_subclass_with_meta__(cls, model=None, _meta=None, registry=None, convert_choices_to_enum=True, **options):
        assert is_valid_django_model(model), (
            'You need to pass a valid Django Model in {}.Meta, received "{}".'
        ).format(cls.__name__, model)

        if hasattr(model, '_parler_meta'):
            if not _meta:
                _meta = DjangoObjectTypeOptions(cls)

            if not registry:
                registry = get_global_registry()

            fields = OrderedDict()
            for name, parler_model in model._parler_meta.get_fields_with_model():
                field = parler_model._meta.get_field(name)
                field.null = field.null or field.blank

                _convert_choices_to_enum = convert_choices_to_enum
                if not isinstance(_convert_choices_to_enum, bool):
                    if name in _convert_choices_to_enum:
                        _convert_choices_to_enum = True
                    else:
                        _convert_choices_to_enum = False

                converted = convert_django_field_with_choices(
                    field, registry, convert_choices_to_enum=_convert_choices_to_enum
                )
                fields[name] = converted
            
            parler_fields = yank_fields_from_attrs(
                fields,
                _as=Field,
            )

            _meta.fields = parler_fields

        super().__init_subclass_with_meta__(model=model, _meta=_meta, registry=registry, convert_choices_to_enum=convert_choices_to_enum, **options)


@classmethod
def __init_subclass_with_meta__(cls, **options):
options.setdefault("_meta", ArticleTypeOptions(cls))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this is the intended way to override _meta and why it can't be part of ArticleBaseType.Meta.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually after implementing my option for Parler. Probably I can modify this test. Basically in the __init_subclass_with_meta__ we can inject more fields that are part of the model but are not part of the Meta fields of the Django model.

You can see the real life example I put in the comments. I can modify this example to match that one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zbyte64 I updated the test with a more realistic scenario. Please let me know what do you think :)

@firaskafri
Copy link
Collaborator

@sebsasto updates?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants