Open berycz opened 6 months ago
@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
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.
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.
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?
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
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.
@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.
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 %}
https://docs.djangoproject.com/en/stable/ref/models/instances/#django.db.models.Model.clean
and it probably should be called?