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

Add ability for model types to be placed on input fields [WIP] feedback wanted #160

Closed
wants to merge 5 commits into from

Conversation

colanconnon
Copy link

@colanconnon colanconnon commented Apr 28, 2017

Add the ability to have a model type on the input class in mutations for more code reuse. This is my first time contributing so some feedback is welcome! I am borrowing some ideas from #121

@coveralls
Copy link

coveralls commented Apr 28, 2017

Coverage Status

Coverage decreased (-0.3%) to 93.476% when pulling 854e872 on colanconnon:master into 1e34dfb on graphql-python:master.

@coveralls
Copy link

coveralls commented Apr 28, 2017

Coverage Status

Coverage increased (+0.1%) to 93.898% when pulling a719580 on colanconnon:master into 1e34dfb on graphql-python:master.

@coveralls
Copy link

coveralls commented Apr 28, 2017

Coverage Status

Coverage increased (+0.1%) to 93.898% when pulling 3da4b2b on colanconnon:master into 1e34dfb on graphql-python:master.

@picturedots
Copy link

picturedots commented May 22, 2017

Thanks for this -- tried and it and I get errors when the reducer runs
unhashable type: 'String' --
/usr/local/lib/python3.5/site-packages/graphql/type/typemap.py in reducer

  1.     if type.name in map: 
    

But it also might be an instance of this error: #139 or related to the fact the graphene reducer doesn't recognize the type DjanngoModelInput when running is_graphene_type() .

@syrusakbary syrusakbary force-pushed the master branch 2 times, most recently from bdc7189 to f93251b Compare September 1, 2017 08:12
@spockNinja
Copy link
Contributor

Thanks for the idea. An easy way to generate Input based on models is definitely useful.

I'll point out a few similar alternatives.

  1. There is already support for mutations that utilize existing DRF Serializers. Docs here
  2. There is another PR in the works to do the same with native Django Forms.

Both of those options allow for validation and such to be handled by the serializer/form. They also have a default mutate that does what you would expect, validating and saving the serializer/form. So I think those two approaches should cover this functionality.

Are there any advantages of this approach over those two solutions that I am not thinking of?

Donnyyyyy
Donnyyyyy approved these changes Jul 24, 2018
@zbyte64 zbyte64 mentioned this pull request Oct 18, 2018
@firaskafri
Copy link
Collaborator

@colanconnon still interested to work on this?

@phalt
Copy link
Contributor

phalt commented May 3, 2019

I'd like to tidy up some of the PRs, so if we still want to work on this let me know or I will close this in 1 week.

@phalt phalt closed this May 8, 2019
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.

7 participants