Skip to content

feat(agreements): Budget Team edit-award screen (OPS-2280) - #6169

Draft
josbell wants to merge 6 commits into
mainfrom
OPS-2280/budget-team-edit-award-screen
Draft

feat(agreements): Budget Team edit-award screen (OPS-2280)#6169
josbell wants to merge 6 commits into
mainfrom
OPS-2280/budget-team-edit-award-screen

Conversation

@josbell

@josbell josbell commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

What changed

Adds a dedicated Edit Award Approval screen for Budget Team members reviewing a pending award approval request (Step 6). Previously the Edit button on the Award Approval review page navigated to the generic full-agreement editor; now it goes to a focused form showing only the award-specific fields (Vendor, Contract #, Award Amount, Award Date, CLINs) pre-filled from the submitted step 6 data.

Key changes:

  • New route /agreements/:id/edit-awardEditAwardApproval page + useEditAwardApproval hook. Seeds fields from step 6, saves via PATCH /procurement-tracker-steps/:id without touching approval_requested/approval_status.
  • AwardRequestForm — new shared presentational component extracted from RequestAwardApproval. Both request and edit pages render it. A mode prop ("request" | "edit") controls instruction copy and hides the Notes textarea in edit mode.
  • OPS-2280 bypass backout — removes the frontend budgetTeamBypasses modal-suppression guard in EditAgreementAndBudgetLines and the backend budget_team_can_bypass direct-write path in BudgetLineItemService. Budget Team financial BLI edits now always route through the change-request workflow (consistent with other roles during award approval).
  • Figma alignment — title, intro text, and section instruction strings updated to match the "Step 6 - Award - Edit CLINs" spec.
  • Test data — agreement 13 advanced to step 6 award-approval-pending for local testing; description and BLI null fields fixed.
  • Documents 401useGetDocumentsByAgreementIdQuery in usePreAwardApprovalData set to skip: true until the Azure storage backend is configured (feature already disabled in UI).

Issue

OPS-2280

How to test

  1. Rebuild the Docker stack (docker compose up --build) to pick up the seed data changes for agreement 13.
  2. Log in as a Budget Team user.
  3. Navigate to /agreements/13/review-award (Award Approval review page for agreement 13, which is seeded at step 6 with a pending award approval).
  4. Click Edit — confirm it navigates to /agreements/13/edit-award (not the generic editor).
  5. Confirm the page is titled "Edit Award Approval", shows the Figma intro text, and pre-fills Vendor, Contract #, Award Amount, Award Date, and CLINs from the seed data.
  6. Confirm no Notes textarea is present.
  7. Edit a field (e.g. Award Amount), click Save Changes — confirm it returns to the review page with the updated values and the approval status is still pending.
  8. Confirm the unsaved-changes blocker fires if you navigate away with edits pending.
  9. Navigate to /agreements/13/award-approval (request form) — confirm "Add …" instruction copy and Notes textarea are still present.
  10. Log in as a non-Budget-Team user and navigate directly to /agreements/13/edit-award — confirm the "Access Denied" alert is shown.

A11y impact

  • No accessibility-impacting changes in this PR

Storybook

  • N/A — change is page-specific or non-visual

Screenshots

See Figma spec: "Step 6 - Award - Edit CLINs"

Definition of Done Checklist

  • OESA: Code refactored for clarity
  • OESA: Dependency rules followed
  • Automated unit tests updated and passed
  • Automated integration tests updated and passed
  • Automated quality tests updated and passed
  • Automated load tests updated and passed
  • Automated a11y tests updated and passed
  • Automated security tests updated and passed
  • 90%+ Code coverage achieved
  • Form validations updated

Links

  • [Figma: Step 6 - Award - Edit CLINs](Step 6 - Award - Edit CLINs.pdf)

@josbell

josbell commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Code review

Found 4 issues:

  1. Missing role gate on EditAwardApproval — any authenticated user can load the page and attempt to mutate award fields. The sibling ApproveAwardApproval correctly computes hasPermission = BUDGET_TEAM || SYSTEM_OWNER and renders an "Access Denied" alert when false; EditAwardApproval has no such check and is not wrapped in a RoleProtectedRoute.

export const EditAwardApproval = () => {
const { id } = useParams();
const agreementId = Number(id);
const { setAlert } = useAlert();
const [selectedBudgetLineId, setSelectedBudgetLineId] = useState(null);

  1. CLIN seeding race condition — the seeding useEffect guard at line 132 checks !step6 and vendors.length === 0 but not !agreement. If the trackers/vendors queries resolve before the agreement query, the effect fires with allBudgetLines = [], seeds no CLINs, and then immediately sets isSeeded = true. When the agreement arrives later and allBudgetLines populates, the isSeeded guard prevents re-execution — existing CLIN assignments on the BLIs are permanently skipped.

// Run only once to avoid overwriting user edits on re-renders.
useEffect(() => {
if (isSeeded || !step6 || vendors.length === 0) return;
if (step6.vendor_id) {
const vendor = vendors.find((v) => v.id === step6.vendor_id);
setSelectedVendor(vendor || null);
}
if (step6.contract_number) setContractNumber(step6.contract_number);
if (step6.award_amount != null) setAwardAmount(String(step6.award_amount));
if (step6.award_date) setAwardDate(formatApiDateForDisplay(step6.award_date));
if (step6.requestor_notes) setNotes(step6.requestor_notes);
// Seed CLIN assignments from existing budget-line clin_id values
const existingClins = {};
allBudgetLines.forEach((bli) => {
if (bli.clin_id) {
existingClins[bli.id] = bli.clin_id;
}
});
if (Object.keys(existingClins).length > 0) {
setClinAssignments(existingClins);
}
setIsSeeded(true);
}, [isSeeded, step6, vendors, allBudgetLines]);

  1. handleCancel confirm missing flushSync — when the user confirms "Leave without saving" from the cancel modal, setIsNavigating(true) is called without flushSync before navigate(returnTo) (line 321). React may batch the state update, leaving isNavigating = false when the router transition fires, causing useBlocker to intercept the intentional navigation and show a second modal. Both handleSave (line 290) and the blocker handler (line 210) in the same file correctly use flushSync.

secondaryButtonText: "Continue editing",
handleConfirm: () => {
setShowModal(false);
setIsNavigating(true);
navigate(returnTo);
},
closeModal: () => {
setShowModal(false);

  1. Shared Vest suite singleton, no suite.reset() on mountEditAwardApproval.hooks.js imports the same module-level suite singleton as RequestAwardApproval.hooks.js (line 17) and initializes with useState(suite.get()) (line 73) without resetting it first. If a user visits the request form, triggers validation errors, then navigates to /edit-award, the edit form opens with phantom error messages from stale suite state before the user touches anything. ApproveAwardApproval.hooks.js calls suite.reset() on mount and unmount to prevent exactly this.

import suite from "./RequestAwardApproval.suite";

// Validation
const [validationResult, setValidationResult] = useState(suite.get());
const [updateProcurementTrackerStep] = useUpdateProcurementTrackerStepMutation();

const [validatorRes, setValidatorRes] = useState(() => suite.get());
useEffect(() => {
suite.reset();
setValidatorRes(suite.get());
return () => {
suite.reset();
};
}, []);

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant