fix: When applying filters, no longer reset the fiscal year selected in the dropdown on BLI page by rajohnson90 · Pull Request #5447 · HHS/OPRE-OPS

rajohnson90 · 2026-04-07T20:07:47Z

What changed

Removed some js code that ran when filters.fiscalYears was undefined that caused the selected fiscal year to return to the current FY after applying filters.

Issue

#5278

How to test

Navigate to the BLI page
Select fiscal year 2044
Select any combination of bli type filters. I chose draft, planned, and obligated.
Hit apply
Verify that the fiscal year in the dropdown on the upper right is still 2044 and you see BLIs.

A11y impact

No accessibility-impacting changes in this PR
Accessibility changes included and validated against WCAG 2.1 AA intent
Any temporary suppression includes A11Y-SUPPRESSION metadata (owner, expires, rationale)

Definition of Done Checklist

…reset when changing filter

Copilot

Pull request overview

Fixes the BLI list page so applying filters no longer resets the fiscal-year dropdown back to the current FY when filters.fiscalYears becomes undefined (OPS-5278).

Changes:

Removed/reset logic that forced the fiscal year shortcut back to the current FY when filters.fiscalYears is undefined.
Updated fiscal year resolution logic so the selected FY can still be used when filters don’t provide fiscal years.
Adjusted CanCard LineGraph-related unit test assertions.

Reviewed changes

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

File	Description
`frontend/src/pages/budgetLines/list/BudgetLineItemList.jsx`	Removes `undefined`-triggered FY reset and updates resolved FY behavior to preserve dropdown selection.
`frontend/src/components/CANs/CanCard/CanCard.test.jsx`	Refactors/relocates some LineGraph data assertions in tests.

Comments suppressed due to low confidence (2)

frontend/src/pages/budgetLines/list/BudgetLineItemList.jsx:51

The behavior change here is specifically to preserve the dropdown-selected fiscal year when filters.fiscalYears becomes undefined (e.g., after applying other filters). There are existing RTL tests for BudgetLineItemList (see frontend/src/pages/budgetLines/list/BudgetLineItemList.test.jsx) that cover fiscalYears: undefined defaulting to current FY, but there is no regression test for the shortcut-active path (user selects FY via the dropdown, then filters.fiscalYears is undefined, and the resolved FY should remain the selected year). Please add a test covering that scenario to prevent regressions.

    useEffect(() => {
        if (filters.fiscalYears === null) {
            setIsFiscalYearShortcutActive(false);
            setFiscalYearShortcut("All");
        } else if ((filters.fiscalYears ?? []).length > 1) {
            setIsFiscalYearShortcutActive(false);
            setFiscalYearShortcut("Multi");
        } else if ((filters.fiscalYears ?? []).length === 1) {
            setIsFiscalYearShortcutActive(false);
            setFiscalYearShortcut(filters.fiscalYears[0].id);
        } else if ((filters.fiscalYears ?? []).length === 0 && !isFiscalYearShortcutActive) {
            setFiscalYearShortcut(getCurrentFiscalYear());

frontend/src/pages/budgetLines/list/BudgetLineItemList.jsx:96

resolvedFiscalYears is annotated as possibly returning undefined, but with the current logic it only returns null or an array (the undefined branch was removed and the final return filters.fiscalYears is only reached when it is a non-empty array). Updating the JSDoc type to match the actual return values will avoid misleading consumers and future refactors.

    /** @type {Array<{id: number | string, title: number | string}> | null | undefined} */
    const resolvedFiscalYears = useMemo(() => {
        const currentFiscalYear = getCurrentFiscalYear();
        if (filters.fiscalYears === null) {
            return null;
        } else if ((filters.fiscalYears ?? []).length === 0) {
            const fallbackFiscalYear = isFiscalYearShortcutActive ? fiscalYearShortcut : currentFiscalYear;
            return [{ id: Number(fallbackFiscalYear), title: Number(fallbackFiscalYear) }];
        }
        return filters.fiscalYears;
    }, [filters.fiscalYears, isFiscalYearShortcutActive, fiscalYearShortcut]);

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

Copilot · 2026-04-08T15:42:42Z

        await waitFor(() => {
            const lineGraph = screen.getByTestId("mock-line-graph");
            const graphData = JSON.parse(lineGraph.getAttribute("data-graph-data"));

            // The second segment (available) should use funding.total_funding, not top-level total_funding
            expect(graphData[1].value).toBe(mockCanFundingData.funding.total_funding);
-            expect(graphData[1].value).not.toBeUndefined();
        });
+
+        const lineGraph = screen.getByTestId("mock-line-graph");
+        const graphData = JSON.parse(lineGraph.getAttribute("data-graph-data"));
+        expect(graphData[1].value).not.toBeUndefined();
    });


The new assertions re-query and re-parse data-graph-data outside the existing waitFor. This is redundant (the earlier expectations already prove the data is present) and can make the test harder to read. Consider keeping all assertions in the same waitFor block (or using a single parse) so the test is both simpler and less timing-sensitive.

Copilot · 2026-04-08T15:42:43Z

    it("passes correct total_funding to the spending/available LineGraph", async () => {
        render(
            <BrowserRouter>
                <CanCard
                    canId={mockCan.id}
                    fiscalYear={mockFiscalYear}
                />
            </BrowserRouter>
        );

        await waitFor(() => {
            const lineGraph = screen.getByTestId("mock-line-graph");
            const graphData = JSON.parse(lineGraph.getAttribute("data-graph-data"));

            // The second segment (available) should use funding.total_funding, not top-level total_funding
            expect(graphData[1].value).toBe(mockCanFundingData.funding.total_funding);
-            expect(graphData[1].value).not.toBeUndefined();
        });
+
+        const lineGraph = screen.getByTestId("mock-line-graph");
+        const graphData = JSON.parse(lineGraph.getAttribute("data-graph-data"));
+        expect(graphData[1].value).not.toBeUndefined();
    });



This PR description is focused on the BLI fiscal-year filter behavior, but this file also changes CanCard chart tests. If these test adjustments are required, consider mentioning them in the PR description (or splitting them into a separate PR) to keep scope and review intent clear.

fpigeonjr

Review Summary

The core logic fix is correct — removing the filters.fiscalYears === undefined branch from the useEffect and resolvedFiscalYears memo properly prevents the fiscal-year dropdown from snapping back to the current FY when other filters are applied. The undefined case now correctly falls through to the length === 0 path, preserving the existing behavior for the inactive-shortcut scenario.

That said, there are a few issues worth addressing before merging:

1. Missing regression test for the exact bug scenario (Medium)

The specific regression — user selects a non-current FY via the dropdown shortcut (isFiscalYearShortcutActive = true, fiscalYearShortcut = 2044), then applies other filters causing fiscalYears to become undefined, and the resolved FY should remain 2044 — is not covered by any test. The existing test at line 295 ("uses current fiscal year when fiscalYears is undefined") passes but exercises the shortcut-inactive path, not the broken one. A test covering this exact scenario would pin the fix and guard against regressions.

2. `CanCard.test.jsx` assertions moved outside `waitFor` (Low)

The assertions for graphData[1].value and graphData[1].percent were moved outside their respective waitFor blocks. This is fragile — if the component renders asynchronously, the outer DOM query may run before the element is updated. The existing assertions inside waitFor already prove the data is present, making the outer duplicates weaker and redundant. These changes are also out of scope for this PR's stated purpose and should be explained in the PR description or split out.

3. Stale JSDoc annotation (Minor)

The resolvedFiscalYears memo on line 86 is still annotated as Array | null | undefined, but undefined is no longer a possible return value after this change. Should be updated to Array | null.

rajohnson90 · 2026-04-09T13:52:37Z

The 'waitFor' assertions were moved because my linter wont let me commit code as long as the CanCard.test.jsx has multiple assertions within one waitFor block. Am I the only one encountering this??

rajohnson90 · 2026-04-09T14:44:26Z

A cypress test was added to pin the desired behavior for the fiscal year drop down w/ filters. I can't change the cancard assertions because the original code causes a linting error that prevents me from committing.

fpigeonjr

Follow-Up Review

The second commit (12534de) addresses the main blocker from my previous review by adding an E2E test that exercises the exact bug scenario: select FY 2044 via the dropdown shortcut, apply a BLI status filter, and verify the page doesn't crash.

Remaining minor items (non-blocking):

E2E test has no assertion — the new test in budgetLineItemsList.cy.js exercises the interaction but never asserts cy.get("#fiscal-year-select").should("have.value", "2044"). It won't catch a regression, but an E2E test is better than nothing and the unit test rename clarifies intent.
CanCard.test.jsx assertions outside waitFor — still fragile and out of scope, but low risk in practice.
Stale JSDoc — resolvedFiscalYears is still annotated | undefined though that return path was removed.

The core logic fix is correct and the intent of the regression test is right. These remaining items are worth a follow-up ticket but don't block shipping this fix.

Approving.

Reviewed and submitted on Frank's behalf as his coding agent.

github-actions · 2026-04-09T20:12:31Z

🎉 This PR is included in version 1.355.7 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

fix: removing behavior when fiscalYears is undefined to prevent year …

a8709f9

…reset when changing filter

rajohnson90 self-assigned this Apr 7, 2026

rajohnson90 requested review from Santi-3rd, fpigeonjr, johndeange, josbell and weimiao67 as code owners April 7, 2026 20:07

Santi-3rd approved these changes Apr 8, 2026

View reviewed changes

fpigeonjr requested review from Copilot April 8, 2026 15:38

Copilot started reviewing on behalf of fpigeonjr April 8, 2026 15:39 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

weimiao67 approved these changes Apr 8, 2026

View reviewed changes

This comment was marked as outdated.

Sign in to view

fpigeonjr requested changes Apr 8, 2026

View reviewed changes

test: adding test for fixed fiscal year dropdown

12534de

rajohnson90 requested a review from fpigeonjr April 9, 2026 14:43

fpigeonjr approved these changes Apr 9, 2026

View reviewed changes

test: add assert to test and update doc

51a21ad

rajohnson90 merged commit 4894a01 into main Apr 9, 2026
58 checks passed

rajohnson90 deleted the OPS-5278/filter-resets-fiscal-year branch April 9, 2026 20:11

github-actions Bot added the released label Apr 9, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: When applying filters, no longer reset the fiscal year selected in the dropdown on BLI page#5447

fix: When applying filters, no longer reset the fiscal year selected in the dropdown on BLI page#5447
rajohnson90 merged 3 commits intomainfrom
OPS-5278/filter-resets-fiscal-year

rajohnson90 commented Apr 7, 2026 •

edited by jonnalley

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

This comment was marked as outdated.

Uh oh!

fpigeonjr left a comment

Uh oh!

rajohnson90 commented Apr 9, 2026

Uh oh!

rajohnson90 commented Apr 9, 2026

Uh oh!

fpigeonjr left a comment

Uh oh!

Uh oh!

github-actions Bot commented Apr 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

rajohnson90 commented Apr 7, 2026 • edited by jonnalley Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

Issue

How to test

A11y impact

Definition of Done Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

fpigeonjr left a comment

Choose a reason for hiding this comment

Review Summary

1. Missing regression test for the exact bug scenario (Medium)

2. CanCard.test.jsx assertions moved outside waitFor (Low)

3. Stale JSDoc annotation (Minor)

Uh oh!

rajohnson90 commented Apr 9, 2026

Uh oh!

rajohnson90 commented Apr 9, 2026

Uh oh!

fpigeonjr left a comment

Choose a reason for hiding this comment

Follow-Up Review

Uh oh!

Uh oh!

github-actions Bot commented Apr 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rajohnson90 commented Apr 7, 2026 •

edited by jonnalley

Loading

2. `CanCard.test.jsx` assertions moved outside `waitFor` (Low)