Merge pull request #5580 from HHS/procurement-tracker-ui-fixes

Procurement Tracker UI Minor Fixes

Author: josbell

backend/ops_api/Pipfile

@@ -24,7 +24,7 @@ PyYAML = "==6.0.3"
 requests = "==2.33.1"
 sqlalchemy = "==2.0.49"
 sqlalchemy-continuum = "==1.6.0"
-deepdiff = "*"
+deepdiff = "==9.0.0"
 
 [dev-packages]
 black = {extras = ["d"], version = "==26.3.1"}

backend/ops_api/Pipfile.lock

@@ -26,6 +26,34 @@
 from ops_api.ops.validation.procurement_tracker_steps_validator import ProcurementTrackerStepsValidator
 
 
+def escape_markdown(text: str) -> str:
+    """
+    Escape Markdown metacharacters for safe plain-text display.
+
+    Ensures user-supplied text is displayed literally in both plain-text
+    and ReactMarkdown contexts, preventing unintended formatting or links.
+
+    Args:
+        text: The text to escape
+
+    Returns:
+        Text with Markdown metacharacters escaped
+    """
+    # Escape backslash first to prevent double-escaping
+    text = text.replace("\\", "\\\\")
+    # Escape Markdown syntax characters
+    text = text.replace("*", "\\*")  # Bold/italic
+    text = text.replace("_", "\\_")  # Italic/bold
+    text = text.replace("[", "\\[")  # Links
+    text = text.replace("]", "\\]")
+    text = text.replace("`", "\\`")  # Code
+    text = text.replace("#", "\\#")  # Headers
+    text = text.replace("+", "\\+")  # Lists
+    text = text.replace("-", "\\-")  # Lists
+    text = text.replace("!", "\\!")  # Images
+    return text
+
+
 class ProcurementTrackerStepService:
     """Service for procurement tracker step operations."""
 
@@ -362,8 +390,11 @@ def _handle_approval_notifications(
                 )
                 if step.pre_award_approval_reviewer_notes and step.pre_award_approval_reviewer_notes.strip():
                     reviewer_notes_text = step.pre_award_approval_reviewer_notes.strip()
-                    # Use 5 backticks to safely contain any triple-backtick sequences in notes
-                    message += f"\n\nNotes:\n`````\n{reviewer_notes_text}\n`````"
+                    # Escape Markdown metacharacters to ensure notes display literally in both
+                    # SimpleAlert (plain text) and NotificationCenter/LogItem (ReactMarkdown).
+                    # Prevents user-supplied markdown from rendering as formatting or clickable links.
+                    reviewer_notes_text = escape_markdown(reviewer_notes_text)
+                    message += f"\n\nNotes:\n{reviewer_notes_text}"
 
                 notification_service.create(
                     {

backend/ops_api/ops/services/procurement_tracker_steps.py

@@ -275,7 +275,6 @@ def test_approval_response_includes_reviewer_notes_in_notification(auth_client,
     assert notification is not None, "Notification should be created for approval response"
     assert reviewer_notes in notification.message, f"Reviewer notes should be in message. Got: {notification.message}"
     assert "Notes:" in notification.message, "Message should include 'Notes:' label"
-    assert "```" in notification.message, "Notes should be wrapped in code block"
 
 
 def test_decline_response_includes_reviewer_notes_in_notification(auth_client, test_pre_award_step, loaded_db):
@@ -306,7 +305,6 @@ def test_decline_response_includes_reviewer_notes_in_notification(auth_client, t
     assert notification is not None, "Notification should be created for decline response"
     assert reviewer_notes in notification.message, f"Reviewer notes should be in message. Got: {notification.message}"
     assert "Notes:" in notification.message, "Message should include 'Notes:' label"
-    assert "```" in notification.message, "Notes should be wrapped in code block"
 
 
 def test_approval_response_excludes_empty_reviewer_notes(auth_client, test_pre_award_step, loaded_db):
@@ -398,12 +396,12 @@ def test_reviewer_notes_prevent_markdown_injection(auth_client, test_pre_award_s
     ).first()
 
     assert notification is not None, "Notification should be created"
-    # Verify notes are wrapped in 5-backtick code block (prevents Markdown rendering)
-    assert "`````" in notification.message, "Notes should be wrapped in 5-backtick code block"
-    # Verify the raw Markdown syntax is preserved as plain text
-    assert "**Bold**" in notification.message, "Markdown syntax should be preserved literally"
-    assert "[link]" in notification.message, "Link syntax should be preserved literally"
-    assert "```code```" in notification.message, "Triple backticks should be preserved literally"
+    # Verify notes are included in the message
+    assert "Notes:" in notification.message, "Message should include 'Notes:' label"
+    # Verify the Markdown syntax is escaped to prevent rendering
+    assert "\\*\\*Bold\\*\\*" in notification.message, "Asterisks should be escaped"
+    assert "\\[link\\]" in notification.message, "Brackets should be escaped"
+    assert "\\`\\`\\`code\\`\\`\\`" in notification.message, "Backticks should be escaped"
 
 
 def test_reviewer_notes_backtick_injection_prevented(auth_client, test_pre_award_step, loaded_db):
@@ -432,9 +430,9 @@ def test_reviewer_notes_backtick_injection_prevented(auth_client, test_pre_award
     ).first()
 
     assert notification is not None, "Notification should be created"
-    # Verify 5-backtick fence is used
-    assert notification.message.count("`````") == 2, "Should have opening and closing 5-backtick fences"
-    # Verify triple backticks are contained within the fence (appear in raw form)
-    assert "```" in notification.message, "Triple backticks should be preserved"
-    # Verify the markdown after triple backticks is also preserved literally
-    assert "**This should NOT render as bold**" in notification.message, "Markdown after backticks should be literal"
+    # Verify notes are included in the message
+    assert "Notes:" in notification.message, "Message should include 'Notes:' label"
+    # Verify triple backticks are escaped
+    assert "\\`\\`\\`" in notification.message, "Triple backticks should be escaped"
+    # Verify the markdown after triple backticks is also escaped
+    assert "\\*\\*This should NOT render as bold\\*\\*" in notification.message, "Asterisks should be escaped"

backend/ops_api/tests/ops/procurement_tracker/test_procurement_tracker_steps_duplicate_notifications.py

@@ -446,6 +446,8 @@ describe("Awarded Agreement", () => {
         cy.get("#agreementNotes").clear();
         cy.get("#agreementNotes").type("Adding notes as agreement team member.");
         cy.get("[data-cy='continue-btn']").click();
+        // wait for page to finish loading after save
+        cy.url().should("not.include", "mode=edit");
         // verify notes are added
         cy.get("[data-cy='details-notes']").should("contain", "Adding notes as agreement team member.");
         checkAgreementHistory();
@@ -475,6 +477,8 @@ describe("Awarded Agreement", () => {
         cy.get("#agreementNotes").clear();
         cy.get("#agreementNotes").type("Adding notes as agreement power user.");
         cy.get("[data-cy='continue-btn']").click();
+        // wait for page to finish loading after save
+        cy.url().should("not.include", "mode=edit");
         // verify notes are added
         cy.get("[data-cy='details-notes']").should("contain", "Adding notes as agreement power user.");
         checkAgreementHistory();

frontend/cypress/e2e/agreementDetailsEdit.cy.js

@@ -1,3 +1,4 @@
+import React from "react";
 import Accordion from "../../../UI/Accordion";
 import { fromUpperCaseToTitleCase } from "../../../../helpers/utils";
 import { PROCUREMENT_STEP_STATUS } from "../ProcurementTracker.constants";
@@ -63,43 +64,41 @@ const getStepState = (step, activeStepNumber) => {
 
 /**
  * @param {StepBuilderAccordionProps} props
+ * @param {React.Ref} ref - Forwarded ref to the accordion container
  * @returns {import("react").ReactElement}
  */
-const StepBuilderAccordion = ({
-    step,
-    totalSteps,
-    activeStepNumber,
-    children,
-    isClosed = false,
-    isReadOnly = false,
-    level = 3
-}) => {
-    const stepState = getStepState(step, activeStepNumber);
-    const readOnlyClassName = isReadOnly ? "step-builder-accordion__heading--read-only" : "";
+const StepBuilderAccordion = React.forwardRef(
+    ({ step, totalSteps, activeStepNumber, children, isClosed = false, isReadOnly = false, level = 3 }, ref) => {
+        const stepState = getStepState(step, activeStepNumber);
+        const readOnlyClassName = isReadOnly ? "step-builder-accordion__heading--read-only" : "";
 
-    const heading = (
-        <div
-            className={`step-builder-accordion__heading step-builder-accordion__heading--${stepState} ${readOnlyClassName}`}
-            data-testid={`step-builder-heading-${step?.id}`}
-        >
-            <span className="step-builder-accordion__step-count">
-                <span className="step-builder-accordion__step-number">{step?.step_number}</span>{" "}
-                <span className="step-builder-accordion__step-total">of {totalSteps}</span>
-            </span>{" "}
-            <span className="step-builder-accordion__step-label">{formatStepLabel(step?.step_type)}</span>
-        </div>
-    );
+        const heading = (
+            <div
+                className={`step-builder-accordion__heading step-builder-accordion__heading--${stepState} ${readOnlyClassName}`}
+                data-testid={`step-builder-heading-${step?.id}`}
+            >
+                <span className="step-builder-accordion__step-count">
+                    <span className="step-builder-accordion__step-number">{step?.step_number}</span>{" "}
+                    <span className="step-builder-accordion__step-total">of {totalSteps}</span>
+                </span>{" "}
+                <span className="step-builder-accordion__step-label">{formatStepLabel(step?.step_type)}</span>
+            </div>
+        );
 
-    return (
-        <Accordion
-            heading={heading}
-            dataCy={`step-builder-accordion-${step?.id}`}
-            isClosed={isClosed}
-            level={level}
-        >
-            {children}
-        </Accordion>
-    );
-};
+        return (
+            <Accordion
+                ref={ref}
+                heading={heading}
+                dataCy={`step-builder-accordion-${step?.id}`}
+                isClosed={isClosed}
+                level={level}
+            >
+                {children}
+            </Accordion>
+        );
+    }
+);
+
+StepBuilderAccordion.displayName = "StepBuilderAccordion";
 
 export default StepBuilderAccordion;

frontend/src/components/Agreements/ProcurementTracker/StepBuilderAccordion/StepBuilderAccordion.jsx

@@ -19,9 +19,10 @@ import accordion from "@uswds/uswds/js/usa-accordion";
  * @param {string} [props.id] - Optional id for anchor linking.
  * @param {string} [props.dataCy] - Data attribute for testing.
  * @param {(isOpen: boolean) => void} [props.onToggle] - Optional callback fired on toggle.
+ * @param {React.Ref} ref - Forwarded ref to the accordion container div.
  * @returns {JSX.Element} - The rendered accordion component.
  */
-const Accordion = ({ heading, children, level = 4, isClosed = false, id, dataCy, onToggle }) => {
+const Accordion = React.forwardRef(({ heading, children, level = 4, isClosed = false, id, dataCy, onToggle }, ref) => {
     const accordionId = React.useId();
     const AccordionHeading = `h${level}`;
     const [isOpen, setIsOpen] = React.useState(!isClosed);
@@ -57,7 +58,14 @@ const Accordion = ({ heading, children, level = 4, isClosed = false, id, dataCy,
             id={id}
             className={`usa-accordion ${isOpen ? "padding-bottom-6" : ""}`}
             style={{ lineHeight: "inherit" }}
-            ref={accordionRef}
+            ref={(node) => {
+                accordionRef.current = node;
+                if (typeof ref === "function") {
+                    ref(node);
+                } else if (ref) {
+                    ref.current = node;
+                }
+            }}
             data-cy={dataCy}
         >
             <AccordionHeading className="usa-accordion__heading">
@@ -86,6 +94,8 @@ const Accordion = ({ heading, children, level = 4, isClosed = false, id, dataCy,
             </div>
         </div>
     );
-};
+});
+
+Accordion.displayName = "Accordion";
 
 export default Accordion;