[Django] #33832: Support M2M validation using signals

65 views
Skip to first unread message

Django

unread,
Jul 7, 2022, 8:31:58 AM7/7/22
to django-...@googlegroups.com
#33832: Support M2M validation using signals
-------------------------------------+-------------------------------------
Reporter: ldeluigi | Owner: nobody
Type: New | Status: new
feature |
Component: | Version: 4.0
Uncategorized | Keywords: validation m2m
Severity: Normal | signal manytomany
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
MyModel has one ManyToManyField that links to AnotherModel.


For validation purposes I need to check the model after m2m relationships
have been set with custom logic based on the db (I can't check it before
m2m fields are populated)


At the moment, I'm raising ValidationError inside a signal receiver:

{{{
@receiver(m2m_changed, sender=MyModel.relationship.through)
def validator_from_signal(sender, instance, action, reverse, model,
**kwargs):
if action == 'post_add':
if reverse and not check_reverse(instance):
msg = f'AnotherModel {instance.id} violates validation
checks.'
logging.error(msg)
raise ValidationError(msg)
elif not reverse and not check(instance):
msg = f'MyModel {instance.id} violates validation checks.'
logging.error(msg)
raise ValidationError(msg)
}}}

It works, thanks to the transaction behaviour which in case of exception
rollbacks everything, but if the user submits a wrong MyModel or
AnotherModel (because of ManyToMany illegal relationships), in particular
with the Admin Form, they get a 500 internal server error. Which isn't
very user friendly...


I know this is the expected behaviour, but I'd like the Admin form to
handle ValidationError(s) and/or IntegrityError(s) from signal as normal
ValidationError. I can't implement this check inside the clean() method:
1. because m2m fields are not populated on the database at that stage
2. because it would only work for the admin form

What do you think?

--
Ticket URL: <https://code.djangoproject.com/ticket/33832>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Jul 8, 2022, 12:24:19 AM7/8/22
to django-...@googlegroups.com
#33832: Support M2M validation using signals
-------------------------------------+-------------------------------------
Reporter: ldeluigi | Owner: nobody
Type: New feature | Status: closed
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution: invalid
Keywords: validation m2m | Triage Stage:
signal manytomany | Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* status: new => closed
* resolution: => invalid
* component: Uncategorized => contrib.admin


Comment:

Thanks for the ticket, however `IntegrityError`s are not masked anywhere
in the admin and they shouldn't be. Moreover, signals are the last resort
and could probably be avoided in you case. Please see
TicketClosingReasons/UseSupportChannels for ways to get help with Django
usage, because it's a support request as it stands.

--
Ticket URL: <https://code.djangoproject.com/ticket/33832#comment:1>

Django

unread,
Jul 8, 2022, 1:13:32 PM7/8/22
to django-...@googlegroups.com
#33832: Support M2M validation using signals
-------------------------------------+-------------------------------------
Reporter: ldeluigi | Owner: nobody
Type: New feature | Status: closed
Component: contrib.admin | Version: 4.0
Severity: Normal | Resolution: invalid
Keywords: validation m2m | Triage Stage:
signal manytomany | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by ldeluigi):

If someone stumbles upon this I managed to fix my issue with this
monkeypatch of
[https://github.com/django/django/blob/eb3699ea775548a22e0407ad12bf8cbdeaf95ff5/django/contrib/admin/options.py#L1748]:

{{{
from django.contrib.admin.options import *
from django.contrib.admin.exceptions import DisallowedModelAdminToField
from django.core.exceptions import (
PermissionDenied,
ValidationError,
)
from django.contrib.admin.utils import (
flatten_fieldsets,
unquote,
)
from django.forms.formsets import all_valid
from django.contrib.admin import helpers
from django.utils.translation import gettext as _

class NewModelAdmin(ModelAdmin):
def _changeform_view(self, request, object_id, form_url,
extra_context):
to_field = request.POST.get(TO_FIELD_VAR,
request.GET.get(TO_FIELD_VAR))
if to_field and not self.to_field_allowed(request, to_field):
raise DisallowedModelAdminToField(
"The field %s cannot be referenced." % to_field
)

model = self.model
opts = model._meta

if request.method == "POST" and "_saveasnew" in request.POST:
object_id = None

add = object_id is None

if add:
if not self.has_add_permission(request):
raise PermissionDenied
obj = None

else:
obj = self.get_object(request, unquote(object_id), to_field)

if request.method == "POST":
if not self.has_change_permission(request, obj):
raise PermissionDenied
else:
if not self.has_view_or_change_permission(request, obj):
raise PermissionDenied

if obj is None:
return self._get_obj_does_not_exist_redirect(request,
opts, object_id)

fieldsets = self.get_fieldsets(request, obj)
ModelForm = self.get_form(
request, obj, change=not add,
fields=flatten_fieldsets(fieldsets)
)
if request.method == "POST":
form = ModelForm(request.POST, request.FILES, instance=obj)
formsets, inline_instances = self._create_formsets(
request,
form.instance if add else obj,
change=not add,
)
form_validated = form.is_valid()
if form_validated:
new_object = self.save_form(request, form, change=not add)
else:
new_object = form.instance
if all_valid(formsets) and form_validated:
try:
self.save_model(request, new_object, form, not add)
self.save_related(request, form, formsets, not add)
change_message = self.construct_change_message(
request, form, formsets, add
)
if add:
self.log_addition(request, new_object,
change_message)
return self.response_add(request, new_object)
else:
self.log_change(request, new_object,
change_message)
return self.response_change(request, new_object)
except ValidationError as e:
form_validated = False
form._update_errors([e])
else:
form_validated = False
else:
if add:
initial = self.get_changeform_initial_data(request)
form = ModelForm(initial=initial)
formsets, inline_instances = self._create_formsets(
request, form.instance, change=False
)
else:
form = ModelForm(instance=obj)
formsets, inline_instances = self._create_formsets(
request, obj, change=True
)

if not add and not self.has_change_permission(request, obj):
readonly_fields = flatten_fieldsets(fieldsets)
else:
readonly_fields = self.get_readonly_fields(request, obj)
adminForm = helpers.AdminForm(
form,
list(fieldsets),
# Clear prepopulated fields on a view-only form to avoid a
crash.
self.get_prepopulated_fields(request, obj)
if add or self.has_change_permission(request, obj)
else {},
readonly_fields,
model_admin=self,
)
media = self.media + adminForm.media

inline_formsets = self.get_inline_formsets(
request, formsets, inline_instances, obj
)
for inline_formset in inline_formsets:
media = media + inline_formset.media

if add:
title = _("Add %s")
elif self.has_change_permission(request, obj):
title = _("Change %s")
else:
title = _("View %s")
context = {
**self.admin_site.each_context(request),
"title": title % opts.verbose_name,
"subtitle": str(obj) if obj else None,
"adminform": adminForm,
"object_id": object_id,
"original": obj,
"is_popup": IS_POPUP_VAR in request.POST or IS_POPUP_VAR in
request.GET,
"to_field": to_field,
"media": media,
"inline_admin_formsets": inline_formsets,
"errors": helpers.AdminErrorList(form, formsets),
"preserved_filters": self.get_preserved_filters(request),
}

# Hide the "Save" and "Save and continue" buttons if "Save as New"
was
# previously chosen to prevent the interface from getting
confusing.
if (
request.method == "POST"
and not form_validated
and "_saveasnew" in request.POST
):
context["show_save"] = False
context["show_save_and_continue"] = False
# Use the change template instead of the add template.
add = False

context.update(extra_context or {})

return self.render_change_form(
request, context, add=add, change=not add, obj=obj,
form_url=form_url
)
}}}


using this modified ModelAdmin called NewModelAdmin will catch
ValidationError(s) during the save process and properly handle those.


My code overrides the entire method but changes only these lines:
https://github.com/django/django/blob/eb3699ea775548a22e0407ad12bf8cbdeaf95ff5/django/contrib/admin/options.py#L1796-L1807

--
Ticket URL: <https://code.djangoproject.com/ticket/33832#comment:2>

Reply all
Reply to author
Forward
0 new messages