feat(admin): Ballot Count Report Builder #4019
Conversation
…sion, but do not use those dimensions in existing tally report builder
currently it assumes it will be used from the tally report builder, but that will change soon. so make it more generic, and include some information about the report printed
…d naming the csv export button is not easily re-used for the ballot count reports, so just creating separate components
…SV' for uniformity across buttons and across report types
… the existing button for tally reports
almost all uses of this will be removed in the next PR, it was easier to just rename this and make a new method rather than try to update existing usages, because grout is picky and it's always a lot of mocking work
this makes the ballot count reports match the tally reports, and allows us to always include the election title
in subsequent commit, will be used for the report builder. in subsequent PRs, will be used for pre-defined ballot count report pages as well
… which is more readable
adghayes
changed the title
adghayes
force-pushed
the
drew/ballot-count-report-builder
branch
from
46f1472 to
69003aa
Compare
adghayes
requested review from
arsalansufi and
kofi-q
and removed request for
a team and
arsalansufi
|
@kofi-q don't worry too much about the button-full layout on the reports screen, next PR is gonna completely remix that. |
| const reportProperties = { | ||
| filter: JSON.stringify(filter), | ||
| groupBy: JSON.stringify(groupBy), | ||
| } as const; |
There was a problem hiding this comment.
This is a brute force approach to adding information in logging, but I think it's valuable to include this information and, due to the infrequency with which logs will be looked at, not worth investing time in something prettier.
|
|
||
| if (reportRank === 2) { | ||
| if (precinctId && votingMethod) { | ||
| if (partyId) { |
There was a problem hiding this comment.
A lot of the churn here is just moving things around to make this a little more readable, may wan to review side by side.
| ); | ||
| if (reportRank === 2) { | ||
| // Party + Other | ||
| if (partyId) { |
There was a problem hiding this comment.
These cases were not relevant before because the tally reports cannot have a partyId, but the ballot count reports can.
| }${titleWithoutOfficiality}`; | ||
| const customFilter = !titleGeneration.isOk() ? filter : undefined; | ||
|
|
||
| return BallotCountReport({ |
There was a problem hiding this comment.
haven't seen this render pattern before - is it solving a particular problem? wondering if it ends up bypassing some of React's component lifecycle management in unexpected ways
There was a problem hiding this comment.
Hm, I didn't notice I was doing anything special in terms of rendering patterns. I think of Reports as a locally defined React component, which wraps BallotCountReport. It just generates the component, which is either saved as state in the larger component as the preview, or passed to the print methods. I had to abstract the reports component since it has multiple destinations within the Viewer. Do you see another way to do it?
There was a problem hiding this comment.
Ah, I meant that BallotCountReport is defined as a component and usually, we'd render that as:
return (
<BallotCountReport
title={title}
electionDefinition={electionDefinition}
// etc
/>
);whereas, this is just calling BallotCountReport() directly as a function here - IIRC, there may be some subtle differences and probably not worth breaking pattern unless there's a need to (if for nothing at all, maybe just for code consistency)
There was a problem hiding this comment.
Oh, I see. Will change it in an upcoming PR where I touch this file!
definitely review by commit
Overview
Closes #3991. Adds a ballot count report builder. Adds infrastructure for preset ballot count reports in next PR.
Demo Video or Screenshot
ballot-count-report-builder.mov
Testing Plan
Added high automated test coverage for new components and tested manually.
Checklist