From fedb74ab095b183df8fe93bb44681b810702a30f Mon Sep 17 00:00:00 2001 From: Jeremy Thurgood Date: Mon, 22 Jan 2024 16:15:13 +0200 Subject: [PATCH 1/2] Report page validation errors during import --- home/import_content_pages.py | 9 +++++++-- home/tests/bad-whatsapp-template-category.csv | 3 +++ home/tests/test_content_import_export.py | 15 +++++++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 home/tests/bad-whatsapp-template-category.csv diff --git a/home/import_content_pages.py b/home/import_content_pages.py index 7ab82faf..79f0ee95 100644 --- a/home/import_content_pages.py +++ b/home/import_content_pages.py @@ -9,6 +9,7 @@ from typing import Any from uuid import uuid4 +from django.core.exceptions import ValidationError # type: ignore from openpyxl import load_workbook from taggit.models import Tag # type: ignore from treebeard.exceptions import NodeAlreadySaved # type: ignore @@ -394,8 +395,12 @@ def save(self, parent: Page) -> None: self.add_quick_replies_to_page(page) self.add_triggers_to_page(page) - with contextlib.suppress(NodeAlreadySaved): - parent.add_child(instance=page) + try: + with contextlib.suppress(NodeAlreadySaved): + parent.add_child(instance=page) + 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() diff --git a/home/tests/bad-whatsapp-template-category.csv b/home/tests/bad-whatsapp-template-category.csv new file mode 100644 index 00000000..bda6c527 --- /dev/null +++ b/home/tests/bad-whatsapp-template-category.csv @@ -0,0 +1,3 @@ +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,659,ma_import-export,,MA_import export,,,,,,,,,,,,,,,,,,,,,,English,,,,,, +Sub 1.1,1,660,locale-import,MA_import export,Locale import,,,import per locale,this is the english message..edit,template,Marketing,,,,,,,,,,,,,,,,English,,[],,,, diff --git a/home/tests/test_content_import_export.py b/home/tests/test_content_import_export.py index 6cdaa656..d1bafd72 100644 --- a/home/tests/test_content_import_export.py +++ b/home/tests/test_content_import_export.py @@ -924,6 +924,21 @@ def test_missing_related_pages(self, newcsv_impexp: ImportExport) -> None: "'English'" ) + 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 + """ + with pytest.raises(ImportException) as e: + newcsv_impexp.import_file("bad-whatsapp-template-category.csv") + + assert e.value.row_num == 3 + # FIXME: Find a better way to represent this. + assert ( + e.value.message + == "Validation error: {'whatsapp_template_category': [\"Value 'Marketing' is not a valid choice.\"]}" + ) + # "old-xlsx" has at least three bugs, so we don't bother testing it. @pytest.fixture(params=["old-csv", "new-csv", "new-xlsx"]) From 0d1ee7475eb10ec3d08422bfb10bb97c3fb1e5e4 Mon Sep 17 00:00:00 2001 From: Jeremy Thurgood Date: Mon, 22 Jan 2024 16:38:35 +0200 Subject: [PATCH 2/2] Fix no-translation-key test and report CPI validation errors as well --- home/import_content_pages.py | 8 ++++++-- home/tests/test_content_import_export.py | 15 ++++++++++++--- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/home/import_content_pages.py b/home/import_content_pages.py index 79f0ee95..658be256 100644 --- a/home/import_content_pages.py +++ b/home/import_content_pages.py @@ -232,8 +232,12 @@ def create_content_page_index_from_row(self, row: "ContentRow") -> None: if row.translation_tag or locale != self.default_locale(): index.translation_key = row.translation_tag locale = self.locale_from_display_name(row.locale) - with contextlib.suppress(NodeAlreadySaved): - self.home_page(locale).add_child(instance=index) + try: + with contextlib.suppress(NodeAlreadySaved): + self.home_page(locale).add_child(instance=index) + except ValidationError as err: + # FIXME: Find a better way to represent this. + raise ImportException(f"Validation error: {err}") index.save_revision().publish() diff --git a/home/tests/test_content_import_export.py b/home/tests/test_content_import_export.py index d1bafd72..1424f05e 100644 --- a/home/tests/test_content_import_export.py +++ b/home/tests/test_content_import_export.py @@ -11,7 +11,6 @@ import pytest from django.core import serializers # type: ignore -from django.core.exceptions import ValidationError # type: ignore from django.core.files.images import ImageFile # type: ignore from openpyxl import load_workbook from pytest_django.fixtures import SettingsWrapper @@ -821,13 +820,23 @@ def test_no_translation_key_nondefault(self, newcsv_impexp: ImportExport) -> Non HomePage.add_root(locale=pt, title="Home (pt)", slug="home-pt") # A ContentPageIndex without a translation key fails - with pytest.raises(ValidationError): + with pytest.raises(ImportException) as e: newcsv_impexp.import_file("no-translation-key-cpi.csv") + assert e.value.row_num == 4 + # FIXME: Find a better way to represent this. + assert "translation_key" in e.value.message + assert "“” is not a valid UUID." in e.value.message + # A ContentPage without a translation key fails - with pytest.raises(ValidationError): + with pytest.raises(ImportException) as e: newcsv_impexp.import_file("no-translation-key-cp.csv") + assert e.value.row_num == 5 + # FIXME: Find a better way to represent this. + assert "translation_key" in e.value.message + assert "“” is not a valid UUID." in e.value.message + def test_invalid_locale_name(self, newcsv_impexp: ImportExport) -> None: """ Importing pages with invalid locale names should raise an error that results