Merge branch 'main' of https://github.com/praekeltfoundation/contentrepo

 into MNCH-1030-whatsapp-list-messages

home/import_content_pages.py

@@ -235,12 +235,11 @@ def create_content_page_index_from_row(self, row: "ContentRow") -> None:
         try:
             with contextlib.suppress(NodeAlreadySaved):
                 self.home_page(locale).add_child(instance=index)
+            index.save_revision().publish()
         except ValidationError as err:
             # FIXME: Find a better way to represent this.
             raise ImportException(f"Validation error: {err}")
 
-        index.save_revision().publish()
-
     def create_shadow_content_page_from_row(
         self, row: "ContentRow", row_num: int
     ) -> None:
@@ -402,12 +401,11 @@ def save(self, parent: Page) -> None:
         try:
             with contextlib.suppress(NodeAlreadySaved):
                 parent.add_child(instance=page)
+            page.save_revision().publish()
         except ValidationError as err:
             # FIXME: Find a better way to represent this.
             raise ImportException(f"Validation error: {err}", self.row_num)
 
-        page.save_revision().publish()
-
     def add_web_to_page(self, page: ContentPage) -> None:
         page.enable_web = self.enable_web
         page.title = self.title

home/models.py

@@ -876,6 +876,26 @@ def clean(self):
         # The variable check is only for templates
         if self.is_whatsapp_template and len(self.whatsapp_body.raw_data) > 0:
             whatsapp_message = self.whatsapp_body.raw_data[0]["value"]["message"]
+
+            right_mismatch = re.findall(r"(?<!\{){[^{}]*}\}", whatsapp_message)
+            left_mismatch = re.findall(r"\{{[^{}]*}(?!\})", whatsapp_message)
+            mismatches = right_mismatch + left_mismatch
+
+            if mismatches:
+                errors.setdefault("whatsapp_body", []).append(
+                    StreamBlockValidationError(
+                        {
+                            0: StreamBlockValidationError(
+                                {
+                                    "message": ValidationError(
+                                        f"Please provide variables with matching braces. You provided {mismatches}."
+                                    )
+                                }
+                            )
+                        }
+                    )
+                )
+
             vars_in_msg = re.findall(r"{{(.*?)}}", whatsapp_message)
             non_digit_variables = [var for var in vars_in_msg if not var.isdecimal()]
 

home/templates/home/content_page.html

@@ -0,0 +1,12 @@
+{% extends "base.html" %}
+{% load wagtailcore_tags %}
+
+{% block body_class %}template-contentpage{% endblock %}
+
+
+{% block content %}
+    <h1>{{ page.title }}</h1>
+    <h2>{{ page.subtitle }}</h2>
+    {% include_block page.body %}
+
+{% endblock %}

home/tests/bad-cpi-translation-key.csv

@@ -0,0 +1,2 @@
+structure,message,page_id,slug,parent,web_title,web_subtitle,web_body,whatsapp_title,whatsapp_body,whatsapp_template_name,whatsapp_template_category,example_values,variation_title,variation_body,sms_title,sms_body,ussd_title,ussd_body,messenger_title,messenger_body,viber_title,viber_body,translation_tag,tags,quick_replies,triggers,locale,next_prompt,buttons,image_link,doc_link,media_link,related_pages
+Menu 1,0,1303,appointment-reminders,,Appointment reminders,,,,,,,,,,,,,,,,,,BADUUID,,,,English,,,,,,

home/tests/bad-whatsapp-template-vars.csv

@@ -0,0 +1,4 @@
+structure,message,page_id,slug,parent,web_title,web_subtitle,web_body,whatsapp_title,whatsapp_body,whatsapp_template_name,whatsapp_template_category,example_values,variation_title,variation_body,sms_title,sms_body,ussd_title,ussd_body,messenger_title,messenger_body,viber_title,viber_body,translation_tag,tags,quick_replies,triggers,locale,next_prompt,buttons,image_link,doc_link,media_link,related_pages
+Menu 1,0,1303,appointment-reminders,,Appointment reminders,,,,,,,,,,,,,,,,,,8cff04aa-cf08-4120-88b4-e2269b7d5d80,,,,English,,,,,,
+Sub 1.13,1,1467,mnch_appointment_child_2,Appointment reminders,Appointment child 2,,,mnch_appointment_child_2,"Hi {{1}}
+{{2}} {{4}}",mnch_appointment_child_2,MARKETING,Helen,,,,,,,,,,,0c9857a5-e381-49d7-b2fd-123348c4a373,,,,English,,,,,,

home/tests/good-cpi-translation-key.csv

@@ -0,0 +1,2 @@
+structure,message,page_id,slug,parent,web_title,web_subtitle,web_body,whatsapp_title,whatsapp_body,whatsapp_template_name,whatsapp_template_category,example_values,variation_title,variation_body,sms_title,sms_body,ussd_title,ussd_body,messenger_title,messenger_body,viber_title,viber_body,translation_tag,tags,quick_replies,triggers,locale,next_prompt,buttons,image_link,doc_link,media_link,related_pages
+Menu 1,0,1303,appointment-reminders,,Appointment reminders,,,,,,,,,,,,,,,,,,8cff04aa-cf08-4120-88b4-e2269b7d5d80,,,,English,,,,,,

home/tests/good-whatsapp-template-vars.csv

@@ -0,0 +1,4 @@
+structure,message,page_id,slug,parent,web_title,web_subtitle,web_body,whatsapp_title,whatsapp_body,whatsapp_template_name,whatsapp_template_category,example_values,variation_title,variation_body,sms_title,sms_body,ussd_title,ussd_body,messenger_title,messenger_body,viber_title,viber_body,translation_tag,tags,quick_replies,triggers,locale,next_prompt,buttons,image_link,doc_link,media_link,related_pages
+Menu 1,0,1303,appointment-reminders,,Appointment reminders,,,,,,,,,,,,,,,,,,8cff04aa-cf08-4120-88b4-e2269b7d5d80,,,,English,,,,,,
+Sub 1.13,1,1467,mnch_appointment_child_2,Appointment reminders,Appointment child 2,,,mnch_appointment_child_2,"Hi {{1}}
+{{2}} {{3}}",mnch_appointment_child_2,MARKETING,Helen,,,,,,,,,,,0c9857a5-e381-49d7-b2fd-123348c4a373,,,,English,,,,,,

home/tests/test_content_import_export.py

@@ -936,7 +936,7 @@ def test_missing_related_pages(self, newcsv_impexp: ImportExport) -> None:
     def test_invalid_wa_template_category(self, newcsv_impexp: ImportExport) -> None:
         """
         Importing a WhatsApp template with an invalid category should raise an
-        error that results in an error message that gets sent back to the user
+        error that results in an error message that gets sent back to the user.
         """
         with pytest.raises(ImportException) as e:
             newcsv_impexp.import_file("bad-whatsapp-template-category.csv")
@@ -948,6 +948,77 @@ def test_invalid_wa_template_category(self, newcsv_impexp: ImportExport) -> None
             == "Validation error: {'whatsapp_template_category': [\"Value 'Marketing' is not a valid choice.\"]}"
         )
 
+    def test_invalid_wa_template_vars(self, newcsv_impexp: ImportExport) -> None:
+        """
+        Importing a WhatsApp template with invalid variables should raise an
+        error that results in an error message that gets sent back to the user.
+        """
+        with pytest.raises(ImportException) as e:
+            newcsv_impexp.import_file("bad-whatsapp-template-vars.csv")
+
+        assert e.value.row_num == 3
+        # FIXME: Find a better way to represent this.
+        assert (
+            e.value.message
+            == "Validation error: {'whatsapp_body': ['Validation error in StreamBlock']}"
+        )
+
+    def test_invalid_wa_template_vars_update(self, newcsv_impexp: ImportExport) -> None:
+        """
+        Updating a valid WhatsApp template with invalid variables should raise
+        an error that results in an error message that gets sent back to the
+        user. The update validation happens in a different code path from the
+        initial import.
+        """
+        newcsv_impexp.import_file("good-whatsapp-template-vars.csv")
+
+        # Update an existing page, which does the validation in
+        # `page.save_revision()` rather than `parent.add_child()`.
+        with pytest.raises(ImportException) as e:
+            newcsv_impexp.import_file("bad-whatsapp-template-vars.csv", purge=False)
+
+        assert e.value.row_num == 3
+        # FIXME: Find a better way to represent this.
+        assert (
+            e.value.message
+            == "Validation error: {'whatsapp_body': ['Validation error in StreamBlock']}"
+        )
+
+    def test_cpi_validation_failure(self, newcsv_impexp: ImportExport) -> None:
+        """
+        Importing a ContentPageIndex with an invalid translation key should
+        raise an error that results in an error message that gets sent back to
+        the user.
+        """
+        with pytest.raises(ImportException) as e:
+            newcsv_impexp.import_file("bad-cpi-translation-key.csv")
+
+        assert e.value.row_num == 2
+        # FIXME: Find a better way to represent this.
+        assert (
+            e.value.message
+            == "Validation error: {'translation_key': ['“BADUUID” is not a valid UUID.']}"
+        )
+
+    def test_cpi_validation_failure_update(self, newcsv_impexp: ImportExport) -> None:
+        """
+        Updating a valid ContentPageIndex with an invalid translation key
+        should raise an error that results in an error message that gets sent
+        back to the user. The update validation happens in a different code
+        path from the initial import.
+        """
+        newcsv_impexp.import_file("good-cpi-translation-key.csv")
+
+        with pytest.raises(ImportException) as e:
+            newcsv_impexp.import_file("bad-cpi-translation-key.csv", purge=False)
+
+        assert e.value.row_num == 2
+        # FIXME: Find a better way to represent this.
+        assert (
+            e.value.message
+            == "Validation error: {'translation_key': ['“BADUUID” is not a valid UUID.']}"
+        )
+
 
 # "old-xlsx" has at least three bugs, so we don't bother testing it.
 @pytest.fixture(params=["old-csv", "new-csv", "new-xlsx"])

home/tests/test_models.py

@@ -277,7 +277,7 @@ def test_clean_text_valid_variables(self):
                         "WA Title",
                         [
                             WABlk(
-                                "{{2}}{{1}} {{foo}}",
+                                "{{2}}{{1}} {{foo}} {{mismatch1} {mismatch2}}",
                             )
                         ],
                     )

requirements-dev.txt

@@ -1,7 +1,7 @@
-black
-ruff
+black==24.1.1
+ruff==0.1.14
 responses
-pytest
+pytest==8.0.0
 pytest-django
 pytest-cov
 pytest-xdist