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

implement HTML JSON form style per spec #2682

Closed
wants to merge 4 commits into from

Conversation

sheppard
Copy link
Contributor

Here's a first pass at #2148. I was able to make it work with the existing tests with minor modifications (to both the algorithm and the tests). The next thing to do is to add more tests to handle the new uses that are now possible.

Let me know if you would me to change the comment style - this is basically a line-for-line translation of the spec to Python, including numbers referencing each step in the spec. I wouldn't normally code like that, but I then again I don't usually get to implement a spec from scratch either.

There were a couple of ambiguities in the spec that I resolved by looking at what I assume is the reference implementation:
https://github.com/darobin/formic/blob/gh-pages/json/json.js
I'll try to follow up with the authors to get those ambiguities clarified.

@tomchristie
Copy link
Member

Wow. Okay will try to sit down and properly review some time. Interesting stuff.

@sheppard sheppard force-pushed the json-forms branch 2 times, most recently from 83b1f7c to 30dce52 Compare August 6, 2015 14:38
@sheppard
Copy link
Contributor Author

sheppard commented Aug 6, 2015

Rebased. FWIW, I've been using this in production for a couple of wq.db-based projects and have found find it quite useful for populating nested serializers with simple HTML forms (e.g. in these tests).

There are a couple of things I haven't addressed yet:

  • Handling of files per spec. In practice, I don't really think we want or need to convert form-submitted files to JSON/base64 on the server. If anything, it would be nice to be able to convert the other way (HTML-JSON-form-file to UploadedFile) to support clients that already understand HTML JSON forms.
  • Prevent sparse array attack (Sparse Arrays darobin/formic#15). The spec doesn't have a limit yet so we'd need to make one up. We could cut it off at maybe 1000 sequential nulls?

@sheppard sheppard mentioned this pull request Sep 18, 2015
42 tasks
@sheppard sheppard force-pushed the json-forms branch 2 times, most recently from 186e355 to 303a304 Compare December 30, 2015 21:48
@xordoquy
Copy link
Collaborator

@sheppard I'm trying to address the oldest PR.
It seems that the JSON form submission has been canceled, the associated page is marked as unmaintained. Does this mean we could still use this PR or should we cancel it ?

@sheppard
Copy link
Contributor Author

It's certainly up in the air given the status of the spec. The main argument for keeping it is just that it's a bit more robust than what we have now (see #2148).

While I was waiting for this to sort out, I went ahead and put the implementation in a standalone package (html-json-forms). It might make sense to just link to that package for now and see how much interest there is.

@tomchristie
Copy link
Member

@sheppard - That sounds like a great option - works for me! Let's close this in favor of linking in the docs. We can always re-assess at a later date.

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.

3 participants