Re: [Django] #13794: Django does not respect to_field's model on an inline model admin

Django

unread,

May 5, 2011, 2:40:33 PM5/5/11

to django-...@googlegroups.com

#13794: Django does not respect to_field's model on an inline model admin
-------------------------------------+-------------------------------------
Reporter: | Owner: gautier
sebastien@… | Status: assigned
Type: Bug | Component: Forms
Milestone: | Severity: Normal
Version: 1.2 | Keywords: admin inline
Resolution: | to_field formset
Triage Stage: Accepted | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 1 | Easy pickings: 0
-------------------------------------+-------------------------------------
Changes (by patchhammer):

* needs_better_patch: 0 => 1
* easy: => 0

Comment:

fix_inline_model_with_to_field.diff fails to apply cleanly on to trunk

--
Ticket URL: <http://code.djangoproject.com/ticket/13794#comment:5>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Jan 10, 2013, 4:47:02 PM1/10/13

to django-...@googlegroups.com

#13794: Django does not respect to_field's model on an inline model admin
-------------------------------------+-------------------------------------

Reporter: sebastien@… | Owner: gautier
Type: Bug | Status: assigned
Component: Forms | Version:
Severity: Normal | 1.5-beta-1

Keywords: admin inline | Resolution:
to_field formset | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by anonymous):

* needs_better_patch: 1 => 0
* version: 1.2 => 1.5-beta-1

Comment:

I updated the patch so that is does not change behavior at all for no
to_field formsets. Any feedback or suggestions welcome.

This bug has been present for a while and would be nice to get fixed.

--
Ticket URL: <https://code.djangoproject.com/ticket/13794#comment:7>

Django

unread,

Jan 22, 2013, 12:34:55 PM1/22/13

to django-...@googlegroups.com

#13794: Django does not respect to_field's model on an inline model admin
-------------------------------------+-------------------------------------
Reporter: sebastien@… | Owner: gautier

Type: Bug | Status: new

* status: assigned => new

--
Ticket URL: <https://code.djangoproject.com/ticket/13794#comment:8>

Django

unread,

Jan 28, 2013, 2:46:19 PM1/28/13

to django-...@googlegroups.com

#13794: Django does not respect to_field's model on an inline model admin
-------------------------------------+-------------------------------------
Reporter: sebastien@… | Owner: gautier
Type: Bug | Status: new
Component: Forms | Version:
Severity: Normal | 1.5-beta-1
Keywords: admin inline | Resolution:
to_field formset | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Old description:

> The problem occurs in the function __unicode__ of ModelB
>
> When editing a ModelA instance in admin site, all ModelB instances have a
> modela_id set to the pk of the ModelA instance, instead of the value of
> the to_field's field. And, when accessing to the field modela of a ModelB
> instance, a DoesNotExist exception is raised.
>
> This problem does not occus in the shell :
> {{{
> In [3]: ModelA.objects.all()[0].modelb_set.all()[0].modela_id
> Out[3]: u'TRUC'
>
> In [6]: ModelB.objects.all()[0].modela_id
> Out[6]: u'TRUC'
> }}}
>

> See below to reproduce
>
> {{{
> models.py
>
> class ModelA(models.Model):
> code = models.CharField(max_length=20, unique=True)
>
> class ModelB(models.Model):
> modela = models.ForeignKey(ModelA, to_field="code")
> position = models.IntegerField()
>
> def __unicode__(self):
> return u"%s" % self.modela
>
> admin.py
>
> class ModelBInlineAdmin(admin.TabularInline):
> model = ModelB
>
> class ModelAAdmin(admin.ModelAdmin):
> model = ModelA
> inlines = [ModelBInlineAdmin]
>
> admin.site.register(ModelA, ModelAAdmin)
>
> }}}

New description:

The problem occurs in the function `__unicode__` of `ModelB`

When editing a `ModelA` instance in admin site, all `ModelB` instances
have a `modela_id` set to the `pk` of the `ModelA` instance, instead of
the value of the `to_field`'s field. And, when accessing to the field
modela of a `ModelB` instance, a `DoesNotExist` exception is raised.

This problem does not occur in the shell :
{{{
In [3]: ModelA.objects.all()[0].modelb_set.all()[0].modela_id
Out[3]: u'TRUC'

In [6]: ModelB.objects.all()[0].modela_id
Out[6]: u'TRUC'
}}}

See below to reproduce

{{{
# models.py

class ModelA(models.Model):
code = models.CharField(max_length=20, unique=True)

class ModelB(models.Model):
modela = models.ForeignKey(ModelA, to_field="code")
position = models.IntegerField()

def __unicode__(self):
return u"%s" % self.modela

# admin.py

class ModelBInlineAdmin(admin.TabularInline):
model = ModelB

class ModelAAdmin(admin.ModelAdmin):
model = ModelA
inlines = [ModelBInlineAdmin]

admin.site.register(ModelA, ModelAAdmin)
}}}

--

Comment (by aaugustin):

Added a bit of markup for reaadability.

--
Ticket URL: <https://code.djangoproject.com/ticket/13794#comment:9>

Django

unread,

Jan 28, 2013, 5:57:01 PM1/28/13

to django-...@googlegroups.com

#13794: Django does not respect to_field's model on an inline model admin
-------------------------------------+-------------------------------------
Reporter: sebastien@… | Owner: gautier
Type: Bug | Status: new
Component: Forms | Version:
Severity: Normal | 1.5-beta-1
Keywords: admin inline | Resolution:
to_field formset | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by aaugustin):

I asked Florian (who reported #11043) to have a look; he didn't have much
time but here are his initial thoughts:

> that's confusing code; when you look at django/forms/models.py line 707
-- do you have any idea where self.fk should come from?
{{{
FormSet = modelformset_factory(model, **kwargs)
FormSet.fk = fk
}}}
> both patches seem to fix the problem (or at least the attached
testcase), which makes me think that the testcase isn't good enough, might
have to trigger validation
> I can't figure out why get_queryset is affected by _construct_form
> ah constructing a formset executes the query and then changes the
instances, now on to figure out if there could be a problem if that data
is deleted :/
>
https://code.djangoproject.com/attachment/ticket/13794/fix_inline_model_with_to_field.diff
this patch looks better, the updated one seems wrong or useless

--
Ticket URL: <https://code.djangoproject.com/ticket/13794#comment:10>

Django

unread,

Jan 28, 2013, 6:50:34 PM1/28/13

to django-...@googlegroups.com

#13794: Django does not respect to_field's model on an inline model admin
-------------------------------------+-------------------------------------
Reporter: sebastien@… | Owner: gautier
Type: Bug | Status: new
Component: Forms | Version:
Severity: Normal | 1.5-beta-1
Keywords: admin inline | Resolution:
to_field formset | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by anonymous):

The patch 'to_field_formsets.patch' was an attempt to fix the problem in
the safest way possible. All tests pass with line 719 removed, but the
comment above suggests that line should not be removed. If you are
convinced that line isn't needed then fix_inline_model_with_to_field.diff
is fine. I just wanted to provide an option that is guaranteed not to
break any working code but the fix the problem.

If you think this needs more investigation please let me know and I'll try
to find out if that line is just a relic of the old way or if it is
actually used.

--
Ticket URL: <https://code.djangoproject.com/ticket/13794#comment:11>

Django

unread,

Jan 29, 2013, 2:49:27 PM1/29/13

to django-...@googlegroups.com

#13794: Django does not respect to_field's model on an inline model admin
-------------------------------------+-------------------------------------
Reporter: sebastien@… | Owner: gautier
Type: Bug | Status: new
Component: Forms | Version:
Severity: Normal | 1.5-beta-1
Keywords: admin inline | Resolution:
to_field formset | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by anonymous):

After further investigation it seems that main functional difference
between the two is that not setting the fk value on form construction
(fix_inline_model_with_to_field.diff) results in the relationship field
not being available on a new instance until after full_clean, specifically
construct_instance, is called on the formset. Setting the fk value on
construction (to_field_formsets.patch), means that the fk value and
subsequently the relationship is available when dealing with a new item.

Adding this line to the test in to_field_formsets.path will make the test
pass for that patch but fail for fix_inline_model_with_to_field.diff:

{{{
self.assertEqual(formset[1].instance.user_id, "guido")
}}}

So there is a slight chance of fix_inline_model_with_to_field.diff
breaking currently working code while to_field_formsets.patch will not.
But as far as I can tell this undocumented so it's really your call either
way.

The problem is that right now, this does have potential to create invalid
relationships, leading to data corruption. If the value set in
_construct_form happens to be the same as a valid pk value of that same
table, then the save happens silently resetting all your relationships to
point to the wrong record.

--
Ticket URL: <https://code.djangoproject.com/ticket/13794#comment:12>

Django

unread,

Mar 25, 2013, 1:30:12 PM3/25/13

to django-...@googlegroups.com

#13794: Django does not respect to_field's model on an inline model admin
-------------------------------------+-------------------------------------
Reporter: sebastien@… | Owner: gautier
Type: Bug | Status: new
Component: Forms | Version:

Severity: Release blocker | 1.5-beta-1

Keywords: admin inline | Resolution:
to_field formset | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by anonymous):

* severity: Normal => Release blocker

--
Ticket URL: <https://code.djangoproject.com/ticket/13794#comment:13>

Django

unread,

Mar 25, 2013, 2:27:50 PM3/25/13

to django-...@googlegroups.com

#13794: Django does not respect to_field's model on an inline model admin
-------------------------------------+-------------------------------------
Reporter: sebastien@… | Owner: gautier
Type: Bug | Status: new
Component: Forms | Version:

Severity: Normal | 1.5-beta-1

Keywords: admin inline | Resolution:
to_field formset | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by aaugustin):

* severity: Release blocker => Normal

Comment:

Why would this be a release blocker?

--
Ticket URL: <https://code.djangoproject.com/ticket/13794#comment:14>

unread,

Jan 14, 2014, 12:00:28 PM1/14/14

to django-...@googlegroups.com

#13794: Django does not respect to_field's model on an inline model admin
-------------------------------------+-------------------------------------
Reporter: sebastien@… | Owner: gautier
Type: Bug | Status: new

* version: 1.5-beta-1 => master

Comment:

Updated patch to apply against master. Would be really nice to get this
fixed. Again this bug can have the nasty side effect of reseting your

relationships to point to the wrong record.

--
Ticket URL: <https://code.djangoproject.com/ticket/13794#comment:18>

Django

unread,

Jun 13, 2014, 6:16:53 PM6/13/14

to django-...@googlegroups.com

#13794: Django does not respect to_field's model on an inline model admin
-------------------------------------+-------------------------------------
Reporter: sebastien@… | Owner: gautier
Type: Bug | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: admin inline | Triage Stage: Accepted
to_field formset | Needs documentation: 0

Has patch: 1 | Patch needs improvement: 1

Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Changes (by timo):

* needs_better_patch: 0 => 1

Comment:

The latest version of of the patch doesn't include the test from the
earlier version. Please uncheck "Patch needs improvement" if someone can
update it. Thanks.

--
Ticket URL: <https://code.djangoproject.com/ticket/13794#comment:19>

Django

unread,

Jul 1, 2014, 1:21:06 PM7/1/14

to django-...@googlegroups.com

#13794: Django does not respect to_field's model on an inline model admin
-------------------------------------+-------------------------------------
Reporter: sebastien@… | Owner: gautier
Type: Bug | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: admin inline | Triage Stage: Accepted
to_field formset | Needs documentation: 0

Has patch: 1 | Patch needs improvement: 0

Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Changes (by anonymous):

* needs_better_patch: 1 => 0

Comment:

Ok patch updated with the test.

--
Ticket URL: <https://code.djangoproject.com/ticket/13794#comment:20>

Django

unread,

Jul 8, 2014, 12:36:27 PM7/8/14

to django-...@googlegroups.com

#13794: Django does not respect to_field's model on an inline model admin
-------------------------------------+-------------------------------------
Reporter: sebastien@… | Owner: gautier
Type: Bug | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: admin inline | Triage Stage: Accepted
to_field formset | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by timo):

No tests fail if I remove the line `fk_value = getattr(fk_value, 'pk',
fk_value)`. Could you clarify if that's needed and add a test if so?

--
Ticket URL: <https://code.djangoproject.com/ticket/13794#comment:21>

Django

unread,

Jul 8, 2014, 2:22:45 PM7/8/14

to django-...@googlegroups.com

#13794: Django does not respect to_field's model on an inline model admin
-------------------------------------+-------------------------------------
Reporter: sebastien@… | Owner: gautier
Type: Bug | Status: new
Component: Forms | Version: master
Severity: Normal | Resolution:
Keywords: admin inline | Triage Stage: Accepted
to_field formset | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by anonymous):

It's an edge case of an edge case. It was done to match what happens in
save_new. Added a test.

--
Ticket URL: <https://code.djangoproject.com/ticket/13794#comment:22>

Django

unread,

Jul 9, 2014, 8:09:05 AM7/9/14

to django-...@googlegroups.com

#13794: Django does not respect to_field's model on an inline model admin
-------------------------------------+-------------------------------------
Reporter: sebastien@… | Owner: gautier

Type: Bug | Status: closed
Component: Forms | Version: master
Severity: Normal | Resolution: fixed

Changes (by Tim Graham <timograham@…>):

* status: new => closed
* resolution: => fixed

Comment:

In [changeset:"5e2c4a4bd1f86962842783e0b42ada7b9c14c247"]:
{{{
#!CommitTicketReference repository=""
revision="5e2c4a4bd1f86962842783e0b42ada7b9c14c247"
Fixed #13794 -- Fixed to_field usage in BaseInlineFormSet.

Thanks sebastien at clarisys.fr for the report and gautier
for the patch.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/13794#comment:23>

Django

unread,

Jul 10, 2014, 4:40:39 PM7/10/14

to django-...@googlegroups.com

#13794: Django does not respect to_field's model on an inline model admin
-------------------------------------+-------------------------------------
Reporter: sebastien@… | Owner: gautier
Type: Bug | Status: closed
Component: Forms | Version: master
Severity: Normal | Resolution: fixed
Keywords: admin inline | Triage Stage: Accepted
to_field formset | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by anonymous):

Thanks for the commit, any chance of this making it into 1.7? It is a
potentially destructive bug so it would be nice to see it in a stable
version sooner rather than later.

--
Ticket URL: <https://code.djangoproject.com/ticket/13794#comment:24>

Django

unread,

Jul 10, 2014, 5:24:51 PM7/10/14

to django-...@googlegroups.com

#13794: Django does not respect to_field's model on an inline model admin
-------------------------------------+-------------------------------------
Reporter: sebastien@… | Owner: gautier
Type: Bug | Status: closed
Component: Forms | Version: master
Severity: Normal | Resolution: fixed
Keywords: admin inline | Triage Stage: Accepted
to_field formset | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by claudep):

+1 to backport.

--
Ticket URL: <https://code.djangoproject.com/ticket/13794#comment:25>

Django

unread,

Jul 10, 2014, 5:41:25 PM7/10/14

to django-...@googlegroups.com

#13794: Django does not respect to_field's model on an inline model admin
-------------------------------------+-------------------------------------
Reporter: sebastien@… | Owner: gautier
Type: Bug | Status: closed
Component: Forms | Version: master
Severity: Normal | Resolution: fixed
Keywords: admin inline | Triage Stage: Accepted
to_field formset | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"736e289445aea5083da04ddd8734c3ff20310408"]:
{{{
#!CommitTicketReference repository=""
revision="736e289445aea5083da04ddd8734c3ff20310408"
[1.7.x] Fixed #13794 -- Fixed to_field usage in BaseInlineFormSet.

Thanks sebastien at clarisys.fr for the report and gautier
for the patch.

Backport of 5e2c4a4bd1 from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/13794#comment:26>

Django

unread,

Jul 12, 2014, 1:25:57 PM7/12/14

to django-...@googlegroups.com

#13794: Django does not respect to_field's model on an inline model admin
-------------------------------------+-------------------------------------
Reporter: sebastien@… | Owner: gautier
Type: Bug | Status: closed
Component: Forms | Version: master
Severity: Normal | Resolution: fixed
Keywords: admin inline | Triage Stage: Accepted
to_field formset | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by ramiro):

IMO, as the fixed issue can cause data loss in formsets (and not only in
contrib.admin inlines like its summary mentions) it qualifies for getting
backported to all the currently supported branches.

I've pushed the backport commits to:

-
https://github.com/ramiro/django/commit/bd7baed03f023e22933579283c0e9ef222dda212
-
https://github.com/ramiro/django/commit/4a3c5904c1d69737bfeb475405294e3614ea2c25
-
https://github.com/ramiro/django/commit/c89684cf2f8ac378846c226bee1c858172c1edc1

Opinions?

unread,

Jul 14, 2014, 8:15:47 PM7/14/14

to django-...@googlegroups.com

#13794: Django does not respect to_field's model on an inline model admin
-------------------------------------+-------------------------------------
Reporter: sebastien@… | Owner: gautier
Type: Bug | Status: closed
Component: Forms | Version: master
Severity: Normal | Resolution: fixed
Keywords: admin inline | Triage Stage: Accepted
to_field formset | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by Ramiro Morales <cramm0@…>):

In [changeset:"291e837bda016d8556c2f6a4712729955a1667d0"]:
{{{
#!CommitTicketReference repository=""
revision="291e837bda016d8556c2f6a4712729955a1667d0"
[1.5.x] Revert "Fixed #13794 -- Fixed to_field usage in
BaseInlineFormSet."