Giter Club home page Giter Club logo

Comments (11)

joeydebreuk avatar joeydebreuk commented on June 8, 2024 1

Looks great!
Also happy with Django model 'FieldTypesModel' has no field 'unknownField' 🥳

Some ideas:

enable / disable fields in generated queries / mutations
API below is not ideal, but just an example of what one might want to accomplish.

GeneratedQuery = strawberry_django.queries(User, exclude=["list"])  # no list
GeneratedQuery = strawberry_django.queries(User, include=["list"])  # only list
GeneratedMutation = strawberry_django.mutations(User, exclude=["create", "delete"])  # you get the point

Make is easy to extend / customize mutations
A hard one, but important. If this to complicated to we might find ourselves implementing every mutation from scratch.

As I am sure you are aware, there are a lot of cases, where one might want to extend / customize the default behavior. This is already pretty easy to do with strawberry fields in output_types. But for mutations its a lot harder. examples:

  • input type could be different from update type
  • input field (eg m2m ID list) needs to be checked for authorization with current user
  • input has to be validated as a whole
  • updating of 1 input field is some specific relation that needs to be saved in custom way
  • want to do some action before / after every update / create / delete
  • ...

Optional functions like validate_input , prepare_update_input and on_post_save will come a long way. Probably have to add these as the repo progresses, but just something to consider.

queryset authorization
It might be helpful to somehow configure authorization filters for a queryset / model, not only at the resolver of the ModelResolver or its equivalent. But also for relational field (strawberry_django.relation_field()).

But maybe its confusing because its not very explicit. Not sure.

pagination
Maybe its an idea to take (optional) pagination in mind. At the root query, but also at related queries. This is what I'm using now: https://gist.github.com/joeydebreuk/2e1333fb8da82220bca5300ee81d225c

from strawberry-django.

la4de avatar la4de commented on June 8, 2024 1

@joeydebreuk, Most of the requested features have been implement. Please let us know how well new API fits to your needs. :)

from strawberry-django.

la4de avatar la4de commented on June 8, 2024 1

strawberry-graphql-django v0.1.0 has been published to the pypi!

Run pip install strawberry-graphql-django and start experimenting the new API. =)

from strawberry-django.

joeydebreuk avatar joeydebreuk commented on June 8, 2024 1

Awesome! Automatically generated fields in mutations for many-to-many !!

from strawberry-django.

la4de avatar la4de commented on June 8, 2024

Looks great!
Also happy with Django model 'FieldTypesModel' has no field 'unknownField' 🥳

Some ideas:

Thanks for your feedback. Updated based on your feedback. I could not cover all ideas yet but at least some of them :)

from strawberry-django.

patrick91 avatar patrick91 commented on June 8, 2024

Wow this is neat! 👏

Just to clarify one things, as we want to merge this into strawberry itself, will be rename the package to be strawberry.django?

so you'd have:

@strawberry.django.type(models.User, fields=['id', 'name'])
class User:
    # types can be extended
    @strawberry.field
    def name_upper(root) -> str:
        return root.name.upper()

right?


what is a strawberry_django.relation_field()?


Validators are interesting, I wonder if we should make them part of the strawberry API somehow, I'll ping @BryceBeagle and @jkimbo (maybe we can also push this to a separate issue/discussion).


strawberry_django.mutations(User, UserInput)

this is interesting I think we should flesh out some of the use cases. I guess we should allow to specify what kind of mutations one would like too.


@la4de @BryceBeagle @jkimbo what do you think of jumping on a call for this? thought it might faster to review and share ideas, otherwise we can keep discussing here.

from strawberry-django.

jkimbo avatar jkimbo commented on June 8, 2024

Some really interesting ideas here @la4de , well done! Definitely worth having a call @patrick91 but I'll write down some comments anyway:

[1] I'm a bit confused about the relation_field as well. Why is it necessary? If I did this would it work?

@strawberry_django.type(models.User, fields=['id', 'name', 'group'])
class User:
    pass

Although I do like the idea of having a way of remapping an existing model field to a new name on in the Strawberry type. I don't think it needs to be restricted to relational fields though. e.g.

@strawberry_django.type(models.User, fields=['id'])
class User:
    full_name: str = strawberry_django.field(source="name")

[2] I think fields should be a required param when defining a type btw to prevent accidentally exposing fields you don't want to.

[3] Input types. Does the example mean that you can only define one input type per model?

[4] It might be nice to provide a way of converting a Django Form as an input type? Maybe combining it with a mutation so that it can run the validator methods.

[5] The validators and pre/post processors is super interesting. I especially like the use of the field as the decorator to tie the functions together. It solves one of the issues I was having with decorators where it wasn't clear how to apply to fields that don't define a resolver. I'm going to have to think a bit more about it though because I wonder how it fits into the core Strawberry API.

[6] The query generator function should probably be called get_by_pk (I assume thats what it's doing). That means in future you could expand it to be get_by_filters (maybe using Django-filters).

e.g.:

@strawberry.type
class Query:
    get_my_user: User = strawberry_django.queries.get_by_pk(User)

Also what is the behaviour if it can't find a user with that pk? Error or return None?

[7] I think the mutations API is going to be hard for the reasons that @joeydebreuk points out. Does the mutations.create do update as well for example or is that separate?

[8] The hooks API is really neat but I think passing functions are arguments to .list and .create might be simpler and less "magic". Also it's always good to only have an API where there is only 1 (obvious) way of doing things.

[9] Why is the type register needed? I like the concept but I can't think of a use case where you would need it. In the pydantic example you just register the types and it errors if it can't find a corresponding type.


Realise thats a lot of comments but hopefully they are helpful! Very excited about this though. It's looking really good.

from strawberry-django.

joeydebreuk avatar joeydebreuk commented on June 8, 2024

This is looking great @la4de.

I really love this approach. Its brilliant because its flexible and explicit. Yet very simple and readable.

@strawberry.type
class Mutation(GeneratedMutation):
    create_group = strawberry_django.mutations.create(Group, GroupInput)

Your approach to validation and mutation hooks also looks great. Allthough I think a generic validation method on input object is also useful. eg in some cases field A validation is based on the value of field B.

One small comment about this:

@strawberry_django.input(models.User, fields=['group', 'tag'])
class UserInput:
    name: str = strawberry_django.field()

Personally I would expect to have all fields in fields=['group', 'tag', 'name'], so a reader doesn't have to read the fields list, and the Input fields, and put them together.

One thing I didn't mention before...

The filter filters: [String!] couples the query directly to the models' shape, and without schema validation.
I think we need to find a better alternative. Maybe Django Filters can help here.

But maybe we should put all feature requests in separate issues, so this can be finished up first.

Edit:
Pagination issue
Filter issue

from strawberry-django.

la4de avatar la4de commented on June 8, 2024

Thank you @patrick91, @jkimbo and @joeydebreuk for your feedback. I really appreciate it. It's good idea to have a call and discuss about the ideas, details and challenges.

I need a bit more time to answer to all your questions but here are few items.

Yes, strawberry_django would be strawberry.django if we end up merging the packages.

You all liked concept of validators. I fully agree that it should came from strawberry core. I start prototyping it soon so that everyone can start playing with it :)

strawberry_django.relation_field() is shortcut for strawberry_django.field(resolver=strawberry_django.queries.resolvers.get_relation_resolver()). Maybe that's something we could hide, handle internally and add relation resolver to the field if it's pointing to the relation field of django model. Need to think this a bit more.

strawberry_django.mutations() generates all mutations but they can be generated separately by using strawberry_django.mutations.create(), strawberry_django.mutations.update(), or strawberry_django.mutations.delete() functions. I like the idea to make strawberry_django.mutations() configurable so that user can define which mutations are generated. @joeydebreuk proposed that already earlier.

It is possible to generate as many types types as you want from single model. However there has to method to configure which type is used where, especially if you have multiple types for the same model. TypeRegister solves that problem. You can define multiple TypeRegisters, register types there and pass type register to type, query and mutation generators. I need to add another example to clarify that.

I thought the same that using strings to define filters may not be the final. However that was the easiest way to get it done and I think that's fine for prototype. That's something we need to think a bit more.

It seems that v0.1 release is behind the corner. I got so good feedback that maybe we should release that soon after some API tunings and start collection feedback from the users.

Talk to your later. Thanks for your time :)

from strawberry-django.

joeydebreuk avatar joeydebreuk commented on June 8, 2024

Awesome, quick job. I'll check it out.

from strawberry-django.

nrbnlulu avatar nrbnlulu commented on June 8, 2024

Why is this closed?

from strawberry-django.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.