Skip to content

Conversation

@toolness
Copy link
Collaborator

@toolness toolness commented Aug 19, 2018

This attempts to use Graphene-Django's integration with Django Forms to validate our login form.

Advantages

This could be useful going forward because:

  • Our forms will need to have validation, and that validation will need to be done by a trusted agent (i.e. server-side). Django Forms are basically built to solve this kind of problem, though obviously we wouldn't really be needing their rendering functionality.

  • We need to have a mechanism for reporting per-field errors and non-field errors, which this solution gives us.

  • Using Django Forms nicely moves validation into a business logic layer that is separate from the specifics of GraphQL, which is recommended by the GraphQL documentation.

  • I'm still vague on this one, but using Django Forms might make it easier to eventually add progressive enhancement, allowing users without JS (or with malfunctioning JS) to use the site.

Risks

The Graphene-Django documentation for forms integration states:

Note: the API is experimental and will likely change in the future.

So that doesn't exactly fill me with confidence. And aside from that, the documentation itself seems to either be wrong or out-of-date or something, so I had additional problems getting this working (though I did contribute some changes upstream at graphql-python/graphene-django#499).

But on the other hand, the codebase for the forms integration isn't that big and it's fairly easy to understand, so we could potentially contribute upstream or fork the code.

To do

  • Add tests.
  • Show per-field/non-field errors (currently we just show a window.alert with a generic message).
  • Graphene-Django's form field validation errors use Django's snake-case names for form fields (e.g. phone_number), rather than GraphQL's lower-camel-case names (e.g. phoneNumber). See if we can fix this.
  • File an issue to eventually remove the fixInvalidGlobaltypesReferences() function added here once Apollo 1.7.1 or later is released. Filed as Get rid of temporary code once Apollo 1.7.1 or later is released #42.

@toolness toolness changed the title [WIP] Use a Django Form to validate the login form Use a Django Form to validate the login form Aug 20, 2018
@toolness toolness merged commit aeace83 into master Aug 21, 2018
@toolness toolness deleted the django-forms branch August 21, 2018 00:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants