Adds ability to transfer data from temporary crashes when they are deleted via modal by mateoclarke · Pull Request #1968 · cityofaustin/vision-zero

mateoclarke · 2026-02-19T16:44:41Z

Associated issues

Adds the ability to transfer data from a temporary crash record to another crash before deleting it. When a user clicks "Delete" on a temporary crash, a modal opens that lets them:

Search for a target crash by Crash ID or primary address
Transfer any combination of: notes, FRB recommendation, ~~edited card fields (Summary/Flags/Other)~~, lat/lon and victim photo
Skip transfer and delete the temp crash without transferring anything

The victim photo transfer is handled via a new Flask API endpoint that copies the image in S3 and updates the target person's metadata — this avoids permission issues with accessing image metadata through GraphQL.
Also refactors the repeated @cross_origin header lists in server.py into a reusable CORS_HEADERS constant.

One limitation of the victim photo transfer is that it only works if the source and target crash each only have one fatality (which is the majority of cases). Otherwise, extra logic would be required to try to match persons between the source and target crashes, which could be considered as a future enhancement.

Testing

URL to test: Local

Steps to test:

Start up your local stack, including the user/image API by running: ./vision-zero local-stack-up
Navigate to a temporary crash record (one with a T prefix in the record locator). You can use the table filter to only show temp records (down at the bottom of toggles under "Internal"). Or you can create a new Temporary record yourself.
- Check the "Record history" at the bottom of the page. If there isn't much, edit some fields like: notes, FRB recommendation, latitude, longitude, fields on the summary, flags, and other cards.
Click the Delete button in the yellow banner
Test transfer flow:
- Search for a target crash by typing a Crash ID or address
- Select a result from the dropdown
- Verify the "The following will be transferred" summary lists the correct edit items
- Click Delete and confirm you are redirected to the target crash
- On the target crash, verify the transferred data is present (notes, recommendations, and victim photo if both crashes had exactly one fatality)
Test skip-transfer flow:
- Open the delete modal again on a different temp crash
- Toggle "I don't want to transfer data"
- Click Delete and confirm the crash is deleted and you are redirected to /crashes
Test photo transfer specifically:
- Find a temp crash with a fatality. Using that crash ID, search the fatality list table and navigate to the temp crash's related fatality page.
- Upload a photo to the victim record
- Search for a target crash that also has exactly one fatality
- Verify "Victim photo" appears in the transfer summary
- Complete the transfer and verify the photo appears on the target crash's fatality
- Bonus: Try it again but select a target record without any fatalities or with more than 1 fatality and confirm the photo doesn't transfer.

Ship list

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

netlify · 2026-02-19T16:46:01Z

✅ Deploy Preview for atd-vze-staging ready!

Name	Link
🔨 Latest commit	`68f1950`
🔍 Latest deploy log	https://app.netlify.com/projects/atd-vze-staging/deploys/69a76f9baff1920008d21e36
😎 Deploy Preview	https://deploy-preview-1968--atd-vze-staging.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

mateoclarke · 2026-02-24T20:02:20Z

editor/components/CrashIsTemporaryBanner.tsx

+      <DeleteTemporaryCrashModal
+        show={showDeleteModal}
+        onHide={() => setShowDeleteModal(false)}
+        crash={crash}


Here's where we pass in the full crash data for the modal.

mateoclarke · 2026-02-24T20:04:08Z

editor/components/CrashIsTemporaryBanner.tsx


 interface CrashIsTemporaryBannerProps {
-  crashId: number;
+  crash: Crash;


Decided to update this component to use the full crash data object instead of just an ID to avoid a duplicative fetch once the delete/transfer modal opens. Now we can pass the full object into the delete modal.

mateoclarke · 2026-02-25T04:06:22Z

api/server.py

 EDITOR_ROLE_NAME = "editor"
 MAX_IMAGE_SIZE_MEGABYTES = 5
 CORS_URL = "*"
+CORS_HEADERS = ["Content-Type", "Authorization", "Access-Control-Allow-Origin", CORS_URL]


This wasn't in scope, but when I was in this file, I just noticed how much this array was being repeated for every route and thought I could DRY it up a bit. Hope that's ok. I think there was one route that differed, so I assumed that's intentional and left it as it.

I'm glad you did this -- you cleaned up a lot of repetition, I think this is good.

mateoclarke · 2026-02-25T04:07:07Z

api/server.py

+    return _transfer_person_image(source_person_id, target_person_id, s3)
+
+
+@app.route("/images/crash_diagram/<record_locator>", methods=["GET"])


this is the important part of the changes in this file.

mateoclarke · 2026-02-25T17:08:56Z

api/utils/images.py

    return new_image_obj_key


+def _transfer_person_image(source_person_id, target_person_id, s3):


I was a little confused about the use of the underscore prefixes in this file, but I went with the flow and named this method similarly. My understanding is that the leading _ signals (but doesn't enforce) a private method. Meaning, this handler method should be internal to the module and not exported for use in other files. See this article for a longer explanation.

But we do import this method, along with a bunch of similar handler methods in server.py#32. I missed this detail in the initial code review, which probably would've been the appropriate time to discuss our use of this convention. But it's an interesting pythonic detail so worth noting now too.

+1, I have heard of this convention and recognize it when I see it. Thanks for pointing it out -- maybe a spin off issue for the backlog to polish it up?

Hmm yeah thanks for going with the flow. I agree these are not private methods in conventional sense and would support removing the leading underscore.

frankhereford

Wow, thanks for this big chunk of work Mateo. There's a lot going on here -- I spent an hour reading over the code, and I think it's really well done. In particular, I really like the type-ahead component.

It tested out perfectly as I worked through the steps you provided.

Of my comments, I would consider the request about the updated_by audit field to be the most important.

I don't see anything that should hold this back from shipping; thanks again. 🚢 🚢 🚢

frankhereford · 2026-03-03T15:25:33Z

api/utils/images.py

    return new_image_obj_key


+def _transfer_person_image(source_person_id, target_person_id, s3):


+1, I have heard of this convention and recognize it when I see it. Thanks for pointing it out -- maybe a spin off issue for the backlog to polish it up?

frankhereford · 2026-03-03T15:42:04Z

api/utils/images.py

+    source_image_source = make_hasura_request(
+        query=GET_PERSON_IMAGE_METADATA_FULL,
+        variables={"person_id": source_person_id},
+    )["people_by_pk"]["image_source"]


There is a try/except guardrail in use around the S3 call, but not around these other API calling functions. Is this OK, should there be more exception handling?

I see we only have a pass in the HasuraAPIError so if there is a problem, our flask app is going to return a 200 if there is an upstream problem with the graphql endpoint. I think based on that, it seems reasonable to want to trap not getting back the data expected so a 500 with a description can be sent, instead of a potentially app-breaking empty message.

In particular, I'm talking about the API failing here, not the potential follow-on NoneType error.

As a distant, secondary thought, if you were to split the API call and the deep accessing of a value out of the returned dictionary -- splitting those two operations into two "lines" of code seems like a marginally better way to represent the two steps being taken.

I agree it's odd we don't have any handlers for the hasura api calls. I believe, Frank, the pass you're seeing in HasuraAPIError is required for the way you must construct custom classes that inherit from Exception. It will definitely throw an error if raised.

But yes you're totally right this will just short-circuit the app and flask will return a generic 500 AFAIK.

Feels like tech-debt work scoping outside of this work?

Just want to cosign Frank's comments here, I don't know enough to be able to assess how likely it could be for this to fail, but just wondering if we should protect it anyway.

frankhereford · 2026-03-03T15:44:17Z

api/server.py

 EDITOR_ROLE_NAME = "editor"
 MAX_IMAGE_SIZE_MEGABYTES = 5
 CORS_URL = "*"
+CORS_HEADERS = ["Content-Type", "Authorization", "Access-Control-Allow-Origin", CORS_URL]


I'm glad you did this -- you cleaned up a lot of repetition, I think this is good.

frankhereford · 2026-03-03T16:27:09Z

editor/components/CrashSearchTypeahead.tsx

I really like how this component works in terms of user interface. In particular, I immediately went to try to edit the string presented in the <input type=text> field and am impressed how it resets itself as soon as I try edit anything as that would invalidate the match. It's a nice touch, it makes it very hard to use this incorrectly.

frankhereford · 2026-03-03T16:40:53Z

editor/utils/transferTempCrash.ts

+  if (Object.keys(cardUpdates).length > 0) {
+    await mutations.updateCrash(
+      { id: targetCrashId, updates: cardUpdates },
+      { skip_updated_by_setter: true }


Is it possible to not skip setting the updated_by audit field here and in other places in the file?

I see that it's used in a few places in this file -- line 54 for example. There may be a reason why it's being omitted in these new transferring mutations that I'm not seeing of course.

In terms of figuring out who may have deleted a temp crash, the outgoing Temp crash gets the deleting user's email recorded, but that crash becomes much harder to access since the change is a DELETE. So I think getting the user's email in the audit record in the target crash is important to keep record of who did the deletion, or at least who caused those fields to be updated.

I agree it'd be good to go ahead and set updated_by whenever possible. Although these fields don't exist on the recommendations table. Just opened #27204 for that.

roseeichelmann · 2026-03-04T23:17:48Z

editor/queries/crash.ts

+          {
+            _or: [
+              { record_locator: { _ilike: $searchPattern } }
+              { address_display: { _ilike: $searchPattern } }


i tried to search for a crash by starting with typing in the address (there were many crashes with that address) and then started to type out the crash ID, but that yields 0 results bc it has to be one or the other ... it would be cool to be able to do that but i cant even think of how that could be implemented with SQL ilike pattern matching so 🤷

chiaberry

If there is a temp crash created, but I only input info into the initial create modal (case ID, address, unit) and I try to transfer the data to another crash, should I see anything listed under what information is going to be moved?

chiaberry · 2026-03-04T23:07:18Z

api/utils/images.py

+    source_image_source = make_hasura_request(
+        query=GET_PERSON_IMAGE_METADATA_FULL,
+        variables={"person_id": source_person_id},
+    )["people_by_pk"]["image_source"]


Just want to cosign Frank's comments here, I don't know enough to be able to assess how likely it could be for this to fail, but just wondering if we should protect it anyway.

chiaberry · 2026-03-04T23:08:52Z

api/utils/images.py

+    make_hasura_request(
+        query=UPDATE_PERSON_IMAGE_METADATA,
+        variables={
+            "person_id": target_person_id,
+            "object": {
+                "image_s3_object_key": target_obj_key,
+                "image_source": source_image_source,
+                "image_original_filename": source_original_filename,
+                "updated_by": get_user_email(),
+            },
+        },
+    )


along the lines of what Frank said above, but do we need to check the response at all from this request?

make_hasura_request will throw if there's an HTTP error, so I think this falls into the same bucket of adding better error message handing to these calls which to me feels out of scope.

roseeichelmann · 2026-03-04T23:33:54Z

editor/components/DeleteTemporaryCrashModal.tsx

+  // Detect photo existence via the API
+  const [hasPhotoToTransfer, setHasPhotoToTransfer] = useState(false);
+  useEffect(() => {
+    if (!show || !tempFatalityId) {


this show was ambiguous to me.. could you make this var name more descriptive (showModal)? i had to root around to figure out what this was referring to

roseeichelmann · 2026-03-04T23:38:42Z

editor/components/DeleteTemporaryCrashModal.tsx

+  );
+
+  useEffect(() => {
+    if (!show) return;


Why do you need this useEffect + resetting these values here if you already reset them in the handleClose function? ( i could be overlooking something obvious)

roseeichelmann

followed all the test steps and this works great! wahoo thanks mateo, this is such a cool feature ✨

i will say that i found searching by address kind of unhelpful -- a lot of addresses have more than the limit of 20 crashes in the query, so i couldnt even find the crash i was looking for when doing that. but maybe in some select cases it could be helpful. i just wonder if it would be more straightforward to only search by Crash ID?

chiaberry · 2026-03-05T14:52:15Z

If there is a temp crash created, but I only input info into the initial create modal (case ID, address, unit) and I try to transfer the data to another crash, should I see anything listed under what information is going to be moved?

Okay I see that the transferred items list comes from editedCardFields, so if I havent edited any fields nothing will appear in that list. I was confused by the modal saying data would be transferred, but there isnt anything to transfer. Maybe if the list is empty, there could be a message saying nothing will be transferred and the record will be deleted?

johnclary

@mateoclarke this feature is looking so good. It looks great and works great. And thank you for filling in the blanks on details that were missed in scoping. I'm happy to discuss any of my feedback—let me know 🙏

johnclary · 2026-03-04T19:51:54Z

api/utils/images.py

    return new_image_obj_key


+def _transfer_person_image(source_person_id, target_person_id, s3):


Hmm yeah thanks for going with the flow. I agree these are not private methods in conventional sense and would support removing the leading underscore.

johnclary · 2026-03-04T19:58:51Z

api/utils/images.py

+        )
+
+    source_image_source = make_hasura_request(
+        query=GET_PERSON_IMAGE_METADATA_FULL,


I think you can safely update _get_person_image_metadata() to return image_source as well, and then you can remove this graphql API call entirely?

it's annoying that python doesn't have fancy js destructing so you'll have to update all the places that use _get_person_image_metadata()—seems worth it though.

johnclary · 2026-03-04T20:04:54Z

api/utils/images.py

+    source_image_source = make_hasura_request(
+        query=GET_PERSON_IMAGE_METADATA_FULL,
+        variables={"person_id": source_person_id},
+    )["people_by_pk"]["image_source"]


I agree it's odd we don't have any handlers for the hasura api calls. I believe, Frank, the pass you're seeing in HasuraAPIError is required for the way you must construct custom classes that inherit from Exception. It will definitely throw an error if raised.

But yes you're totally right this will just short-circuit the app and flask will return a generic 500 AFAIK.

Feels like tech-debt work scoping outside of this work?

johnclary · 2026-03-04T20:07:32Z

api/utils/images.py

+        variables={"person_id": source_person_id},
+    )["people_by_pk"]["image_source"]
+
+    ext = source_obj_key.rsplit(".", 1)[-1] if "." in source_obj_key else "jpg"


it feels kinda odd to fallback to jpg here. we might want to just throw an error since we should never encounter source_obj_key without a file extension?

johnclary · 2026-03-04T20:13:57Z

api/server.py

 )
+@cross_origin(headers=CORS_HEADERS)
+@requires_auth
+def transfer_person_image(source_person_id, target_person_id):


Love to see a new API route come into being! What do you think about renaming this to copy_person_image? Transfer suggests to me that the original image is being moved, but we are leaving the original image in place.

One thing that's missing for this new route is a test or two. Would you mind adding test coverage?

johnclary · 2026-03-05T16:40:27Z

editor/utils/transferTempCrash.ts

+    );
+  }
+
+  if (photo.shouldTransfer && photo.sourcePersonId && photo.targetPersonId) {


i am realizing that we might should transfer the person's name as well. it seems like the VZ-entered name is potentially more reliable than the crash report. Out of scope fore now—I'll flag that as a possible follow-up item.

johnclary · 2026-03-05T16:48:09Z

editor/utils/transferTempCrash.ts

+  deleteRec: MutateFn
+): Promise<void> {
+  if (targetRec) {
+    const partnerPksToDelete = (targetRec.recommendations_partners ?? []).map(


thank you for considering the fact that there could be partners on the target record 😵‍💫

Rather than deleting existing partners, I think I'd prefer that we preserve those and add any partners from the temp record that aren't already present on the target.

johnclary · 2026-03-05T16:53:28Z

editor/utils/transferTempCrash.ts

+ * Execute all data transfers from a temporary crash to a target crash.
+ * Each step is independently conditional — only runs if there is data to transfer.
+ */
+export async function executeTransfer(config: TransferConfig): Promise<void> {


I really like the way you've bundled all the mutations together. Obviously, if for some reason one of these mutations fails after an earlier one completes we will have a minor headache on our hands, but I can't think of a way to avoid this without a special SQL function or API route that handles this transactionally.

johnclary · 2026-03-05T16:54:15Z

editor/components/CrashSearchTypeahead.tsx

johnclary · 2026-03-05T17:06:35Z

editor/components/DeleteTemporaryCrashModal.tsx

+                {transferItems.map((item) => (
+                  <li key={item}>{item}</li>
+                ))}
+              </ul>


Thanks @chiaberry for mentioning the missing user feedback when there is no data to transfer.

@mateoclarke when there is nothing to transfer, I guess we can hide the crash search and toggle and instead show a message that says:

No transferrable data was found on this temporary crash record

With the bg-light rounded div 👇

adds modal to transfer and delete temp crash record

0b25ad5

mateoclarke marked this pull request as draft February 19, 2026 16:44

mateoclarke commented Feb 24, 2026

View reviewed changes

mateoclarke and others added 6 commits February 24, 2026 15:04

transfers victim photo data

c90ef63

Merge branch 'main' into mateo/26663_transfer_temp_data

f95c19e

fix merge conflict resolution

2f0a64b

refactor CORS headers into reusable array

a235d9b

no updated crash diagram data to transfer

7666e47

Also transfer updated lat/lons

6e00869

mateoclarke commented Feb 25, 2026

View reviewed changes

refactor DeleteTemporaryCrashModal

76d0b1e

mateoclarke commented Feb 25, 2026

View reviewed changes

mateoclarke added 7 commits February 25, 2026 10:12

rename CrashSearchResult

4b5ebe8

continue rename CrashSearchResult

5322cf1

continues continue rename CrashSearchResult

5c46b5f

simplify edited field labels dedupe

cb8afe2

comments update

9b9a6cd

make sure only non-null edits transfer

0ae17fc

refactor not null check

6ff5883

mateoclarke requested review from chiaberry, frankhereford, johnclary and mddilley and removed request for johnclary and mddilley February 25, 2026 19:11

mateoclarke requested review from Charlie-Henry, roseeichelmann and tillyw February 25, 2026 19:12

mateoclarke marked this pull request as ready for review February 25, 2026 19:22

frankhereford approved these changes Mar 3, 2026

View reviewed changes

reduce the list of fields we transfer

68f1950

roseeichelmann reviewed Mar 4, 2026

View reviewed changes

chiaberry reviewed Mar 4, 2026

View reviewed changes

roseeichelmann reviewed Mar 4, 2026

View reviewed changes

roseeichelmann approved these changes Mar 4, 2026

View reviewed changes

johnclary reviewed Mar 5, 2026

View reviewed changes

		return _transfer_person_image(source_person_id, target_person_id, s3)


		@app.route("/images/crash_diagram/<record_locator>", methods=["GET"])

		return new_image_obj_key


		def _transfer_person_image(source_person_id, target_person_id, s3):

Conversation

mateoclarke commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Associated issues

Testing

Ship list

Uh oh!

netlify bot commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for atd-vze-staging ready!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateoclarke Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

frankhereford left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

johnclary Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

johnclary Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roseeichelmann Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chiaberry left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roseeichelmann Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roseeichelmann left a comment

Choose a reason for hiding this comment

Uh oh!

chiaberry commented Mar 5, 2026

Uh oh!

johnclary left a comment

Choose a reason for hiding this comment

mateoclarke commented Feb 19, 2026 •

edited

Loading

netlify bot commented Feb 19, 2026 •

edited

Loading

mateoclarke Feb 24, 2026 •

edited

Loading

johnclary Mar 4, 2026 •

edited

Loading

johnclary Mar 5, 2026 •

edited

Loading

roseeichelmann Mar 4, 2026 •

edited

Loading

roseeichelmann Mar 4, 2026 •

edited

Loading

johnclary Mar 4, 2026 •

edited

Loading

johnclary Mar 5, 2026 •

edited

Loading

johnclary Mar 5, 2026 •

edited

Loading

johnclary Mar 5, 2026 •

edited

Loading