Skip to content
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
69 changes: 69 additions & 0 deletions nofos/bloom_nofos/error_helpers.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from django.core.exceptions import ValidationError
from django.shortcuts import render

DOCUMENT_STRUCTURE_RECOVERY_STEPS = (
Expand All @@ -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,
*,
Expand All @@ -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(
Expand All @@ -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(
Expand Down
9 changes: 9 additions & 0 deletions nofos/bloom_nofos/templates/import_error.html
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@
<h1 class="font-heading-xl margin-y-0">{{ error_title }}</h1>
<p>{{ error_summary }}</p>

{% if error_details %}
<dl>
{% for detail in error_details %}
<dt class="text-bold">{{ detail.label }}</dt>
<dd class="margin-left-0 margin-bottom-2">{{ detail.value }}</dd>
{% endfor %}
</dl>
{% endif %}

{% if recovery_steps %}
<p>What to do next:</p>
<ol class="usa-list">
Expand Down
60 changes: 60 additions & 0 deletions nofos/compare/test_views.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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):
Expand Down
33 changes: 33 additions & 0 deletions nofos/compare/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
33 changes: 33 additions & 0 deletions nofos/composer/tests/test_views.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import os
import uuid
from unittest.mock import patch

Expand All @@ -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
Expand All @@ -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",
Expand Down Expand Up @@ -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):
Expand Down
18 changes: 18 additions & 0 deletions nofos/composer/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
Binary file not shown.
Loading
Loading