Open mhdismail opened 2 years ago
Hey @mhdismail ,
I get your point, but that is not that simple.
We could do what you say and automatically create an enum for that field, similar to what graphene-django
does. But the problem here is that you would not have access to that enum to use it in other places (e.g. to use it as an argument in a mutation).
The reason to why I chose to use https://github.com/bellini666/django-choices-field for that are those:
1) This lib started with some adaptations and improvements I've made on top of strawberry-graphql-django
for some personal projects I have. I use that choices-field on those, and thus I added support for it there.
2) The main reason, and also the main reason to why I wrote django-choices-field
itself, was to have a better static typing support and also to allow retrieving the original enum from the field. That also fits perfectly into the way strawberry
works since once I define an enum using models.TextChoices
for example, I can have it decorated with @strawberry.enum
and use it at any type/input, without any "magical" conversion happening.
Ok thanks, that makes sense, and it's good to know that you can wrap the TextChoices with @strawberry.enum
I have another quick question, maybe I can open a new issue, but do you think we can apply the Permission on the level of the class as an alternative to per field? For example with DRF, when you have a set of related actions in a class, you can set the permission for that entire class. For example, I'm working with the Order model, and I want to set IsAuthenticated to be applicable for all the queries/mutations there, and in case I set the directives for a single field it will take precedence over the class one.
Currently I'm splitting my code in a relative way, and then merging them into app level queries/mutations then I merge them into the root one.
Hey again @bellini666 I have a set of other quick questions after trying to cover all the things I might need for a project I need to work on.
List[Union[FooType, BarType]]
, I have a search query that I need to handle depending on the selections, I made it work this way, is this the right approach? or we can probably add support to resolve the type like with Graphene?def get_foo_bar(info: Info) -> List[Union[FooType, BarType]]:
items = []
for selected_field in info.selected_fields:
for selection in selected_field.selections:
if selection.type_condition == FooType.__name__:
items += list(Foo.objects.all())
elif selection.type_condition == BarType.__name__:
items += list(Bar.objects.all())
return items
Are we able to override the permissions logic you have in place? I would like to return an unauthorized error if you don't have access to a certain query/mutation? In the current case, it returns an empty list.
Regarding the response metadata, I couldn't figure out how we can custom the response data and add some metadata like count, next page, prev page.., but I'm assuming this is a strawberry-django thing?
Edit: Sorry if I'm spamming this way, but I thought it's better than opening new issues.
I have another quick question, maybe I can open a new issue, but do you think we can apply the Permission on the level of the class as an alternative to per field? For example with DRF, when you have a set of related actions in a class, you can set the permission for that entire class. For example, I'm working with the Order model, and I want to set IsAuthenticated to be applicable for all the queries/mutations there, and in case I set the directives for a single field it will take precedence over the class one.
I actually started developing a way to add permission directives to types, but although they work well for queries, mutations are hard to ensure due to the fact that we would need to check arguments and things like that.
I decided for my personal projects that it was better and more clear to add the permission directives directly into the queries/mutations fields, so I stopped developing that feature. I don't have anything against it, but it has some hard issues to be properly solved T_T.
It seems Union doesn't work properly if the query is automated? For example return type is List[Union[FooType, BarType]], I have a search query that I need to handle depending on the selections, I made it work this way, is this the right approach? or we can probably add support to resolve the type like with Graphene?
The problem here is that you are trying to solve a problem that graphql isn't able to solve.
I mean, it is not the servers responsibility to filter the content based on what type you are querying. If you return something that is a list of FooType | BarType
, if you only retrieve FooType
fields you would still receive BarType
by the definition of how graphql works, you would just have an empty result.
The way I would approach here would be to always return everything or to add arguments to the query like include_foo: bool = True, include_bar: bool = True
(or both False
by default), that way the client can ask for which types to be included for optimization.
Are we able to override the permissions logic you have in place? I would like to return an unauthorized error if you don't have access to a certain query/mutation? In the current case, it returns an empty list.
Sure! You can create a subclass of HasPermDirective
, decorate it with @schema_directive
, override what you want in it and use it directly :)
I have a custom HasTenantPerm
in a project of mine, subclass of HasPermDirective
, which checks if the user has permissions for the current tenant in the session he is logged in.
Regarding the response metadata, I couldn't figure out how we can custom the response data and add some metadata like count, next page, prev page.., but I'm assuming this is a strawberry-django thing?
You can use the relay
interface for that: https://blb-ventures.github.io/strawberry-django-plus/quickstart/#relay-support
It has an interface already integrated with django, so you can use gql.django.connection
for example to have a paginated relay interface for any django model.
There are some examples in the demo code, all using the relay interface.
Thanks!
I actually started developing a way to add permission directives to types, but although they work well for queries, mutations are hard to ensure due to the fact that we would need to check arguments and things like that.
Wouldn't passing the info/context be enough and leave the rest for the developer?
The way I would approach here would be to always return everything or to add arguments to the query like include_foo: bool = True, include_bar: bool = True (or both False by default), that way the client can ask for which types to be included for optimization.
That's a better idea I agree, but strawberry has a similar example on their documentation, that's why I was checking what is lacking from the django integration.
You can use the relay interface for that: https://blb-ventures.github.io/strawberry-django-plus/quickstart/#relay-support
It took me sometime to understand the entire thing, and now I got the relay code working with the mutations. I got confused a bit with the gql.NodeInput as I was using it even without the relay.node.
I do have another point to raise after more testing, and I wanted to confirm with you.
Currently if you have a reverse foreign relation and you have the following type domains: Optional[List[DomainInputPartial]]
, add works fine, set and remove do not work in one case. You're treating both as m2m, so this function will fail update_m2m(info, instance, field, value)
if you have a foreign key that is not nullable, because .remove will not exist for the RelatedManager.
For ForeignKey objects, this method only exists if null=True. If the related field can’t be set to None (NULL), then an object can’t be removed from a relation without being added to another. In the above example, removing e from b.entry_set() is equivalent to doing e.blog = None, and because the blog ForeignKey doesn’t have null=True, this is invalid.
You'd have to call a filter and delete for both the set/remove to work.
https://github.com/blb-ventures/strawberry-django-plus/blob/9f06e1169f6ce696a9439bec52abd546ef380b29/strawberry_django_plus/types.py#L208
Hey there, I'm currently trying the lib here, I'm enjoying it so far, hopefully it gets merged with the main lib soon.
Regarding the above, can't you just do something like the following
Shouldn't that be enough? Assuming you're able to get access to to the model there.