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.

Sign up for GitHub

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

Jump to bottom

[Frontend][Review Metrics] Review Metrics Playtesting follow-ups #32

Merged
merged 5 commits into from
Sep 21, 2022
Merged

[Frontend][Review Metrics] Review Metrics Playtesting follow-ups #32

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dc9b06f
fix overwritten values count
terryttsai Sep 20, 2022
03cdcc5
round decimals to 2 places
terryttsai Sep 21, 2022
788b41d
allow continuing if upload has errors
terryttsai Sep 21, 2022
eccdddc
remove rounding decimals
terryttsai Sep 21, 2022
3080909
no metrics to review
terryttsai Sep 21, 2022
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
26 changes: 11 additions & 15 deletions publisher/src/components/DataUpload/UploadErrorsWarnings.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,7 @@ export const UploadErrorsWarnings: React.FC<UploadErrorsWarningsProps> = ({
removeSnakeCase(selectedSystem).toLowerCase()}
</a>
</span>{" "}
system. To continue, please resolve the errors in your file and
reupload.
system.
terryttsai marked this conversation as resolved.
Show resolved Hide resolved
</UserPromptDescription>
</>
)}
Expand All @@ -164,19 +163,16 @@ export const UploadErrorsWarnings: React.FC<UploadErrorsWarningsProps> = ({
<UploadErrorButtonWrapper>
<Button onClick={resetToNewUpload}>New Upload</Button>

{/* (TODO(#15195): Placeholder - this should navigate to the confirmation component */}
{hasWarningsOnly && (
<Button
onClick={() =>
navigate("/review-metrics", {
state: metrics,
replace: true,
})
}
>
Continue
</Button>
)}
<Button
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a screenshot showing what this page looks like now if there are errors? I think there will be two calls to action -- a blue button on the left that says New Upload and a blue button on the right that says Continue. Is that right? That's fine for now (we need input on product and design on this) but just want to make sure I know the current behavior. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added screenshot in the description

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only the button on the right is blue

onClick={() =>
navigate("/review-metrics", {
state: metrics,
replace: true,
})
}
>
Continue
</Button>
</UploadErrorButtonWrapper>

{/* Messages */}
Expand Down
95 changes: 59 additions & 36 deletions publisher/src/components/ReviewMetrics/ReviewMetrics.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,6 @@ const ReviewMetrics: React.FC = observer(() => {
}, {} as { [key: string]: number });

metric.datapoints.forEach((dp) => {
if (dp.old_value !== null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for this fix!! Are the duplicate values on the backend causing any other issues? We should definitely fix the root cause on the backend, just curious if there are any other patches we need to do on the frontend in the meantime.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, this is the issue to fix: #31, and on the frontend, I'm rooting out duplicate datapoints before using it and hadn't found any more issues with this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds good!

overwrittenValuesCount += 1;
}
if (dp.disaggregation_display_name && dp.dimension_display_name) {
if (!disaggregationRowData[dp.disaggregation_display_name]) {
disaggregationRowData[dp.disaggregation_display_name] = {};
Expand All @@ -122,11 +119,23 @@ const ReviewMetrics: React.FC = observer(() => {
dp.dimension_display_name
] = [];
}
disaggregationRowData[dp.disaggregation_display_name][
dp.dimension_display_name
][startDatesIndexLookup[dp.start_date]] = dp;
} else {
if (
!disaggregationRowData[dp.disaggregation_display_name][
dp.dimension_display_name
][startDatesIndexLookup[dp.start_date]]
) {
disaggregationRowData[dp.disaggregation_display_name][
dp.dimension_display_name
][startDatesIndexLookup[dp.start_date]] = dp;
if (dp.old_value !== null) {
overwrittenValuesCount += 1;
}
}
} else if (!aggregateRowData[startDatesIndexLookup[dp.start_date]]) {
aggregateRowData[startDatesIndexLookup[dp.start_date]] = dp;
if (dp.old_value !== null) {
overwrittenValuesCount += 1;
}
}
});

Expand All @@ -137,7 +146,7 @@ const ReviewMetrics: React.FC = observer(() => {
<SectionTitle>{metric.display_name}</SectionTitle>
{overwrittenValuesCount > 0 && (
<SectionTitleOverwrites>
* {overwrittenValuesCount} Overwrite
* {overwrittenValuesCount} Overwritten Value
terryttsai marked this conversation as resolved.
Show resolved Hide resolved
{overwrittenValuesCount !== 1 ? "s" : ""}
</SectionTitleOverwrites>
)}
Expand Down Expand Up @@ -208,14 +217,10 @@ const ReviewMetrics: React.FC = observer(() => {
</DatapointsTableDetailsRowHead>
<DatapointsTableDetailsRowBody>
<DatapointsTableDetailsRow>
{aggregateRowData.map((dp, index) => (
{aggregateRowData.map((dp, index) =>
// row data could be null, so no distinct key given in that case
// eslint-disable-next-line react/no-array-index-key
<DatapointsTableDetailsCell key={index}>
{dp.value}
{dp.old_value !== null ? <OrangeText>*</OrangeText> : ""}
</DatapointsTableDetailsCell>
))}
renderDatapointsValue(index, dp.value, dp.old_value)
)}
</DatapointsTableDetailsRow>
{Object.entries(disaggregationRowData).map(
([disaggregation, dimension]) => (
Expand All @@ -225,17 +230,9 @@ const ReviewMetrics: React.FC = observer(() => {
.sort(([a], [b]) => sortDatapointDimensions(a, b))
.map(([key, dps]) => (
<DatapointsTableDetailsRow key={key}>
{dps.map((dp, index) => (
// eslint-disable-next-line react/no-array-index-key
<DatapointsTableDetailsCell key={index}>
{dp.value}
{dp.old_value !== null ? (
<OrangeText>*</OrangeText>
) : (
""
)}
</DatapointsTableDetailsCell>
))}
{dps.map((dp, index) =>
renderDatapointsValue(index, dp.value, dp.old_value)
)}
</DatapointsTableDetailsRow>
))}
</React.Fragment>
Expand All @@ -248,6 +245,22 @@ const ReviewMetrics: React.FC = observer(() => {
);
};

const renderDatapointsValue = (
key: React.Key,
value: string | number | null,
oldValue: string | number | null
) => {
if (value === null) {
return null;
}
return (
<DatapointsTableDetailsCell key={key}>
{value}
{oldValue !== null ? <OrangeText>*</OrangeText> : ""}
</DatapointsTableDetailsCell>
);
};

return (
<Container>
<DataUploadHeader transparent={false}>
Expand All @@ -264,20 +277,30 @@ const ReviewMetrics: React.FC = observer(() => {
</DataUploadHeader>
<MainPanel>
<Heading>
Review <span>{filteredMetrics.length}</span> Metrics
{filteredMetrics.length > 0 ? (
<>
Review <span>{filteredMetrics.length}</span> Uploaded Metrics
</>
) : (
"No Metrics to Review"
)}
</Heading>
<Subheading>
Take a moment to review the changes. If you believe there is an error,
please contact the Justice Counts team via{" "}
<a href="mailto:[email protected]">
[email protected]
</a>
{filteredMetrics.length > 0 ? (
<>
Your data has been successfully uploaded. Take a moment to review
the changes. If you believe there is an error, please contact the
Justice Counts team via{" "}
<a href="mailto:[email protected]">
[email protected]
</a>
</>
) : (
"Uploaded file contains no metrics to review."
)}
</Subheading>
{filteredMetrics.map((metric, idx) => {
if (metric.datapoints.length > 0) {
return renderSection(metric, idx);
}
return null;
return renderSection(metric, idx);
terryttsai marked this conversation as resolved.
Show resolved Hide resolved
})}
</MainPanel>
</Container>
Expand Down
4 changes: 2 additions & 2 deletions publisher/src/shared/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,8 +211,8 @@ export interface RawDatapoint {
metric_display_name: string | null;
disaggregation_display_name: string | null;
dimension_display_name: string | null;
value: string | number;
old_value: string | number;
value: string | number | null;
old_value: string | number | null;
is_published: boolean;
frequency: ReportFrequency;
}
Expand Down