Fixed and implemented generic bi-directional m2m form class to node edit

ninecms/forms.py

@@ -43,13 +43,9 @@ class ContentTypePermissionsForm(forms.Form):
     )
 
 
-class ContentNodeEditForm(forms.ModelForm):
+class ContentNodeEditForm(ManyToManyModelForm):
     """ Node edit or create form """
-    terms = forms.ModelMultipleChoiceField(
-        TaxonomyTerm.objects.all(),
-        widget=FilteredSelectMultiple("Terms", True),
-        required=False,
-    )
+    terms = ModelBiMultipleChoiceField(TaxonomyTerm.objects.all(), double_list="Terms")
 
     def __init__(self, *args, **kwargs):
         """ Get user object to check if has full_html permission
@@ -63,8 +59,6 @@ def __init__(self, *args, **kwargs):
         except AttributeError:
             self.current_user = kwargs.pop('user', None)
         super(ContentNodeEditForm, self).__init__(*args, **kwargs)
-        if self.instance.pk:
-            self.initial['terms'] = self.instance.terms.values_list('pk', flat=True)
 
     def clean(self):
         """ Override clean form to bleach
@@ -80,22 +74,6 @@ def clean(self):
                 cleaned_data[field] = sanitize(cleaned_data[field], full_html=full_html)
         return cleaned_data
 
-    def save(self, *args, **kwargs):
-        """ Handle saving of related terms
-        :param args
-        :param kwargs
-        :return: instance
-        """
-        instance = super(ContentNodeEditForm, self).save(*args, **kwargs)
-        if instance.pk:
-            for term in instance.terms.all():
-                if term not in self.cleaned_data['terms']:
-                    instance.terms.remove(term)
-            for term in self.cleaned_data['terms']:
-                if term not in instance.terms.all():
-                    instance.terms.add(term)
-        return instance
-
     class Meta:
         """ Form model and fields """
         model = Node

ninecms/tests/tests_content_login.py

@@ -27,8 +27,9 @@
 from guardian.models import GroupObjectPermission
 from ninecms.forms import ContentNodeEditForm, ImageForm, FileForm, VideoForm, PageTypeForm
 from ninecms.tests.setup import create_front, create_basic, create_user, create_image, create_block_simple, \
-    get_front_title, assert_front, data_login, data_node, get_basic_title, create_video, create_file, data_page_type
-from ninecms.models import PageType, Node, PageLayoutElement, ContentBlock
+    get_front_title, assert_front, data_login, data_node, get_basic_title, create_video, create_file, data_page_type, \
+    create_terms
+from ninecms.models import PageType, Node, PageLayoutElement, ContentBlock, TaxonomyTerm
 import os
 
 
@@ -165,6 +166,31 @@ def test_content_node_edit_form_valid(self):
         self.assertEqual(form.cleaned_data['body'],
                          '<div> </div>&lt;script&gt;alert("This is a test.");&lt;/script&gt;')
 
+    def test_content_node_edit_form_m2m_valid(self):
+        """ Test that a node form's reverse m2m is valid
+        :return: None
+        """
+        create_terms(())
+        # assign 2 tags
+        data = data_node(self.node_rev_front.node.page_type_id, self.admin)
+        data['terms'] = (1, 2)
+        form = ContentNodeEditForm(data=data, user=self.admin)
+        r = form.is_valid()
+        self.assertEqual(r, True)
+        self.assertEqual(list(form.cleaned_data['terms']), list(TaxonomyTerm.objects.all()))
+        form.save()
+        self.assertEqual(len(form.errors), 0)
+        self.assertEqual(len(form.instance.terms.all()), 2)
+
+        # remove 1 tag
+        data['terms'] = (1,)
+        form = ContentNodeEditForm(data=data, user=self.admin, instance=form.instance)
+        r = form.is_valid()
+        self.assertEqual(r, True)
+        form.save()
+        self.assertEqual(len(form.errors), 0)
+        self.assertEqual(len(form.instance.terms.all()), 1)
+
     def test_image_form_valid_sanitize(self):
         """ Test forms sanitize; Test image form
         Form is invalid as it requires image filename etc.

ninecms/utils/manytomany.py

@@ -50,7 +50,7 @@ def __init__(self, *args, **kwargs):
         # self.fields['blocks'].widget = RelatedFieldWidgetWrapper(self.fields['blocks'].widget, rel, admin.site)
 
     def save(self, *args, **kwargs):
-        """ Handle saving of related blocks
+        """ Handle saving of related
         :param args
         :param kwargs
         :return: instance
@@ -62,13 +62,14 @@ def save(self, *args, **kwargs):
                 field = self.base_fields[field_name]
                 if type(field).__name__ == 'ModelBiMultipleChoiceField':
                     # the m2m records, eg if model field is `blocks`, this would be `instance.blocks.all()`
-                    records = getattr(self.instance, field_name).all()
+                    recordset = getattr(self.instance, field_name)
+                    records = recordset.all()
                     # remove records that have been removed in form
                     for record in records:
                         if record not in self.cleaned_data[field_name]:
-                            instance.blocks.remove(record)
+                            recordset.remove(record)
                     # add records that have been added in form
                     for record in self.cleaned_data[field_name]:
                         if record not in records:
-                            instance.blocks.add(record)
+                            recordset.add(record)
         return instance