Django

Code

Ticket #8241 (closed: fixed)

Opened 2 years ago

Last modified 2 years ago

Primary ForeignKeys don't work with FormSets

Reported by: sciyoshi Assigned to: brosner
Milestone: 1.0 Component: django.contrib.admin
Version: SVN Keywords:
Cc: sciyoshi@gmail.com, ville@unessa.net Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 1 Patch needs improvement: 0

Description

Using this UserProfile? as an example: 

class UserProfile(models.Model):
    user = models.OneToOneField(primary_key=True)
    nickname = models.CharField(max_length=30)

from django.contrib.auth.models import User
from django.contrib.auth.admin import UserAdmin
from django.contrib.admin import site, StackedInline

class UserProfileAdmin(StackedInline)
    model = UserProfile
    max_num = 1

UserAdmin.inlines = UserAdmin.inlines + [UserProfileAdmin]

site.unregister(User)
site.register(User, UserAdmin)

Trying to save a User from the admin causes a KeyError?: 'user_id' because the primary key doesn't get rendered in the form.

Attachments

8241.patch (2.2 kB) - added by sciyoshi on 08/13/08 11:44:46.
onetoone_admin.8461.diff (2.7 kB) - added by semenov on 08/21/08 16:12:32.
onetoone_admin.8541.diff (3.2 kB) - added by semenov on 08/25/08 12:19:37.

Change History

08/11/08 16:26:48 changed by sciyoshi

  • needs_better_patch changed.
  • needs_tests changed.
  • needs_docs changed.

That should be 

user = models.OneToOneField(User, primary_key=True)

08/12/08 13:27:28 changed by jacob

  • stage changed from Unreviewed to Accepted.
  • component changed from Uncategorized to Admin interface.
  • milestone set to 1.0.

(follow-up: ↓ 4 ) 08/13/08 04:51:11 changed by mremolt

Isn't this a duplicate for or at least related to #7947 ?

(in reply to: ↑ 3 ) 08/13/08 11:19:34 changed by sciyoshi

Replying to mremolt:

Isn't this a duplicate for or at least related to #7947 ?

I don't think so; applying the patch from that ticket doesn't fix the bug, and the same behavior also occurs if we have 

user = models.ForeignKey(User, primary_key=True)

I think the problem could be fixed somewhere around add_fields in BaseModelFormSet?: 

    def add_fields(self, form, index):
        """Add a hidden field for the object's primary key."""
        if self.model._meta.has_auto_field:
            self._pk_field_name = self.model._meta.pk.attname
            form.fields[self._pk_field_name] = IntegerField(required=False, widget=HiddenInput)
        super(BaseModelFormSet, self).add_fields(form, index)

If the primary key isn't an AutoField? it doesn't get added to the fields here...

08/13/08 11:44:26 changed by sciyoshi

The following patch fixes it for me (for OneToOneFields? the patch from #7949 needs to be applied first), but probably breaks other things; namely, model_formsets fails. If anybody has ideas on how to better implement this let me know...

08/13/08 11:44:46 changed by sciyoshi

  • attachment 8241.patch added.

08/13/08 11:44:57 changed by sciyoshi

  • needs_better_patch set to 1.
  • has_patch set to 1.
  • needs_tests set to 1.

08/21/08 16:12:02 changed by semenov

  • needs_better_patch deleted.

sciyoshi, first of all, thanks for your patch. It inspired me for further analysis of the problem. :)

From what I've discovered, your patch suffers from the following:

1. It curcumvents into the naming convention of form fields and adds a form field named user_id (i.e., taken as field.attname, not as field.name). At first glance, that seems to be ok: the field is filled by model_to_dict(), passed thru browser POST, but then ignored by save_instance(). Alas, that would break things if a model had TWO fields like user=OneToOneField?() and user_id=CharField?(). We shouldn't be messing with the naming convention: all form fields should always use field.name as their names.

2. The patch breaks BaseModelFormSet?.add_fields() contract, which currently adds a new field if (and only if) a primary key is an AutoField? -- what I consider to be a desired behaviour.

I'm attaching the updated patch (vs [8461]), which resolves the both problems.

08/21/08 16:12:32 changed by semenov

  • attachment onetoone_admin.8461.diff added.

08/21/08 16:16:48 changed by semenov

Also, I didn't get the idea behind parent_link, which was in your patch, and also was discussed in #7947. I replaced that with: 

if not kwargs.get('primary_key', False): 
    kwargs['editable'] = False

which I admit might be wrong for some obscure reason...

08/25/08 12:19:22 changed by semenov

If anybody cares, I'm attaching the updated patch against [8541], since [8528] broke things again (but in a different way).

08/25/08 12:19:37 changed by semenov

  • attachment onetoone_admin.8541.diff added.

08/25/08 13:03:17 changed by semenov

Note: I really don't like my drill down until we find a "real" pk approach; but I don't have enough deep knowledge of django/db internals, so that was the only way that seemed correct to me (more or less).

08/26/08 00:44:29 changed by Uninen

  • cc changed from sciyoshi@gmail.com to sciyoshi@gmail.com, ville@unessa.net.

08/29/08 11:29:46 changed by brosner

  • status changed from new to closed.
  • resolution set to invalid.

It appears this ticket has been caught in the middle of some commit rapid fire. In the fire the issue reported here is fixed. The issue that semenov brings up is not related, but I think should be fixed. Can you please open a ticket talking about the namespace stomping. Please reopen if the issue reported has not been fixed on trunk with detailed and concise use cases.

08/29/08 13:44:21 changed by brosner

  • status changed from closed to reopened.
  • resolution deleted.

Ok. I take back that this is fixed. I had a bad test case. Reopening.

08/29/08 13:44:28 changed by brosner

  • owner changed from nobody to brosner.
  • status changed from reopened to new.

08/31/08 04:50:51 changed by brosner

  • status changed from new to closed.
  • resolution set to fixed.

Fixed in [8756].

Add/Change #8241 (Primary ForeignKeys don't work with FormSets)




Change Properties
Action