Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report validation errors during import #234

Merged
merged 2 commits into from
Jan 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions home/import_content_pages.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -231,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()

Expand Down Expand Up @@ -394,8 +399,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()

Expand Down
3 changes: 3 additions & 0 deletions home/tests/bad-whatsapp-template-category.csv
Original file line number Diff line number Diff line change
@@ -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,,[],,,,
30 changes: 27 additions & 3 deletions home/tests/test_content_import_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -924,6 +933,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"])
Expand Down