-
Notifications
You must be signed in to change notification settings - Fork 768
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 Django form mutations #217
Add Django form mutations #217
Conversation
3 similar comments
That looks good! @syrusakbary do you have time to check this? :) |
bdc7189
to
f93251b
Compare
This change looks great. There was some work towards translating form fields already done in form_converter.py. It looks like it is only currently being used for filter fields. I think I like having it in /forms/converter though. So how about we drop the old file, make sure it didn't have anything the new one doesn't, and fix the filter-fields to import from this file. |
6cde395
to
666ddb2
Compare
I just re-ran the build on this PR. I believe it is compatible with Graphene/Graphene-Django 2.0. |
I'm loving this PR. I need to take some time to review it. Also might be useful to have an utility
Which is equivalent to write by hand:
Thoughts? |
Thanks, Syrus! I like the idea of creating an I'll whip up those changes for this PR. |
graphene_django/forms/mutation.py
Outdated
|
||
@classmethod | ||
def mutate_and_get_payload(cls, root, info, **input): | ||
form = cls._meta.form_class(data=input) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on some of the other requests coming in on the SerializerMutation
stuff, I have a few suggestions here for extensibility.
- Add a
get_form
classmethod that can be overridden to do any custom form logic. - Add a
get_form_kwargs
classmethod for a simpler override use case where you just want to send some extra kwargs to the form class.
Hey guys, I just pushed some changes to this PR. They include...
The last thing I haven't added is the |
This PR is looking fantastic. I think you're right, we probably shouldn't inherit from Also note that I believe in 2.0, |
Awesome.. this looks very promising. :) |
Hey @grantmcconnaughey, your work on this PR is great, I'd love to have it in graphene-django. I think that this feature is essential in any larger Django project that aims at having comprehensive API while keeping the codebase clean and DRY. |
Thanks, this looks awesome and full of great ideas. Is there any chance of work being done on this to bring it up into 2.0? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is look great!
form will make mutate easy way!
I want to merge this PR but I want to do some changes before it, such as:
I will open another PR adding the changes on top of this PR :) |
The refactor from As I want to make a release but not block this feature, I will merge #450 (once tests pass), while making clear in the docs that the API is experimental, so people can start trying it. |
This PR adds support for mutations based on Django forms. It's pretty similar to the serializer mutations over in #186. I based most of the changes off of that PR so the code should look pretty similar after being merged.
Here's how to use them:
The mutation will have one argument called
'input'
. That input will expect aname
.There is also a mutation that supports
ModelForm
s. This mutation will look up theDjangoObjectType
for the forms model and return it after the form saves.To-Do