Code

Opened 3 years ago

Last modified 3 years ago

#15665 new Bug

Inline admins are broken when primary key is not an AutoField and not editable.

Reported by: sebastian_noack Owned by: nobody
Component: contrib.admin Version: 1.2
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

For each inline object, a hidden field for its primary key (if it is an AutoField) is included in the form. But if you inherit from a concrete model that has a custom (not AutoField) primary key, you end up in a situation where you have a primary key which is a OneToOneField that is not editable in the admin.

from django.db import models

class Event(models.Model):
    name = models.CharField(max_length=100, primary_key=True)

    def __unicode__(self):
        return self.name

class Person(models.Model):
    name = models.CharField(max_length=100, primary_key=True)

    def __unicode__(self):
        return self.name

class Visitor(Person):
    event = models.ForeignKey(Event)

    def __unicode__(self):
        return u"'%s' at '%s'" % (self.person_ptr, self.event)
from django.contrib import admin

class VisitorInline(admin.StackedInline):
    model = Visitor

class EventAdmin(admin.ModelAdmin):
    inlines = [VisitorInline]

admin.site.register(Event, EventAdmin)

In that case there is no element in the form that holds the primary key and you get the Exception below, when you try to save an existing object in the admin.

Traceback (most recent call last):

  File "/home/sebastian/djacap/fam/src/famtest/django/core/servers/basehttp.py", line 280, in run
    self.result = application(self.environ, self.start_response)

  File "/home/sebastian/djacap/fam/src/famtest/django/core/servers/basehttp.py", line 674, in __call__
    return self.application(environ, start_response)

  File "/home/sebastian/djacap/fam/src/famtest/django/core/handlers/wsgi.py", line 241, in __call__
    response = self.get_response(request)

  File "/home/sebastian/djacap/fam/src/famtest/django/core/handlers/base.py", line 141, in get_response
    return self.handle_uncaught_exception(request, resolver, sys.exc_info())

  File "/home/sebastian/djacap/fam/src/famtest/django/core/handlers/base.py", line 165, in handle_uncaught_exception
    return debug.technical_500_response(request, *exc_info)

  File "/home/sebastian/djacap/fam/src/famtest/django/views/debug.py", line 58, in technical_500_response
    html = reporter.get_traceback_html()

  File "/home/sebastian/djacap/fam/src/famtest/django/core/handlers/base.py", line 100, in get_response
    response = callback(request, *callback_args, **callback_kwargs)

  File "/home/sebastian/djacap/fam/src/famtest/django/contrib/admin/options.py", line 239, in wrapper
    return self.admin_site.admin_view(view)(*args, **kwargs)

  File "/home/sebastian/djacap/fam/src/famtest/django/utils/decorators.py", line 76, in _wrapped_view
    response = view_func(request, *args, **kwargs)

  File "/home/sebastian/djacap/fam/src/famtest/django/views/decorators/cache.py", line 69, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)

  File "/home/sebastian/djacap/fam/src/famtest/django/contrib/admin/sites.py", line 190, in inner
    return view(request, *args, **kwargs)

  File "/home/sebastian/djacap/fam/src/famtest/django/utils/decorators.py", line 21, in _wrapper
    return decorator(bound_func)(*args, **kwargs)

  File "/home/sebastian/djacap/fam/src/famtest/django/utils/decorators.py", line 76, in _wrapped_view
    response = view_func(request, *args, **kwargs)

  File "/home/sebastian/djacap/fam/src/famtest/django/utils/decorators.py", line 17, in bound_func
    return func(self, *args2, **kwargs2)

  File "/home/sebastian/djacap/fam/src/famtest/django/db/transaction.py", line 299, in _commit_on_success
    res = func(*args, **kw)

  File "/home/sebastian/djacap/fam/src/famtest/django/contrib/admin/options.py", line 890, in change_view
    queryset=inline.queryset(request))

  File "/home/sebastian/djacap/fam/src/famtest/django/forms/models.py", line 705, in __init__
    queryset=qs)

  File "/home/sebastian/djacap/fam/src/famtest/django/forms/models.py", line 429, in __init__
    super(BaseModelFormSet, self).__init__(**defaults)

  File "/home/sebastian/djacap/fam/src/famtest/django/forms/formsets.py", line 47, in __init__
    self._construct_forms()

  File "/home/sebastian/djacap/fam/src/famtest/django/forms/formsets.py", line 97, in _construct_forms
    self.forms.append(self._construct_form(i))

  File "/home/sebastian/djacap/fam/src/famtest/django/forms/models.py", line 718, in _construct_form
    form = super(BaseInlineFormSet, self)._construct_form(i, **kwargs)

  File "/home/sebastian/djacap/fam/src/famtest/django/forms/models.py", line 446, in _construct_form
    pk = self.data[pk_key]

  File "/home/sebastian/djacap/fam/src/famtest/django/utils/datastructures.py", line 235, in __getitem__
    raise MultiValueDictKeyError("Key %r not found in %r" % (key, self))

MultiValueDictKeyError: "Key 'visitor_set-0-person_ptr' not found in <QueryDict: {u'visitor_set-__prefix__-event': [u'FOSDEM'], u'name': [u'FOSDEM'], u'visitor_set-0-name': [u'Me'], u'visitor_set-3-event': [u'FOSDEM'], u'visitor_set-3-name': [u''], u'visitor_set-1-event': [u'FOSDEM'], u'visitor_set-1-name': [u''], u'visitor_set-2-event': [u'FOSDEM'], u'visitor_set-2-name': [u''], u'visitor_set-INITIAL_FORMS': [u'1'], u'visitor_set-TOTAL_FORMS': [u'4'], u'csrfmiddlewaretoken': [u'85dd78ff278138c4c70f2d1c8107bd95'], u'visitor_set-0-event': [u'FOSDEM'], u'_continue': [u'Save and continue editing'], u'visitor_set-MAX_NUM_FORMS': [u''], u'visitor_set-__prefix__-name': [u'']}>"

I found following line in admin/edit_inline/tabular.html and admin/edit_inline/stacked.html.

{% if inline_admin_form.has_auto_field %}{{ inline_admin_form.pk_field.field }}{% endif %}

Removing the check for has_auto_field fixed the error described above, but leads to wrong behavior, when modifying the value of the concrete base class's primary key (the name field in the example above) in the inline admin, resulting in a new entry.

I have reproduced that issue with django 1.2.3, 1.2.4, 1.2.5 and the latest version from SVN.

Attachments (0)

Change History (8)

comment:1 Changed 3 years ago by lukeplant

  • Type set to Bug

comment:2 Changed 3 years ago by lukeplant

  • Severity set to Normal

comment:3 Changed 3 years ago by anonymous

  • Component changed from Database layer (models, ORM) to contrib.admin
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:3 Changed 3 years ago by anonymous

  • Component changed from Database layer (models, ORM) to contrib.admin
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:4 Changed 3 years ago by lukeplant

  • Triage Stage changed from Unreviewed to Accepted

Accepted on the basis that we should do something better than crash. What exactly to do is tricky - it is similar to the case of editing the primary key value, which can result a new row rather than a changed row, IIRC.

comment:5 Changed 3 years ago by sebastian_noack

I was able to work around that issue and make the objects from the inline admin editable including their primary key by following hack. It deletes all objects and re-adds them again. In my opinion django must support that case by default. But maybe their is a better way as first removing all objects.

from django.contrib import admin
from django.db import transaction

class SuperAdmin(admin.ModelAdmin):
    inlines = [SubSubInline]

    def save_model(self, *args, **kwargs):
        super(SuperAdmin, self).save_model(*args, **kwargs)
        self._saved = True

    def change_view(self, request, *args, **kwargs):
        if request.method != 'POST':
            return super(SuperAdmin, self).change_view(request, *args, **kwargs)

        transaction.enter_transaction_management()
        transaction.managed(True)

        # ModelAdmin.change_view is decorated with autocommit. So we
        # have to lazy patch the transaction API in order to bypass
        # transaction management from within ModelAdmin.change_view.
        # So we can rollback the transaction afterwards if needed.
        transaction_api = (transaction.enter_transaction_management,
                           transaction.leave_transaction_management,
                           transaction.commit, transaction.rollback)

        transaction.enter_transaction_management = lambda *args, **kwargs: None
        transaction.leave_transaction_management = lambda *args, **kwargs: None
        transaction.commit                       = lambda *args, **kwargs: None
        transaction.rollback                     = lambda *args, **kwargs: None

        try:
            for inline in self.inline_instances:
                prefix = inline.get_formset(request).get_default_prefix()

                request.POST = request.POST.copy()
                request.POST[prefix + '-INITIAL_FORMS'] = '0'

                inline.queryset(request).delete()

            return super(SuperAdmin, self).change_view(request, *args, **kwargs)
        finally:
            (transaction.enter_transaction_management,
             transaction.leave_transaction_management,
             transaction.commit, transaction.rollback) = transaction_api

            if getattr(self, '_saved', False):
                del self._saved
                transaction.commit()
            else:
                transaction.rollback()
            transaction.leave_transaction_management()

comment:6 Changed 3 years ago by lukeplant

Probably related: #2259

comment:7 Changed 3 years ago by dArignac

  • Easy pickings unset
  • UI/UX unset

Looks like an duplicate of #13696

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as new
The owner will be changed from nobody to anonymous. Next status will be 'assigned'
as The resolution will be set. Next status will be 'closed'
Author


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

 
Note: See TracTickets for help on using tickets.