Ability to view temp records in the crashes list and also soft-delete them #1495

roseeichelmann · 2024-07-31T22:08:37Z

Associated issues

Closes cityofaustin/atd-data-tech#18429

Testing

URL to test:
Local

Steps to test:

Start up your VZ data model, run the migrations and apply metadata from this branch. You can run the cris import script to get some more data.
Create some temp records with varying units, injuries, etc.
Test deleting the temp record. In the database make sure the crash and all its associated units and people records all had their is_deleted columns updated to true.
Make sure the temp records you deleted are no longer showing up in the crashes list page, the fatalities page, or in the dashboard totals.
Will crash if you try to go to the old temp crash details page but I think there is another issue open to address routing to the 404 page so i think we can address this in that PR

Ship list

Check migrations for any conflicts with latest migrations in master branch
Confirm Hasura role permissions for necessary access
Code reviewed
Product manager approved

…te the units and crash for the crash id

roseeichelmann · 2024-08-01T16:43:41Z

atd-vze/src/views/Tools/CreateCrashRecord.js

@@ -231,7 +232,7 @@ const CreateCrashRecord = ({ client }) => {
                      id="hf-case-id"
                      name="hf-case-id"
                      placeholder="Enter Case ID..."
-                      autocomplete="off"
+                      autoComplete="off"


fixes dom warning

roseeichelmann · 2024-08-01T16:44:24Z

atd-vze/src/views/Tools/CreateCrashRecord.js

@@ -193,6 +193,7 @@ const CreateCrashRecord = ({ client }) => {
            created_by: userEmail,
            cris_schema_version: "2023",
            is_temp_record: true,
+            private_dr_fl: false,


default to private_dr_fl = false on temp record creation, meaning the crash will show up in our socrata totals and in the crashes list page, etc

perfect. glad you mentioned Socrata—I think we should the is_temp_record column to public datasets. i'll add it to the todo list :)

roseeichelmann · 2024-08-01T16:44:59Z

atd-toolbox/data_model/python/sql_templates/crashes_list_view.sql

@@ -419,6 +419,7 @@ left join
 left join
    lookups.injry_sev_lkp
    on lookups.injry_sev_lkp.id = crash_injury_metrics_view.crash_injry_sev_id
+where crashes.is_deleted = false


crashes list view only has records that havent been soft deleted now

…here anymore

roseeichelmann · 2024-08-01T17:29:45Z

atd-toolbox/data_model/python/sql_templates/crashes_list_view.sql

@@ -173,6 +173,7 @@ create or replace view person_injury_metrics_view as (
    left join
        public.crashes as crashes
        on units.crash_id = crashes.id
+    where people.is_deleted = false


updating all of these views to filter out soft deleted records

i made a todo that we should index those columns 👍

roseeichelmann · 2024-08-01T19:08:38Z

atd-vze/src/views/Crashes/crashDataMap.js

@@ -3,7 +3,7 @@ export const createCrashDataMap = isTempRecord => {
    {
      title: "Details",
      fields: {
-        crash_id: {
+        record_locator: {


now the temp records wont have an empty space here on the crash details page

roseeichelmann · 2024-08-01T19:10:53Z

atd-vze/src/views/Fatalities/fatalityGridTableParameters.js

@@ -5,13 +5,13 @@ export const fatalityGridTableColumns = {
    label_table: "Year",
    type: "Int",
  },
-  cris_crash_id: {
+  record_locator: {


also to show the temp crash ids of temp records

…htenment

roseeichelmann · 2024-08-01T20:29:23Z

atd-vze/src/views/Tools/CreateCrashRecordTable.js

@@ -147,15 +135,15 @@ const CreateCrashRecordTable = ({ crashesData, loading, error }) => {
                          </Link>
                        </td>
                        <td>{item.case_id}</td>
-                        <td>{item.crash_timestamp}</td>
+                        <td>{formatDateTimeString(item.crash_timestamp)}</td>


previously these werent formatted and were showing up as ugly ISO strings

johnclary

@roseeichelmann this looks great and works great—so nice to have this missing piece in place.

I have a tiny change request to fix that delete confirmation text.

These other things can happily be out of scope, but I want to ask if you would address them in this PR and/or give your feedback:

I don't like this text that says SECONDARY ADDRESS MISSING. I think it would be better to not render anything if the secondary address is null.

It would be nice to change the text that appears in the crash diagram card. For temp records, the text can say "Crash diagrams are not available for user-created crash records" 👇

Lastly—I'm not sure about this one—I noticed that people records do not get created unless the injury severity was fatal or serious. But units do get created no matter what. See this example, where we have three units but only two person records, because the fatal and serious injury count were both 0 for unit 2:

I think ideally we should create a person record with no injury in these cases. Right? I also wonder what we're doing in production today...

I'm happy to open separate issues for these tasks ☝️

johnclary · 2024-08-02T14:07:15Z

atd-vze/src/queries/tempRecords.js

+      }
+    }
+    update_people_cris(
+      where: { units_cri: { crash_id: { _eq: $recordId } } }


awesome to see this nested where! i'm going to rename the relationship units_cri to units_cris in a forthcoming PR that branches from here. that's an artifact of Hasura trying to guess at pluralization 🤖

i was gonna point out the cri thing and ask if we should rename but i forgot, glad you pointed it out!

multiple cri = cris. this whole thing is hilarious to me. Thanks for the laugh, Hasura! 😄 And, thanks y'all for fixing. 🚀

johnclary · 2024-08-02T14:11:02Z

atd-vze/src/views/Fatalities/fatalityGridTableParameters.js

    primary_key: true,
    searchable: true,
    sortable: true,
    label_search: "Search by Crash ID",
    label_table: "Crash ID",
-    type: "Int",
+    type: "String",


ah yes, we should index record_locator as well. i'm tracking that ✅

johnclary · 2024-08-02T14:11:57Z

atd-vze/src/views/Tools/CreateCrashRecord.js

@@ -193,6 +193,7 @@ const CreateCrashRecord = ({ client }) => {
            created_by: userEmail,
            cris_schema_version: "2023",
            is_temp_record: true,
+            private_dr_fl: false,


perfect. glad you mentioned Socrata—I think we should the is_temp_record column to public datasets. i'll add it to the todo list :)

johnclary · 2024-08-02T14:15:59Z

atd-vze/src/views/Tools/CreateCrashRecordTable.js

-  const openModalDelete = crashId => {
-    setDeleteId(crashId);
+  const openModalDelete = recordId => {
+    setDeleteId(recordId);
    toggleModalDelete();


Would you mind updating the warning message inside the modal? Remove "This cannot be undone" since that is no longer true.

johnclary · 2024-08-02T14:16:22Z

atd-vze/src/views/Tools/CreateCrashRecordTable.js

@@ -147,15 +135,15 @@ const CreateCrashRecordTable = ({ crashesData, loading, error }) => {
                          </Link>
                        </td>
                        <td>{item.case_id}</td>
-                        <td>{item.crash_timestamp}</td>
+                        <td>{formatDateTimeString(item.crash_timestamp)}</td>


johnclary · 2024-08-02T14:43:39Z

Here's the issue to fix the 404 handler: cityofaustin/atd-data-tech#18479

roseeichelmann · 2024-08-02T17:51:36Z

@johnclary i addressed all your requested changes, as for your last point, not adding any people unless they are injuries/fatalities is how it currently works in production. i think the vz team truly just cares about injuries and fatalities for temp records and it was a conscious choice to only create ppl records for that. plus, there could be 5 people in a unit and if none of them had any injuries or fatalities and we just auto added 1 person to the unit that wouldnt make much sense either?

johnclary

🚢 🚢 🚢 🚢 🚢 🚢 🚢 🚢

johnclary · 2024-08-02T19:27:20Z

atd-vze/src/views/Crashes/Crash.js

-          )}
+          <h2 className="h2 mb-3">{`${primaryAddress ? primaryAddress : ""} ${
+            secondaryAddress ? "& " + secondaryAddress : ""
+          }`}</h2>


johnclary · 2024-08-02T19:28:48Z

@roseeichelmann great points—I agree with you wrt to those person records 🙏

mddilley

I tested all of the steps, and it is all working as expected. So nice to see these soft deletes!!! This PR really shows off the record locator too! Good stuff!! 🚢 😎 🙌

mddilley · 2024-08-06T00:06:06Z

atd-vze/src/views/Crashes/CrashDiagram.js

+        (isTempRecord ? (
+          <CardBody>
+            <p>
+              Crash diagrams are not available for user-created crash records.


i like this detail!

mddilley · 2024-08-06T00:08:39Z

atd-vze/src/views/Tools/CreateCrashRecordTable.js

-  //     });
-  // };
+  /**
+   * Soft deletes the temporary crash and all its associated unit and people records


thanks for being more specific about the types of records affected in this comment. the old comment threw me for a loop!

mddilley · 2024-08-06T00:13:10Z

atd-vze/src/views/Tools/CreateCrashRecordTable.js

+      variables: { recordId: deleteId, updatedBy: userEmail },
+    })
+      .then(() => {
+        setSuccessfulNewRecordId(null);


at first i was confused about why this needs to be set to null but now i see that this makes the green alert go away if it happens to be there after previously adding a temp record. 👍

yup you got it!

roseeichelmann added 5 commits July 31, 2024 17:06

update metadata permissions etc

8bfb5ee

add is_deleted = false where clause

fbd5886

soft delete mutations

4efedfa

handle soft delete

771726d

default to private dr false on temp record creation

1c59d64

roseeichelmann added the WIP Work in progress label Jul 31, 2024

roseeichelmann added 8 commits August 1, 2024 11:24

add/remove permissions for these tables

b1a4eb1

create a separate soft delete mutation for person records

9fc34eb

pass useremail

e53e78f

loop thru each unit and soft delete all the person records, then dele…

0479b04

…te the units and crash for the crash id

fix dom warning

b551433

dont actually need to query person id here

c1f9c75

update comment

121a69c

format these

c1e6764

roseeichelmann commented Aug 1, 2024

View reviewed changes

roseeichelmann added 5 commits August 1, 2024 12:24

remove is_temp_record to fix inconsistent metadata this doesnt exist …

d90f9d5

…here anymore

add permissions

72fad68

adjust all other views to filter out soft deleted records

4a9180a

add record locator to fatalities view, filter out soft deleted records

337fbd1

use record locator instead of crash id

58d2748

roseeichelmann commented Aug 1, 2024

View reviewed changes

wow there was such a better more simple way to do this i had an enlig…

e7bafc2

…htenment

roseeichelmann commented Aug 1, 2024

View reviewed changes

roseeichelmann removed the WIP Work in progress label Aug 1, 2024

roseeichelmann requested review from frankhereford and chiaberry August 1, 2024 20:29

roseeichelmann requested review from johnclary, tillyw and mddilley August 1, 2024 20:29

roseeichelmann added the WIP Work in progress label Aug 1, 2024

roseeichelmann removed request for frankhereford, chiaberry, johnclary, tillyw and mddilley August 1, 2024 20:41

toggle off successful record alert whenever temp record is deleted

52c42a8

roseeichelmann removed the WIP Work in progress label Aug 1, 2024

roseeichelmann requested review from frankhereford, chiaberry, johnclary, tillyw and mddilley August 1, 2024 20:57

johnclary requested changes Aug 2, 2024

View reviewed changes

roseeichelmann added 4 commits August 2, 2024 12:47

query is_temp_record

7f16699

remove secondary address is missing

8eaa702

different message from missing crash diagram if a temp record

378d0cb

remove the this cannot be undone message

a1b32e8

roseeichelmann requested a review from johnclary August 2, 2024 17:49

johnclary approved these changes Aug 2, 2024

View reviewed changes

mddilley approved these changes Aug 6, 2024

View reviewed changes

roseeichelmann merged commit bc645cf into data-model-v2 Aug 6, 2024

roseeichelmann deleted the 18429_temp_records_soft_delete branch August 6, 2024 16:41

johnclary mentioned this pull request Oct 21, 2024

Remove crashes edits layer #1572

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ability to view temp records in the crashes list and also soft-delete them #1495

Ability to view temp records in the crashes list and also soft-delete them #1495

roseeichelmann commented Jul 31, 2024 •

edited by johnclary

Loading

roseeichelmann Aug 1, 2024

roseeichelmann Aug 1, 2024

johnclary Aug 2, 2024

roseeichelmann Aug 1, 2024

roseeichelmann Aug 1, 2024

johnclary Aug 2, 2024

roseeichelmann Aug 1, 2024

roseeichelmann Aug 1, 2024

roseeichelmann Aug 1, 2024

johnclary Aug 2, 2024

johnclary left a comment •

edited

Loading

johnclary Aug 2, 2024

roseeichelmann Aug 2, 2024

mddilley Aug 5, 2024

johnclary Aug 2, 2024

johnclary Aug 2, 2024

johnclary Aug 2, 2024

johnclary Aug 2, 2024

johnclary commented Aug 2, 2024

roseeichelmann commented Aug 2, 2024

johnclary left a comment

johnclary Aug 2, 2024

johnclary commented Aug 2, 2024

mddilley left a comment

mddilley Aug 6, 2024

mddilley Aug 6, 2024

mddilley Aug 6, 2024

roseeichelmann Aug 6, 2024

Ability to view temp records in the crashes list and also soft-delete them #1495

Ability to view temp records in the crashes list and also soft-delete them #1495

Conversation

roseeichelmann commented Jul 31, 2024 • edited by johnclary Loading

Associated issues

Testing

Ship list

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnclary left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnclary commented Aug 2, 2024

roseeichelmann commented Aug 2, 2024

johnclary left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnclary commented Aug 2, 2024

mddilley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

roseeichelmann commented Jul 31, 2024 •

edited by johnclary

Loading

johnclary left a comment •

edited

Loading