Fix various things in the crash recommendations form #1542

roseeichelmann · 2024-09-04T02:45:00Z

Associated issues

Closes cityofaustin/atd-data-tech#18468

Testing

URL to test:
Netlify

Steps to test:

Go to our staging website to see the buggy behavior of this component. Click on a crash without a fatality, update an injury to fatality to show the recommendation form. Try to add recommendation text or update, see that the mutation fails. Add a status, see that an empty partner record is created. Go to another crash, make it a fatality, and this time start by adding partners and then recommendation text, this actually works as expected.
Now test the netlify preview. See that the updates section isnt there until you create a recommendation. If you first create a recommendation text or status, a partner wont be created yet. Now add partners, status, etc.
Go to a new crash and this time add partners first, a recommendation record will be created when you add a partner. The update section will then display. Add text to all fields, status, etc.

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

roseeichelmann · 2024-09-04T17:40:27Z

atd-vze/src/queries/recommendations.js

-        recommendations_partners: { data: { partner_id: $partner_id } }
-      }
-    ) {
+    insert_recommendations(objects: [$recommendationRecord]) {


previously we had a bug where if you created a recommendation by just filling out the status, a recommendation record would be created along with a partner record that just had null as the partner id, so then you would end up with something that looks like this

we dont want to create a partner record until the user actually adds a partner. so im changing this mutation to use an object variable that may or may not contain partner data so we arent creating empty partners

Makes sense - thanks for tightening this up for less cruft in the DB.

Thanks for fixing this. Would you rename the Add button to Save? And can you disable the button until the input has been modified?

One other tiny detail—would you mind removing the colon (:) from the Coordination Parter and Status labels?

roseeichelmann · 2024-09-04T17:42:39Z

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

-                onAdd={onAdd}
-                onEdit={onEdit}
-              />
+          {doesRecommendationRecordExist && (


dont show the update section until a recommendation has been created

would you hide this field until rec_text is not null? otherwise this appears after you save a coordination parter or status.

roseeichelmann · 2024-09-04T18:35:12Z

atd-vze/src/queries/recommendations.js

@@ -15,23 +15,9 @@ export const GET_RECOMMENDATION_LOOKUPS = gql`

 export const INSERT_RECOMMENDATION = gql`
  mutation InsertRecommendation(
-    $text: String


mutation was failing silently if you try to create a recommendation by first adding recommendation text because this mutation was expecting the variable text but we were feeding it rec_text

roseeichelmann · 2024-09-04T18:39:38Z

atd-vze/src/views/Crashes/Recommendations/RecommendationMultipleSelectDropdown.js

+    const valuesObject = {
+      recommendations_partners: {
+        // Mutation expects lookup IDs as integers
+        data: { [field]: parseInt(selectedItem.id) },


alternatively i could have this valuesObject conditionally formatted based on whether doesRecommendationRecordExist, so it only gets this nested insert format if the rec record doesnt exist yet, and we wouldnt have to specify valuesObject.recommendations_partners.data in the onAddPartner function

roseeichelmann · 2024-09-04T18:41:43Z

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

@@ -74,7 +75,7 @@ const Recommendations = ({ crashPk, recommendation, refetch }) => {
  const onAddPartner = valuesObject => {
    const recommendationPartnerRecord = {
      recommendationRecordId,
-      ...valuesObject,
+      ...valuesObject.recommendations_partners.data,


dont want that nested insert format if the recommendation record already exists and we are only inserting a partner record. like i mentioned in the comment above, i could handle this logic in that component instead and format the valuesObject there based on whether the recommendation record exists yet

roseeichelmann · 2024-09-04T18:48:37Z

@johnclary the issue says to have visual feedback when the form is saving (does this mean a spinner?) so i guess you mean whenever the user clicks the add button or edits and saves, but none of our other components like notes or related records tables have any kind of feedback when you edit fields? could you expand maybe on what you would want to see? personally i think its fine as is

frankhereford

🚢🚢🚢 This is absolutely an improvement. Thank you @roseeichelmann!

I have been thinking about the possibilities you raised about using logic in the component to control the use of the nested record creation in the graphql. I am not sure, so don't give this too much weight, but I think I like it the way you have it. Simpler code, less deeply buried logic, and the application seems to be working as I would expect it. I like it!

frankhereford · 2024-09-04T18:57:57Z

atd-vze/src/queries/recommendations.js

-        recommendations_partners: { data: { partner_id: $partner_id } }
-      }
-    ) {
+    insert_recommendations(objects: [$recommendationRecord]) {


Makes sense - thanks for tightening this up for less cruft in the DB.

chiaberry

i think I am following the alternative way of doing this, but the way you have it here makes sense to me. Thank you for fixing the silent failure, I tried adding things in different order and it all checks out

johnclary

Thanks @roseeichelmann!

I think that back when i created this issue i did not realize that editing the coordination partners or status input triggers an insert/update immediately—which was adding to my confusion about potential bugginess.

I requested feedback in order to tighten up the save behavior of the text and text updates.

I agree we can leaving the loading feedback out of scope. If we were going to do a bigger refactor of this form, it would be great to rework this so that it behaves like one big form—you edit any field then click "Save" to save changes. I'd also like to make the parters and status inputs look like normal bootstrap inputs (with outlines, focus effects, etc), but the i can see multiselect library doesn't make that easy. It'd be nice to replace it with react-select for a superior UX. I'll drop that in a backlog issue.

johnclary · 2024-09-05T17:53:13Z

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

-                onAdd={onAdd}
-                onEdit={onEdit}
-              />
+          {doesRecommendationRecordExist && (


would you hide this field until rec_text is not null? otherwise this appears after you save a coordination parter or status.

johnclary · 2024-09-05T17:55:18Z

atd-vze/src/queries/recommendations.js

-        recommendations_partners: { data: { partner_id: $partner_id } }
-      }
-    ) {
+    insert_recommendations(objects: [$recommendationRecord]) {


Thanks for fixing this. Would you rename the Add button to Save? And can you disable the button until the input has been modified?

One other tiny detail—would you mind removing the colon (:) from the Coordination Parter and Status labels?

…database

roseeichelmann · 2024-09-06T00:43:43Z

atd-vze/src/views/Crashes/Recommendations/RecommendationTextInput.js

@@ -42,7 +44,7 @@ const RecommendationTextInput = ({
  };

  // Recommendation record does not exist yet, adding a value will create record
-  const isAddingRecommendation =
+  const isCreatingRecord =


make it more apparent that we are talking about this is going to create a recommendation record, not talking about just adding recommendation text

roseeichelmann · 2024-09-06T00:44:27Z

atd-vze/src/views/Crashes/Recommendations/recommendationsDataMap.js

-    partner_id: {
-      label: "Coordination Partner:",
-      lookupOptions: "atd__coordination_partners_lkp",
-      key: "coord_partner_desc",


this was a duplication pretty much of the first object and its not actually doing anything?

roseeichelmann · 2024-09-06T00:44:57Z

atd-vze/src/views/Crashes/Recommendations/RecommendationTextInput.js

-
+    const isEmptyString = !inputValue.trim();
+    // Save null to database if user entered empty string or only spaces/newlines
+    const valuesObject = { [field]: isEmptyString ? null : inputValue };


previously we were allowing empty strings to be saved to the database.

roseeichelmann · 2024-09-06T00:46:43Z

@johnclary this is ready for re-review! i agree about making this one big form in the future. i disabled the save button until there is input and also made it so that users cant save empty strings to the database, including just spaces or newlines.

question though, what if someone has rec text and update text, then they delete whats in the rec text. do we want the update section to still disappear even though it has content or should it show? currently i have it so that the update section will always show up if there is update text

…on text, fix border showing up underneath recommendation when there is no update section

chiaberry · 2024-09-06T13:27:30Z

I was re-testing this PR after the changes, and your comment made me want to try "deleting" the rec text, by updating it to be an empty string. It feels weird having the text flash again before the area turning into an input again.

But I dont know how often the recommendation text would be deleted vs being updated. So maybe its not worth trying to fix the UX right now

…etch

johnclary

🚢 thanks @roseeichelmann !!

roseeichelmann added 4 commits September 3, 2024 21:40

formatting

d5da23e

use objects instead of explicit variables

c25ca81

conditionally create values object based on whether record exists

592cfb7

move variables

eb04434

roseeichelmann added the WIP Work in progress label Sep 4, 2024

roseeichelmann added 3 commits September 4, 2024 12:33

change this var name

38777d3

add comments and remove conditional

7d1dfff

access properties needed from values object for partner record creation

ee46043

roseeichelmann commented Sep 4, 2024

View reviewed changes

roseeichelmann requested review from johnclary, mddilley, frankhereford, mateoclarke, chiaberry and tillyw September 4, 2024 18:51

frankhereford approved these changes Sep 4, 2024

View reviewed changes

roseeichelmann removed the WIP Work in progress label Sep 4, 2024

chiaberry approved these changes Sep 5, 2024

View reviewed changes

johnclary requested changes Sep 5, 2024

View reviewed changes

roseeichelmann added 3 commits September 5, 2024 19:39

dont show update section unless recommendation text exists

b8f6281

remove colon, remove random extra data map entry thats not being used

fe1f98e

disable save button until there is input, dont send empty strings to …

f8cf02d

…database

roseeichelmann commented Sep 6, 2024

View reviewed changes

roseeichelmann requested a review from johnclary September 6, 2024 00:45

show update section if there is an update or if there is recommendati…

e16fdcc

…on text, fix border showing up underneath recommendation when there is no update section

roseeichelmann added 2 commits September 6, 2024 14:28

always show updates section, chain edit state setting until after ref…

824c6db

…etch

change to add

a72adaa

johnclary approved these changes Sep 10, 2024

View reviewed changes

roseeichelmann merged commit e56e3c6 into master Sep 10, 2024
8 checks passed

roseeichelmann deleted the 18468_fix_recommendations branch September 10, 2024 17:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix various things in the crash recommendations form #1542

Fix various things in the crash recommendations form #1542

roseeichelmann commented Sep 4, 2024 •

edited

Loading

roseeichelmann Sep 4, 2024

frankhereford Sep 4, 2024

johnclary Sep 5, 2024

roseeichelmann Sep 4, 2024

johnclary Sep 5, 2024

roseeichelmann Sep 4, 2024

roseeichelmann Sep 4, 2024

roseeichelmann Sep 4, 2024 •

edited

Loading

roseeichelmann commented Sep 4, 2024 •

edited

Loading

frankhereford left a comment

frankhereford Sep 4, 2024

chiaberry left a comment

johnclary left a comment •

edited

Loading

johnclary Sep 5, 2024

johnclary Sep 5, 2024

roseeichelmann Sep 6, 2024

roseeichelmann Sep 6, 2024

roseeichelmann Sep 6, 2024

roseeichelmann commented Sep 6, 2024 •

edited

Loading

chiaberry commented Sep 6, 2024

johnclary left a comment

Fix various things in the crash recommendations form #1542

Fix various things in the crash recommendations form #1542

Conversation

roseeichelmann commented Sep 4, 2024 • edited 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

roseeichelmann Sep 4, 2024 • edited Loading

Choose a reason for hiding this comment

roseeichelmann commented Sep 4, 2024 • edited Loading

frankhereford left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chiaberry left a 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

roseeichelmann commented Sep 6, 2024 • edited Loading

chiaberry commented Sep 6, 2024

johnclary left a comment

Choose a reason for hiding this comment

roseeichelmann commented Sep 4, 2024 •

edited

Loading

roseeichelmann Sep 4, 2024 •

edited

Loading

roseeichelmann commented Sep 4, 2024 •

edited

Loading

johnclary left a comment •

edited

Loading

roseeichelmann commented Sep 6, 2024 •

edited

Loading