Admin Edit Inline templates don' t output hidden field for PK when it isn't an autoField and has editable=False

[evan.reiser@…]

In my project I'm using a UUID field where I generate the uuid myself in python, and don't rely on any sequences on the database side. Because of this I just use a CharField field, and when used i set it to be the primary key. However because of this usage the field does not have 'has_auto_field' == True, This causes the edit inline forms for existing objects to not output the hidden field to hold the PK for the existing object.

Right now this template requires the inline_admin_form.has_auto_field == True, for it to output the pk_field.field, however I think this should always output the primary key for the object.

This line occurs on line 30 of contrib/admin/templates/admin/edit_inline/tabular.html in django 1.2.1

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

contrib/admin/templates/admin/edit_inline/tabular.html

        <td class="original">
          {% if inline_admin_form.original or inline_admin_form.show_url %}<p>
          {% if inline_admin_form.original %} {{ inline_admin_form.original }}{% endif %}
          {% if inline_admin_form.show_url %}<a href="../../../r/{{ inline_admin_form.original_content_type_id }}/{{ inline_admin_form.original.id }}/">{% trans "View on site" %}</a>{% endif %}
            </p>{% endif %}
          {% if inline_admin_form.has_auto_field %}{{ inline_admin_form.pk_field.field }}{% endif %}
          {{ inline_admin_form.fk_field.field }}
          {% spaceless %}
          {% for fieldset in inline_admin_form %}
            {% for line in fieldset %}
              {% for field in line %}
                {% if field.is_hidden %} {{ field.field }} {% endif %}
              {% endfor %}
            {% endfor %}
          {% endfor %}
          {% endspaceless %}
        </td>

[Karen Tracey]

The problem description does not mention what the problem is, exactly. It states how some code behaves in a particular circumstance, but leaves out what the negative effect of that behavior might be? We have tests that verify that inlines with explicit non-auto PK fields can be created, saved, edited and saved again in the admin (see ​http://code.djangoproject.com/ticket/13696), so I'm struggling to understand what exactly the problem reported here is? What exactly is not working properly?

[Evan Reiser <evan.reiser@…>]

It seems to be that this this problem happens for models which have a primary key field that is not based on the AutoField (such as a CharField).

When the primary key field is not based on an AutoField 'has_auto_field' is not set to True, therefore it will not output the primary key in the form. (as you can see on line 30 of the tabular.html

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

These inline forms should still work for any primary key (in my opinion), not only when the field is based on an AutoField.
You said there area already tests in place to check for this, but according to the template, I'm not sure how this would be possible unless the field is set to be "has_auto_field" = True.

It seems that changing line 30 to this fixes the problem:

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

[Karen Tracey]

But...it is only AutoFields that are specifically not part of the form by default, so it is only in that case that the admin has to do something special to ensure they are on the form. I'm still missing the problem you are actually observing, vs. the specifics of the reasons for it that you seem to have diagnosed. Could you step back and describe the problem with the admin behavior that you are actually seeing? As I said, we have tests that verify that inlines with non-auto-field PKs can be added and updated. I've also manually verified that myself. I can add and update inlines with explicit non-auto character field PKs. I don't see any exceptions raised or incorrect data saved. So I am still struggling to understand what problem you are observing that would require a change in the admin template here.

[David Wolever]

For what it's worth, I've been burned by this too.

To clarify a bit: it occurs when the id field is not an AutoField and editable=False. To use the example from the documentation:

import random
from django.db import models as m

class Author(m.Model):
    name = m.CharField(max_length=100)

class Book(m.Model):
    id = m.CharField(max_length=32, primary_key=True, editable=False)
    author = m.ForeignKey(Author)
    title = m.CharField(max_length=100)

    def save(self, *args, **kwargs):
        if not self.id:
            self.id = str(random.random())
        super(Book, self).save(*args, **kwargs) 

In the Author admin page, there is no reference to the ids of the related Books (clean up a bit):

<td class="original">
  <p>Book object</p>
  <!-- this line *should* be here, but isn't: <input type="hidden" name="book_set-0-id" value="0.23411" id="id_book_set-0-id" /> -->
  <input type="hidden" name="book_set-0-author" value="1" id="id_book_set-0-author" />
</td>

If this can be confirmed, I'll supply tests.

[David Wolever]

As a warning: the “obvious” work around for this is to have custom ID fields “pretend” to be auto fields by doing something like this:

def contribute_to_class(...): # XXX BAD XXX
    cls._meta.has_auto_field = True
    ls._meta.auto_field = self

However, this doesn't quite work either, because line 525 of /Users/wolever/Verso/Repos/web/env/massuni-web/lib/python2.6/site-packages/django/db/models/base.py in save_base(update_pk = bool(meta.has_auto_field and not pk_set)), sets the update_pk flag, which essentially causes:

if update_pk:
    instance.id = get_last_inserted_id()

And, at least with the SQLite backend, the result of get_last_inserted_id is the row number (or similar), *not* the value of the id field.

So, in short, this issue can't be hacked around by setting has_auto_field.

[Russell Keith-Magee]

The combination of editable=False and a non-auto primary key field appears to be the key here. On first inspection, it looks to me like the inlines should be looking for a "is primary key and not editable", rather than "is AutoField", as the condition for displaying the hidden ID column.

[evan.reiser@…]

This is a very easy fix, all you need to do is change this line:

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

to this:

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

[Alexander Herrmann]

the patch for the issue

[Ramiro Morales]

Replying to dArignac:

Status changed from assigned to closed

Thanks for the patch! But there is no need to close the ticket as fixed, the has patch one is the way to mark it has a fix a fix ready for review.

Fixed is used when/if the issue is actually fixed either by applying it (or another fix) to the code (by a core committer) . Otherwise it will be completely under the radar for people looking for unsolved tickets or patches to review and commit, causing effectively the exact contrary effect to the one you intended when you worked in the issue in the first place.

See ​https://docs.djangoproject.com/en/dev/internals/contributing/triaging-tickets/#closing-tickets

[Alexander Herrmann]

Oh ok, sorry, so I got it wrong. Will not happen next time!

Replying to ramiro:

Replying to dArignac:

Status changed from assigned to closed

Thanks for the patch! But there is no need to close the ticket as fixed, the has patch one is the way to mark it has a fix a fix ready for review.

Fixed is used when/if the issue is actually fixed either by applying it (or another fix) to the code (by a core committer) . Otherwise it will be completely under the radar for people looking for unsolved tickets or patches to review and commit, causing effectively the exact contrary effect to the one you intended when you worked in the issue in the first place.

See ​https://docs.djangoproject.com/en/dev/internals/contributing/triaging-tickets/#closing-tickets

[Issac Kelly]

Tested against 1.4-pre-alpha and still fixes this issue.

[Ramiro Morales]

The note in this comment: https://code.djangoproject.com/browser/django/trunk/django/forms/models.py?rev=17373#L637 seems to indicate that AutoField's editable value isn't False so I don't think this patch is ready for commit. The condition in the if test would need to be a bit more complex than the two proposed so far: Checking for the field being a PK field (as implemented in the patch) or checking of it being a PK and not editable.

[Alexander Herrmann]

I don't get where exactly the problem now is. If I include an AutoField setting its property "editable" to True, it still cannot be edited within the AdminSite, there is also no problem saving this. For all custom Fields being primary keys (primary_key=True) the current fix does work. Please explain, thanks!

[brianglass]

I am affected by a very similar problem (though technically unrelated) in the same template. When I have a model that has a field that is not named id, the "View on site" link in the inline editing section is missing the pk in the url. So the link ends up going to nowhere. The relevant code is:

{% if inline_admin_form.show_url %}<a href="../../../r/{{ inline_admin_form.original_content_type_id }}/{{ inline_admin_form.original.id }}/">{% trans "View on site" %}</a>{% endif %}

Instead of {{ inline_admin_form.original.id }}, I think it should be {{ inline_admin_form.original.pk }}.

[evan.reiser@…]

Brianglass: Yes. I agree. It's such an obvious and easy fix, its too bad that there is so much bureaucracy around changing it.

The underlying problem is that the code is assuming you always have primary keys named "id" that are auto incrementing, which obviously isn't the case.

[anonymous]

We've been bitten by this as well. This bug has been replicated in Django grappelli. So for anyone that bumps into this bug you can override admin/edit_inline/tabular.html and/or admin/edit_inline/stacked.html similarly to the patch that's been posted here.

Also note that django_extensions' UUIDField contains a hack to work around this bug. It pretends to be an auto field, which doesn't work well, as pointed out by wolever above. I'm trying to get that hack removed from django-extensions: ​https://github.com/django-extensions/django-extensions/pull/332

[Karen Tracey]

Pull request with slightly altered fix: ​https://github.com/django/django/pull/1381

The proposed fix duplicated the pk field on the inlines if there was a non-auto EDITABLE pk field already rendered. The pull request tests for that (previous fix would fail that test) in addition to testing for the non-auto non-editable pk.

[Karen Tracey <kmtracey@…>]

In 3aad955ea8db1592fad0012155eaa25b72e50dc5:

Fixed #13696 -- ensured inline pk field is rendered

[Karen Tracey <kmtracey@…>]

In 706e542eb5db0d14277f34566d351b875e645bac:

[1.6.x] Fixed #13696 -- ensured inline pk field is rendered

Backport of 3aad955ea8db1592fad0012155eaa25b72e50dc5 from master.