Fix a bug with nested items

server/books/schema/mutations/contentblock.py

@@ -8,10 +8,12 @@ from graphql_relay import from_global_id
 from api.utils import get_object, get_errors
 from books.models import ContentBlock, Chapter, SchoolClass
 from books.schema.inputs import ContentBlockInput
+from core.logger import get_logger
 from ..nodes import ContentBlockNode
 from core.utils import set_hidden_for, set_visible_for
 from .utils import handle_content_block, handle_text, set_user_defined_block_type
 
+logger = get_logger(__name__)
 
 class MutateContentBlock(relay.ClientIDMutation):
     class Input:
@@ -58,8 +60,8 @@ class MutateContentBlock(relay.ClientIDMutation):
             errors = get_errors(e)
             raise errors
         except Exception as e:
-            errors = ['Error: {}'.format(e)]
-            raise Exception(errors)
+            logger.error(e)
+            raise e
 
 
 class AddContentBlock(relay.ClientIDMutation):

server/books/schema/mutations/utils.py

@@ -11,6 +11,10 @@
 import bleach
 import re
 
+from typing import List, Union
+
+from wagtail.core.blocks import StreamValue
+
 from api.utils import get_object
 from assignments.models import Assignment
 from books.models import ContentBlock
@@ -20,6 +24,18 @@ class AssignmentParameterException(Exception):
     pass
 
 
+class ContentTypeException(Exception):
+    pass
+
+
+def get_previous_item(previous_contents: Union[StreamValue, List[dict]], item: dict):
+    if isinstance(previous_contents, StreamValue):
+        contents = previous_contents.raw_data
+    else:
+        contents = previous_contents
+    return next((c for c in contents if c.get('id', None) == item['id']), None)
+
+
 def handle_text(text):
     is_list = bool(re.search(r'<ul>', text))
     if is_list:
@@ -46,6 +62,17 @@ ALLOWED_BLOCKS = (
 
 
 def handle_content_block(content, context=None, module=None, allowed_blocks=ALLOWED_BLOCKS, previous_contents=None):
+    """
+    Handle different content types of a content block
+
+    :param content: The current content block to be handled
+    :param context: The request context of the current request, contains the user
+    :param module: The module where this content block comes from, used for assignments
+    :type module: books.models.Module
+    :param allowed_blocks: List of allowed types of blocks
+    :type allowed_blocks: list(str)
+    :param previous_contents: The original content list from where this content block came from before being modified by the user
+    """
     # todo: add all the content blocks
     # todo: sanitize user inputs!
     if content['type'] not in allowed_blocks:
@@ -117,18 +144,21 @@ def handle_content_block(content, context=None, module=None, allowed_blocks=ALLO
             }
         }
     elif content['type'] == 'content_list_item':
-        return {
-            'type': 'content_list_item',
-            'value': [handle_content_block(c, context, module) for c in content['contents']]
-        }
+        previous_item = get_previous_item(previous_contents=previous_contents, item=content)
+        value = [handle_content_block(c, context, module, previous_contents=previous_item['value']) for c in
+                 content['contents']]
+        content['value'] = value
+        return content
     elif content['type'] == 'readonly' and previous_contents is not None:
         # get first item that matches the id
         # users can re-order readonly items, but we won't let them change them otherwise, so we just take the
         # item from before and ignore anything else
-        previous_content = next((c for c in previous_contents.raw_data if c['id'] == content['id']), None)
-        return previous_content
+        previous_item = get_previous_item(previous_contents=previous_contents, item=content)
+        if previous_item is None:
+            raise ContentTypeException('Readonly item found that is not allowed here')
+        return previous_item
 
-    return None
+    raise ContentTypeException('Type did not match a case')
 
 
 def set_user_defined_block_type(block_type):