OPS 1518/pre award approval approver side

[josbell]

What changed

Implemented the approver side of the pre-award approval workflow for procurement tracker step 5. Division Directors, Budget Team members, and System Owners can now review and respond to pre-award approval requests submitted by agreement managers.

Backend:

• Added database schema fields for approval responses: approval_status, approval_responded_by, approval_responded_date, approval_reviewer_notes
• Added API schemas with validation (status must be APPROVED/DECLINED, notes max 150 chars)
• Implemented service layer logic with server-controlled fields
• Added validation rules: authorization check and business logic (can't respond without request, can't respond twice, notes required when declining)
• Built notification system: notifies reviewers when approval requested, notifies submitter when approved/declined
• Added event/history tracking for all approval state transitions

Frontend:

• Created /agreements/:id/review-pre-award route and page component
• Built approver review UI with agreement details, budget lines (grouped by services component), CAN impact review, and Final Consensus Memo documents
• Implemented notes section (read-only submitter notes + editable reviewer notes)
• Added confirmation modals for approve/decline actions
• Integrated permission checks (BUDGET_TEAM, SYSTEM_OWNER, or REVIEWER_APPROVER roles)
• Added alerts for already-processed requests and error handling
• Created shared `PreAwardBudgetLinesReviewAccordion` component
• Created shared `usePreAwardApprovalData` hook for data fetching

Additional improvements:

• Updated requester page to use shared components and data hook
• Enhanced change request list to show pre-award approval requests
• Added pre-award review card component for change requests view
• Fixed duplicate notification bug in procurement tracker steps
• Updated SimpleAlert to support line breaks in messages

Issue

Closes #1518

How to test

Prerequisites

1. Start Docker stack: `docker compose up --build`
2. Ensure you have a user with Division Director, Budget Team, or System Owner role

Test Steps

Figma for Procurement Tracker Step 5 Requester/ Approver

As the Requester:

1. Navigate to agreement with at least one BLI in executing status (13 or 14 in dev)
2. Complete Procurement Tracker steps up to step 5
3. Click on Request Pre-Award Approval BTN
4. Verify form looks as illustrated in Figma
5. Complete form and submit

As the Approver (Division Director/Budget Team/System Owner):

6. Navigate to the Change Requests list or check notifications for a pre-award approval request
7. Click the notification or request link to navigate to `/agreements/:id/review-pre-award`
8. Verify the page loads with:
   • Agreement metadata accordion
   • Budget lines grouped by services component (all BLI statuses included)
   • Review Executing Total section showing summary
   • CAN Impact section showing all budget lines per UX requirements
   • Review Final Consensus Memo section (if documents uploaded in step 5)
   • Notes section with read-only Submitter's Notes and editable Reviewer's Notes textarea
9. Test Decline flow:
   • Leave Reviewer's Notes blank and click "Decline"
   • Verify validation error appears (notes required)
   • Enter notes (max 150 characters)
   • Click "Decline" and confirm in modal
   • Verify success message appears and redirect to /agreements
10. Test Approve flow:

• Return to the review page for a different agreement
• Optionally enter Reviewer's Notes
• Click "Approve" and confirm in modal
• Verify success message appears and redirect to /agreements

11. Test permissions:

• Try to access the page as a user without proper roles
• Verify access denied message appears

12. Test already processed:

• Try to access a review page for an already approved/declined request
• Verify alert appears explaining it's already been processed

As the Requester:

11. After approval/decline, check notifications
12. Navigate to the agreement's Procurement Tracker tab
13. Verify appropriate alert banner appears (green for approved, red for declined)
14. For declined requests, verify reviewer notes are displayed
15. Verify history events appear in agreement history panel

Backend Tests:
```bash
cd backend/ops_api
pytest tests/ops/procurement_tracker/test_procurement_tracker_model.py -v
pytest tests/ops/procurement_tracker/test_procurement_tracker_steps_duplicate_notifications.py -v
```

Frontend Tests:
```bash
cd frontend
bun run test --watch=false src/pages/agreements/pre-award-approval/
bun run lint
```

[Copilot]

Pull request overview

Adds the “approver side” of Step 5 (Pre-Award) approval workflow, enabling authorized reviewers to review an agreement’s executing BLIs/CAN impact, approve/decline with notes, and persist/notify outcomes via the procurement tracker step API.

Changes:

• Frontend: introduces a new review/approve page + route for /agreements/:id/review-pre-award, plus updated requester-side banner logic.
• Backend: adds PRE_AWARD approval response fields (DB/model/schema), validation rules, notifications, and agreement history events.
• Tests/migrations: adds an Alembic migration and new frontend/backend tests for the new behavior.

Reviewed changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
frontend/src/pages/agreements/pre-award-approval/RequestPreAwardApproval.jsx	Uses isApprovalPending to drive the “in review” banner.
frontend/src/pages/agreements/pre-award-approval/RequestPreAwardApproval.hooks.js	Adds approval state derivations (pending/approved) and exports isApprovalPending.
frontend/src/pages/agreements/pre-award-approval/index.js	Exports the new ApprovePreAwardApproval page.
frontend/src/pages/agreements/pre-award-approval/ApprovePreAwardApproval.jsx	New approver UI: review accordions, notes, approve/decline actions, permission gating.
frontend/src/pages/agreements/pre-award-approval/ApprovePreAwardApproval.hooks.js	New hook to fetch data, enforce permissions, and submit approve/decline updates.
frontend/src/pages/agreements/pre-award-approval/ApprovePreAwardApproval.test.jsx	Component tests for the new approver page.
frontend/src/pages/agreements/pre-award-approval/ApprovePreAwardApproval.hooks.test.js	Hook tests for permissions, filtering, and step/doc extraction.
frontend/src/index.jsx	Registers the new /agreements/:id/review-pre-award route.
backend/ops_api/tests/ops/procurement_tracker/test_procurement_tracker_model.py	Adds model tests for new response fields + serialization rules.
backend/ops_api/ops/validation/rules/procurement_tracker_step.py	Adds authorization + business validation rules for approval responses.
backend/ops_api/ops/validation/procurement_tracker_steps_validator.py	Wires new response validators into the validator set.
backend/ops_api/ops/services/procurement_tracker_steps.py	Maps new response fields, sets server-controlled response metadata, emits notifications.
backend/ops_api/ops/schemas/procurement_tracker_steps.py	Adds schema fields + patch validation for approval responses.
backend/models/procurement_tracker.py	Adds DB columns + relationship + to_dict() mapping for PRE_AWARD response fields.
backend/models/agreement_history.py	Adds agreement history events for request/approve/decline actions.
backend/alembic/versions/2026_03_23_1600-d6e7f8a9b0c1_add_pre_award_approval_response_fields.py	Migration adding PRE_AWARD response columns + FK.
backend/.claude/stories/OPS-1518-pre-award-approval-approver.md	Story/implementation notes for OPS-1518.
.gitignore	Adds .claude to ignored paths.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 47 out of 49 changed files in this pull request and generated 7 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 47 out of 51 changed files in this pull request and generated 9 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

reviewer_notes is validated with Length(max=500) here, but the PR description and UI enforce a 150 character limit. This creates an API/UI contract mismatch (clients can submit >150 chars and have it accepted). Align backend validation with the intended limit (and consider adding the same length validation on the response schema if you want strict consistency).

[josbell]

✅ Fixed in cf53878

Changed reviewer_notes validation from max=500 to max=150 characters to match the frontend textarea limit and user expectations. This ensures consistent validation between API and UI.

[Copilot]

PreAwardApprovalResponseValidationRule treats any existing pre_award_approval_status as “already responded”. Since the API schema and pending-approvals query both allow/consider "PENDING", any step persisted with status PENDING would be impossible to respond to (it would fail validation as “already been pending”). Either remove PENDING as a stored status or update this rule to treat PENDING the same as None (not yet responded).

Suggested change

	# Check if already responded
	if procurement_tracker_step.pre_award_approval_status:
	raise ValidationError(
	{
	"approval_status": f"This approval request has already been {procurement_tracker_step.pre_award_approval_status.lower()}."
	# Check if already responded. Treat PENDING the same as no response yet.
	current_approval_status = procurement_tracker_step.pre_award_approval_status
	if current_approval_status and current_approval_status != "PENDING":
	raise ValidationError(
	{
	"approval_status": f"This approval request has already been {current_approval_status.lower()}."

[josbell]

✅ Fixed in cf53878

Updated validation to only block terminal states (APPROVED/DECLINED), treating PENDING the same as None:

current_approval_status = procurement_tracker_step.pre_award_approval_status
if current_approval_status in ["APPROVED", "DECLINED"]:
    raise ValidationError(...)

This prevents potential lockout if PENDING status ever exists in the database while allowing re-response for non-terminal states.

[Copilot]

_get_approval_reviewers() includes division directors/deputies regardless of roles, but get_pending_approvals_for_user() only returns results for users with BUDGET_TEAM, SYSTEM_OWNER, or REVIEWER_APPROVER. This can lead to users receiving a “Pre-Award Approval Request” notification but never seeing the item in the pending approvals list. Consider aligning the two methods (e.g., allow directors/deputies even without REVIEWER_APPROVER, or restrict notification recipients to those roles).

Suggested change

	# For REVIEWER_APPROVER role, filter by division director/deputy
	if "REVIEWER_APPROVER" in user_role_names:
	# Add joins to reach division table
	stmt = (
	stmt.outerjoin(Agreement.budget_line_items)
	.outerjoin(BudgetLineItem.can)
	.outerjoin(CAN.portfolio)
	.outerjoin(Portfolio.division)
	.where(
	or_(
	Division.division_director_id == user_id,
	Division.deputy_division_director_id == user_id,
	)
	)
	)
	results = self.db_session.execute(stmt.distinct()).scalars().all()
	return list(results)
	# User has no review permissions
	return []
	# For all other users, allow access when they are the division director/deputy
	# for the related agreement. This keeps the pending approvals list aligned
	# with reviewer notification recipients.
	stmt = (
	stmt.outerjoin(Agreement.budget_line_items)
	.outerjoin(BudgetLineItem.can)
	.outerjoin(CAN.portfolio)
	.outerjoin(Portfolio.division)
	.where(
	or_(
	Division.division_director_id == user_id,
	Division.deputy_division_director_id == user_id,
	)
	)
	)
	results = self.db_session.execute(stmt.distinct()).scalars().all()
	return list(results)

[josbell]

✅ Fixed in cf53878

Aligned get_pending_approvals_for_user() with notification recipient logic by adding a fallback for all users:

# For all other users, allow access when they are the division director/deputy
# for the related agreement. This keeps the pending approvals list aligned
# with reviewer notification recipients.
stmt = (
    stmt.outerjoin(Agreement.budget_line_items)
    .outerjoin(BudgetLineItem.can)
    .outerjoin(CAN.portfolio)
    .outerjoin(Portfolio.division)
    .where(
        or_(
            Division.division_director_id == user_id,
            Division.deputy_division_director_id == user_id,
        )
    )
)

Division directors/deputies now see pending approvals even without REVIEWER_APPROVER role, preventing the scenario where users receive notifications but don't see items in their list.

[Copilot]

useChangeRequestTotal skips the change-requests query when userId is falsy, but still always calls useGetPendingPreAwardApprovalsQuery(). During auth hydration / logged-out states this can trigger unnecessary 401s and error handling. Consider adding a matching skip: !userId (or otherwise gating the query) for the pending pre-award approvals request.

[josbell]

✅ Fixed in cf53878

Added skip: !userId to useGetPendingPreAwardApprovalsQuery:

const { data: preAwardApprovals } = useGetPendingPreAwardApprovalsQuery(undefined, { skip: !userId });

This matches the existing pattern for useGetChangeRequestsListQuery and prevents unnecessary 401 errors during auth hydration or logged-out states.

[Copilot]

This page now passes allBudgetLines into the shared review accordion. Previously the requester flow reviewed only executing BLIs, and the PR description mentions “executing status only”. Because groupByServicesComponent does not filter by status, this will display non-executing BLIs and may confuse the requester about what’s being approved. Consider filtering to executing BLIs for the review section (and, if needed elsewhere, return executingBudgetLines from usePreAwardApprovalData).

[josbell]

✅ Intentional per UX requirements

Updated PR description to clarify: All BLI statuses (not just executing) are intentionally included per UX requirements from Figma. The review section shows all budget lines to give approvers complete context, while the "Executing Total" section specifically highlights executing status BLIs.

The allBudgetLines parameter provides comprehensive agreement budget information as designed by UX.

[Copilot]

AgreementCANReviewAccordion adds budget line amounts into CAN pending totals when isApprovePage is false (see its !isApprovePage branch), so passing allBudgetLines here will include non-executing BLIs in the CAN impact calculation. If pre-award review should reflect executing BLIs only, pass a filtered list (or set action/isApprovePage appropriately) to avoid inflating CAN totals.

[josbell]

✅ Intentional per UX requirements

Updated PR description to clarify: According to Figma UX requirements, the CAN Impact section should include all budget lines, not just executing status. This provides approvers with complete visibility into the CAN budget impact of the agreement.

The allBudgetLines parameter is correct as designed.

[Copilot]

The “Download document” button rendered for existing memo documents has no click handler or link target, so it appears interactive but does nothing. If download isn’t implemented yet, consider disabling this button (like the empty-state) and/or adding a clear “coming soon” affordance; otherwise wire it up to the document download URL/action.

[josbell]

✅ Intentional - disabled with clear affordance

The download button is intentionally disabled with appropriate UX indicators:

• disabled attribute
• cursor: "not-allowed" style
• title="Document upload coming soon" tooltip
• aria-label="Download document (disabled)" for screen readers

This provides a clear "coming soon" affordance as requested. The button will be enabled when document download functionality is implemented in a future story.

[Copilot]

hasPermission requires one of BUDGET_TEAM/SYSTEM_OWNER/REVIEWER_APPROVER even if the current user is the division director/deputy for a CAN on the agreement. Backend authorization allows directors/deputies by ID regardless of role, so a director without REVIEWER_APPROVER could be incorrectly blocked in the UI. Consider allowing director/deputy matching by ID even when the role list doesn’t include REVIEWER_APPROVER (or update backend to match the role requirement).

[josbell]

✅ Fixed in cf53878

Simplified permission check to align with backend authorization logic:

// For BUDGET_TEAM and SYSTEM_OWNER, permission granted regardless of division
if (userRoleNames.includes("BUDGET_TEAM") || userRoleNames.includes("SYSTEM_OWNER")) {
    return true;
}

// For all other users (including REVIEWER_APPROVER or no specific role),
// check if user is division director/deputy for any CAN in the budget lines.
// This aligns with backend authorization and notification recipient logic.
return allBudgetLines.some(
    (bli) =>
        bli.can?.portfolio?.division?.division_director_id === userId ||
        bli.can?.portfolio?.division?.deputy_division_director_id === userId
);

Removed the REVIEWER_APPROVER role requirement for division directors, allowing director/deputy matching by ID even without that role.

[Copilot]

The comment says “Export RootState type for use with useSelector”, but this file only declares a local JSDoc typedef and doesn’t actually export a RootState symbol that other files can import. If you want consumers to be able to reference it via import("../store").RootState, consider adding an explicit exported typedef pattern (or adjust the comment to avoid implying it’s exported).

[josbell]

✅ Fixed in ea63580

Clarified the RootState comment to accurately reflect the JSDoc import pattern:

/**
 * RootState type for use with useSelector via JSDoc imports.
 * Usage: @type {import('./store').RootState}
 * @typedef {ReturnType<typeof store.getState>} RootState
 */

The comment now documents the actual usage pattern and removes misleading "export" language.

[rajohnson90]

The code looks good, but it's missing an update for the openapi file. There's a claude skill for this so it should be pretty quick.

[josbell]

The code looks good, but it's missing an update for the openapi file. There's a claude skill for this so it should be pretty quick.

Thanks for catching this! Updated openapi file. Let me know if I missed anything else.
@rajohnson90

[github-actions]

🤖 Auto-formatting applied!

I detected that this PR was edited via the GitHub web UI and had some formatting issues. I automatically applied fixes using:

• Prettier (frontend)
• Black + isort (backend)

The fixes have been committed to this branch. For future contributions, consider setting up the local development environment to run formatters before pushing - it avoids this extra round-trip.

[github-actions]

🎉 This PR is included in version 1.361.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀