CMS Forms Flexible Imports

CHANGELOG.md

@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Consistent labelling on import forms
 - Fix class_name warning: RemovedInWagtail60Warning
 - Helper texts for some ordered content sets
+- CMS Forms Flexible imports and Better Error Handling
 ### Removed
 - Locale field on exports
 -->

home/import_assessments.py

@@ -1,5 +1,6 @@
 import contextlib
 import csv
+import re
 from dataclasses import dataclass, field, fields
 from queue import Queue
 from typing import Any
@@ -15,6 +16,7 @@
 from home.models import Assessment, ContentPage, HomePage  # type: ignore
 
 AssessmentId = tuple[str, Locale]
+MANDATORY_HEADERS = ["title", "question", "slug", "generic_error", "locale"]
 
 
 class ImportAssessmentException(Exception):
@@ -96,9 +98,32 @@ def save_assessments(self) -> None:
             )
 
     def parse_file(self) -> list["AssessmentRow"]:
+        """
+        Converts the iterator to a list of rows.
+        If there are rows:
+            a. Extracts the original headers from the first row.
+            b. Converts the original headers to snake_case and creates a mapping from original headers to snake_case headers.
+            c. Validates that the snake_case headers contain all mandatory headers.
+            d. Transforms each row to use snake_case headers.
+        """
+        row_iterator = parse_file(self.file_content, self.file_type)
+        rows = [row for _, row in row_iterator]
+
+        if rows:
+            original_headers = rows[0].keys()
+            headers_mapping = {
+                header: self.to_snake_case(header) for header in original_headers
+            }
+            snake_case_headers = list(headers_mapping.values())
+            self.validate_headers(snake_case_headers, MANDATORY_HEADERS, row_num=1)
+            transformed_rows = [
+                {headers_mapping[key]: value for key, value in row.items()}
+                for row in rows
+            ]
+
         return [
-            AssessmentRow.from_flat(row, i)
-            for i, row in parse_file(self.file_content, self.file_type)
+            AssessmentRow.from_flat(row, i + 2)
+            for i, row in enumerate(transformed_rows)
         ]
 
     def set_progress(self, message: str, progress: int) -> None:
@@ -185,6 +210,21 @@ def create_shadow_assessment_from_row(
         )
         assessment.questions.append(question)
 
+    def validate_headers(
+        self, headers: list[str], MANDATORY_HEADERS: list[str], row_num: int
+    ) -> None:
+        missing_headers = [
+            header for header in MANDATORY_HEADERS if header not in headers
+        ]
+        if missing_headers:
+            raise ImportAssessmentException(
+                message=f"Missing mandatory headers: {', '.join(missing_headers)}",
+                row_num=row_num,
+            )
+
+    def to_snake_case(self, s: str) -> str:
+        return re.sub(r"[\W_]+", "_", s).lower().strip("_")
+
 
 # Since wagtail page models are so difficult to create and modify programatically,
 # we instead store all the pages as these shadow objects, which we can then at the end
@@ -360,6 +400,18 @@ class AssessmentRow:
     def fields(cls) -> list[str]:
         return [field.name for field in fields(cls)]
 
+    @classmethod
+    def check_missing_fields(cls, row: dict[str, str], row_num: int) -> None:
+        """
+        Checks for missing required fields in the row and raises an exception if any is missing.
+        """
+        missing_fields = [field for field in MANDATORY_HEADERS if field not in row]
+        if missing_fields:
+            raise ImportAssessmentException(
+                f"The import file is missing required fields: {', '.join(missing_fields)}",
+                row_num,
+            )
+
     @classmethod
     def from_flat(cls, row: dict[str, str], row_num: int) -> "AssessmentRow":
         """
@@ -376,27 +428,21 @@ def from_flat(cls, row: dict[str, str], row_num: int) -> "AssessmentRow":
             key: value for key, value in row.items() if value and key in cls.fields()
         }
 
-        # Create default value with correct length for answer responses if not specified
+        cls.check_missing_fields(row, row_num)
+
         answers = deserialise_list(row.pop("answers", ""))
         answer_responses = deserialise_list(row.pop("answer_responses", ""))
         if not answer_responses:
             answer_responses = [""] * len(answers)
 
-        try:
-            return cls(
-                tags=deserialise_list(row.pop("tags", "")),
-                answers=answers,
-                scores=[float(i) for i in deserialise_list(row.pop("scores", ""))],
-                answer_semantic_ids=deserialise_list(
-                    row.pop("answer_semantic_ids", "")
-                ),
-                answer_responses=answer_responses,
-                **row,
-            )
-        except TypeError:
-            raise ImportAssessmentException(
-                "The import file is missing some required fields."
-            )
+        return cls(
+            tags=deserialise_list(row.pop("tags", "")),
+            answers=answers,
+            scores=[float(i) for i in deserialise_list(row.pop("scores", ""))],
+            answer_semantic_ids=deserialise_list(row.pop("answer_semantic_ids", "")),
+            answer_responses=answer_responses,
+            **row,
+        )
 
 
 def get_content_page_id_from_slug(slug: str, locale: Locale) -> int:

home/tests/import-export-data/assessment_missing_related_page.csv

@@ -1,4 +1,4 @@
-title,question_type,tags,slug,locale,high_result_page,high_inflection,medium_result_page,medium_inflection,low_result_page,generic_error,questions,error,min,max,answers,scores,answer_semantic_ids,question_semantic_id
-Fake Test,categorical_question,"Random, selection",fake-test,en,fake-page,4,fake-page,3,fake-page,We did not quite get that,Select a number for the first question,Please retry,,,"A,B","4.0,1.0","test-1,test-2",question_semantic_id
-Fake Test,categorical_question,"Random, selection",fake-test,en,fake-page,4,fake-page,3,fake-page,We did not quite get that,Select a number for the second question,We did not quite get that,,,"A,B,C","1.0,3.0,4.0","test-1,test-2,test-3",question_semantic_id
-Fake Test,categorical_question,"Random, selection",fake-test,en,fake-page,4,fake-page,3,fake-page,We did not quite get that,Select a number for the third question,Sorry we did not quite get that,,,"A,B,C","2.0,1.0,2.0","test-1,test-2,test-3",question_semantic_id
+title,question_type,tags,slug,locale,high_result_page,high_inflection,medium_result_page,medium_inflection,low_result_page,skip_threshold,generic_error,question,error,min,max,answers,scores,answer_semantic_ids,question_semantic_id
+Fake Test,categorical_question,"Random, selection",fake-test,en,fake-page,4,fake-page,3,fake-page,0,We did not quite get that,Select a number for the first question,Please retry,,,"A,B","4.0,1.0","test-1,test-2",question_semantic_id
+Fake Test,categorical_question,"Random, selection",fake-test,en,fake-page,4,fake-page,3,fake-page,0,We did not quite get that,Select a number for the second question,We did not quite get that,,,"A,B,C","1.0,3.0,4.0","test-1,test-2,test-3",question_semantic_id
+Fake Test,categorical_question,"Random, selection",fake-test,en,fake-page,4,fake-page,3,fake-page,0,We did not quite get that,Select a number for the third question,Sorry we did not quite get that,,,"A,B,C","2.0,1.0,2.0","test-1,test-2,test-3",question_semantic_id

home/tests/import-export-data/assessments_missing_generic_error.csv

@@ -0,0 +1,6 @@
+title,question_type,tags,slug,version,locale,high_result_page,high_inflection,medium_result_page,medium_inflection,low_result_page,skip_threshold,skip_high_result_page,generic_error,question,explainer,error,min,max,answers,scores,answer_semantic_ids,question_semantic_id,answer_responses
+Freetext Question,freetext_question,draft-assessment,Draft-assessment,v1.0,en,,,,,,0,,,Is this a draft assessment,,,,,,,,draf-assessment,
+Random French,freetext_question,,random-french,v1.0,fr,high-inflection,5,,3,,0,,Sorry we didn't quite get that.,What do you not like about France,We need to know this,,,,,,,france-notlike,
+Integer Question,integer_question,test-min-max-range,test-min-max-range,v1.0,en,,5,,3,,0,,This is a generic error,Lowest temeprature you're experienced,We need to know some things,This is an error message,0,30,,,,lowest-temperature,
+Weather Trivia,integer_question,weather-trivia,weather-trivia,v1.0,en,high-inflection,5,medium-score,1,low-score,0,,"Sorry, we didn't quite get that.",What's the coldest weather you're experienced?,We need to know some things,Your reply should be between {min} and {max},50,70,,,,coldest-weather,
+Draft Assessment 2,freetext_question,draft-assessment,Draft-assessment-2,v1.0,en,,,,,,0,,This is a generic error for draft page,Is this a draft assessment,,,,,,,,draf-assessment,

home/tests/import-export-data/assessments_missing_locale.csv

@@ -0,0 +1,6 @@
+title,question_type,tags,slug,version,locale,high_result_page,high_inflection,medium_result_page,medium_inflection,low_result_page,skip_threshold,skip_high_result_page,generic_error,question,explainer,error,min,max,answers,scores,answer_semantic_ids,question_semantic_id,answer_responses
+Freetext Question,freetext_question,draft-assessment,Draft-assessment,v1.0,en,,,,,,0,,This is a generic error for draft page,Is this a draft assessment,,,,,,,,draf-assessment,
+Random French,freetext_question,,random-french,v1.0,fr,high-inflection,5,,3,,0,,Sorry we didn't quite get that.,What do you not like about France,We need to know this,,,,,,,france-notlike,
+Integer Question,integer_question,test-min-max-range,test-min-max-range,v1.0,en,,5,,3,,0,,This is a generic error,Lowest temeprature you're experienced,We need to know some things,This is an error message,0,30,,,,lowest-temperature,
+Weather Trivia,integer_question,weather-trivia,weather-trivia,v1.0,,high-inflection,5,medium-score,1,low-score,0,,"Sorry, we didn't quite get that.",What's the coldest weather you're experienced?,We need to know some things,Your reply should be between {min} and {max},50,70,,,,coldest-weather,
+Draft Assessment 2,freetext_question,draft-assessment,Draft-assessment-2,v1.0,en,,,,,,0,,This is a generic error for draft page,Is this a draft assessment,,,,,,,,draf-assessment,

home/tests/import-export-data/assessments_missing_title.csv

@@ -0,0 +1,6 @@
+title,question_type,tags,slug,version,locale,high_result_page,high_inflection,medium_result_page,medium_inflection,low_result_page,skip_threshold,skip_high_result_page,generic_error,question,explainer,error,min,max,answers,scores,answer_semantic_ids,question_semantic_id,answer_responses
+Freetext Question,freetext_question,draft-assessment,Draft-assessment,v1.0,en,,,,,,0,,This is a generic error for draft page,Is this a draft assessment,,,,,,,,draf-assessment,
+Random French,freetext_question,,random-french,v1.0,fr,high-inflection,5,,3,,0,,Sorry we didn't quite get that.,What do you not like about France,We need to know this,,,,,,,france-notlike,
+,integer_question,test-min-max-range,test-min-max-range,v1.0,en,,5,,3,,0,,This is a generic error,Lowest temeprature you're experienced,We need to know some things,This is an error message,0,30,,,,lowest-temperature,
+Weather Trivia,integer_question,weather-trivia,weather-trivia,v1.0,en,high-inflection,5,medium-score,1,low-score,0,,"Sorry, we didn't quite get that.",What's the coldest weather you're experienced?,We need to know some things,Your reply should be between {min} and {max},50,70,,,,coldest-weather,
+Draft Assessment 2,freetext_question,draft-assessment,Draft-assessment-2,v1.0,en,,,,,,0,,This is a generic error for draft page,Is this a draft assessment,,,,,,,,draf-assessment,

home/tests/import-export-data/assessments_multiple_missing_headers.csv

@@ -0,0 +1,6 @@
+,question_type,tags,,version,,high_result_page,high_inflection,medium_result_page,medium_inflection,low_result_page,skip_threshold,skip_high_result_page,,question,explainer,error,min,max,answers,scores,answer_semantic_ids,question_semantic_id,answer_responses
+Freetext Question,freetext_question,draft-assessment,Draft-assessment,v1.0,en,,,,,,0,,This is a generic error for draft page,Is this a draft assessment,,,,,,,,draf-assessment,
+Random French,freetext_question,,random-french,v1.0,fr,high-inflection,5,,3,,0,,Sorry we didn't quite get that.,What do you not like about France,We need to know this,,,,,,,france-notlike,
+Test min max range,integer_question,test-min-max-range,test-min-max-range,v1.0,en,,5,,3,,0,,This is a generic error,Lowest temeprature you're experienced,We need to know some things,This is an error message,0,30,,,,lowest-temperature,
+Weather Trivia,integer_question,weather-trivia,weather-trivia,v1.0,en,high-inflection,5,medium-score,1,low-score,0,,"Sorry, we didn't quite get that.",What's the coldest weather you're experienced?,We need to know some things,Your reply should be between {min} and {max},50,70,,,,coldest-weather,
+Draft Assessment 2,freetext_question,draft-assessment,Draft-assessment-2,v1.0,en,,,,,,0,,This is a generic error for draft page,Is this a draft assessment,,,,,,,,draf-assessment,

home/tests/import-export-data/broken_assessment.csv

@@ -1,2 +1,2 @@
 this,is,not,a,valid,content,csv
-"For real, it's totally not."
+"For real, it's totally not.",,,,,,

home/tests/import-export-data/snake_case_assessments.csv

@@ -0,0 +1,5 @@
+title,question type,tags,slug,version,locale,High result page,High inflection,Medium result page,Medium inflection,Low result page,Skip threshold,Skip high result page,Generic error,question,explainer,error,min,max,answers,scores,Answer semantic ids,Question semantic id,Answer responses
+Freetext Question,freetext_question,draft-assessment,Draft-assessment,v1.0,en,high-inflection,5,medium-score,3,,0,,This is a generic error for draft page,Is this a draft assessment,,,,,,,,draf-assessment,
+Test min max range,integer_question,test-min-max-range,test-min-max-range,v1.0,en,high-inflection,5,medium-score,3,,0,,This is a generic error,Lowest temeprature you're experienced,We need to know some things,This is an error message,0,30,,,,lowest-temperature,
+Weather Trivia,integer_question,weather-trivia,weather-trivia,v1.0,en,high-inflection,5,medium-score,1,low-score,0,,"Sorry, we didn't quite get that.",What's the coldest weather you're experienced?,We need to know some things,Your reply should be between {min} and {max},50,70,,,,coldest-weather,
+Draft Assessment 2,freetext_question,draft-assessment,Draft-assessment-2,v1.0,en,high-inflection,5,medium-score,3,,0,,This is a generic error for draft page,Is this a draft assessment,,,,,,,,draf-assessment,

home/tests/test_assessment_import_export.py

@@ -425,6 +425,41 @@ def test_single_assessment(self, impexp: ImportExport) -> None:
         imported = impexp.get_assessment_json()
         assert imported == orig
 
+    def test_snake_case_assessments(self, csv_impexp: ImportExport) -> None:
+        """
+        Importing a csv with spaces in header names and uppercase text should be coverted to snake case
+        and pass, provided that the converted text are valid cms headers.
+        Here we check that the headers are changed to snake case, the assessment is saved
+        and finally we check that the saved assessment has the correct headers.
+
+        (This uses snake_case_assessments.csv.)
+        """
+        csv_impexp.import_file("snake_case_assessments.csv")
+        imported_assessments = Assessment.objects.all()
+        assert imported_assessments.exists()
+        assessment_model = Assessment
+        model_fields = [field.name for field in assessment_model._meta.get_fields()]
+
+        expected_fields = {
+            "title": True,
+            "slug": True,
+            "high_inflection": True,
+            "medium_inflection": True,
+            "high_result_page": True,
+            "medium_result_page": True,
+            "low_result_page": True,
+            "skip_high_result_page": True,
+            "skip_threshold": True,
+            "generic_error": True,
+            "questions": True,
+        }
+
+        for field_name, should_exist in expected_fields.items():
+            if should_exist:
+                assert (
+                    field_name in model_fields
+                ), f"Field '{field_name}' not found in Assessment model."
+
 
 @pytest.mark.usefixtures("result_content_pages")
 @pytest.mark.django_db()
@@ -462,7 +497,10 @@ def test_import_error(self, csv_impexp: ImportExport) -> None:
         csv_bytes = csv_impexp.import_file("assessment_simple.csv")
         with pytest.raises(ImportAssessmentException) as e:
             csv_impexp.import_file("broken_assessment.csv")
-        assert e.value.message == "The import file is missing some required fields."
+        assert (
+            e.value.message
+            == "Missing mandatory headers: title, question, slug, generic_error, locale"
+        )
         content = csv_impexp.export_assessment()
         src, dst = csv_impexp.csvs2dicts(csv_bytes, content)
         assert dst == src
@@ -618,6 +656,52 @@ def test_invalid_medium_score(self, csv_impexp: ImportExport) -> None:
         )
         assert e.value.row_num == 2
 
+    def test_multiple_missing_headers(self, csv_impexp: ImportExport) -> None:
+        """
+        Importing a CSV with multiple missing headers should return an error
+        stating all the mandatory headers that are missing
+        """
+        with pytest.raises(ImportAssessmentException) as e:
+            csv_impexp.import_file("assessments_multiple_missing_headers.csv")
+        assert (
+            e.value.message == "Missing mandatory headers: title, slug, "
+            "generic_error, locale"
+        )
+        assert e.value.row_num == 1
+
+    def test_missing_title(self, csv_impexp: ImportExport) -> None:
+        """
+        Importing a CSV with a missing title field should return an error
+        that a title is missing
+        """
+        with pytest.raises(ImportAssessmentException) as e:
+            csv_impexp.import_file("assessments_missing_title.csv")
+        assert e.value.message == "The import file is missing required fields: title"
+        assert e.value.row_num == 4
+
+    def test_missing_locale(self, csv_impexp: ImportExport) -> None:
+        """
+        Importing a CSV with a missing locale field should return an error
+        that a locale is mmissing
+        """
+        with pytest.raises(ImportAssessmentException) as e:
+            csv_impexp.import_file("assessments_missing_locale.csv")
+        assert e.value.message == "The import file is missing required fields: locale"
+        assert e.value.row_num == 5
+
+    def test_missing_generic_error(self, csv_impexp: ImportExport) -> None:
+        """
+        Importing a CSV with a missing generic error field should return an error
+        that a generic error is mmissing
+        """
+        with pytest.raises(ImportAssessmentException) as e:
+            csv_impexp.import_file("assessments_missing_generic_error.csv")
+        assert (
+            e.value.message
+            == "The import file is missing required fields: generic_error"
+        )
+        assert e.value.row_num == 2
+
 
 @pytest.mark.usefixtures("result_content_pages")
 @pytest.mark.django_db()