Updated the block id initialization (#5552)

The current block id generation only sets the id as the block is serialized for storage in the database, which means that the id is unavailable in the block until it is pulled back from the database. In my debugging this caused the id to be set to new values up to 3 times when saving a brand new page (each time with a new id).
This updated logic applies the new id to the actual block which makes it available right away and prevents the id from being regenerated.

CHANGELOG.txt

@@ -33,6 +33,7 @@ Changelog
  * Fix: Prevent tab labels on page edit view from being cut off on very narrow screens (Kevin Howbrook)
  * Fix: Very long words in page listings are now broken where necessary (Kevin Howbrook)
  * Fix: Language chosen in user preferences no longer persists on subsequent requests (Bojan Mihelac)
+ * Fix: Prevent new block IDs from being assigned on repeated calls to `StreamBlock.get_prep_value` (Colin Klein)
 
 
 2.6.1 (05.08.2019)

CONTRIBUTORS.rst

@@ -395,6 +395,7 @@ Contributors
 * Sarath Kumar Somana
 * Dani Hodovic
 * Janne Alatalo
+* Colin Klein
 
 Translators
 ===========

docs/releases/2.7.rst

@@ -57,6 +57,7 @@ Bug fixes
  * Prevent tab labels on page edit view from being cut off on very narrow screens (Kevin Howbrook)
  * Very long words in page listings are now broken where necessary (Kevin Howbrook)
  * Language chosen in user preferences no longer persists on subsequent requests (Bojan Mihelac)
+ * Prevent new block IDs from being assigned on repeated calls to ``StreamBlock.get_prep_value`` (Colin Klein)
 
 
 Upgrade considerations

wagtail/core/blocks/stream_block.py

@@ -432,20 +432,22 @@ class StreamValue(Sequence):
                 # value (stream_data_item here) is still valid
                 prep_value_item = stream_data_item
 
+                # As this method is preparing this value to be saved to the database,
+                # this is an appropriate place to ensure that each block has a unique id.
+                prep_value_item['id'] = prep_value_item.get('id', str(uuid.uuid4()))
+
             else:
                 # convert the bound block back into JSONish data
                 child = self[i]
+                # As this method is preparing this value to be saved to the database,
+                # this is an appropriate place to ensure that each block has a unique id.
+                child.id = child.id or str(uuid.uuid4())
                 prep_value_item = {
                     'type': child.block.name,
                     'value': child.block.get_prep_value(child.value),
                     'id': child.id,
                 }
 
-            # As this method is preparing this value to be saved to the database,
-            # this is an appropriate place to ensure that each block has a unique id.
-            if not prep_value_item.get('id'):
-                prep_value_item['id'] = str(uuid.uuid4())
-
             prep_value.append(prep_value_item)
 
         return prep_value

wagtail/core/tests/test_blocks.py

@@ -2748,6 +2748,12 @@ class TestStreamBlock(WagtailTestUtils, SimpleTestCase):
         # ID to this block, as it didn't have one already.
         self.assertTrue(jsonish_value[1]['id'])
 
+        # Calling get_prep_value again should preserve existing IDs, including the one
+        # just assigned to block 1
+        jsonish_value_again = block.get_prep_value(value)
+        self.assertEqual(jsonish_value[0]['id'], jsonish_value_again[0]['id'])
+        self.assertEqual(jsonish_value[1]['id'], jsonish_value_again[1]['id'])
+
     def test_get_prep_value_not_lazy(self):
         stream_data = [
             ('heading', 'this is my heading', '0000'),