Avoiding Signals Best Praces (esp. with m2m fields)

Using Django Mystery Errors
1

First, I’m not sure if this is the right sub-forum to put this inquiry; I’m not seeking guidance on resolving a mystery error, but rather I’m seeking guidance on how to implement best practices and avoid using signals. I have a few cases for the following Supplier model:

class Supplier(StatefulProcurementSupplier):
   """
        NB: StatefulProcurementSupplier includes, inter alia, id = UUID4
   """
    name = models.CharField(
        max_length=128,
        blank=False,
        unique=True,
    )
    number = models.PositiveIntegerField(
        default=0,
        unique=True,
        verbose_name='#',
    )
    business_type = models.ForeignKey(
        "BusinessType",
        on_delete=models.SET_NULL,
        null=True,
        related_name="suppliers",
        verbose_name='Business Type',
    )
    classifications = models.ManyToManyField(
        "Classification",
        related_name="suppliers",
        verbose_name='Classification',
    )
    signed_form = models.FileField(
        upload_to=document_directory_path,
        null=True,
        blank=True,
        validators=[validate_file_extension],
    )
  1. How to have the number automatically increment when a user creates a new Supplier? I have explored using an AutoField, but this field inherits the AutoFieldMixin, which checks if the field is a PK; if it isn’t, an error is thrown. I want to keep the pk/id as a UUID. I have the following pre-save signal:
@receiver(pre_save, sender=Supplier)
def set_supplier_number(sender, instance, **kwargs):
    instance.set_number()

Which utilizes the following Supplier method:

    def set_number(self):
        if (not self.pk) or (self.number == 0): # Ensure that the instance is new and assign a number to it; if instance isn't newly-created, don't execute.
            max_number = Supplier.objects.aggregate(max_number=Max("number"))[
                "max_number"
            ]
            if max_number:
                max_number += 1
            else:
                max_number = 1
            self.number = max_number

I tried using the above in an overwritten Supplier.save() method, but due to other complications (Item #3), I had to replace the save() method with a pre-save signal (this one), post-save signal (#2), and m2m_changed signals (#3). I believe that when I resolve #3, I can implement best practices and avoid signals entirely. This is what I’d like to do.

  1. Whenever a Supplier object is created or updated, due to business logic and the need for application of the four-eyes principle, there is a need to create a historical model of that object so that privileged users can compare the two most recent historical objects and approve or reject the proposed changes.

Accordingly, the Supplier has a create_historical_object method which is called upon post_save (note: the below, because it is used in a signal, replaces 'self 'with ‘instance’; when I refactor, I’ll use ‘self’):

@receiver(post_save, sender=Supplier)
def save_supplier_post_save(sender, instance, **kwargs):
    create_supplier_history(instance)

def create_supplier_history(instance):
    classifications = instance.classifications.all()
    history = SupplierHistory.get_queryset(historical_object=instance)
    version = len(history) + 1
    supplier_history = SupplierHistory.objects.create(
        historical_object=instance,
        name=instance.name,
        number=instance.number,
        business_type=instance.business_type,
        signed_form=instance.signed_form,
        version=version,
        state=instance.state,
        created_by=instance.created_by,
        created_date=instance.created_date,
        stateful_comment=instance.stateful_comment,
    )
    for classification in classifications:
        supplier_history.classifications.add(classification)
  1. This is the meat of the problem: as mentioned earlier, there is a need for privileged users to be able to approve changes made by less-privileged users, and so there is a need to see current and proposed versions (e.g., the SupplierHistory). However, the m2m relation between a Supplier and Classifications present a challenge, and I believe this is because:

[quote="KenWhitesell, post:2, topic:19071"] saving membership in a many-to-many relationship does not result in saving *either* of the two models joined by that relationship. An M2M relationship is expressed as an entry in the join table with foreign keys to both related models. [/quote]

Accordingly, the SupplierHistory instance must be saved after any change to the related m2m field, and so a signal was written for this:

@receiver(m2m_changed, sender=Supplier.classifications.through)
def save_supplier_m2m_changed(sender, instance, action, **kwargs):
    if action in ['post_add', 'post_remove', 'post_clear']:
        create_supplier_history(instance)

Now, given the above use case, am I stuck with using signals here?

Is this one of the few use cases where signals are both appropriate and required?

Or does a more senior Django developer have guidance about how to eliminate the use of signals and rely entirely on methods (e.g., an overwritten save() method, with other helper methods) for the Supplier object?

2

You could override the save method to include the logic for setting the number and creating historical records to your Supplier model:

class Supplier(StatefulProcurementSupplier):
    name = models.CharField(max_length=128, blank=False, unique=True)
    number = models.PositiveIntegerField(default=0, unique=True, verbose_name='#')
    business_type = models.ForeignKey("BusinessType", on_delete=models.SET_NULL, null=True, related_name="suppliers", verbose_name='Business Type')
    classifications = models.ManyToManyField("Classification", related_name="suppliers", verbose_name='Classification')
    signed_form = models.FileField(upload_to=document_directory_path, null=True, blank=True, validators=[validate_file_extension])

    def save(self, *args, **kwargs):
        if not self.pk or self.number == 0:
            self.set_number()
        super(Supplier, self).save(*args, **kwargs)
        self.create_historical_object()

    def set_number(self):
        max_number = Supplier.objects.aggregate(max_number=Max("number"))["max_number"]
        self.number = max_number + 1 if max_number else 1

    def create_historical_object(self):
        classifications = self.classifications.all()
        history = SupplierHistory.get_queryset(historical_object=self)
        version = len(history) + 1
        supplier_history = SupplierHistory.objects.create(
            historical_object=self,
            name=self.name,
            number=self.number,
            business_type=self.business_type,
            signed_form=self.signed_form,
            version=version,
            state=self.state,
            created_by=self.created_by,
            created_date=self.created_date,
            stateful_comment=self.stateful_comment,
        )
        for classification in classifications:
            supplier_history.classifications.add(classification)

As for the many-to-many changes, you could create a custom method in the Supplier model:

def update_classifications(self, new_classifications):
        self.classifications.set(new_classifications)
        self.create_historical_object()

Then, when creating or updating a Supplier instance, use the save method for initial save and the update_classifications method for updating m2m relationships.

# Creating a new Supplier
supplier = Supplier(name="New Supplier", business_type=some_business_type, created_by=some_user)
supplier.save()
supplier.update_classifications([classification1, classification2])

# Updating an existing Supplier
supplier = Supplier.objects.get(id=some_id)
supplier.name = "Updated Supplier"
supplier.save()
supplier.update_classifications([classification3, classification4])