Newforms-Admin: cleaner way to allow developers to do Form pre-save/post-save work

[Alen Ribic]

At the moment, it doesn't seem that a developers can do pre-save/post-
save work cleanly when dealing with Forms in NewForms Admin using
django.newforms.models.ModelForm.
For instance, if I'd like to send a notification email to a client
once a form has been submitted for the first time (add_view), I'd have
to override the models.ModelForm.save() method myself which inst very
clean way to go about it.

My suggestion here would be to introduce some small changes at the
django.newforms.models.BaseModelForm class level demonstrated in the
diff below. This way the developers can overwrite one or all of the
event methods namely pre_save(), post_save().

Index: django/newforms/models.py
===================================================================
--- django/newforms/models.py (revision 7512)
+++ django/newforms/models.py (working copy)
@@ -262,6 +262,12 @@
BaseForm.__init__(self, data, files, auto_id, prefix,
object_data,
error_class, label_suffix, empty_permitted)

+ def pre_save(self, is_new_data):
+ pass
+
+ def post_save(self, is_new_data):
+ pass
+
def save(self, commit=True):
"""
Saves this ``form``'s cleaned_data into model instance
@@ -272,10 +278,22 @@
"""
if self.instance.pk is None:
fail_message = 'created'
+ new_data = True
else:
fail_message = 'changed'
- return save_instance(self, self.instance, self._meta.fields,
fail_message, commit)
+ new_data = False

+ # do some pre-save logic here
+ self.pre_save(new_data)
+
+ result_instance = save_instance(self, self.instance,
self._meta.fields, fail_message, commit)
+
+ # do some post-save logic here
+ if result_instance is not None: self.post_save(new_data)
+
+ return result_instance
+
+
class ModelForm(BaseModelForm):
__metaclass__ = ModelFormMetaclass


Regards,
-Alen

[phillc]

whats wrong with overridng save, then calling the super.save() ?

[Alen Ribic]

> whats wrong with overridng save, then calling the super.save() ?

Technically, nothing wrong :-). It would be cleaner, in my opinion,
if the developer just defined a pre_save() and post_save() methods
without needing to wrap and return super.save() calls all the time.

Also, there is the repetitive code of checking if the instance.pk
'already exists' so that if you need to distinguish between the new or
existing data. This signals to me that this code should perhaps be
more abstract (further up the class hierarchy).

regards,
-Alen