Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forecast - Actual column split #3405

Closed
wants to merge 18 commits into from
8 changes: 7 additions & 1 deletion web/src/features/moreCast2/components/ColumnDefBuilder.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export const DEFAULT_FORECAST_COLUMN_WIDTH = 120

// Defines the order in which weather models display in the datagrid.
export const ORDERED_COLUMN_HEADERS: WeatherDeterminateType[] = [
WeatherDeterminate.ACTUAL,
WeatherDeterminate.HRDPS,
WeatherDeterminate.HRDPS_BIAS,
WeatherDeterminate.RDPS,
Expand Down Expand Up @@ -75,7 +76,12 @@ export class ColumnDefBuilder implements ColDefGenerator, ForecastColDefGenerato
? ORDERED_COLUMN_HEADERS
: ORDERED_COLUMN_HEADERS.filter(header => !header.endsWith('_BIAS'))
return fields.map(header =>
this.generateColDefWith(`${this.field}${header}`, header, this.precision, DEFAULT_COLUMN_WIDTH)
this.generateColDefWith(
`${this.field}${header}`,
header,
this.precision,
header.includes('Actual') ? DEFAULT_FORECAST_COLUMN_WIDTH : DEFAULT_COLUMN_WIDTH
)
)
}

Expand Down
30 changes: 19 additions & 11 deletions web/src/features/moreCast2/components/GridComponentRenderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
GridValueSetterParams
} from '@mui/x-data-grid'
import { ModelChoice } from 'api/moreCast2API'
import { createLabel } from 'features/moreCast2/util'
import { createWeatherModelLabel } from 'features/moreCast2/util'

const NOT_AVAILABLE = 'N/A'

Expand Down Expand Up @@ -44,13 +44,14 @@ export class GridComponentRenderer {
precision: number,
field: string
): string => {
const actualField = this.getActualField(field)
const actual = params.row[actualField]
if (field.includes('grass')) {
brettedw marked this conversation as resolved.
Show resolved Hide resolved
const actualField = this.getActualField(field)
const actual = params.row[actualField]

if (!isNaN(actual)) {
return Number(actual).toFixed(precision)
if (!isNaN(actual)) {
return Number(actual).toFixed(precision)
}
}

const value = params?.value?.value

if (isNaN(value)) {
Expand All @@ -63,17 +64,24 @@ export class GridComponentRenderer {
// The value of field will be precipForecast, rhForecast, tempForecast, etc.
// We need the prefix to help us grab the correct 'actual' field (eg. tempACTUAL, precipACTUAL, etc.)
const actualField = this.getActualField(field)
const isActual = !isNaN(params.row[actualField])

const isGrassField = field.includes('grass')

const isActual = !isNaN(params.row[actualField])
// We can disable a cell if an Actual exists or the forDate is before today.
// Both forDate and today are currently in the system's time zone
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know this is relatively simple, but could we pull this out into a helper function? Would make it easier to test and keep renderForecastCellWith focused on the rendering.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep I can do that. I was also thinking the logic on line 85, setting value might be better to put in the predictionItemValueFormatter....it would keep formatting in the Formatter. Worth changing?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah I think that's a good call too

const today = new Date()
today.setHours(0, 0, 0, 0)
const isPreviousDate = params.row['forDate'] < today

return (
<TextField
disabled={isActual}
disabled={isActual || isPreviousDate}
size="small"
label={isGrassField ? '' : createLabel(isActual, params.row[field].choice)}
value={params.formattedValue}
label={isGrassField ? '' : createWeatherModelLabel(params.row[field].choice)}
InputLabelProps={{
shrink: true
}}
value={!isActual && params.formattedValue === NOT_AVAILABLE && !isPreviousDate ? '' : params.formattedValue}
></TextField>
)
}
Expand Down
12 changes: 9 additions & 3 deletions web/src/features/moreCast2/components/TabbedDataGrid.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import { AlertColor, List, Stack } from '@mui/material'
import { styled } from '@mui/material/styles'
import { GridCellParams, GridColDef, GridColumnVisibilityModel, GridEventListener } from '@mui/x-data-grid'
import { ModelChoice, ModelType, submitMoreCastForecastRecords } from 'api/moreCast2API'
import {
ModelChoice,
ModelType,
WeatherDeterminate,
WeatherDeterminateType,
submitMoreCastForecastRecords
} from 'api/moreCast2API'
import { DataGridColumns, columnGroupingModel } from 'features/moreCast2/components/DataGridColumns'
import ForecastDataGrid from 'features/moreCast2/components/ForecastDataGrid'
import ForecastSummaryDataGrid from 'features/moreCast2/components/ForecastSummaryDataGrid'
Expand Down Expand Up @@ -272,8 +278,8 @@
// occurs on a cell in a weather model field/column and row where a forecast is being created (ie. the
// row has no actual value for the weather parameter of interest)
const handleCellDoubleClick = (params: GridCellParams) => {
const headerName = params.colDef.headerName as ModelType
if (!headerName || headerName === ModelChoice.ACTUAL || headerName === ModelChoice.FORECAST) {
const headerName = params.colDef.headerName as WeatherDeterminateType

Check warning on line 281 in web/src/features/moreCast2/components/TabbedDataGrid.tsx

View check run for this annotation

Codecov / codecov/patch

web/src/features/moreCast2/components/TabbedDataGrid.tsx#L281

Added line #L281 was not covered by tests
if (!headerName || headerName === WeatherDeterminate.ACTUAL || headerName === WeatherDeterminate.FORECAST) {
// A forecast or actual column was clicked, or there is no value for headerName, nothing to do
return
}
Expand Down
28 changes: 24 additions & 4 deletions web/src/features/moreCast2/components/colDefBuilder.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ describe('ColDefBuilder', () => {
})
)
})
it('should generate all col defs correcty', () => {
it('should generate all col defs correctly', () => {
const colDefs = colDefBuilder.generateColDefs()

const expected = [
Expand All @@ -62,7 +62,7 @@ describe('ColDefBuilder', () => {
headerName: determinate,
sortable: false,
type: 'number',
width: DEFAULT_COLUMN_WIDTH
width: determinate === WeatherDeterminate.ACTUAL ? DEFAULT_FORECAST_COLUMN_WIDTH : DEFAULT_COLUMN_WIDTH
})
)
)
Expand Down Expand Up @@ -146,14 +146,34 @@ describe('ColDefBuilder', () => {
)
expect(
forecastColDef.renderCell({ row: { testField: { choice: ModelChoice.GDPS, value: 1 } }, formattedValue: 1 })
).toEqual(<TextField disabled={false} label={ModelChoice.GDPS} size="small" value={1} />)
).toEqual(
<TextField
disabled={false}
InputLabelProps={{
shrink: true
}}
label={ModelChoice.GDPS}
size="small"
value={1}
/>
)

expect(
forecastColDef.renderCell({
row: { testField: { choice: ModelChoice.GDPS, value: 1 }, testActual: 2 },
formattedValue: 1
})
).toEqual(<TextField disabled={false} label={ModelChoice.GDPS} size="small" value={1} />)
).toEqual(
<TextField
disabled={false}
InputLabelProps={{
shrink: true
}}
label={ModelChoice.GDPS}
size="small"
value={1}
/>
)
expect(forecastColDef.valueFormatter({ value: 1.11 })).toEqual('1.1')
expect(
forecastColDef.valueGetter({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,18 +113,18 @@ describe('GridComponentRenderer', () => {
expect(actualField).toEqual('testActual')
})

it('should return an actual over a prediction if it exists', () => {
it('should return an actual over a prediction if it exists for grass curing', () => {
const itemValue = gridComponentRenderer.valueGetter(
{
row: {
testForecast: { choice: ModelChoice.GDPS, value: 1.11 },
testActual: 2.22
grassCuringForecast: { choice: ModelChoice.GDPS, value: 10.0 },
grassCuringActual: 20.0
},
value: { choice: ModelChoice.GDPS, value: 1.11 }
value: { choice: ModelChoice.NULL, value: 10.0 }
},
1,
'testForecast'
'grassCuringForecast'
)
expect(itemValue).toEqual('2.2')
expect(itemValue).toEqual('20.0')
})
})
Loading