feat: blocker modal conditional messages

[Santi-3rd]

What changed

The save & exit modal notifies the user if their changes require Division Director approval.

Issue

#4763

How to test

• Go to an agreement that has planned BLIs
• Change the dollar amount on a planned BLI
• Try to navigate away from the page with unsaved changes. This should trigger the correct modal.
• Exit the modal by pressing the Esc key, Leave Without Saving and Save & Send to Approval. Verify that each action works as expected.
• e2e tests pass

Screenshots

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

[Copilot]

Pull request overview

This pull request implements conditional messaging in the save & exit modal to notify users when their changes require Division Director approval. The modal now displays different messages and button text based on whether budget line items have financial snapshot changes (changes to amounts in planned BLIs) that require approval.

• Added conditional modal content based on whether changes require Division Director approval
• Introduced a new handleFinancialSnapshotChangesViaBlocker function to handle approval workflow when saving via the navigation blocker
• Lifted hasUnsavedChanges state management from ServicesComponents hook to parent components for better state coordination

Reviewed changes

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

Show a summary per file

File	Description
SaveChangesAndExitModal.jsx	Added inline style adjustment (width: 101%) to modal content container
ServicesComponents.jsx	Added setHasUnsavedChanges and hasUnsavedChanges props to support parent state management
ServicesComponents.hooks.js	Removed local hasUnsavedChanges state, now managed by parent component
CreateBLIsAndSCs.jsx	Passed setHasUnsavedChanges and hasUnsavedChanges props to ServicesComponents, removed redundant state reset
CreateBLIsAndSCs.hooks.js	Added conditional modal content logic, new blocker handler function, and reorganized blocker useEffect to show appropriate messages based on financial snapshot changes
saveChangesToEdits.cy.js	Added comprehensive e2e test for approval workflow via blocker modal, updated test assertions to verify correct modal descriptions
budgetChangeRequest.cy.js	Removed unnecessary alert close action in test flow

Comments suppressed due to low confidence (1)

frontend/src/components/BudgetLineItems/CreateBLIsAndSCs/CreateBLIsAndSCs.hooks.js:924

• When savedViaModal is true and there are financial snapshot changes, both handleFinancialSnapshotChangesViaBlocker (line 913) and showSuccessMessage (line 924) will call setAlert, potentially causing duplicate alerts or one alert overwriting the other. The success message should only be displayed by one of these functions to avoid confusion.

                    if (isThereAnyBLIsFinancialSnapshotChanged && !isSuperUser && !savedViaModal) {
                        await handleFinancialSnapshotChanges(existingBudgetLineItemsWithIds);
                    } else if (isThereAnyBLIsFinancialSnapshotChanged && !isSuperUser && savedViaModal) {
                        await handleFinancialSnapshotChangesViaBlocker(existingBudgetLineItemsWithIds);
                    } else {
                        await handleRegularUpdates(existingBudgetLineItemsWithIds);
                    }
                    await handleDeletions();
                }
                suite.reset();
                budgetFormSuite.reset();
                datePickerSuite.reset();
                resetForm();
                setIsEditMode(false);
                showSuccessMessage(isThereAnyBLIsFinancialSnapshotChanged, savedViaModal);

---

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

[Copilot]

The useEffect dependency array is incomplete. The effect uses modalContent and handleSave but doesn't include them in the dependencies (only includes blocker, setModalProps, and setIsEditMode). This means if tempBudgetLines changes (which affects modalContent) or if handleSave is redefined, the modal will use stale values. The modal content could be incorrect, or the save handler might use outdated closures, potentially leading to incorrect behavior when the blocker is triggered.

[Copilot]

The modalContent object is computed on every render based on hasFinancialSnapshotChanges, which depends on tempBudgetLines. However, this computation happens outside the useEffect, meaning the modal content is evaluated immediately when the component renders, not when the blocker is triggered. If tempBudgetLines changes after the blocker is triggered but before the modal is shown, the modal content may not reflect the current state accurately.

[Copilot]

The handleFinancialSnapshotChangesViaBlocker function shows a generic success message without the detailed budget change messages that are shown in the regular handleFinancialSnapshotChanges flow and in showSuccessMessage. When users save via the blocker modal, they won't see the same detailed "Pending Changes" information that they would see when saving via the normal Save & Exit button, creating an inconsistent user experience.

[fpigeonjr]

999:8 warning React Hook React.useEffect has missing dependencies: 'handleSave' and 'modalContent'. Either include them or remove the dependency array

seeing this linter warning

[fpigeonjr]

• agreement 9, two budget lines that are planned
• ✅ esc worked to cancel
• bug: total is not updating after amount(prob not related to PR changes)
• ✅ navigate to agreements list and edits went to Division Director
• ✅ navigating to destination works

[fpigeonjr]

Claude Code's review:

PR Review: feat: blocker modal conditional messages

Overview

This PR enhances the save & exit modal to conditionally notify users when their changes to planned budget line items require Division Director approval. The implementation includes comprehensive E2E tests and updates to the approval workflow.

Critical Issues

1. Duplicate Alert Bug 🔴

In CreateBLIsAndSCs.hooks.js around lines 907-924, there's a potential for duplicate alerts:

if (isThereAnyBLIsFinancialSnapshotChanged && !isSuperUser && savedViaModal) {
await handleFinancialSnapshotChangesViaBlocker(existingBudgetLineItemsWithIds); // Sets alert
}
// ...
showSuccessMessage(isThereAnyBLIsFinancialSnapshotChanged, savedViaModal); // Also sets alert

Both handleFinancialSnapshotChangesViaBlocker and showSuccessMessage set alerts. When savedViaModal=true with financial snapshot changes, you'll get two competing alerts.

Fix: Skip showSuccessMessage when the blocker handler was used, or have showSuccessMessage handle the blocker case specially.

2. React Hooks Dependency Warning 🔴

The blocker useEffect at line ~978 is missing dependencies:

React.useEffect(() => {
if (blocker.state === "blocked") {
// Uses: modalContent, handleSave, setShowSaveChangesModal, setHasUnsavedChanges
// But only lists: [blocker, setModalProps, setIsEditMode]
}
}, [blocker, setModalProps, setIsEditMode]);

This will cause stale closure bugs. All referenced variables should be in the dependency array, or use useCallback to stabilize the handlers.

3. Suspicious Modal Width Hack 🟡

In SaveChangesAndExitModal.jsx:95:

Why 101%? This inline style suggests a layout bug being masked. This should be investigated and fixed properly with CSS.

Code Quality Issues

4. Unnecessary Indentation Changes 🟡

Lines 823-945 in CreateBLIsAndSCs.hooks.js show large indentation changes that make the diff harder to review. These appear to be unintentional reformatting. Consider reverting to minimize diff noise.

5. Inconsistent Button Text 🟡

• Modal heading: "Save changes before leaving?" vs "Save changes before closing?"
• Button text: "Leave without saving" vs "Exit Without Saving"

Pick one terminology and stick with it for consistency.

[github-actions]

🎉 This PR is included in version 1.253.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀