Add Closure to CSV Download #1249
Conversation
There was a problem hiding this comment.
Nice, clear, correct implementation. I like seeing a new JS test.
While this does match the spec of the feature as listed in the issue, when I take a look at the CSV after closing a facility and reopening one I see an opportunity to make this a little friendlier to less technical users
An alternate formatting I came up with is to output the string "CLOSED" in the is_closed column if Facility.is_closed is True, otherwise leave the column blank. This reduces the number of possible states from 3 to 2 and avoids some of the "double negativeness" of having True representing closed. I'm interested in what you think about this alternate formatting, if you have another suggestion, or think it is more appropriate to keep it as is currently implemented.
| @@ -38,6 +38,7 @@ export function logDownload() { | |||
|
|
|||
| const vectorTileFlagIsActive = get(featureFlags, 'flags.vector_tile', false); | |||
| const ppeIsActive = get(featureFlags, 'flags.ppe', false); | |||
| const reportsAreActive = get(featureFlags, 'flags.report_a_facility', false); | |||
There was a problem hiding this comment.
👍 Nice integration with our new switch
TaiWilkin
force-pushed
the
tw/add-report-waffle-switch
branch
from
5397050
to
3283fc6
Compare
|
I agree that 'CLOSED' is going to be clearer for non-technical users. I've updated the code to use 'CLOSED' when True and otherwise display nothing, and updated the test to check for handling both True and False. |
Add the facility closure status to the CSV downloads in the case that the report_a_facility switch is on.
TaiWilkin
force-pushed
the
tw/add-is-closed-csv
branch
from
d4957e1
to
104ddc1
Compare
Overview
Add the facility closure status to the CSV downloads in the case that
the
report_a_facilityswitch is on.Connects #1248
Demo
Testing Instructions
./scripts/serverChecklist
fixup!commits have been squashed