Code

Opened 2 years ago

Closed 4 months ago

Last modified 6 weeks ago

#17413 closed New feature (fixed)

Serialization or getting non-HTML version of form errors is not easy

Reported by: kmike Owned by: loic84
Component: Forms Version: master
Severity: Normal Keywords: form errors serialization
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

See e.g. http://forrst.com/posts/Convert_Django_form_errors_to_JSON-ijw

What do you think about adding errors_dict attribute to BaseForm? A rough draft:

def _get_errors_dict(self):
    return dict(
        (k, map(unicode, v))
        for (k,v) in self.errors.iteritems()
    )
errors_dict = property(_get_errors_dict)

Attachments (1)

17413.diff (530 bytes) - added by merb 9 months ago.

Download all attachments as: .zip

Change History (18)

comment:1 Changed 2 years ago by aaugustin

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

comment:2 Changed 13 months ago by apollo13

  • Triage Stage changed from Design decision needed to Accepted

Would be nice to have.

comment:3 Changed 9 months ago by merb

Wouldn't it be better to rewrite

class ErrorDict(dict)

and add as_json or as_dict?
Currently it has as_text and as_ul

This is how the ErrorDict got returned, so a simple

    @property
    def errors(self):
        "Returns an ErrorDict for the data provided for the form"
        if self._errors is None:
            self.full_clean()
        return self._errors

The next snippet would call the json.dumps of the error dict.

print json.dumps(form.errors.as_dict())

I apped a patch that would do the following. If this is wished we only need a small test.

I also made a Pull Request on github:
https://github.com/django/django/pull/1406

Last edited 9 months ago by merb (previous) (diff)

Changed 9 months ago by merb

comment:4 Changed 9 months ago by merb

  • Owner changed from nobody to merb
  • Status changed from new to assigned

comment:5 Changed 9 months ago by merb

  • Easy pickings set
  • Has patch set
  • Needs documentation set
  • Needs tests set

comment:6 Changed 9 months ago by merb

  • Patch needs improvement set

comment:7 Changed 9 months ago by merb

As i asked here: https://github.com/django/django/pull/1406
What's better, to have a errors_dict or a to_json() in the ErrorDict()

the first would make some things like this happen: json.dumps(errors_dict) the second would be form.errors.to_json()

comment:8 Changed 8 months ago by loic84

It's probably an unusual request (and hopefully an acceptable one) but I'd like to put the ErrorDict.to_json() effort on-hold.

I'll write more thoroughly about it next week on the ML but here is a quick explanation of my rationale.

Some background:

  • The ValidationError refactor [f34cfec0fa1243b4a3b9865b961a2360f211f0d8] enabled pertaining error metadata like error code and error params.
  • This fixed #20199 and #16986 by allowing metadata to flow freely between the Model layer and the Form layer.
  • My current work on #20867 will enable raising exceptions with all metadata from Form.clean().

The issue:

While ValidationError is now a capable carrier for errors with their metadata, all information is lost as soon as it reaches ErrorDict and ErrorList. I'd like to fix that by making those two classes capable of handling instances of ValidationError. This is the last piece of the puzzle, but it'll require a small backward incompatibility that will need to be discussed (basically changing the output of ValidationError.__str__) to allow Error(Dict|List) itself to remain backward compatible.

Once this has happened, to_json() would become much more useful because on top of having the error message, it will also have the error code, which will be very useful on the JS side. If we introduce to_json() now, we'll be unable to make this change because of backward compatibility.

comment:9 Changed 8 months ago by merb

Yeah that's okai for me. I had an working version these days, but my hard drive crashed. I first need to recover it. (and hope it will work) just write here when you are done.

comment:10 Changed 8 months ago by loic84

Some updates:

  • #20867 is shaping up nicely and provides the groundwork for this feature, @timo mentioned he'll review it once 1.6 has been released.
  • The experimental branch https://github.com/loic/django/compare/forms.errordict made ErrorDict aware of metadata, and implements as_json() to give a useful representation of the form errors along with all metadata (see test case).

This will be quite useful to anyone who want to further process the errors with JavaScript.

Now there are some remaining challenges in term of backward compatibility, I'll be focussing on that from now on.

comment:11 Changed 8 months ago by loic84

Now with backward compatibility: https://github.com/django/django/pull/1483.

Still missing tons of docs, especially since this builds on top of #20867 and we haven't decided yet on what will be the public API.

I also want to take this opportunity to create an exhaustive test suite for ErrorDict and ErrorList, these are tested indirectly by a gazillion of other tests, but I'd like to have a proper reference test suite for them.

comment:12 Changed 8 months ago by loic84

  • Easy pickings unset
  • Owner changed from merb to loic84
  • Version set to master

comment:13 Changed 5 months ago by loic84

Following the merge of #20867, I've been able to resume work on this PR:

https://github.com/django/django/pull/1483

Feedback welcome.

comment:14 Changed 4 months ago by bouke

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Accepted to Ready for checkin

comment:15 Changed 4 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 3ce9829b615336b0f3ac39b080c27fc8cf5af483:

Fixed #17413 -- Serialization of form errors along with all metadata.

comment:16 Changed 4 months ago by Tim Graham <timograham@…>

In 2fd7fc134cf0c0685ceac22fd858509aa43f819f:

Refs #17413 -- Added isinstance backward compatibility on ErrorList.

comment:17 Changed 6 weeks ago by Florian Apolloner <florian@…>

In 34236efc5e569ce7d42f7d52dd798e59f95457f8:

Reworked ErrorDict.as_json() to prevent unnecessary serialization/deserialization step.

Thanks @apollo13 for the suggestion. Refs #17413.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.