Skip to content

Commit b47f926

Browse files
author
Ada Ball
committed
Merge branch 'main' into ab/app-358/nbs-custom-testing
2 parents 26b18e0 + 33d0fb9 commit b47f926

18 files changed

Lines changed: 841 additions & 2339 deletions

File tree

‎apps/modernization-api/src/main/java/gov/cdc/nbs/report/ReportSpecBuilder.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,10 @@ public ReportSpec build() {
7777
String dataSourceName =
7878
dataSourceNameUtils.buildDataSourceName(reportConfig.dataSource().name());
7979
List<ReportColumn> columns = fetchColumns();
80+
List<List<String>> columnMap = null;
81+
if (columns != null) {
82+
columnMap = columns.stream().map(c -> List.of(c.name(), c.title())).toList();
83+
}
8084

8185
String selectClause = buildSelectClause(columns);
8286
String fromClause = String.format("FROM %s", dataSourceName);
@@ -89,7 +93,14 @@ public ReportSpec build() {
8993
String.join(" ", selectClause, fromClause, whereClause, orderByClause).trim();
9094

9195
return new ReportSpec(
92-
isExport, isBuiltin, reportTitle, libraryName, dataSourceName, subsetQuery, daysValue);
96+
isExport,
97+
isBuiltin,
98+
reportTitle,
99+
libraryName,
100+
dataSourceName,
101+
subsetQuery,
102+
columnMap,
103+
daysValue);
93104
}
94105

95106
private Integer extractDaysValue() {

‎apps/modernization-api/src/main/java/gov/cdc/nbs/report/models/ReportSpec.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package gov.cdc.nbs.report.models;
22

33
import com.fasterxml.jackson.annotation.JsonProperty;
4+
import java.util.List;
45

56
public record ReportSpec(
67
@JsonProperty(value = "is_export", required = true) boolean isExport,
@@ -9,4 +10,5 @@ public record ReportSpec(
910
@JsonProperty(value = "library_name", required = true) String libraryName,
1011
@JsonProperty(value = "data_source_name", required = true) String dataSourceName,
1112
@JsonProperty(value = "subset_query", required = true) String subsetQuery,
13+
@JsonProperty(value = "column_map", required = false) List<List<String>> columnMap,
1214
@JsonProperty(value = "days_value") Integer daysValue) {}

‎apps/modernization-api/src/test/java/gov/cdc/nbs/report/ReportServiceTest.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,7 @@ void executeReport_should_return_response_when_report_exists_and_runner_is_pytho
565565
"nbs_custom",
566566
"[NBS_ODSE].[dbo].[PHCDemographic]",
567567
"SELECT * FROM [NBS_ODSE].[dbo].[PHCDemographic]",
568+
null,
568569
null);
569570
try (MockedConstruction<ReportSpecBuilder> specBuilderMock =
570571
mockConstruction(

‎apps/modernization-api/src/test/java/gov/cdc/nbs/report/models/ReportSpecTest.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ void should_create_report_spec() {
1616
"nbs_custom",
1717
"nbs_rdb.investigation",
1818
"SELECT * FROM [NBS_ODSE].[dbo].[NBS_configuration]",
19+
null,
1920
11);
2021

2122
assertThat(reportSpec.isBuiltin()).isTrue();

‎apps/modernization-ui/package-lock.json‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎apps/modernization-ui/src/apps/report/run/ReportConfigurationPage.tsx‎

Lines changed: 41 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,23 +14,45 @@ import layoutStyles from '../layout/layout.module.scss';
1414
import { Required } from 'design-system/entry';
1515
import { InPageNavigation } from 'design-system/inPageNavigation';
1616

17-
const SECTIONS = [
17+
const BASIC_SECTIONS = [
18+
{
19+
title: 'Time',
20+
id: 'basic-time',
21+
filterTypes: ['BAS_TIM_RANGE', 'BAS_TIM_RANGE_LIST', 'BAS_MM_YYYY_RANGE', 'BAS_TIM_RANGE_CUSTOM', 'BAS_DAYS'],
22+
},
23+
{
24+
title: 'Condition',
25+
id: 'basic-condition',
26+
filterTypes: ['BAS_CON_LIST', 'BAS_CVG_LIST'],
27+
},
28+
{
29+
title: 'Geographic area',
30+
id: 'basic-geography',
31+
filterTypes: ['BAS_JUR_LIST'],
32+
},
1833
{
19-
title: 'Basic filters',
20-
id: 'basic-filters',
21-
hasData: (config: ReportConfiguration) => config.basicFilters.length > 0,
34+
title: 'Other filters',
35+
id: 'basic-other',
36+
filterTypes: ['BAS_TXT', 'BAS_STD_HIV_WRKR'],
37+
},
38+
];
39+
40+
const SECTIONS = [
41+
...BASIC_SECTIONS.map(({ title, id, filterTypes }) => ({
42+
title,
43+
id,
44+
hasData: (config: ReportConfiguration) =>
45+
config.basicFilters.some((f) => filterTypes.includes(f.filterType.type!)),
2246
Component: ({ config, id, title }: { config: ReportConfiguration; id: string; title: string }) => (
23-
<CurrentStateProvider
24-
stateFilter={config.basicFilters.find((f) => f.filterType.code?.startsWith(STATE_FILTER_CODE))}
25-
>
26-
<Card id={id} title={title} collapsible={true}>
27-
{config.basicFilters.map((filter, i) => (
47+
<Card id={id} title={title} collapsible={true}>
48+
{config.basicFilters
49+
.filter((filter) => filterTypes.includes(filter.filterType.type!))
50+
.map((filter, i) => (
2851
<BasicFilter key={`basic_filter_${i}`} filter={filter} columns={config.columns} />
2952
))}
30-
</Card>
31-
</CurrentStateProvider>
53+
</Card>
3254
),
33-
},
55+
})),
3456
{
3557
title: 'Advanced filter',
3658
id: 'advanced-filter',
@@ -87,9 +109,13 @@ const ReportConfigurationPage = ({
87109
<InPageNavigation sections={sectionData.map(({ id, title }) => ({ id, label: title }))} />
88110
</aside>
89111
<form className={layoutStyles.columnContent}>
90-
{sectionData.map(({ id, title, Component }) => (
91-
<Component key={id} config={config} id={id} title={title} />
92-
))}
112+
<CurrentStateProvider
113+
stateFilter={config.basicFilters.find((f) => f.filterType.code?.startsWith(STATE_FILTER_CODE))}
114+
>
115+
{sectionData.map(({ id, title, Component }) => (
116+
<Component key={id} config={config} id={id} title={title} />
117+
))}
118+
</CurrentStateProvider>
93119
</form>
94120
</ReportLayout>
95121
);

‎apps/modernization-ui/src/apps/report/run/ReportRunPage.spec.tsx‎

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -379,12 +379,14 @@ describe('report run page', () => {
379379
.mocked(generated.ReportControllerService.exportReport)
380380
.mockResolvedValue(MOCK_RESULT);
381381

382-
const { getByRole, findByRole, findByLabelText, container } = renderWithRouter();
382+
const { getByRole, findByRole, findAllByText, findByLabelText, container } = renderWithRouter();
383383

384384
expect(getByRole('status')).toHaveTextContent('Loading');
385385

386386
expect(mockConfigApi).toHaveBeenCalled();
387387

388+
expect(await findAllByText('Other filters')).toHaveLength(2);
389+
388390
const input = await findByLabelText('Full Name');
389391
await userEvent.type(input, 'test');
390392

@@ -494,12 +496,14 @@ describe('report run page', () => {
494496
.mocked(generated.ReportControllerService.exportReport)
495497
.mockResolvedValue(MOCK_RESULT);
496498

497-
const { getByRole, findByRole, findByLabelText, container } = renderWithRouter();
499+
const { getByRole, findByRole, findAllByText, findByLabelText, container } = renderWithRouter();
498500

499501
expect(getByRole('status')).toHaveTextContent('Loading');
500502

501503
expect(mockConfigApi).toHaveBeenCalled();
502504

505+
expect(await findAllByText('Time')).toHaveLength(2);
506+
503507
expect(await findByLabelText('Full Name')).toBeVisible();
504508
const fromInput = await findByLabelText('From');
505509
const toInput = await findByLabelText('To');
@@ -621,12 +625,14 @@ describe('report run page', () => {
621625
.mocked(generated.ReportControllerService.exportReport)
622626
.mockResolvedValue(MOCK_RESULT);
623627

624-
const { getByRole, findByRole, findByLabelText, container } = renderWithRouter();
628+
const { getByRole, findByRole, findAllByText, findByLabelText, container } = renderWithRouter();
625629

626630
expect(getByRole('status')).toHaveTextContent('Loading');
627631

628632
expect(mockConfigApi).toHaveBeenCalled();
629633

634+
expect(await findAllByText('Time')).toHaveLength(2);
635+
630636
expect(await findByLabelText('Full Name')).toBeVisible();
631637
const fromInput = await findByLabelText('From');
632638
const toInput = await findByLabelText('To');
@@ -756,12 +762,14 @@ describe('report run page', () => {
756762
.mocked(generated.ReportControllerService.exportReport)
757763
.mockResolvedValue(MOCK_RESULT);
758764

759-
const { getByRole, findByRole, findByLabelText, container } = renderWithRouter();
765+
const { getByRole, findByRole, findAllByText, findByLabelText, container } = renderWithRouter();
760766

761767
expect(getByRole('status')).toHaveTextContent('Loading');
762768

763769
expect(mockConfigApi).toHaveBeenCalled();
764770

771+
expect(await findAllByText('Time')).toHaveLength(2);
772+
765773
expect(await findByLabelText('Full Name')).toBeVisible();
766774
const fromMonthInput = await findByLabelText('From Month');
767775
await userEvent.selectOptions(fromMonthInput, '1');
@@ -905,12 +913,14 @@ describe('report run page', () => {
905913
{ value: '04', name: 'Arizona' },
906914
]);
907915

908-
const { getByRole, findByRole, findByLabelText, container } = renderWithRouter();
916+
const { getByRole, findByRole, findAllByText, findByLabelText, container } = renderWithRouter();
909917

910918
expect(getByRole('status')).toHaveTextContent('Loading');
911919

912920
expect(mockConfigApi).toHaveBeenCalled();
913921

922+
expect(await findAllByText('Geographic area')).toHaveLength(2);
923+
914924
expect(await findByRole('option', { name: 'Georgia' })).toBeVisible();
915925

916926
// component refreshes when options populates, so can't do this earlier
@@ -1220,12 +1230,14 @@ describe('report run page', () => {
12201230
.mockResolvedValue(MOCK_RESULT);
12211231
vi.mocked(options.selectableResolver).mockImplementation(mockOptionApiImpl);
12221232

1223-
const { getByRole, findByRole, findByLabelText, container } = renderWithRouter();
1233+
const { getByRole, findByRole, findAllByText, findByLabelText, container } = renderWithRouter();
12241234

12251235
expect(getByRole('status')).toHaveTextContent('Loading');
12261236

12271237
expect(mockConfigApi).toHaveBeenCalled();
12281238

1239+
expect(await findAllByText('Geographic area')).toHaveLength(2);
1240+
12291241
expect(await findByRole('option', { name: 'Dekalb County' })).toBeVisible();
12301242

12311243
// component refreshes when options populates, so can't do this earlier
@@ -1552,12 +1564,14 @@ describe('report run page', () => {
15521564
.mockResolvedValue(MOCK_RESULT);
15531565
vi.mocked(options.selectableResolver).mockImplementation(mockOptionApiImpl);
15541566

1555-
const { getByRole, findByRole, findByLabelText, container } = renderWithRouter();
1567+
const { getByRole, findByRole, findAllByText, findByLabelText, container } = renderWithRouter();
15561568

15571569
expect(getByRole('status')).toHaveTextContent('Loading');
15581570

15591571
expect(mockConfigApi).toHaveBeenCalled();
15601572

1573+
expect(await findAllByText('Condition')).toHaveLength(2);
1574+
15611575
expect(await findByRole('option', { name: '2019 Novel Coronavirus' })).toBeVisible();
15621576

15631577
// component refreshes when options populates, so can't do this earlier
@@ -1829,12 +1843,14 @@ describe('report run page', () => {
18291843
.mockResolvedValue(MOCK_RESULT);
18301844
vi.mocked(useConceptOptions).mockReturnValue(mockOptionApiImpl);
18311845

1832-
const { getByRole, findByRole, findByLabelText, container } = renderWithRouter();
1846+
const { getByRole, findByRole, findAllByText, findByLabelText, container } = renderWithRouter();
18331847

18341848
expect(getByRole('status')).toHaveTextContent('Loading');
18351849

18361850
expect(mockConfigApi).toHaveBeenCalled();
18371851

1852+
expect(await findAllByText('Condition')).toHaveLength(2);
1853+
18381854
expect(await findByRole('option', { name: '100 - Chancroid' })).toBeVisible();
18391855

18401856
expect(useConceptOptions).toHaveBeenCalledWith('CASE_DIAGNOSIS_STD', { lazy: false });
@@ -2103,12 +2119,14 @@ describe('report run page', () => {
21032119
.mocked(generated.ReportControllerService.exportReport)
21042120
.mockResolvedValue(MOCK_RESULT);
21052121

2106-
const { getByRole, findByRole, findByLabelText, container } = renderWithRouter();
2122+
const { getByRole, findByRole, findAllByText, findByLabelText, container } = renderWithRouter();
21072123

21082124
expect(getByRole('status')).toHaveTextContent('Loading');
21092125

21102126
expect(mockConfigApi).toHaveBeenCalled();
21112127

2128+
expect(await findAllByText('Time')).toHaveLength(2);
2129+
21122130
const input = await findByLabelText('Duplicate Investigations Time Frame');
21132131
await user.type(input, '5');
21142132

@@ -2322,12 +2340,14 @@ describe('report run page', () => {
23222340
.mockResolvedValue(MOCK_RESULT);
23232341
vi.mocked(options.selectableResolver).mockImplementation(mockOptionApiImpl);
23242342

2325-
const { getByRole, findByRole, findByLabelText, container } = renderWithRouter();
2343+
const { getByRole, findByRole, findAllByText, findByLabelText, container } = renderWithRouter();
23262344

23272345
expect(getByRole('status')).toHaveTextContent('Loading');
23282346

23292347
expect(mockConfigApi).toHaveBeenCalled();
23302348

2349+
expect(await findAllByText('Other filters')).toHaveLength(2);
2350+
23312351
expect(await findByRole('option', { name: 'Jyn Erso' })).toBeVisible();
23322352

23332353
// component refreshes when options populates, so can't do this earlier

‎apps/modernization-ui/src/design-system/input/numeric/NumericInput.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ type NumericInputProps = {
88
sizing?: Sizing;
99
error?: string;
1010
required?: boolean;
11+
helperText?: string;
1112
} & NumericProps;
1213

1314
const NumericInput = ({
@@ -18,6 +19,7 @@ const NumericInput = ({
1819
error,
1920
required,
2021
placeholder,
22+
helperText,
2123
...remaining
2224
}: NumericInputProps) => {
2325
return (
@@ -28,6 +30,7 @@ const NumericInput = ({
2830
htmlFor={id}
2931
required={required}
3032
error={error}
33+
helperText={helperText}
3134
>
3235
<Numeric id={id} placeholder={placeholder} {...remaining} />
3336
</EntryWrapper>

‎apps/report-execution/src/errors.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,16 @@ def __init__(self, library_name: str, is_builtin: bool):
1818
super().__init__(message, 422)
1919

2020

21+
class MissingColumnError(BaseReportExecutionError):
22+
"""Required columns are missing."""
23+
24+
def __init__(self, missing_columns: list[str]):
25+
message = f"""
26+
Required columns {', '.join(missing_columns)} are missing from column selection
27+
"""
28+
super().__init__(message, 422)
29+
30+
2131
class ResultTooBigError(BaseReportExecutionError):
2232
"""The returned results are larger than allowed by configuration."""
2333

‎apps/report-execution/src/execute_report.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ def execute_report(report_spec: models.ReportSpec):
2424
subset_query=report_spec.subset_query,
2525
data_source_name=report_spec.data_source_name,
2626
days_value=report_spec.days_value,
27+
column_map=report_spec.column_map,
2728
)
2829

2930
check_valid_result(result, report_spec)

0 commit comments

Comments
 (0)