"Editable = False" + "Edit Inline" in new forms admin

[magneto]

An error involved which now seems realted to the editable = false + edit inline

class HandleOption(models.Model):
      handle_option_id = models.PositiveIntegerField(primary_key=True, editable = false)
      name = models.CharField(max_length=100)

class Product(models.Model):
      product_id = models.PositiveIntegerField(primary_key=True, editable = false)
      name = models.CharField(max_length=100)

class ProductHandling(models.Model):
	product_handling_id = models.PositiveIntegerField(primary_key=True, editable = false)
	product = models.ForeignKey(Product)
	handle_option = models.ForeignKey(HandleOption, core = True)
	
# Admin bits

class ProductHandling_Inline(admin.StackedInline):
	model = ProductHandling

class ProductOptions(admin.ModelAdmin):
	inlines = [ProductHandling_Inline]

The form renders properly, However on 'Save' we get a trace back

Traceback:
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/django/core/handlers/base.py" in get_response
  87.                 response = callback(request, *callback_args, **callback_kwargs)
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/django/contrib/admin/sites.py" in root
  155.                 return self.model_page(request, *url.split('/', 2))
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/django/views/decorators/cache.py" in _wrapped_view_func
  44.         response = view_func(request, *args, **kwargs)
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/django/contrib/admin/sites.py" in model_page
  172.         return admin_obj(request, rest_of_url)
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/django/contrib/admin/options.py" in __call__
  251.             return self.change_view(request, unquote(url))
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/django/contrib/admin/options.py" in change_view
  550.                 return self.save_change(request, form, inline_formsets)
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/django/db/transaction.py" in _commit_on_success
  198.                 res = func(*args, **kw)
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/django/contrib/admin/options.py" in save_change
  390.                 formset.save()
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/django/forms/models.py" in save
  339.         return self.save_existing_objects(commit) + self.save_new_objects(commit)
File "/Library/Frameworks/Python.framework/Versions/2.5/lib/python2.5/django/forms/models.py" in save_existing_objects
  355.             obj = existing_objects[form.cleaned_data[self.model._meta.pk.attname]]

Exception Type: KeyError at /admin/products/product/647/
Exception Value: None

The variables at the time of save at the traceback point

###
# existing_objects
###
{
3221L: <ProductHandling>, 
3222L: <ProductHandling>,
3223L: <ProductHandling>, 
}

###
# form.cleaned_data
###
{'DELETE': False, 'product_handling_id': None, 'handle_option': <HandleOption: [Postprocessing] - frame - Black Metal Frame>} 

###
# self.model._meta.pk.attname
###
product_handling_id

IF i Take OUT editable

class ProductHandling(models.Model):
	product_handling_id = models.PositiveIntegerField(primary_key=True)
	product = models.ForeignKey(Product)
	handle_option = models.ForeignKey(HandleOption, core = True)

All functions well, BUT there is a 'blank' row in the admin form (obviously a <input hidden> but with an attribute Label present)

seems in the edit_inline case where a field is not editable, but is the explicit primary Key (not the usual automatic Primary key), it should be treated in the same fashion as the auto primary key field

[Brian Rosner]

Ok, what needs to be fixed here is the BaseModelFormSet always makes the primary key as a hidden field. That is wrong and should understand the difference between an automatically created primary key and a custom override. When it is overidden such as your case above it should always allow the field to edited by the user. It would be wrong to treat them the same since Django cannot provide the value implicitly. Therefore the exception you see in the editable=False case is correct. You would have to provide it yourself.

[Brian Rosner]

Actually let me re-think this. I am missing something here.

[magneto]

basically if 'editable = True' (or not set)

the 'inline edit' _does_ hide the field because it is a primary Key, so that is correct .. what is werid is that is shows the 'label' for it still

i.e. in the above examples

product handling id | <input hidden>
handle option | <select menu>

it really should be just

<input hidden>
handle option | <select menu>

but if editable = false, it forgets to add the input hidden
so the form skeleton is just

handle_option | <select menu>

thus the 'save aux elements' in the form.cleaned_data is lacking the PK to save it

[Brian Rosner]

I disagree. When editable=True on a custom primary key and non-AutoField the field should *show* on the form. There is no way of providing that value otherwise unless of course you provide it at the code level. The correct fix and I stand by what I said earlier is that custom primary keys should always show on the form.

[magneto]

Sure, Editable = True can certainly show the input box .. not much to disagree about there, that's just one part of the bug.

The other part is the 'editable = false',

In the inline Model, being a Primary Key, should always slap a hidden field regardless of the edit-ability of it

the above patch still does not address that little issue.

It's almost as if any place there is an " if not editable " it needs to be conditioned with " if not editable and not primary_key "

here is a small little patch .. however, it still shows the 'hidden' row in the inline stacked presentation .. but at least the PK is in the form and so it can update/save the object

[magneto]

Force the display of editable=false, pk=true for inline models (the field ends up being hidden)

[Brian Rosner]

I still think we are not coming to the right agreement here. Let me outline each case as I see it:

• If the primary key was automatically generated it is always hidden.
• If the primary key was defined by hand *and* is a of type AutoField (aka self.model._meta.has_auto_field) it is always hidden regardless of the editable value.
• If the primary key was defined by hand, is of some other type, and editable=True (or not given) it is always shown.
• If the primary key was defined by hand, is of some other type, and editable=False then it is *not* shown, but if not given a value before saving the database will complain (correct behavior).

There doesn't need to be any changes to newforms outside of a formset because the bug is formset specific. newforms handles everything correctly. I hope I am not repeating what you are saying, but your patch indicates fixing the wrong thing and doesn't even appear correct.

[magneto]

ya, the patch is not 'right' and is more to demonstrate what i think 'should' happen (the PK shows up, but is not editable).

but the issue involves your last statement

• If the primary key was defined by hand, is of some other type, and editable=False then it is *not* shown, but if not given a value before saving the database will complain (correct behavior).

When one edits Inline, the PK still needs to be there for the saving to work. Its not the DB complaining, its the Form Validator complaining it cannot find the PK as it wants to update things.

#this works (does NOT show the editbox)
	product_handling_id = models.AutoField(primary_key=True)
# this works (but shows the edit box)
	product_handling_id = models.PositiveIntegerField(primary_key=True)
#this fails
	product_handling_id = models.AutoField(primary_key=True, editable = False)
#this fails
	product_handling_id = models.PositiveIntegerField(primary_key=True, editable = False)

If you try the admin from the above example the PK field never makes it into the form, thus makeing it editable when it should not be (PositiveIntegerField vs AutoField is really a Database Optimization, as the default AutoField translates into an IntField).

Soooo. It seems the Field definitions should allow for a keyword that says "I Am The AutoField", which given the proximity to v1.0 probably won't happen.

PositiveIntegerField(primary_key = True, auto_field = True)

[magneto]

Ok, in order to effect the my auto_field = True Field option, here is a bigger patch, that does just that

used exactly like

my_col = models.PositiveIntegerField(primary_key = True, auto_field = True)

And this will be treated like any other "AutoField" field, except now we get to specify a custom db_type,

I Am aware i can probably just subclass AutoField, so if y'all think this is too silly, just mark the thing as fixed using the little patch brosner posted

[magneto]

(bad diff file)

[Brian Rosner]

Ok, so we have finally uncovered that we were dealing with two different issues that are closely related. The patch I posted is indeed fixing part of the problem and I will commit. Either open a new ticket or see if it has been reported before, it likely has about a custom AutoField. It would likely take the route of just subclassing AutoField there.

[Erwin]

The original problem also exists in the following situation

class Fruit(models.Model)
    name = models.Charfield("Name", max_length=50)

class Apple(models.Model)
    organization = models.OneToOneField('Fruit', primary_key=True
    color = models.Charfield("Name", max_length=50)

class AppleInline:
    model = Apple

class FruitAdmin
    inlines = [AppleInline]

In this case, when editing an existing product, the hidden field created doesn't have a value:

<input type="hidden" id="some_value" class="some_value" /> value="" is missing

[Brian Rosner]

Ok, the original problem actually (I know realize) is a duplicate of #7888. Marking as so. However, this ticket also discovered a different bug that I will commit now.

[Brian Rosner]

Actually thinking about it. This really isn't a duplicate, but rather a primary key should not be marked editable=False since that will caused the above error. The built-in AutoField returns None in its formfield method which causes it to not display but is still accessible in the cleaned_data dict. To accomplish this now just subclass the field type and do the same. Otherwise open a new ticket regarding custom primary keys since this describes it as being a formset issue, which it really isn't.

[Brian Rosner]

(In [8179]) Ensure that custom primary keys are always shown on a formset. Refs #7938. Thanks magneto for discovering this problem.

[steve918]

I have a custom primary key that uses an auto-generated id that should not be user editable. As suggested formfield method returns None like so:

class HackInlineCharField(models.CharField):

    def formfield(self, **kwargs):
        return None


class ScreenShot(Model):
    
    id = HackInlineCharField(primary_key=True, max_length=32, default=unique_key)

but that results in the following error:

TemplateSyntaxError at /undroid-admin/stories/article/2/

Caught an exception while rendering: "Key 'id' not found in Form"

Is it currently possible to have non-editable custom primary keys in formsets?

[o`dnik]

Do not make your pk field editable=True!!! It may be a security issue in some cases. Instead, you should
p = form.save(commit=False)
p.custom_pk_field = custom_pk
p.save()
It's simple and works.