Opened 10 years ago

Closed 8 years ago

Last modified 8 years ago

#265 closed defect (wontfix)

Patch: RequiredIfOtherField and friends don't work with edit_inline

Reported by: hugo <gb@…> Owned by: nobody
Component: Core (Other) Version:
Severity: normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: no
Easy pickings: UI/UX:

Description

They look at the all_data hash and that contains all lines of the inline editing table and so they don't find the other field. Maybe this could be changed in that validators on fields that are part of an edit_inline only get passed in a dictionary of those keys that relate to their row, with bare fieldnames (without the table.rownumber prefix)?

That way those combined validators could work with inline tables, too. Otherwise any validator that uses all_data to look at other fields will break.

Change History (14)

comment:1 Changed 10 years ago by hugo <gb@…>

I think this patch does the work as listed above. It works nicely with my project - edit_inline=True fields are now validated, too.

Index: core/formfields.py
===================================================================
--- core/formfields.py  (revision 398)
+++ core/formfields.py  (working copy)
@@ -5,6 +5,22 @@
 
 FORM_FIELD_ID_PREFIX = 'id_'
 
+import re
+
+ROW = re.compile('^(\S+\.\d+\.)(\S)+$')
+def _transform_new_data(field, new_data):
+    """
+    returns the full form if the field isn't a field from edit_inline=True,
+    returns only the matching row otherwise. edit_inline is recognized by
+    the fact that the field name is a dotted notiation
+    """
+    m = ROW.match(field)
+    if m:
+        (p, f) = m.groups()
+       return dict([(f[len(p):], new_data[f]) for f in new_data.keys() if f.startswith(p)])
+    else:
+        return new_data
+
 class EmptyValue(Exception):
     "This is raised when empty data is provided"
     pass
@@ -65,9 +81,9 @@
                     if field.is_required or new_data.get(field.field_name, False) or hasattr(validator, 'always_test'):
                         try:
                             if hasattr(field, 'requires_data_list'):
-                                validator(new_data.getlist(field.field_name), new_data)
+                                validator(new_data.getlist(field.field_name), _transform_new_data(field.field_name, new_data))
                             else:
-                                validator(new_data.get(field.field_name, ''), new_data)
+                                validator(new_data.get(field.field_name, ''), _transform_new_data(field.field_name, new_data))
                         except validators.ValidationError, e:
                             errors.setdefault(field.field_name, []).extend(e.messages)
             # If a CriticalValidationError is raised, ignore any other ValidationErrors

The only problem I see is that it triggers the mechanism on fields having dotted names - I don't know wether there are other situations where this might happen and so miss-trigger. But I think it shouldn't as field names should be SQL names and those only can use dotted notation with namespaces and stuff like that, but never in the form Django uses: table.rownum.fieldname

comment:2 Changed 10 years ago by hugo <gb@…>

An additional idea to reduce possible impact: add a validate_inline setting to a model and only use _transform_new_data if validate_inline==True - that way the speed penalty of filtering the new_data dict only hits models where you explicitely require this handling.

comment:3 Changed 10 years ago by hugo <gb@…>

  • Summary changed from RequiredIfOtherField and friends don't work with edit_inline=True to Patch: RequiredIfOtherField and friends don't work with edit_inline=True

comment:4 Changed 10 years ago by jforcier@…

Either I'm insane or you're missing a space on line 21 of that patch. Additionally, it doesn't seem to fix the problem, at least not as I'm getting it :(

comment:5 Changed 10 years ago by anonymous

  • Cc jforcier@… added

comment:6 Changed 10 years ago by hugo

Yes, correct. There is a space missing. Don't know why it is missing - maybe it's tab/space related (I still had my vim set up for tabs when editing that stuff - switched to spaces now to prevent similar foulups in the i18n branch)

comment:7 Changed 10 years ago by jforcier@…

Also see related ticket #526 (this is a general tip for others, not aimed at you, Hugo).

comment:8 Changed 9 years ago by URL

  • Summary changed from Patch: RequiredIfOtherField and friends don't work with edit_inline=True to Patch: RequiredIfOtherField and friends don't work with edit_inline
  • Type enhancement deleted

comment:9 Changed 9 years ago by anonymous

  • Cc jforcier@… removed
  • Type set to defect

comment:10 Changed 9 years ago by Michael Radziej <mir@…>

Poooh. This ticket has convered a lot of dust. May I simply ask you if this is still an issue?

comment:11 Changed 9 years ago by Gary Wilson <gary.wilson@…>

#1690 marked as a dup of this.

comment:12 Changed 9 years ago by Gary Wilson <gary.wilson@…>

  • Has patch set
  • Needs tests set
  • Triage Stage changed from Unreviewed to Accepted

comment:13 Changed 8 years ago by ubernostrum

  • Resolution set to wontfix
  • Status changed from new to closed

Even if this is still an issue, newforms-admin will completely obsolete it.

comment:14 Changed 8 years ago by ph+django@…

This is indeed still an issue, I just beat my head on it for an hour or so before finding this bug.

Note: See TracTickets for help on using tickets.
Back to Top