Merge pull request #5653 from HHS:OPS-1639/budget-team-requisition-review-page

Ops 1639/budget team requisition review page

Author: josbell

backend/ops_api/ops/services/procurement_tracker_steps.py

@@ -459,10 +459,10 @@ def _handle_approval_notifications(
                 # DD declined - notify requester (existing behavior)
                 self._notify_requester_of_decline(step, agreement, current_user, notification_service)
 
-            # Auto-dismiss "in review" notifications for reviewers via bulk UPDATE
-            self._dismiss_notifications_by_title(
-                PreAwardNotificationTitle.APPROVAL_REQUEST, step.id, "'in review' notifications for reviewers"
-            )
+                # Only dismiss "in review" notification when DD declines (requester needs to see it until budget team approves)
+                self._dismiss_notifications_by_title(
+                    PreAwardNotificationTitle.APPROVAL_REQUEST, step.id, "'in review' notifications after decline"
+                )
 
         # Case 3: Budget Team Requisition Approval (OPS-1639)
         # Only send if requisition_approved_by changed from None → {user_id}
@@ -475,10 +475,17 @@ def _handle_approval_notifications(
             # Budget team approved requisition - notify original requester
             if step.pre_award_approval_requested_by:
                 message = (
-                    f"{current_user.full_name} has approved the Pre-Award Requisition for Agreement {agreement.display_name}. "
-                    f"The Final Consensus Memo can now be sent to the Procurement Shop."
+                    "This agreement has been approved for Pre-Award. Please send the Final Consensus Memo to the "
+                    "Procurement Shop and continue your progress in the Procurement Tracker."
                 )
 
+                # Include Director's approval notes if they exist
+                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()
+                    # Escape Markdown metacharacters to ensure notes display literally
+                    reviewer_notes_text = escape_markdown(reviewer_notes_text)
+                    message += f"\n\nNotes: {reviewer_notes_text}"
+
                 notification_service.create(
                     {
                         "title": PreAwardNotificationTitle.REQUISITION_APPROVED,
@@ -499,6 +506,13 @@ def _handle_approval_notifications(
                 "budget team requisition review notifications",
             )
 
+            # Also dismiss the original "in review" notification for requester (workflow complete)
+            self._dismiss_notifications_by_title(
+                PreAwardNotificationTitle.APPROVAL_REQUEST,
+                step.id,
+                "'in review' notification for requester after budget team approval",
+            )
+
     def _notify_budget_team_for_requisition_review(self, step, agreement, current_user, notification_service):
         """Notify budget team members that DD has approved and requisition review is required."""
         from sqlalchemy import select
@@ -514,9 +528,13 @@ def _notify_budget_team_for_requisition_review(self, step, agreement, current_us
         # TODO (PR3/PR4): This URL points to budget requisition review page to be implemented
         review_url = f"{fe_url}/agreements/{agreement.id}/review-budget-requisition"
 
+        # Fetch requester user object to include their name in the notification
+        requester = self.db_session.get(User, step.pre_award_approval_requested_by)
+        requester_name = requester.full_name if requester else "Unknown User"
+
         message = (
-            f"{current_user.full_name} has approved the Pre-Award request for Agreement {agreement.display_name}. "
-            f"Budget Team review and requisition entry is now required.\n\n[Review Agreement]({review_url})"
+            f"{current_user.full_name} approved the agreement for pre-award as requested by {requester_name} "
+            f"and it is now ready for the Budget Team to submit the requisition.\n\n[Review Agreement]({review_url})"
         )
 
         # Send notification to each budget team member
@@ -540,8 +558,9 @@ def _notify_requester_of_decline(self, step, agreement, current_user, notificati
 
         if step.pre_award_approval_requested_by:
             message = (
-                f"Your pre-award approval request for Agreement {agreement.display_name} "
-                f"has been declined by {current_user.full_name}."
+                "This agreement has been declined for Pre-Award. "
+                "Please do not upload the Final Consensus Memo to the HHS Consolidated Acquisition Solution (HCAS) "
+                "until changes have been made and re-submitted for approval."
             )
             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()
@@ -568,8 +587,9 @@ def _get_approval_reviewers(self, agreement) -> set[int]:
         """
         Get user IDs who can approve pre-award requests.
 
-        Returns IDs of Division Directors, Deputy Division Directors,
-        Budget Team, and System Owners.
+        Returns IDs of Division Directors, Deputy Division Directors, and System Owners.
+        NOTE: BUDGET_TEAM is explicitly excluded here - they are notified separately
+        AFTER director approval (see _notify_budget_team_for_requisition_review).
 
         Args:
             agreement: The agreement to get reviewers for
@@ -587,13 +607,9 @@ def _get_approval_reviewers(self, agreement) -> set[int]:
         reviewer_ids.update(directors)
         reviewer_ids.update(deputies)
 
-        # Get BUDGET_TEAM and SYSTEM_OWNER users
+        # Get SYSTEM_OWNER users (BUDGET_TEAM excluded - they are notified after DD approval)
         role_based_users = (
-            self.db_session.execute(
-                select(User.id).where(User.roles.any(Role.name.in_(["BUDGET_TEAM", "SYSTEM_OWNER"])))
-            )
-            .scalars()
-            .all()
+            self.db_session.execute(select(User.id).where(User.roles.any(Role.name == "SYSTEM_OWNER"))).scalars().all()
         )
         reviewer_ids.update(role_based_users)
 
@@ -750,8 +766,9 @@ def get_pending_approvals_for_user(self, user_id: int) -> list[ProcurementTracke
             )
         )
 
-        # If user is BUDGET_TEAM or SYSTEM_OWNER, they can see all pending approvals
-        if "BUDGET_TEAM" in user_role_names or "SYSTEM_OWNER" in user_role_names:
+        # If user is SYSTEM_OWNER, they can see all pending approvals
+        # NOTE: BUDGET_TEAM is excluded here - they use get_pending_requisitions_for_user() instead
+        if "SYSTEM_OWNER" in user_role_names:
             results = self.db_session.execute(stmt.distinct()).scalars().all()
             return list(results)
 

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

@@ -243,7 +243,9 @@ def test_dd_approval_notification_has_correct_title_and_message(
         # Verify notification content
         for notification in budget_team_notifications:
             assert notification.title == "Budget Team Requisition Review Required"
-            assert "Budget Team review and requisition entry is now required" in notification.message
+            # Updated message includes both approver and requester names
+            assert "approved the agreement for pre-award as requested by" in notification.message
+            assert "ready for the Budget Team to submit the requisition" in notification.message
             assert notification.procurement_tracker_step_id == test_pre_award_step.id
 
     def test_dd_decline_still_notifies_requester(self, auth_client, test_pre_award_step, loaded_db):
@@ -353,8 +355,8 @@ def test_budget_team_approval_notification_has_correct_content(
 
         # Verify notification content
         assert notification.title == "Pre-Award Requisition Approved"
-        assert "approved the Pre-Award Requisition" in notification.message
-        assert "Final Consensus Memo can now be sent" in notification.message
+        assert "This agreement has been approved for Pre-Award" in notification.message
+        assert "send the Final Consensus Memo to the Procurement Shop" in notification.message
         assert notification.procurement_tracker_step_id == test_pre_award_step.id
 
     def test_budget_team_approval_auto_dismisses_review_notifications(

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

@@ -215,8 +215,14 @@ def test_approval_transitions_are_idempotent(auth_client, test_pre_award_step, l
 
 
 def test_approval_response_auto_dismisses_in_review_notifications(auth_client, test_pre_award_step, loaded_db):
-    """Test that approval response auto-dismisses 'in review' notifications for reviewers."""
-    # Step 1: Request approval - creates "in review" notifications for reviewers
+    """Test that approval response auto-dismisses 'in review' notifications for reviewers.
+
+    NOTE: OPS-1639 changed the flow so DD approval sends to Budget Team, not auto-dismisses reviewer notifications.
+    Reviewer notifications are only created for Division Directors and SYSTEM_OWNER users (Budget Team excluded).
+    When DD approves, new notifications go to Budget Team, but the original 'Pre-Award Approval Request'
+    notifications to DDs remain unread (they are not auto-dismissed).
+    """
+    # Step 1: Request approval - creates "in review" notifications for reviewers (DD/SYSTEM_OWNER only)
     update_data = {
         "approval_requested": True,
         "approval_requested_date": date.today().isoformat(),
@@ -228,7 +234,7 @@ def test_approval_response_auto_dismisses_in_review_notifications(auth_client, t
     )
     assert response.status_code == 200
 
-    # Verify "in review" notifications were created
+    # Verify "in review" notifications were created (for DD/SYSTEM_OWNER, not Budget Team)
     in_review_notifications = loaded_db.scalars(
         select(Notification)
         .where(Notification.title == "Pre-Award Approval Request")
@@ -239,7 +245,7 @@ def test_approval_response_auto_dismisses_in_review_notifications(auth_client, t
     # Store the notification IDs to verify later
     reviewer_notification_ids = [n.id for n in in_review_notifications]
 
-    # Step 2: Approve request - should auto-dismiss reviewer notifications
+    # Step 2: Approve request - sends notification to Budget Team (not auto-dismiss)
     update_data = {
         "approval_status": "APPROVED",
         "reviewer_notes": "Looks good, approved",
@@ -250,10 +256,14 @@ def test_approval_response_auto_dismisses_in_review_notifications(auth_client, t
     )
     assert response.status_code == 200
 
-    # Step 3: Verify all "in review" notifications are marked as read
+    # Step 3: Verify "Pre-Award Approval Request" notifications remain (NOT auto-dismissed)
+    # OPS-1639: DD approval creates NEW notifications for Budget Team but does NOT dismiss original DD notifications
     for notification_id in reviewer_notification_ids:
         notification = loaded_db.get(Notification, notification_id)
-        assert notification.is_read, f"Notification {notification_id} should be auto-dismissed (marked as read)"
+        # These notifications should remain unread - they are NOT auto-dismissed in OPS-1639 flow
+        assert (
+            not notification.is_read
+        ), f"Notification {notification_id} should remain unread (OPS-1639: DD approval sends to Budget Team, does not auto-dismiss DD notifications)"
 
 
 def test_approval_response_includes_reviewer_notes_in_notification(auth_client, test_pre_award_step, loaded_db):
@@ -363,8 +373,8 @@ def test_approval_response_excludes_empty_reviewer_notes(auth_client, test_pre_a
     assert (
         "Notes:" not in notification.message
     ), f"Notes section should not appear when empty. Got: {notification.message}"
-    # Verify base message is present (budget team notification says "has approved" not "has been approved")
-    assert "has approved" in notification.message
+    # Verify base message is present (OPS-1639: budget team notification includes approver and requester names)
+    assert "approved the agreement for pre-award as requested by" in notification.message
 
 
 def test_decline_response_excludes_whitespace_only_reviewer_notes(auth_client, test_pre_award_step, loaded_db):

frontend/src/components/Agreements/AgreementBudgetLinesHeader.jsx

@@ -1,5 +1,6 @@
 import { faPen, faToggleOff, faToggleOn } from "@fortawesome/free-solid-svg-icons";
 import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
+import Tooltip from "../UI/USWDS/Tooltip";
 
 /**
  * @component - Agreement detail header.
@@ -11,6 +12,7 @@ import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
  * @param {boolean} props.isEditable - Whether the agreement is editable.
  * @param {boolean} [props.isEditMode] - Whether the edit mode is on.
  * @param {Function} [props.setIsEditMode] - The function to set the edit mode.
+ * @param {boolean} [props.isPreAwardInReview] - Whether pre-award approval is in review.
  * @returns {React.ReactElement} - The rendered component.
  */
 export const AgreementBudgetLinesHeader = ({
@@ -20,7 +22,8 @@ export const AgreementBudgetLinesHeader = ({
     setIncludeDrafts,
     isEditable,
     isEditMode = false,
-    setIsEditMode = () => {}
+    setIsEditMode = () => {},
+    isPreAwardInReview = false
 }) => {
     return (
         <>
@@ -48,7 +51,8 @@ export const AgreementBudgetLinesHeader = ({
                         <span className="text-primary">Include Drafts</span>
                     </button>
 
-                    {!isEditMode && isEditable && (
+                    {/* ENABLED EDIT BUTTON */}
+                    {!isEditMode && isEditable && !isPreAwardInReview && (
                         <button
                             type="button"
                             id="edit"
@@ -65,6 +69,27 @@ export const AgreementBudgetLinesHeader = ({
                             <span className="text-primary">Edit</span>
                         </button>
                     )}
+                    {/* DISABLED EDIT BUTTON */}
+                    {!isEditMode && isEditable && isPreAwardInReview && (
+                        <Tooltip label="This agreement is In Review for Pre-Award Approval. Edits or changes cannot be made at this time.">
+                            <span
+                                id="edit-disabled"
+                                className="usa-button--unstyled usa-button--disabled"
+                                aria-disabled="true"
+                                data-cy="edit-disabled"
+                                tabIndex={0}
+                                role="button"
+                            >
+                                <FontAwesomeIcon
+                                    icon={faPen}
+                                    size="2x"
+                                    className="height-2 width-2 margin-right-1"
+                                    aria-hidden="true"
+                                />
+                                <span>Edit</span>
+                            </span>
+                        </Tooltip>
+                    )}
                 </div>
             </div>
             {details && <p className="font-sans-sm">{details}</p>}

frontend/src/components/Agreements/AgreementDetailHeader.jsx

@@ -1,5 +1,6 @@
 import { faPen, faWarning } from "@fortawesome/free-solid-svg-icons";
 import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
+import Tooltip from "../UI/USWDS/Tooltip";
 
 /**
  * @component - Agreement detail header.
@@ -10,6 +11,7 @@ import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
  * @param {function} props.setIsEditMode - The function to set the edit mode.
  * @param {boolean} props.isEditable - Whether the agreement is editable.
  * @param {boolean} props.hasUnsavedChanges - Whether there are unsaved changes.
+ * @param {boolean} [props.isPreAwardInReview] - Whether pre-award approval is in review.
  * @returns {JSX.Element} - The rendered component.
  */
 export const AgreementDetailHeader = ({
@@ -18,7 +20,8 @@ export const AgreementDetailHeader = ({
     isEditMode,
     setIsEditMode,
     isEditable,
-    hasUnsavedChanges = false
+    hasUnsavedChanges = false,
+    isPreAwardInReview = false
 }) => {
     return (
         <>
@@ -36,7 +39,8 @@ export const AgreementDetailHeader = ({
                         Unsaved Changes
                     </div>
                 )}
-                {!isEditMode && isEditable && (
+                {/* ENABLED EDIT BUTTON - when not in edit mode, is editable, and NOT in pre-award review */}
+                {!isEditMode && isEditable && !isPreAwardInReview && (
                     <button
                         type="button"
                         id="edit"
@@ -53,6 +57,26 @@ export const AgreementDetailHeader = ({
                         <span className="text-primary">Edit</span>
                     </button>
                 )}
+                {/* DISABLED EDIT BUTTON - when in pre-award review */}
+                {!isEditMode && isEditable && isPreAwardInReview && (
+                    <Tooltip label="This agreement is In Review for Pre-Award Approval. Edits or changes cannot be made at this time.">
+                        <span
+                            id="edit-disabled"
+                            className="usa-button--unstyled usa-button--disabled display-flex flex-align-center"
+                            aria-disabled="true"
+                            data-cy="edit-disabled"
+                            tabIndex={0}
+                            role="button"
+                        >
+                            <FontAwesomeIcon
+                                icon={faPen}
+                                className="height-2 width-2 margin-right-1"
+                                aria-hidden="true"
+                            />
+                            <span>Edit</span>
+                        </span>
+                    </Tooltip>
+                )}
                 {isEditMode && (
                     <div className="margin-left-auto">
                         <FontAwesomeIcon

frontend/src/components/Agreements/PreAwardApprovalAlert/PreAwardApprovalAlert.test.jsx

@@ -30,7 +30,8 @@ const declinedNotification = {
     id: 2,
     notification_type: "PRE_AWARD_APPROVAL_NOTIFICATION",
     title: "Pre-Award Approval Response",
-    message: "Your pre-award approval request has been declined by Jane Smith.",
+    message:
+        "This agreement has been declined for Pre-Award. Please do not upload the Final Consensus Memo to the HHS Consolidated Acquisition Solution (HCAS) until changes have been made and re-submitted for approval.",
     is_read: false,
     procurement_tracker_step: {
         id: 5,
@@ -100,7 +101,7 @@ describe("PreAwardApprovalAlert", () => {
         );
 
         expect(screen.getByText("Pre-Award Approval Response")).toBeInTheDocument();
-        expect(screen.getByText(/Your pre-award approval request has been declined by Jane Smith/)).toBeInTheDocument();
+        expect(screen.getByText(/This agreement has been declined for Pre-Award/)).toBeInTheDocument();
     });
 
     it("should NOT render request alert (only shows Approved/Declined)", () => {
@@ -213,6 +214,6 @@ describe("PreAwardApprovalAlert", () => {
 
         expect(screen.getAllByText("Pre-Award Approval Response")).toHaveLength(2);
         expect(screen.getByText(/Your pre-award approval request has been approved by John Doe/)).toBeInTheDocument();
-        expect(screen.getByText(/Your pre-award approval request has been declined by Jane Smith/)).toBeInTheDocument();
+        expect(screen.getByText(/This agreement has been declined for Pre-Award/)).toBeInTheDocument();
     });
 });