Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#9865 closed (fixed)

inline-edited objects with custom PKs cannot be saved

Reported by: bartosak Owned by: nobody
Component: contrib.formtools Version: master
Severity: Keywords: inlineformset_factory
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

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)

inline_custom_pk.diff (3.7 KB) - added by kmtracey 7 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 7 years ago by mtredinnick

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to invalid
  • Status changed from new to 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.

comment:2 Changed 7 years ago by bartosak

  • Resolution invalid deleted
  • Status changed from closed to 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 Changed 7 years ago by bartosak

  • Owner changed from nobody to mtredinnick
  • Status changed from reopened to new

comment:4 Changed 7 years ago by kmtracey

  • Owner changed from mtredinnick to nobody
  • Summary changed from "patients_guardian.pesel may not be NULL" when trying to save related object after "is_valid()" on "save()" method to inline-edited objects with custom PKs cannot be saved
  • Triage Stage changed from Unreviewed to 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).

Changed 7 years ago by kmtracey

comment:5 Changed 7 years ago by brosner

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 Changed 7 years ago by kmtracey

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

(In [9664]) Fixed #9865 -- Allow saving of new inline-edited objects with custom non-auto primary key fields that are not the foreign key to the parent object.

comment:7 Changed 7 years ago by kmtracey

(In [9665]) [1.0.X] Fixed #9865 -- Allow saving of new inline-edited objects with custom non-auto primary key fields that are not the foreign key to the parent object.

r9664 from trunk.

comment:8 Changed 6 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

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