Opened 10 years ago

Closed 10 years ago

Last modified 9 years ago

#81 closed defect (fixed)

Setting primary_key=True on an non-integer field isn't yet supported

Reported by: jmb@… Owned by: adrian
Component: contrib.admin Version:
Severity: major Keywords:
Cc: paul.bowsher@… Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

The admin interface appears to want to keep the primary_key numerical, and so breaks if you have set a field as primary_key=True.

Model:

class Airfield(meta.Model):
    fields = (
        meta.CharField('code', 'airfield code', maxlength=4, primary_key=True),
        meta.CharField('name', 'name', maxlength=50),
    )
    admin = meta.Admin(
        fields = (
            (None, {'fields': ('code', 'name')}),
        ),
    )
    def __repr__(self):
        return self.name

Result from trying to add an item through the admin interface:

There's been an error:

Traceback (most recent call last):

  File "/sw/lib/python2.3/site-packages/django/core/handlers/wsgi.py", line 190, in get_response
    return callback(request, **param_dict)

  File "/sw/lib/python2.3/site-packages/django/views/admin/main.py", line 769, in add_stage
    log.log_action(request.user.id, opts.get_content_type_id(), getattr(new_object, opts.pk.name), repr(new_object), log.ADDITION)

  File "/sw/lib/python2.3/site-packages/django/models/auth.py", line 293, in _module_log_action
    e.save()

  File "/sw/lib/python2.3/site-packages/django/core/meta.py", line 57, in _curried
    return args[0](*(args[1:]+moreargs), **dict(kwargs.items() + morekwargs.items()))

  File "/sw/lib/python2.3/site-packages/django/core/meta.py", line 748, in method_save
    (opts.db_table, ','.join(field_names), ','.join(placeholders)), db_values)

  File "/sw/lib/python2.3/site-packages/django/core/db/base.py", line 10, in execute
    result = self.cursor.execute(sql, params)

ProgrammingError: ERROR:  invalid input syntax for integer: "EGKR"

INSERT INTO auth_admin_log (action_time,user_id,content_type_id,object_id,object_repr,action_flag,change_message) VALUES ('2005-07-19 01:31:40',1,17,'EGKR','Redhill',1,'')

Change History (11)

comment:1 Changed 10 years ago by Manuzhai <mail@…>

Why this is currently not working: the model decides if an object is new by doing

not bool(getattr(self, opts.pk.name))

BUT for non-AutoField PK's, the PK will be filled in by the user, so it thinks the object isn't new when in fact it is, so it tries to update it instead of creating it. In order to fix this, some additional attribute is probably required that keeps track of whether the object is new or not.

comment:2 Changed 10 years ago by hugo <gb@…>

  • milestone set to Version 1.0
  • Summary changed from Setting primary_key=True in a model class causes the admin interface to break to Setting primary_key=True on an non-integer field causes the admin interface to break

This problem arises because the admin tries to log the change in the auth_admin_log. It references objects from there by an integer ID, but your model uses a non-integer primary key. The admin needs to be enhanced to handle non-integer primary keys for this to work (I have a similar problem in one of my play projects, too).

comment:3 Changed 10 years ago by hugo <gb@…>

An alternative solution (as changing the object_id in the admin loggin would require a change to the database schema of the log) the admin logging could allow logging of "unlinked" events - if the key isn't an integer, just store the data with NULL as the key and don't link to the changed object. That way we can at least add objects via admin and get log notifications.

comment:4 Changed 10 years ago by boffbowsh@…

  • Cc paul.bowsher@… added

If you change the field type yourself as a temporary fix, does stuff still break?

comment:5 Changed 10 years ago by adrian

(In [455]) Slightly refactored metasystem -- changed Field pre_save() hooks to pre_save_add(), in preparation for some bigger changes. Refs #81.

comment:6 Changed 10 years ago by adrian

  • Status changed from new to assigned

comment:7 Changed 10 years ago by adrian

  • Summary changed from Setting primary_key=True on an non-integer field causes the admin interface to break to Setting primary_key=True on an non-integer field isn't yet supported

comment:8 Changed 10 years ago by adrian

(In [462]) Refactored the way save() works so that non-integer primary keys are now possible. custom_pk unit tests (from [458]) now pass. Refs #81.

comment:9 Changed 10 years ago by hugo <gb@…>

saving does work now, only admin auth log still produces a traceback and viewing an object in the admin doesn't work if the primary key isn't an integer, due to the too restrictive urlpatterns (they use \d+ - maybe they should use \S+, instead)

comment:10 Changed 10 years ago by adrian

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [469]) Fixed #81 -- Admin now supports primary_key=True for non-integer fields. Note that you'll have to make a change to your database if you're using a previous Django installation and want to use non-integer primary key fields. See the BackwardsIncompatibleChanges wiki page for info.

comment:11 Changed 9 years ago by anonymous

  • milestone Version 1.0 deleted

Milestone Version 1.0 deleted

Note: See TracTickets for help on using tickets.
Back to Top