Re-implement modification notifications in simple_form

The way signals are sent for many2many apparently changed completely
between the python2 and python3 versions of the same Django version,
which broke the way we did this before. And it was always a bit of a
hack...

Instead, reimplement notifications in the simple_form handler. This now
also consolidates regular field notificationss and many2many
notifications in a much cleaner way.

This will, however, *only* have an effect on changes made through
simple_form. Luckily that's the most common way we handle forms, with
the exception being /admin/. So leave the old code in place to handle
the changes through /admin/, as well as the deletion of objects.

In the end the only thing lost is the ability to get m2m differences
when an admin makes changes, and that's the least important of all
notification. And as a bonus, the regular change notifications and in
particular "new item" notifications look a lot nicer.
This commit is contained in:
Magnus Hagander
2019-01-24 13:07:21 +01:00
parent 2c84a8ec63
commit 5d0b64a5ab
3 changed files with 103 additions and 26 deletions

View File

@ -1,9 +1,13 @@
from django import forms
from django.forms import ValidationError
from django.conf import settings
from .models import Organisation
from django.contrib.auth.models import User
from pgweb.util.middleware import get_current_user
from pgweb.mailqueue.util import send_simple_mail
class OrganisationForm(forms.ModelForm):
remove_manager = forms.ModelMultipleChoiceField(required=False, queryset=None, label="Current manager(s)", help_text="Select one or more managers to remove")
@ -44,12 +48,23 @@ class OrganisationForm(forms.ModelForm):
def save(self, commit=True):
model = super(OrganisationForm, self).save(commit=False)
ops = []
if 'add_manager' in self.cleaned_data and self.cleaned_data['add_manager']:
model.managers.add(User.objects.get(email=self.cleaned_data['add_manager'].lower()))
u = User.objects.get(email=self.cleaned_data['add_manager'].lower())
model.managers.add(u)
ops.append('Added manager {}'.format(u.username))
if 'remove_manager' in self.cleaned_data and self.cleaned_data['remove_manager']:
for toremove in self.cleaned_data['remove_manager']:
model.managers.remove(toremove)
ops.append('Removed manager {}'.format(toremove.username))
if ops:
send_simple_mail(
settings.NOTIFICATION_FROM,
settings.NOTIFICATION_EMAIL,
"{0} modified managers of {1}".format(get_current_user().username, model),
"The following changes were made to managers:\n\n{0}".format("\n".join(ops))
)
return model
def apply_submitter(self, model, User):

View File

@ -2,13 +2,22 @@ from django.shortcuts import render, get_object_or_404
from pgweb.util.contexts import render_pgweb
from django.http import HttpResponseRedirect, Http404
from django.template.loader import get_template
from django.db import models
import django.utils.xmlutils
from django.conf import settings
import io
import difflib
from pgweb.mailqueue.util import send_simple_mail
def simple_form(instancetype, itemid, request, formclass, formtemplate='base/form.html', redirect='/account/', navsection='account', fixedfields=None, createifempty=False):
if itemid == 'new':
instance = instancetype()
is_new = True
else:
is_new = False
# Regular form item, attempt to edit it
try:
int(itemid)
@ -28,7 +37,26 @@ def simple_form(instancetype, itemid, request, formclass, formtemplate='base/for
if request.method == 'POST':
# Process this form
form = formclass(data=request.POST, instance=instance)
# Save away the old value from the instance before it's saved
if not is_new:
old_values = {fn: str(getattr(instance, fn)) for fn in form.changed_data if hasattr(instance, fn)}
if form.is_valid():
# We are handling notifications, so disable the ones we'd otherwise send
do_notify = getattr(instance, 'send_notification', False)
instance.send_notification = False
if not getattr(instance, 'approved', True) and not is_new:
# If the object has an "approved" field and it's set to false, we don't
# bother notifying about the changes. But if it lacks this field, we notify
# about everything, as well as if the field exists and the item has already
# been approved.
# Newly added objects are always notified.
do_notify = False
notify = io.StringIO()
r = form.save(commit=False)
r.submitter = request.user
# Set fixed fields. Note that this will not work if the fixed fields are ManyToMany,
@ -43,6 +71,65 @@ def simple_form(instancetype, itemid, request, formclass, formtemplate='base/for
form.apply_submitter(r, request.user)
r.save()
if is_new:
subj = 'A new {0} has been added'.format(instance._meta.verbose_name)
for f in form.fields:
notify.write("{}:\n".format(f))
if instance._meta.get_field(f) in instance._meta.many_to_many:
notify.write("{}\n".format("\n".join([str(x) for x in form.cleaned_data[f]])))
else:
notify.write("{}\n".format(str(form.cleaned_data[f])))
notify.write("\n")
else:
subj = '{0} id {1} has been modified'.format(instance._meta.verbose_name, instance.id)
for fn in form.changed_data:
if not hasattr(instance, fn):
continue
f = instance._meta.get_field(fn)
if f in instance._meta.many_to_many:
# m2m field have separate config of notificatgions
if getattr(instance, 'send_m2m_notification', False):
for f in instance._meta.many_to_many:
if f.name in form.cleaned_data:
old = set([str(x) for x in getattr(instance, f.name).all()])
new = set([str(x) for x in form.cleaned_data[f.name]])
added = new.difference(old)
removed = old.difference(new)
if added or removed:
notify.write("--- {}\n+++ {}\n{}\n{}\n".format(
f.verbose_name,
f.verbose_name,
"\n".join(["+ %s" % a for a in added]),
"\n".join(["- %s" % r for r in removed]),
))
else:
# Regular field!
# Sometimes it shows up as changed even if it hasn't changed, so do
# a second check on if the diff is non-empty.
diffrows = [x for x in
difflib.unified_diff(
old_values[f.name].splitlines(),
str(form.cleaned_data[f.name]).splitlines(),
n=1,
lineterm='',
fromfile=f.verbose_name,
tofile=f.verbose_name,
) if not x.startswith("@@")]
if diffrows:
notify.write("\n".join(diffrows))
notify.write("\n\n")
if do_notify and notify.tell():
send_simple_mail(
settings.NOTIFICATION_FROM,
settings.NOTIFICATION_EMAIL,
"%s by %s" % (subj, request.user.username),
"Title: {0}\n\n{1}".format(
str(instance),
notify.getvalue(),
),
)
form.save_m2m()
return HttpResponseRedirect(redirect)

View File

@ -128,30 +128,6 @@ def my_pre_save_handler(sender, **kwargs):
cont)
def my_m2m_changed_handler(sender, **kwargs):
instance = kwargs['instance']
if getattr(instance, 'send_m2m_notification', False) and get_current_user():
(cl, f) = sender.__name__.split('_')
if not hasattr(instance, '_stored_m2m'):
instance._stored_m2m = {}
if kwargs['action'] == 'pre_clear':
instance._stored_m2m[f] = set([str(t) for t in getattr(instance, f).all()])
elif kwargs['action'] == 'post_add':
newset = set([str(t) for t in getattr(instance, f).all()])
added = newset.difference(instance._stored_m2m.get(f, set()))
removed = instance._stored_m2m.get(f, set()).difference(newset)
subj = '{0} id {1} has been modified'.format(instance._meta.verbose_name, instance.id)
if added or removed:
send_simple_mail(settings.NOTIFICATION_FROM,
settings.NOTIFICATION_EMAIL,
"%s by %s" % (subj, get_current_user()),
"The following values for {0} were changed:\n\n{1}\n{2}\n\n".format(
instance._meta.get_field(f).verbose_name,
"\n".join(["Added: %s" % a for a in added]),
"\n".join(["Removed: %s" % r for r in removed]),
))
def my_pre_delete_handler(sender, **kwargs):
instance = kwargs['instance']
if getattr(instance, 'send_notification', False) and get_current_user():
@ -178,4 +154,3 @@ def register_basic_signal_handlers():
pre_save.connect(my_pre_save_handler)
pre_delete.connect(my_pre_delete_handler)
post_save.connect(my_post_save_handler)
m2m_changed.connect(my_m2m_changed_handler)