model clean/full_clean is probably not called ?

[berycz]

https://docs.djangoproject.com/en/stable/ref/models/instances/#django.db.models.Model.clean

and it probably should be called?

[wdelport-sgi]

@berycz +1 for this to be included.

In the meantime I am trying to get around this with a post_handler. Briefly, I have a use case like below.

A model where I override save to do a full clean before save and with ValidationError raised in the clean.

class myModel(models.Model):
    # model stuff
    
    def clean(self):
        super().clean()
        # check for some conditions and raise
        raise ValidationError()

    def save(self, *args, **kwargs):
        self.full_clean()
        return super().save(*args, **kwargs)

An EditTable using the model above with a post_handler

class myEditTable(EditTable):
    class Meta:
        auto__model = myModel
        columns__select__include = True
        edit_actions__save__include = True
        edit_actions__save__post_handler = my_post_handler

def my_post_handler(table, request, **_):
    for row in table.selection():
        print (row) # confirm that method is being called on save
        print (type(row)) # confirm this is a django model instance
        row.save()

My expectation is that the post_handler should trigger the model validation when saving the model, but it doesn't. I tested this outside of iommi views and can confirm that ValidationError is raised when appropriate for API or ORM calls, just not via an iommi table view. The post_handler is being called for the selected rows, and I can confirm they are Django model instances. The edits are also being made to the records I have edited, but the ValidationError needs to be raised when appropriate. Do you have any idea on how to resolve this? I want to avoid duplicating the model validation logic in the post_handler.

Thanks

[wdelport-sgi]

I think I know why this doesn't work—the post_handler is a post-save handler, which means the model has already been saved when it is called. All my logic in the clean method compares existing database field values to those being updated before the save, but these will be identical if the save has already happened without the clean.

[wdelport-sgi]

Some more on this. I think I can handle this use case with the preprocess_rows option. I can now get the ValidationError raised with the following as a preprocess_rows.

def preprocess_rows_method(rows, **_):
    for row in rows:
        try:
            yield row
        except ValidationError:
            raise

This will correctly raise the ValidationError when the condition arises but will do so as a stack trace. The question is how to get this passed back cleanly in the response so the table displays the error instead of the stack trace.

[boxed]

I think I know why this doesn't work—the post_handler is a post-save handler, which means the model has already been saved when it is called.

The post handler is not post-save, this is incorrect. You can check the code: https://github.com/iommirocks/iommi/blob/master/iommi/edit_table.py#L279

I tested this outside of iommi views and can confirm that ValidationError is raised when appropriate for API or ORM calls, just not via an iommi table view.

This part confuses me. From what I can see in the Django source code, full_clean on the model instance is only called from BaseModelForm._post_clean, meaning it's a feature of the Django form system, and will not be run when you do instance.save().

Have I misunderstood this?

[wdelport-sgi]

Thanks, I'll try make it a little clearer. In my model I have added a full_clean to the save method, i.e.

class myModel(models.Model):
    # model stuff
    
    def clean(self):
        super().clean()
        # check for some conditions and raise
        raise ValidationError()

    def save(self, *args, **kwargs):
        self.full_clean()
        return super().save(*args, **kwargs)

The intended behavior here is that clean will be called irrespective of whether the submission is via a form or via orm.

The benefit is that the API endpoints, forms, and any Python code that creates and saves model instances will pass through the same checks and raise a ValidationError when those checks fail. For example, one check may be that we need to collect a comment if a classification value is changed. I do this as follows:

class myModel(models.Model):
    classification = models.CharField(max_length=50)
    comment = models.TextField(null=True, blank=True)
    
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        # save the value of classification as __original_classification if the instance exists, else pass
        try:
            self.__original_classification = self.classification
        except ObjectDoesNotExist:
            pass

    def clean(self):
        super().clean()
        # check for conditions and raise
        if self.pk is not None:
            if self.__original_classification != self.classification and (self.comments is None or self.comments == ""):
                raise ValidationError(
                    _('Comments are required if any %(field)s value has been altered'),
                    code='invalid',
                    params={'field': 'classification'}
                )

    def save(self, *args, **kwargs):
        self.full_clean()
        return super().save(*args, **kwargs)

So, when a user tries to change the value of classification without adding a comment via form, api, or using Python directly, the same ValidationError is raised.

The next step is to try to make this work with EditTable. To do this I first tried an EditTable without a post_handler.

class myEditTable(EditTable):
    class Meta:
        auto__model = myModel
        columns__select__include = True
        edit_actions__save__include = True

When I select records and save them, the correct ValidationError exception is raised, but it is not handled, so I get the typical stack trace in Django.

This is expected given the model setup above. What I am unsure of is how to handle this in the EditTable. I have tried overriding the edit_table__post_handler method, specifically these lines

iommi/iommi/edit_table.py

Lines 281 to 282 in 5f1a440

	save(table.cells_for_rows(), table.edit_form)
	save(table.cells_for_rows_for_create(), table.create_form)

as follows:

try:
    save(table.cells_for_rows(), table.edit_form)
except ValidationError as error_dict:
    for _e in error_dict.messages:
        print (_e)
        table.edit_form.add_error(_e)
    print (table.edit_form.get_errors())

I can confirm that the above will raise the exception and add the error to the table.edit_form, i.e.

[28/Aug/2024 01:45:34] "GET /results/resultreview/?activity_id=ACTE41DFFFC HTTP/1.1" 200 71228
Comments are required if any classification value has been altered
{'global': {'Comments are required if any classification value has been altered'}}
[28/Aug/2024 01:45:42] "POST /results/resultreview/?activity_id=ACTE41DFFFC HTTP/1.1" 302 0

What I am unsure of is how to return the

{'global': {'Comments are required if any classification value has been altered'}}

back to the view so that it is displayed with the selected EditTable record.

I had originally posted here since I had mistakenly thought the clean wasn't being called, but I am cailling it in the save, so that is not the issue. I spent more time on this and got it to the point that I can raise, but not handle the exception in the EditTable.

I hope that is clearer.

[boxed]

@wdelport-sgi I think what you are talking about is quite different from what this ticket is about. Calling full_clean inside save isn't really how Django was designed, and not something iommi is going to support. You can end up with half-saved changes and other messes if you do it that way.

[wdelport-sgi]

Right, I agree that after exploration, this is not precisely the scope of this issue, but whether I'm calling full_clean in the save or not, the current edit_table__post_handler does not support ValidationError handling. The fix I made is above, i.e.

try:
    save(table.cells_for_rows(), table.edit_form)
except ValidationError as error_dict:
    for _e in error_dict.messages:
        table.edit_form.add_error(_e)
    return

with the following in the Template

{% if table.edit_form.errors %}
   {{ table.edit_form.errors }}
{% endif %}