Django save() method not producing desired output

Question

I am working on patient databases. Two related class, one which stores details of patients, and the other which is just to track total numbers of different types of patients. The tracker class is as follow :

class Case (models.Model):
    id = models.AutoField(primary_key=True)
    time_stamp = models.DateTimeField(null=True, auto_now_add=True)
    is_dicom = models.BooleanField(null=False, blank=True)
    PatientId = models.IntegerField(null=True, blank=True, db_column="patientid")

    def __unicode__(self):
        return "%s" % self.id

This has a 'PatientId' field which refers to two different classes (hence not a foreign key to any one class) and just stores the pk of the referring class as integer. The referring classesa are Patient and PatientJpeg which run similar code on save() :

class Patient(models.Model):
    id = models.AutoField(primary_key=True, db_column='pk')
    case_id = models.OneToOneField(Case, null=True, blank=True, db_column='case_id')
    pat_birthdate = models.CharField(max_length=160, blank=True)
    pat_sex = models.CharField(max_length=160, blank=True)
    created_time = models.DateTimeField(null=True, auto_now_add=True)
    updated_time = models.DateTimeField(null=True, auto_now_add=True) 
    class Meta:
        db_table = 'patient'

    def __unicode__(self):
        return u"%s" % self.id

    def save(self):
        if not(self.case_id):
            x = 1
            while (x < Case.objects.count() + 1):
                if not Case.objects.filter(pk=x).exists():
                        break
                else:
                        x += 1

            newCase = Case(x)
            newCase.is_dicom = True
            newCase.PatientId = self.id
            newCase.save()
            self.case_id = newCase
        super(Patient, self).save()

    class PatientJpeg (models.Model):
        id = models.AutoField(primary_key=True, db_column='pk')
        case_id = models.OneToOneField(Case, null=True, blank=True, db_column='case_id')
        pat_age = models.CharField(max_length=160, null=True, blank=True)
        pat_sex = models.CharField(max_length=160, null=True, blank=True)
        def __unicode__(self):
            return u"%s" % self.id

        def jpeg_paths(self):
            array = []
            for x in self.jpeg_set.all():
                array.append(x.image.path)
            return array

        class Meta:
            db_table = 'patient_jpeg'

        def save(self):
            if not(self.case_id):
                x = 1
                while (x < Case.objects.count() + 1):
                    if not Case.objects.filter(pk=x).exists():
                            break
                    else:
                            x += 1

                newCase = Case(x)
                newCase.is_dicom = False
                newCase.PatientId = self.id
                newCase.save()
                self.case_id = newCase
            super(PatientJpeg, self).save()

The problem is that when I save Patient or PatientJpeg, PatientId field (integer field in Case class) remains null. I have replicated this code in shell and it behaves properly, I dont know why it doesnt behave inside django.

Thanks for looking at the code

Answer 1

This code is, to be polite, pretty horrible. It's doing an insane amount of database access each time you create a new Patient - imagine you had 1000 Cases, just saving a new Patient would result in over 1000 database calls.

What's strange is that I can't understand why you need it. The case ID is an arbitrary number, so there doesn't seem to be any particular reason why you need to iterate through existing case IDs to find a blank one. This is especially so given that the ID is an AutoField, so will be allocated by the database anyway. Just get rid of all that and simply create a new case.

The other problem of course is that self.id does not exist when you first save the Patient. You need to ensure that it is saved before allocating the Case.

def save(self, *args, **kwargs):
    if not(self.case_id):
        if not self.id:
            super(Patient, self).save(*args, **kwargs)
        newCase = Case()
        newCase.is_dicom = True
        newCase.PatientId = self.id
        newCase.save()
        self.case_id = newCase
    super(Patient, self).save(*args, **kwargs)

Other pointers: don't call your one-to-one relationship case_id: it's a case object, not an ID, so just call it case. Also, you should really create an abstract class as the parent of both Patient and PatientJpeg, and put the custom save logic there. Finally, your jpeg_paths method could be replaced by a list comprehension.

Answer 2

Before creating Case you need to save Patient, so i think the save method should be like:

    def save(self):
        if not self.id:
            super(PatientJpeg, self).save()

        if not self.case_id:
            x = 1
            while (x < Case.objects.count() + 1):
                if not Case.objects.filter(pk=x).exists():
                        break
                else:
                        x += 1

            newCase = Case(x)
            newCase.is_dicom = False
            newCase.PatientId = self.id
            newCase.save()
            self.case_id = newCase

        super(PatientJpeg, self).save()

Look also at django post_save signal

Answer 3

The time you call newCase.save(), your PatientJpeg record has not been saved to the database. AutoFields in django are evaluated when you try to save them to database, because id must be genertated by the databse. So when you call newCase.save(), self.id in newCase.PatientId = self.id is None

You must save your data after you call super(PatientJpeg, self).save() so self.id will have a valid not None value

Also I suggest you to use ContentTypes Framework if your ForeignKey might be elated to more than one table.