Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,11 @@ export const PercentileStationResultTable: React.FunctionComponent<Props> = ({ s
const yearRange = years.join(', ')
const [snackbarOpen, setSnackbarOpen] = useState(false)

// biome-ignore lint/correctness/useExhaustiveDependencies: intentional — deps are captured via closure correctly
useEffect(() => {
if (years.length < timeRange) {
setSnackbarOpen(true)
}
}, [years])
}, [years, timeRange])

return (
<div>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { fireEvent, render, screen } from '@testing-library/react'
import type { StationSummaryResponse } from '@wps/api/percentileAPI'
import { describe, expect, it } from 'vitest'
import { PercentileStationResultTable } from './PercentileStationResultTable'

const makeStationResponse = (years: number[]): StationSummaryResponse => ({
ffmc: 88.5,
isi: 9.1,
bui: 72.0,

Check warning on line 9 in web/apps/wps-web/src/features/percentileCalculator/components/percentileStationResultTable.test.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Don't use a zero fraction in the number.

See more on https://sonarcloud.io/project/issues?id=bcgov_wps&issues=AZ7cKtQyL49KgC-oqzJq&open=AZ7cKtQyL49KgC-oqzJq&pullRequest=5533
years,
station: {
code: 101,
name: 'Test Station',
lat: 49.0,

Check warning on line 14 in web/apps/wps-web/src/features/percentileCalculator/components/percentileStationResultTable.test.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Don't use a zero fraction in the number.

See more on https://sonarcloud.io/project/issues?id=bcgov_wps&issues=AZ7cKtQyL49KgC-oqzJr&open=AZ7cKtQyL49KgC-oqzJr&pullRequest=5533
long: -120.0,

Check warning on line 15 in web/apps/wps-web/src/features/percentileCalculator/components/percentileStationResultTable.test.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Don't use a zero fraction in the number.

See more on https://sonarcloud.io/project/issues?id=bcgov_wps&issues=AZ7cKtQyL49KgC-oqzJs&open=AZ7cKtQyL49KgC-oqzJs&pullRequest=5533
ecodivision_name: 'Montane Cordillera',
core_season: { start_month: 5, start_day: 1, end_month: 9, end_day: 30 }
}
})

describe('PercentileStationResultTable snackbar', () => {
it('does not show warning when data covers the full time range', () => {
const years = [2020, 2021, 2022, 2023, 2024]
render(<PercentileStationResultTable stationResponse={makeStationResponse(years)} timeRange={5} />)

expect(screen.queryByText(/Data only available for/)).not.toBeInTheDocument()
})

it('shows warning when fewer years of data are available than the time range', () => {
const years = [2022, 2023, 2024]
render(<PercentileStationResultTable stationResponse={makeStationResponse(years)} timeRange={10} />)

expect(screen.getByText('Data only available for 3 of 10 years')).toBeInTheDocument()
})

it('re-shows the warning after dismissal when timeRange increases beyond available data', () => {
const years = [2022, 2023, 2024]
const { rerender } = render(
<PercentileStationResultTable stationResponse={makeStationResponse(years)} timeRange={2} />
)
// 3 years >= timeRange=2 — no warning
expect(screen.queryByText(/Data only available for/)).not.toBeInTheDocument()

// Increase time range beyond available data — warning appears
rerender(<PercentileStationResultTable stationResponse={makeStationResponse(years)} timeRange={5} />)
expect(screen.getByText('Data only available for 3 of 5 years')).toBeInTheDocument()

// User dismisses the snackbar
fireEvent.click(screen.getByRole('button', { name: 'Close' }))

// Slider increases time range further — warning re-opens
rerender(<PercentileStationResultTable stationResponse={makeStationResponse(years)} timeRange={10} />)
expect(screen.getByText('Data only available for 3 of 10 years')).toBeInTheDocument()
})
})

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it introduces a bit of a behaviour change, where the data will load without hitting Calculate after you adjust the slider. The warning snackbar also just flashes and disappears before being able to read it

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed in: 6bb3652

The root cause was yearRange being listed as a dep of the codesFromQuery effect. When the slider moved, yearRange changed → the effect re-ran → fetchPercentiles fired without a Calculate click.

Fix: store yearRange in a ref that's updated inline during render (yearRangeRef.current = yearRange), then read yearRangeRef.current inside the effect. The effect only depends on
codesFromQuery and dispatch, so it only fires when the URL changes (i.e. Calculate was clicked). The ref always holds the latest value, so the correct year range is still passed to the API.

The snackbar flash was a downstream symptom, the spurious fetch set loading: true, which unmounted the result table and reset its snackbarOpen state. That resolves with the fetch fix.

Added regression tests as well.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you try playing with percentile calc in dev? There's more significant issues now. Changing years and calculating doesn't do anything

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah that's broken, at least we can test this in isolation, this app is a bit tricky.

Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { TimeRangeSlider, yearWhenTheCalculationIsDone } from 'features/percenti
import WxStationDropdown from 'features/percentileCalculator/components/WxStationDropdown'
import { fetchPercentiles, resetPercentilesResult } from 'features/percentileCalculator/slices/percentilesSlice'
import { fetchWxStations } from 'features/stations/slices/stationsSlice'
import React, { useEffect, useState } from 'react'
import React, { useEffect, useMemo, useRef, useState } from 'react'
import { useDispatch } from 'react-redux'
import { useLocation, useNavigate } from 'react-router-dom'

Expand All @@ -24,30 +24,33 @@ const PercentileCalculatorPage = () => {
const location = useLocation()
const navigate = useNavigate()

const codesFromQuery = getStationCodesFromUrl(location.search)
const codesFromQuery = useMemo(() => getStationCodesFromUrl(location.search), [location.search])
const [stationCodes, setStationCodes] = useState<number[]>(codesFromQuery)
const [timeRange, setTimeRange] = useState<number>(defaultTimeRange)
const yearRange = {
start: yearWhenTheCalculationIsDone - (timeRange - 1),
end: yearWhenTheCalculationIsDone
}
const yearRange = useMemo(
() => ({
start: yearWhenTheCalculationIsDone - (timeRange - 1),
end: yearWhenTheCalculationIsDone
}),
[timeRange]
)
const yearRangeRef = useRef(yearRange)
yearRangeRef.current = yearRange

// biome-ignore lint/correctness/useExhaustiveDependencies: intentional — fetch on mount only
useEffect(() => {
dispatch(fetchWxStations(getStations, StationSource.unspecified))
}, [])
}, [dispatch])

// biome-ignore lint/correctness/useExhaustiveDependencies: intentional — deps are captured via closure correctly
useEffect(() => {
if (codesFromQuery.length > 0) {
dispatch(fetchPercentiles(codesFromQuery, defaultPercentile, yearRange))
dispatch(fetchPercentiles(codesFromQuery, defaultPercentile, yearRangeRef.current))
} else {
dispatch(resetPercentilesResult())
}

// Update local state to match with the url query
setStationCodes(codesFromQuery)
}, [location])
}, [codesFromQuery, dispatch])

const onCalculateClick = () => {
// Update the url query with the new station codes
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
import { fireEvent, render, screen, waitFor } from '@testing-library/react'
import { fetchPercentiles } from 'features/percentileCalculator/slices/percentilesSlice'
import { Provider } from 'react-redux'
import { MemoryRouter } from 'react-router-dom'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { createTestStore } from '@/test/testUtils'
import PercentileCalculatorPage from './PercentileCalculatorPage'

const MOCKED_CURRENT_YEAR = 2024

vi.mock('@wps/api/stationAPI', () => ({
getStations: vi.fn(() => Promise.resolve([])),
StationSource: { unspecified: 'unspecified' }
}))

vi.mock('features/stations/slices/stationsSlice', async () => {
const actual = await vi.importActual<typeof import('features/stations/slices/stationsSlice')>(
'features/stations/slices/stationsSlice'
)
return { ...actual, fetchWxStations: vi.fn(() => () => {}) }
})

vi.mock('features/percentileCalculator/slices/percentilesSlice', async () => {
const actual = await vi.importActual<typeof import('features/percentileCalculator/slices/percentilesSlice')>(
'features/percentileCalculator/slices/percentilesSlice'
)
return { ...actual, fetchPercentiles: vi.fn(() => () => {}) }
})

vi.mock('features/percentileCalculator/components/TimeRangeSlider', () => ({
TimeRangeSlider: ({ onYearRangeChange }: { onYearRangeChange: (n: number) => void }) => (
<button type="button" data-testid="change-time-range" onClick={() => onYearRangeChange(5)}>
Change Range
</button>
),
yearWhenTheCalculationIsDone: 2024
}))

vi.mock('features/percentileCalculator/components/WxStationDropdown', () => ({
default: ({ onChange }: { onChange: (codes: number[]) => void }) => (
<button type="button" data-testid="select-station" onClick={() => onChange([101])}>
Select Station
</button>
)
}))

vi.mock('features/percentileCalculator/components/PercentileResults', () => ({ default: () => null }))
vi.mock('features/percentileCalculator/components/PercentileTextfield', () => ({ PercentileTextfield: () => null }))

const renderPage = (search = '') =>
render(
<Provider store={createTestStore()}>
<MemoryRouter initialEntries={[`/${search}`]}>
<PercentileCalculatorPage />
</MemoryRouter>
</Provider>
)

describe('PercentileCalculatorPage', () => {
beforeEach(() => {
vi.mocked(fetchPercentiles).mockClear()
})

it('fetches percentiles on mount when station codes are in the URL', async () => {
renderPage('?codes=101,202')

await waitFor(() => {
expect(fetchPercentiles).toHaveBeenCalledOnce()
expect(fetchPercentiles).toHaveBeenCalledWith([101, 202], 90, expect.any(Object))
})
})

it('does not fetch percentiles again when the time range slider changes', async () => {
renderPage('?codes=101')

await waitFor(() => {
expect(fetchPercentiles).toHaveBeenCalledOnce()
})

fireEvent.click(screen.getByTestId('change-time-range'))

// Allow any potential re-renders to settle
await new Promise(resolve => setTimeout(resolve, 50))

expect(fetchPercentiles).toHaveBeenCalledOnce()
})

it('uses the slider yearRange at the time Calculate is clicked, not the default', async () => {
renderPage()

// Change slider to timeRange=5 before selecting any station
fireEvent.click(screen.getByTestId('change-time-range'))

// Select a station to enable Calculate
fireEvent.click(screen.getByTestId('select-station'))

// Click Calculate — this navigates to ?codes=101
fireEvent.click(screen.getByText('Calculate'))

await waitFor(() => {
expect(fetchPercentiles).toHaveBeenCalledWith([101], 90, {
start: MOCKED_CURRENT_YEAR - 4, // timeRange=5 → start = 2024 - (5-1)
end: MOCKED_CURRENT_YEAR
})
})
})
})
Loading