Defer saving ClusterForm instances until formsets are saved

modelcluster/forms.py

@@ -374,7 +374,10 @@ def save(self, commit=True):
                 save_m2m_now = True
                 break
 
-        instance = super().save(commit=(commit and not save_m2m_now))
+        # Defer a full save until after formsets are saved. Calling an instance.save() with
+        # commit=True will save the instance *and* inlines too early, as modelcluster wants to
+        # save/update all the relations.
+        instance = super().save(commit=False)
 
         # The M2M-like fields designed for use with ClusterForm (currently
         # ParentalManyToManyField and ClusterTaggableManager) will manage their own in-memory
@@ -395,12 +398,16 @@ def save(self, commit=True):
         if save_m2m_now:
             self.save_m2m()
 
-            if commit:
-                instance.save()
-
         for formset in self.formsets.values():
             formset.instance = instance
-            formset.save(commit=commit)
+            formset.save(commit=False)
+
+        if commit:
+            instance.save()
+
+        if not save_m2m_now:
+            self.save_m2m()
+
         return instance
 
     def has_changed(self):

tests/tests/test_cluster_form.py

@@ -113,6 +113,53 @@ class Meta:
         self.assertTrue(Band.objects.filter(name='The Beatles').exists())
         self.assertTrue(BandMember.objects.filter(name='John Lennon').exists())
 
+    def test_update_from_saved_revision(self):
+        class BandForm(ClusterForm):
+            class Meta:
+                model = Band
+                fields = ['name']
+                formsets = ['members']
+
+        john = BandMember(name='John Lennon')
+        paul = BandMember(name='Paul McCartney')
+        beatles = Band(name='The Beatles', members=[
+            john,
+            paul,
+        ])
+        beatles.save()
+
+        # To replicate a typical Wagtail saved revision, set the PK of one of the inlines to None,
+        # as these are saved into Revision content JSON *before* being saved to the database.
+        beatles_serialized = beatles.serializable_data()
+        beatles_serialized['members'][1]['pk'] = None
+        saved_revision = Band.from_serializable_data(beatles_serialized)
+
+        form = BandForm({
+            'name': 'The Beatles',
+
+            'members-TOTAL_FORMS': 3,
+            'members-INITIAL_FORMS': 1,
+            'members-MAX_NUM_FORMS': 1000,
+
+            'members-0-name': 'John Lennon',
+            'members-0-id': john.id,
+
+            'members-1-name': 'Paul McCartney',
+            'members-1-id': '',
+
+            'members-2-name': 'George Harrison',
+            'members-2-id': '',
+        }, instance=saved_revision)
+
+        self.assertTrue(form.is_valid())
+        result = form.save()
+
+        self.assertEqual(result.members.count(), 3)
+        self.assertQuerySetEqual(
+            result.members.values_list('name', flat=True).order_by('name'),
+            ['George Harrison', 'John Lennon', 'Paul McCartney'],
+        )
+
     def test_explicit_formset_list(self):
         class BandForm(ClusterForm):
             class Meta: