Create Vision Zero incidents list page - #2074
Conversation
✅ Deploy Preview for vzv-staging canceled.
|
✅ Deploy Preview for vze-staging canceled.
|
| properties: GeoJsonProperties; | ||
| } | ||
|
|
||
| export default function VzMapPopupContent({ |
There was a problem hiding this comment.
I liked searching for Burnet and then toggling the map and seeing most of the incidents lining up with the street, except one or two. The future incident details page is gonna be great
| id: "other", | ||
| column: "responding_agencies_str", | ||
| operator: "_nlike", | ||
| value: "%$apd$%", |
There was a problem hiding this comment.
nice thanks for connecting the earlier comment to this one. it was a good reminder about why we do a similar thing for team members, etc. in the Moped advanced search 🙏
| id: "has_crash_report", | ||
| column: "record_tables_str", | ||
| operator: "_ilike", | ||
| value: "%$crashes$%", |
There was a problem hiding this comment.
And here's how we find incidents that don't have a crashes record—aka there is no crash report.
There was a problem hiding this comment.
An incident cant both have a crash report and not have one, but you can toggle both on as if thats possible. Would it be a big lift to make it so you can only toggle one at a time?
There was a problem hiding this comment.
It would be a lift, yeah. FWIW this behavior is consistent with our column visibility toggles as well 🤷♂️
There was a problem hiding this comment.
okay thats fine then! i figured it would be a big lift
| * Name of the geojson transformer function to use when converting the input table to geojson | ||
| */ | ||
| geojsonTransformerName: "latLon"; | ||
| geojsonTransformerName: "latLon" | "pointFeature"; |
There was a problem hiding this comment.
This lets us consume the geojson Point that comes out of the vz_incident_records_view rather than reconstructing it via lat/lon. Sorta feels like to the right way to have done this from the beginning.
| path: "incidents", | ||
| label: "Incidents", | ||
| icon: FaRoad, | ||
| allowedRoles: ["editor", "vz-admin"], |
There was a problem hiding this comment.
Only editors and admins will be able to see this view while we're in this alpha stage of development
There was a problem hiding this comment.
When I was testing, I saw that the viewer cannot see the incidents page nav link, but they can navigate to it and to the upload non-CR3s page through a url. I can't remember if role-based routing permissions are implemented or relevant to VZE but wanted to check on that.
There was a problem hiding this comment.
oo i think thats a loophole we didnt consider and should fix, i was also able to replicate this.
question though for john -- why is the Incidents list page not allowed for read only?
There was a problem hiding this comment.
We decided to keep the incident page obscured from read-only users while it's under development in order to avoid confusing users. It should be harmless if they somehow manage to reach the page, right?
Good to have a gut check on non-CR3s. Read-only users don't have update permissions on the non-cr3 table, so if they did hack the routing the page is harmless.
It's a bit sloppy that we don't redirect users away from restricted pages or show an error message. I'll open a backlog issue for that 🙏
mddilley
left a comment
There was a problem hiding this comment.
This looks good to ship to me! I asked a couple questions, and I can turn this into an approval real fast after that. Thanks John! Very nice to see what the shift to incidents looks like in the editor! 🙌
| 'exposed under a common schema for cross-type queries and geo-temporal matching.'; | ||
|
|
||
|
|
||
| CREATE MATERIALIZED VIEW vz_incidents_view AS |
There was a problem hiding this comment.
👍 and I clocked this migration at about 7 Mississippis to apply on my machine
| NULLIF( | ||
| ARRAY_AGG(DISTINCT record_incident_number ORDER BY record_incident_number) | ||
| FILTER (WHERE record_incident_number IS NOT NULL), | ||
| ARRAY[]::text[] | ||
| ) as incident_numbers, | ||
| NULLIF( | ||
| ARRAY_AGG(DISTINCT record_table_name ORDER BY record_table_name) | ||
| FILTER (WHERE record_table_name IS NOT NULL), | ||
| ARRAY[]::text[] | ||
| ) as record_tables, |
There was a problem hiding this comment.
are the incident numbers and record tables expected to have corresponding values in their indices? I'm trying to think through how this data is related, and if filtering out null would offset the index in an unexpected way. I might be misunderstanding how the two columns are related!
There was a problem hiding this comment.
each row will always have a record_table, but sometimes an incident number can be null. not a concern here, but you are picking up on potential weirdness with the way to way grab address and point_feature using the sorting logic below: it's possible that either one of these are null, in which case we could potentially grab the address from one member record and the point from a different record. it's an extreme edge case but definitely a detail to iron out with the VZ team.
| path: "incidents", | ||
| label: "Incidents", | ||
| icon: FaRoad, | ||
| allowedRoles: ["editor", "vz-admin"], |
There was a problem hiding this comment.
When I was testing, I saw that the viewer cannot see the incidents page nav link, but they can navigate to it and to the upload non-CR3s page through a url. I can't remember if role-based routing permissions are implemented or relevant to VZE but wanted to check on that.
| id: "other", | ||
| column: "responding_agencies_str", | ||
| operator: "_nlike", | ||
| value: "%$apd$%", |
There was a problem hiding this comment.
nice thanks for connecting the earlier comment to this one. it was a good reminder about why we do a similar thing for team members, etc. in the Moped advanced search 🙏
| operator: "_ilike", | ||
| wildcard: true, | ||
| }, | ||
| searchFields: [{ label: "Address", value: "address" }], |
There was a problem hiding this comment.
Not related to the code changed in this PR but one thing that was a small speed bump for me when testing the search was being able to know which fields that I could search in the list view. I might have missed something or it might be really obvious to users who are familiar with the app but wanted to mention.
There was a problem hiding this comment.
Similarly to Mike, when I followed the test instructions "Search by address, other columns", I tried guessing what other columns to search on (like putting in "ems") and didnt get any results. I couldnt remember if we could search on incident number of not
There was a problem hiding this comment.
Sorry for that confusion: i forgot that i didn't enable searching on other columns. I added searching by Incident number as well—I think that will be valuable 🙏
roseeichelmann
left a comment
There was a problem hiding this comment.
Its so awesome to see the incidents coming together in the UI, ive gotta be honest its been kind of hard for me to wrap my heard around incidents and incident matching, but already getting to see it laid out in this list page and map view its helping my visual brain connect the dots.
i also have a few comments/questions as well and i do think the issue mike pointed out about read only still being able to navigate to the page should be addressed if its intended for them not to see this page, but i am ready to approve asap as well!
chiaberry
left a comment
There was a problem hiding this comment.
This is looking great. I am also curious about the permissions around the incident list page. Is that temporary until the Incident details pages are implemented? I'm thinking editors+ are more informed about things in progress...
| operator: "_ilike", | ||
| wildcard: true, | ||
| }, | ||
| searchFields: [{ label: "Address", value: "address" }], |
There was a problem hiding this comment.
Similarly to Mike, when I followed the test instructions "Search by address, other columns", I tried guessing what other columns to search on (like putting in "ems") and didnt get any results. I couldnt remember if we could search on incident number of not
| properties: GeoJsonProperties; | ||
| } | ||
|
|
||
| export default function VzMapPopupContent({ |
There was a problem hiding this comment.
I liked searching for Burnet and then toggling the map and seeing most of the incidents lining up with the street, except one or two. The future incident details page is gonna be great
| 'exposed under a common schema for cross-type queries and geo-temporal matching.'; | ||
|
|
||
|
|
||
| CREATE MATERIALIZED VIEW vz_incidents_view AS |
| - name: vz_incident | ||
| using: | ||
| foreign_key_constraint_on: vz_incident_id | ||
| - name: vz_incidents_view |
There was a problem hiding this comment.
I just realized that we should name this vz_incidents_list_view to be consistent with our other record views. i'm going to fix this in later because I already have a branch going.


Associated issues
This PR adds a basic VZ incident listing backed by a materialized view. The page is only visible to editors and admins.
Testing
/incidentslist page. It should load normally. Test all the things:Incident details page for ID #<some-number>.Ship list
mainbranch