#9865 closed (fixed)
inline-edited objects with custom PKs cannot be saved
Reported by: | Wojciech Bartosiak | Owned by: | nobody |
---|---|---|---|
Component: | contrib.formtools | Version: | dev |
Severity: | Keywords: | inlineformset_factory | |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
"patients_guardian.pesel may not be NULL" when trying to save related object after "is_valid()" on "save()" method
my models are:
class Patient(models.Model): pesel = models.DecimalField(verbose_name=_('PESEL'), max_digits=20, decimal_places=0, primary_key=True, unique=True) name = models.CharField(verbose_name=_('Imie'), max_length=20) surname = models.CharField(verbose_name=_('Nazwisko'), max_length=20) age = models.IntegerField(verbose_name=_('Wiek')) def __unicode__(self): return '[ %s ] - %s %s' % (self.pesel, self.surname, self.name); class Meta: verbose_name = _('Pacjent') verbose_name_plural = _('Pacjenci') ordering = ['pesel'] class Guardian(models.Model): FAMILY_RELATIONSHIP = ( ('R', _('Rodzic')), ('s', _('Prawny')), ('o', _('Inne')), ) patient = models.ForeignKey(Patient) pesel = models.DecimalField(verbose_name=_('PESEL'), max_digits=20, decimal_places=0 ,primary_key=True, unique=True) name = models.CharField(verbose_name=_('Imie'), max_length=20) surname = models.CharField(verbose_name=_('Nazwisko'), max_length=20) relationship = models.CharField(verbose_name=_('Pokrewienstwo'), max_length=1, choices=FAMILY_RELATIONSHIP) def __unicode__(self): return '[ %s ] - %s %s' % (self.pesel, self.surname, self.name); class Meta: verbose_name = _('Opiekun') verbose_name_plural = _('Opiekunowie') ordering = ['patient', 'pesel']
and my view is:
@login_required @transaction.commit_on_success def new_patient_and_guardian(request): patient_form = modelform_factory(Patient) guardian_form = inlineformset_factory(Patient, Guardian, extra=1, max_num=1) patient_f=patient_form() guardian_f=guardian_form(clone(request.POST)) error_message = "" if request.method=="POST": patient_f=patient_form(clone(request.POST)) if patient_f.is_valid(): try: patient = patient_f.save() guardian_f=guardian_form(clone(request.POST), instance=patient) if guardian_f.is_valid(): guardian_f.save() transaction.commit() return HttpResponseRedirect(reverse('viomed.patients.views.patient_address_edit', args=[patient.pesel])) except Exception, (msg): error_message = "%s" % (msg) transaction.rollback()
Attachments (1)
Change History (9)
comment:1 by , 16 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
comment:2 by , 16 years ago
Resolution: | invalid |
---|---|
Status: | closed → reopened |
No!
Correct data has been send.
When I'm using Debugger before save method all data are parsed ok. When im trying to save error occurs.
comment:3 by , 16 years ago
Owner: | changed from | to
---|---|
Status: | reopened → new |
comment:4 by , 16 years ago
Owner: | changed from | to
---|---|
Summary: | "patients_guardian.pesel may not be NULL" when trying to save related object after "is_valid()" on "save()" method → inline-edited objects with custom PKs cannot be saved |
Triage Stage: | Unreviewed → Accepted |
Why did you assign this to Malcolm? The general custom around here is for people assign a ticket to their own self when they want to indicate they are working on it so as to avoid having other people duplicate the work. We don't in general assign things to other people, as that's rather like saying "Here, you must work on this problem for me", which is a bit rude given everyone here is a volunteer.
This ticket could have benefited from a clearer description at the outset. You didn't mention, for example, anything about the input you provided on the form. Saying you had provided a pesel value of <whatever> on the form for the guardian you were trying to create would have helped make it clear that apparently the provided value for pesel was getting lost somewhere along the way to or in save(). I realize you said the form passed validation, but it still would have been clearer to explicitly state you had provided a value for the field that was coming up "null" in the error message.
Also, complete examples are always better. You didn't provide the complete view function, nor the template you are using, so anyone trying to recreate has to fill in the blanks. It may be pretty obvious what should be filled in but it is harder to do than simply cut and paste and there's always the danger that the person attempting to recreate fills in the blanks differently than what you actually have in a way that affects the outcome.
All that said, there does seem to be a problem here. Specifically if an inline-edited model has a custom non-autofield primary key that is not the foreign key back to the parent object, then the assigned-in-the-form primary key value is not included when saving, leading to the database raising an error because the primary key field is neither an Auto field nor allowed to be null.
I'll attach a patch with a testcase demonstrating the problem and a potential fix, but this is not code I am very comfortable with so I'd prefer someone else look at it before committing.
The errors from the new test without the code fix are:
====================================================================== FAIL: Doctest: modeltests.model_formsets.models.__test__.API_TESTS ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/kmt/tmp/django/trunk/django/test/_doctest.py", line 2180, in runTest raise self.failureException(self.format_failure(new.getvalue())) AssertionError: Failed doctest test for modeltests.model_formsets.models.__test__.API_TESTS File "/home/kmt/tmp/django/trunk/tests/modeltests/model_formsets/models.py", line unknown line number, in API_TESTS ---------------------------------------------------------------------- File "/home/kmt/tmp/django/trunk/tests/modeltests/model_formsets/models.py", line ?, in modeltests.model_formsets.models.__test__.API_TESTS Failed example: formset.save() Exception raised: Traceback (most recent call last): File "/home/kmt/tmp/django/trunk/django/test/_doctest.py", line 1267, in __run compileflags, 1) in test.globs File "<doctest modeltests.model_formsets.models.__test__.API_TESTS[99]>", line 1, in <module> formset.save() File "/home/kmt/tmp/django/trunk/django/forms/models.py", line 389, in save return self.save_existing_objects(commit) + self.save_new_objects(commit) File "/home/kmt/tmp/django/trunk/django/forms/models.py", line 424, in save_new_objects self.new_objects.append(self.save_new(form, commit=commit)) File "/home/kmt/tmp/django/trunk/django/forms/models.py", line 487, in save_new return save_instance(form, new_obj, exclude=[self._pk_field.name], commit=commit) File "/home/kmt/tmp/django/trunk/django/forms/models.py", line 74, in save_instance instance.save() File "/home/kmt/tmp/django/trunk/django/db/models/base.py", line 328, in save self.save_base(force_insert=force_insert, force_update=force_update) File "/home/kmt/tmp/django/trunk/django/db/models/base.py", line 400, in save_base result = manager._insert(values, return_id=update_pk) File "/home/kmt/tmp/django/trunk/django/db/models/manager.py", line 138, in _insert return insert_query(self.model, values, **kwargs) File "/home/kmt/tmp/django/trunk/django/db/models/query.py", line 894, in insert_query return query.execute_sql(return_id) File "/home/kmt/tmp/django/trunk/django/db/models/sql/subqueries.py", line 309, in execute_sql cursor = super(InsertQuery, self).execute_sql(None) File "/home/kmt/tmp/django/trunk/django/db/models/sql/query.py", line 1756, in execute_sql cursor.execute(sql, params) File "/home/kmt/tmp/django/trunk/django/db/backends/sqlite3/base.py", line 170, in execute return Database.Cursor.execute(self, query, params) IntegrityError: model_formsets_bookwithcustompk.my_pk may not be NULL ---------------------------------------------------------------------- File "/home/kmt/tmp/django/trunk/tests/modeltests/model_formsets/models.py", line ?, in modeltests.model_formsets.models.__test__.API_TESTS Failed example: for book in author.bookwithcustompk_set.all(): print book.title Expected: Les Fleurs du Mal Got nothing ---------------------------------------------------------------------- Ran 559 tests in 387.581s FAILED (failures=1) Destroying test database...
With the code fix, all existing plus the new test pass (tested on sqlite).
by , 16 years ago
Attachment: | inline_custom_pk.diff added |
---|
comment:5 by , 16 years ago
Good job tracking this down Karen. This certainly does look to be fixing the correct problem. The only suggestion I would like to make is that I would much prefer to see a declarative if / else conditional than the boolean logic done here because the former is much easier to read/understand going forward.
comment:6 by , 16 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Please ask support questions on the django-users mailing list, not in Trac. There's no bug here; the error message is telling you that a field requires a value, which is correct from the definitions you have provided.