Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Creating a model with two optional, but one mandatory foreign key

My problem is that I have a model that can take one of two foreign keys to say what kind of model it is. I want it to take at least one but not both. Can I have this still be one model or should I split it into two types. Here is the code:

class Inspection(models.Model):
    InspectionID = models.AutoField(primary_key=True, unique=True)
    GroupID = models.ForeignKey('PartGroup', on_delete=models.CASCADE, null=True, unique=True)
    SiteID = models.ForeignKey('Site', on_delete=models.CASCADE, null=True, unique=True)

    @classmethod
    def create(cls, groupid, siteid):
        inspection = cls(GroupID = groupid, SiteID = siteid)
        return inspection

    def __str__(self):
        return str(self.InspectionID)

class InspectionReport(models.Model):
    ReportID = models.AutoField(primary_key=True, unique=True)
    InspectionID = models.ForeignKey('Inspection', on_delete=models.CASCADE, null=True)
    Date = models.DateField(auto_now=False, auto_now_add=False, null=True)
    Comment = models.CharField(max_length=255, blank=True)
    Signature = models.CharField(max_length=255, blank=True)

The problem is the Inspection model. This should be linked to either a group or a site, but not both. Currently with this set up it needs both.

I'd rather not have to split this up into two nearly identical models GroupInspection and SiteInspection, so any solution that keeps it as one model would be ideal.

like image 567
CalMac Avatar asked Jan 08 '20 09:01

CalMac


People also ask

Can a model have two foreign keys?

A table can have multiple foreign keys based on the requirement.

Can a model have a foreign key to itself?

Self-referencing foreign keys are used to model nested relationships or recursive relationships. They work similar to how One to Many relationships. But as the name suggests, the model references itself.

Can Django model have two primary keys?

Do Django models support multiple-column primary keys? ¶ No. Only single-column primary keys are supported.


Video Answer


3 Answers

I would suggest that you do such validation the Django way

by overriding the clean method of Django Model

class Inspection(models.Model):
    ...

    def clean(self):
        if <<<your condition>>>:
            raise ValidationError({
                    '<<<field_name>>>': _('Reason for validation error...etc'),
                })
        ...
    ...

Note, however, that like Model.full_clean(), a model’s clean() method is not invoked when you call your model’s save() method. it needs to be called manually to validate model's data, or you can override model's save method to make it always call the clean() method before triggering the Model class save method


Another solution that might help is using GenericRelations, in order to provide a polymorphic field that relates with more than one table, but it can be the case if these tables/objects can be used interchangeably in the system design from the first place.

like image 126
Radwan Abu-Odeh Avatar answered Oct 09 '22 20:10

Radwan Abu-Odeh


Django has a new (since 2.2) interface for creating DB constraints: https://docs.djangoproject.com/en/3.0/ref/models/constraints/

You can use a CheckConstraint to enforce one-and-only-one is non-null. I use two for clarity:

class Inspection(models.Model):
    InspectionID = models.AutoField(primary_key=True, unique=True)
    GroupID = models.OneToOneField('PartGroup', on_delete=models.CASCADE, blank=True, null=True)
    SiteID = models.OneToOneField('Site', on_delete=models.CASCADE, blank=True, null=True)

    class Meta:
        constraints = [
            models.CheckConstraint(
                check=~Q(SiteID=None) | ~Q(GroupId=None),
                name='at_least_1_non_null'),
            ),
            models.CheckConstraint(
                check=Q(SiteID=None) | Q(GroupId=None),
                name='at_least_1_null'),
            ),
        ]

This will only enforce the constraint at the DB level. You will need to validate inputs in your forms or serializers manually.

As a side note, you should probably use OneToOneField instead of ForeignKey(unique=True). You'll also want blank=True.

like image 30
Jonathan Richards Avatar answered Oct 09 '22 20:10

Jonathan Richards


As mentionned in comments, the reason that "with this set up it needs both" is just that you forgot to add the blank=True to your FK fields, so your ModelForm (either custom one or the default generated by the admin) will make the form field required. At the db schema level, you could fill both, either one or none of those FKs, it would be ok since you made those db fields nullable (with the null=True argument).

Also, (cf my other comments), your may want to check that your really want to FKs to be unique. This technically turns your one to many relationship into a one to one relationship - you're only allowed one single 'inspection' record for a given GroupID or SiteId (you can't have two or more 'inspection' for one GroupId or SiteId). If that's REALLY what you want, you may want to use an explicit OneToOneField instead (the db schema will be the same but the model will be more explicit and the related descriptor much more usable for this use case).

As a side note: in a Django Model, a ForeignKey field materializes as a related model instance, not as a raw id. IOW, given this:

class Foo(models.Model):
    name = models.TextField()

class Bar(models.Model):
    foo = models.ForeignKey(Foo)


foo = Foo.objects.create(name="foo")
bar = Bar.objects.create(foo=foo)

then bar.foo will resolve to foo, not to foo.id. So you certainly want to rename your InspectionID and SiteID fields to proper inspection and site. BTW, in Python, the naming convention is 'all_lower_with_underscores' for anything else than class names and pseudo-constants.

Now for your core question: there's no specific standard SQL way of enforcing a "one or the other" constraint at the database level, so it's usually done using a CHECK constraint, which is done in a Django model with the model's meta "constraints" option.

This being said, how constraints are actually supported and enforced at the db level depends on your DB vendor (MySQL < 8.0.16 just plain ignore them for example), and the kind of constraint you will need here will not be enforced at the form or model level validation, only when trying to save the model, so you also want to add validation either at the model level (preferably) or form level validation, in both cases in the (resp.) model or form's clean() method.

So to make a long story short:

  • first double-check that you really want this unique=True constraint, and if yes then replace your FK field with a OneToOneField.

  • add a blank=True arg to both your FK (or OneToOne) fields

  • add the proper check constraint in your model's meta - the doc is succint but still explicit enough if you know to do complex queries with the ORM (and if you don't it's time you learn ;-))
  • add a clean() method to your model that checks your have either one or the other field and raises a validation error else

and you should be ok, assuming your RDBMS respects check constraints of course.

Just note that, with this design, your Inspection model is a totally useless (yet costly !) indirection - you'd get the exact same features at a lower cost by moving the FKs (and constraints, validation etc) directly into InspectionReport.

Now there might be another solution - keep the Inspection model, but put the FK as a OneToOneField on the other end of the relationship (in Site and Group):

class Inspection(models.Model):
    id = models.AutoField(primary_key=True) # a pk is always unique !

class InspectionReport(models.Model):
    # you actually don't need to manually specify a PK field,
    # Django will provide one for you if you don't
    # id = models.AutoField(primary_key=True)

    inspection = ForeignKey(Inspection, ...)
    date = models.DateField(null=True) # you should have a default then
    comment = models.CharField(max_length=255, blank=True default="")
    signature = models.CharField(max_length=255, blank=True, default="")


class Group(models.Model):
    inspection = models.OneToOneField(Inspection, null=True, blank=True)

class Site(models.Model):
    inspection = models.OneToOneField(Inspection, null=True, blank=True)

And then you can get all the reports for a given Site or Group with yoursite.inspection.inspectionreport_set.all().

This avoids having to add any specific constraint or validation, but at the cost of an additional indirection level (SQL join clause etc).

Which of those solution would be "the best" is really context-dependent, so you have to understand the implications of both and check how you typically use your models to find out which is more appropriate for your own needs. As far as I'm concerned and without more context (or in doubt) I'd rather use the solution with the less indirection levels, but YMMV.

NB regarding generic relations: those can be handy when you really have a lot of possible related models and / or don't know beforehand which models one will want to relate to your own. This is specially useful for reusable apps (think "comments" or "tags" etc features) or extensible ones (content management frameworks etc). The downside is that it makes querying much heavier (and rather impractical when you want to do manual queries on your db). From experience, they can quickly become a PITA bot wrt/ code and perfs, so better to keep them for when there's no better solution (and/or when the maintenance and runtime overhead is not an issue).

My 2 cents.

like image 4
bruno desthuilliers Avatar answered Oct 09 '22 19:10

bruno desthuilliers