diff --git a/nofos/bloom_nofos/error_helpers.py b/nofos/bloom_nofos/error_helpers.py
index 69d9982c..0cedc317 100644
--- a/nofos/bloom_nofos/error_helpers.py
+++ b/nofos/bloom_nofos/error_helpers.py
@@ -1,3 +1,4 @@
+from django.core.exceptions import ValidationError
from django.shortcuts import render
DOCUMENT_STRUCTURE_RECOVERY_STEPS = (
@@ -6,6 +7,26 @@
)
+class MistaggedHeadingError(ValidationError):
+ """A heading is too long and is likely paragraph text with a heading style."""
+
+ code = "mistagged_heading"
+
+ def __init__(self, *, heading_kind, heading_order, heading_text, max_length):
+ self.heading_kind = heading_kind
+ self.heading_order = heading_order
+ self.heading_text = heading_text
+ self.max_length = max_length
+ super().__init__(
+ (
+ f"{heading_kind.title()} heading {heading_order} exceeds the "
+ f"{max_length}-character limit. This often means a paragraph "
+ "was incorrectly styled as a heading."
+ ),
+ code=self.code,
+ )
+
+
def render_blocking_import_error(
request,
*,
@@ -16,6 +37,7 @@ def render_blocking_import_error(
recovery_steps=None,
retry_url=None,
retry_label="Try the import again",
+ error_details=None,
):
"""Render a safe, actionable error page for a blocked document import."""
return render(
@@ -29,10 +51,57 @@ def render_blocking_import_error(
"recovery_steps": recovery_steps or [],
"retry_url": retry_url,
"retry_label": retry_label,
+ "error_details": error_details or [],
},
)
+def render_mistagged_heading_error(
+ request,
+ error,
+ *,
+ retry_url=None,
+ retry_label="Try the import again",
+):
+ """Render a safe, specific response for a likely mistagged paragraph."""
+ detected_as = f"{error.heading_kind.title()} heading"
+ if error.heading_order not in (None, ""):
+ detected_as = f"{detected_as} {error.heading_order}"
+
+ return render_blocking_import_error(
+ request,
+ title="We found text that may have the wrong heading style",
+ summary=(
+ "The document contains heading text that is too long. This usually "
+ "means a paragraph was formatted as a heading by mistake."
+ ),
+ error_code="IMPORT-HEADING-TOO-LONG",
+ status=422,
+ error_details=[
+ {"label": "Detected as", "value": detected_as},
+ {
+ "label": "Heading character limit",
+ "value": str(error.max_length),
+ },
+ {
+ "label": "Characters found",
+ "value": str(len(error.heading_text)),
+ },
+ {"label": "Affected text", "value": error.heading_text},
+ ],
+ recovery_steps=[
+ "Open the document in Word and find the affected text shown above.",
+ (
+ "If it is paragraph text, change its style to Normal. If it is "
+ "a heading, shorten it or apply the correct heading style."
+ ),
+ "Save the document, then select it again.",
+ ],
+ retry_url=retry_url,
+ retry_label=retry_label,
+ )
+
+
def render_import_server_error(request, *, retry_url=None):
"""Return a sanitized 500 response for an unexpected import failure."""
return render_blocking_import_error(
diff --git a/nofos/bloom_nofos/templates/import_error.html b/nofos/bloom_nofos/templates/import_error.html
index 23f90dd8..1c2b46d4 100644
--- a/nofos/bloom_nofos/templates/import_error.html
+++ b/nofos/bloom_nofos/templates/import_error.html
@@ -8,6 +8,15 @@
{{ error_title }}
{{ error_summary }}
+ {% if error_details %}
+
+ {% for detail in error_details %}
+ - {{ detail.label }}
+ - {{ detail.value }}
+ {% endfor %}
+
+ {% endif %}
+
{% if recovery_steps %}
What to do next:
diff --git a/nofos/compare/test_views.py b/nofos/compare/test_views.py
index 506dccba..6817b94e 100644
--- a/nofos/compare/test_views.py
+++ b/nofos/compare/test_views.py
@@ -1,9 +1,11 @@
import csv
import io
import json
+import os
import uuid
from unittest.mock import MagicMock, patch
+from django.conf import settings
from django.contrib.auth import get_user_model
from django.core.files.uploadedfile import SimpleUploadedFile
from django.test import TestCase
@@ -17,6 +19,14 @@
User = get_user_model()
+MISTAGGED_HEADING_FIXTURE_PATH = os.path.join(
+ settings.BASE_DIR,
+ "nofos",
+ "fixtures",
+ "docx",
+ "mistagged-paragraph-heading.docx",
+)
+
class DuplicateCompareTests(TestCase):
def _make_nofo_with_content(
@@ -353,6 +363,29 @@ def test_post_missing_file_returns_400(self):
self.assertEqual(follow_response.status_code, 200)
self.assertContains(follow_response, "Error: Oops! No fos uploaded.")
+ def test_word_import_identifies_mistagged_paragraph_heading(self):
+ with open(MISTAGGED_HEADING_FIXTURE_PATH, "rb") as fixture:
+ uploaded_file = SimpleUploadedFile(
+ "mistagged-paragraph-heading.docx",
+ fixture.read(),
+ content_type=(
+ "application/vnd.openxmlformats-officedocument."
+ "wordprocessingml.document"
+ ),
+ )
+
+ response = self.client.post(self.url, {"nofo-import": uploaded_file})
+
+ content = response.content.decode("utf-8")
+ self.assertEqual(response.status_code, 422)
+ self.assertIn("IMPORT-HEADING-TOO-LONG", content)
+ self.assertIn(
+ "This entire paragraph was accidentally assigned a heading style in Word.",
+ content,
+ )
+ self.assertNotIn("COMPARE-IMPORT-INVALID", content)
+ self.assertEqual(CompareDocument.objects.count(), 0)
+
@patch("nofos.views.parse_uploaded_file_as_html_string")
@patch("compare.views.create_compare_document")
def test_unexpected_creation_error_uses_safe_500_page(
@@ -399,6 +432,33 @@ def test_import_to_existing_document_uses_safe_500_page(
self.assertIn(f'href="{url}"', content)
self.assertNotIn("private compare-to-doc detail", content)
+ def test_word_import_to_existing_document_identifies_mistagged_heading(self):
+ document = CompareDocument.objects.create(
+ title="Existing comparison", opdiv="CDC", group="bloom"
+ )
+ url = reverse("compare:compare_import_to_doc", kwargs={"pk": document.pk})
+ with open(MISTAGGED_HEADING_FIXTURE_PATH, "rb") as fixture:
+ uploaded_file = SimpleUploadedFile(
+ "mistagged-paragraph-heading.docx",
+ fixture.read(),
+ content_type=(
+ "application/vnd.openxmlformats-officedocument."
+ "wordprocessingml.document"
+ ),
+ )
+
+ response = self.client.post(url, {"nofo-import": uploaded_file})
+
+ content = response.content.decode("utf-8")
+ self.assertEqual(response.status_code, 422)
+ self.assertIn("IMPORT-HEADING-TOO-LONG", content)
+ self.assertIn(
+ "This entire paragraph was accidentally assigned a heading style in Word.",
+ content,
+ )
+ self.assertNotIn("COMPARE-TO-DOC-INVALID", content)
+ self.assertEqual(Nofo.objects.count(), 0)
+
# Edit the title right after importing
class CompareImportTitleViewTests(TestCase):
diff --git a/nofos/compare/views.py b/nofos/compare/views.py
index ec9dccab..4004ab3c 100644
--- a/nofos/compare/views.py
+++ b/nofos/compare/views.py
@@ -4,8 +4,10 @@
from bloom_nofos.error_helpers import (
DOCUMENT_STRUCTURE_RECOVERY_STEPS,
+ MistaggedHeadingError,
render_blocking_import_error,
render_import_server_error,
+ render_mistagged_heading_error,
)
from bloom_nofos.logs import log_exception
from django.contrib import messages
@@ -176,6 +178,21 @@ def handle_nofo_create(self, request, soup, sections, filename, *args, **kwargs)
)
return redirect("compare:compare_import_title", pk=compare_doc.pk)
+ except MistaggedHeadingError as e:
+ log_exception(
+ request,
+ e,
+ level="warning",
+ context=(
+ "CompareImportView:MistaggedHeadingError:" "IMPORT-HEADING-TOO-LONG"
+ ),
+ status=422,
+ )
+ return render_mistagged_heading_error(
+ request,
+ e,
+ retry_url=reverse("compare:compare_import"),
+ )
except ValidationError as e:
log_exception(
request,
@@ -475,6 +492,22 @@ def handle_nofo_create(self, request, soup, sections, filename, *args, **kwargs)
new_nofo_id=new_nofo.pk,
)
+ except MistaggedHeadingError as e:
+ log_exception(
+ request,
+ e,
+ level="warning",
+ context=(
+ "CompareImportToDocView:MistaggedHeadingError:"
+ "IMPORT-HEADING-TOO-LONG"
+ ),
+ status=422,
+ )
+ return render_mistagged_heading_error(
+ request,
+ e,
+ retry_url=self.get_retry_url(),
+ )
except ValidationError as e:
log_exception(
request,
diff --git a/nofos/composer/tests/test_views.py b/nofos/composer/tests/test_views.py
index 8004a9b9..45b9dbbb 100644
--- a/nofos/composer/tests/test_views.py
+++ b/nofos/composer/tests/test_views.py
@@ -1,3 +1,4 @@
+import os
import uuid
from unittest.mock import patch
@@ -8,6 +9,7 @@
ContentGuideSubsection,
)
from composer.views import ComposerSectionView
+from django.conf import settings
from django.contrib.auth import get_user_model
from django.contrib.messages import get_messages
from django.core.exceptions import ValidationError
@@ -20,6 +22,14 @@
PASSWORD = "testpass123"
+MISTAGGED_HEADING_FIXTURE_PATH = os.path.join(
+ settings.BASE_DIR,
+ "nofos",
+ "fixtures",
+ "docx",
+ "mistagged-paragraph-heading.docx",
+)
+
def create_composer_admin_user(
email="composer-admin@example.com",
@@ -187,6 +197,29 @@ def test_creation_validation_error_uses_safe_blocking_page(
self.assertIn(f'href="{self.url}"', content)
self.assertNotIn("private validation detail", content)
+ def test_word_import_identifies_mistagged_paragraph_heading(self):
+ with open(MISTAGGED_HEADING_FIXTURE_PATH, "rb") as fixture:
+ uploaded_file = SimpleUploadedFile(
+ "mistagged-paragraph-heading.docx",
+ fixture.read(),
+ content_type=(
+ "application/vnd.openxmlformats-officedocument."
+ "wordprocessingml.document"
+ ),
+ )
+
+ response = self.client.post(self.url, {"nofo-import": uploaded_file})
+
+ content = response.content.decode("utf-8")
+ self.assertEqual(response.status_code, 422)
+ self.assertIn("IMPORT-HEADING-TOO-LONG", content)
+ self.assertIn(
+ "This entire paragraph was accidentally assigned a heading style in Word.",
+ content,
+ )
+ self.assertNotIn("COMPOSER-IMPORT-INVALID", content)
+ self.assertEqual(ContentGuide.objects.count(), 0)
+
class ComposerImportTitleViewTests(TestCase):
def setUp(self):
diff --git a/nofos/composer/views.py b/nofos/composer/views.py
index 231aab57..0bf98a33 100644
--- a/nofos/composer/views.py
+++ b/nofos/composer/views.py
@@ -3,8 +3,10 @@
from bloom_nofos.error_helpers import (
DOCUMENT_STRUCTURE_RECOVERY_STEPS,
+ MistaggedHeadingError,
render_blocking_import_error,
render_import_server_error,
+ render_mistagged_heading_error,
)
from bloom_nofos.html_diff import has_diff, html_diff
from bloom_nofos.logs import log_exception
@@ -469,6 +471,22 @@ def handle_nofo_create(self, request, soup, sections, filename, *args, **kwargs)
return redirect("composer:composer_import_title", pk=document.pk)
+ except MistaggedHeadingError as e:
+ log_exception(
+ request,
+ e,
+ level="warning",
+ context=(
+ "ComposerImportView:MistaggedHeadingError:"
+ "IMPORT-HEADING-TOO-LONG"
+ ),
+ status=422,
+ )
+ return render_mistagged_heading_error(
+ request,
+ e,
+ retry_url=reverse("composer:composer_import"),
+ )
except ValidationError as e:
log_exception(
request,
diff --git a/nofos/nofos/fixtures/docx/mistagged-paragraph-heading.docx b/nofos/nofos/fixtures/docx/mistagged-paragraph-heading.docx
new file mode 100644
index 00000000..c9b282aa
Binary files /dev/null and b/nofos/nofos/fixtures/docx/mistagged-paragraph-heading.docx differ
diff --git a/nofos/nofos/nofo.py b/nofos/nofos/nofo.py
index b5c19ee7..8869840e 100644
--- a/nofos/nofos/nofo.py
+++ b/nofos/nofos/nofo.py
@@ -12,6 +12,7 @@
import mammoth
import markdown
import requests
+from bloom_nofos.error_helpers import MistaggedHeadingError
from bloom_nofos.s3.utils import (
get_image_url_from_s3,
remove_file_from_s3,
@@ -385,32 +386,17 @@ def _get_document_field_name(SectionModel, document):
# NOFO Section has "nofo", Compare docs have "document"
return "nofo" if hasattr(SectionModel, "nofo") else "document"
- def _get_validation_message(validation_error, obj):
- obj_type = obj._meta.verbose_name.title()
- name_max_length = obj._meta.get_field("name").max_length
+ def _raise_document_validation_error(validation_error, obj, heading_kind):
+ name_errors = validation_error.error_dict.get("name", [])
+ if any(error.code == "max_length" for error in name_errors):
+ raise MistaggedHeadingError(
+ heading_kind=heading_kind,
+ heading_order=obj.order,
+ heading_text=obj.name,
+ max_length=obj._meta.get_field("name").max_length,
+ ) from validation_error
- if validation_error.message_dict.get("name", []):
- intro_message = (
- f"Found a {obj_type} name exceeding {name_max_length} characters in length. "
- "This often means a paragraph was incorrectly styled as a heading.\n\n"
- )
- error_message = f"- **Error message**: {validation_error.messages}\n"
- object_type_message = f"- **Type**: {obj_type}\n"
- object_order_message = f"- **{obj_type} order**: {obj.order}\n"
- object_name_message = f"- **{obj_type} name**: {obj.name}\n\n"
- outro_message = f"Note that there may also be other mistagged headings further down in this document."
-
- return (
- f"{intro_message}"
- f"{error_message}"
- f"{object_type_message}"
- f"{object_order_message}"
- f"{object_name_message}"
- f"{outro_message}"
- )
-
- # Generic fallback if it's not a name-related length error
- return str(validation_error)
+ raise ValidationError(str(validation_error)) from validation_error
sections_to_create = []
subsections_to_create = []
@@ -433,7 +419,7 @@ def _get_validation_message(validation_error, obj):
try:
section_obj.full_clean()
except ValidationError as e:
- raise ValidationError(_get_validation_message(e, section_obj)) from e
+ _raise_document_validation_error(e, section_obj, "section")
sections_to_create.append(section_obj)
@@ -512,7 +498,7 @@ def _get_validation_message(validation_error, obj):
try:
subsection_obj.full_clean()
except ValidationError as e:
- raise ValidationError(_get_validation_message(e, subsection_obj)) from e
+ _raise_document_validation_error(e, subsection_obj, "subsection")
subsections_to_create.append(subsection_obj)
diff --git a/nofos/nofos/tests_nofos/test_import.py b/nofos/nofos/tests_nofos/test_import.py
index 5058444d..5e1d05d9 100644
--- a/nofos/nofos/tests_nofos/test_import.py
+++ b/nofos/nofos/tests_nofos/test_import.py
@@ -11,7 +11,7 @@
from django.urls import reverse
from users.models import BloomUser
-from nofos.models import Nofo
+from nofos.models import Nofo, Section, Subsection
from nofos.nofo import (
get_sections_from_soup,
get_subsections_from_sections,
@@ -505,6 +505,13 @@ def setUp(self):
"docx",
"lists--mammoth-warning.docx",
)
+ self.docx_mistagged_heading_fixture_path = os.path.join(
+ settings.BASE_DIR,
+ "nofos",
+ "fixtures",
+ "docx",
+ "mistagged-paragraph-heading.docx",
+ )
def test_strict_mode_warning_is_actionable_and_hides_converter_details(self):
with open(self.docx_warning_fixture_path, "rb") as f:
@@ -527,6 +534,138 @@ def test_strict_mode_warning_is_actionable_and_hides_converter_details(self):
self.assertNotIn("Style ID", content)
self.assertNotIn("Paulsundocumentedstyle", content)
+ def test_long_section_heading_identifies_mistagged_text(self):
+ affected_text = "A" * 251
+ uploaded_file = SimpleUploadedFile(
+ "long-section.html",
+ (
+ "Opportunity name: Test NOFO
"
+ "Opdiv: CDC
"
+ f"{affected_text}
"
+ "Valid subsection
Body
"
+ ).encode("utf-8"),
+ content_type="text/html",
+ )
+
+ response = self.client.post(self.import_url, {"nofo-import": uploaded_file})
+
+ content = response.content.decode("utf-8")
+ self.assertEqual(response.status_code, 422)
+ self.assertIn("IMPORT-HEADING-TOO-LONG", content)
+ self.assertIn("Section heading 1", content)
+ self.assertIn("Heading character limit", content)
+ self.assertIn("251", content)
+ self.assertIn(affected_text, content)
+ self.assertNotIn("IMPORT-CREATE-INVALID", content)
+
+ def test_long_subsection_heading_is_safely_escaped(self):
+ affected_text = ("B" * 401) + ""
+ uploaded_file = SimpleUploadedFile(
+ "long-subsection.html",
+ (
+ "Opportunity name: Test NOFO
"
+ "Opdiv: CDC
"
+ "Valid section
"
+ f"{affected_text.replace('<', '<').replace('>', '>')}
"
+ "Body
"
+ ).encode("utf-8"),
+ content_type="text/html",
+ )
+
+ response = self.client.post(self.import_url, {"nofo-import": uploaded_file})
+
+ content = response.content.decode("utf-8")
+ self.assertEqual(response.status_code, 422)
+ self.assertIn("IMPORT-HEADING-TOO-LONG", content)
+ self.assertIn("Subsection heading 1", content)
+ self.assertIn("400", content)
+ self.assertIn("<script>", content)
+ self.assertNotIn("