Implement soft deletes for crash notes #1540
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
roseeichelmann
requested review from
johnclary,
mddilley,
tillyw,
frankhereford,
mateoclarke and
chiaberry
chiaberry
approved these changes
Sep 5, 2024
johnclary
approved these changes
Sep 5, 2024
There was a problem hiding this comment.
It works perfectly! So nice to have this in place—thank you!
I would like your feedback on a suggestion I made about the styling of the delete confirmation modal.
Also—I noticed this modal closes immediately, regardless of if the delete mutation resolved. I will open a backlog issue to hook the modal close into the mutation request , just like @chiaberry did in cityofaustin/atd-moped#1399
| @@ -286,7 +286,7 @@ function Crash(props) { | |||
| notes={crashRecord?.crash?.crash_notes} | |||
| INSERT_NOTE={INSERT_NOTE} | |||
| UPDATE_NOTE={UPDATE_NOTE} | |||
| DELETE_NOTE={DELETE_NOTE} | |||
| SOFT_DELETE_NOTE={SOFT_DELETE_NOTE} | |||
| @@ -30,7 +30,7 @@ const Notes = ({ | |||
| // declare mutation functions | |||
| const [addNote] = useMutation(INSERT_NOTE); | |||
| const [editNote] = useMutation(UPDATE_NOTE); | |||
| const [deleteNote] = useMutation(DELETE_NOTE); | |||
| const [deleteNote] = useMutation(SOFT_DELETE_NOTE); | |||
There was a problem hiding this comment.
It's not quite right that we show the "Are you sure?" text and note text with the same styling. What do you think about not showing the note text at all in the modal?
There was a problem hiding this comment.
yeah i am very down to remove the note text completely!
mddilley
approved these changes
Sep 6, 2024
| @@ -12,7 +12,7 @@ const Notes = ({ | |||
| notes, | |||
| INSERT_NOTE, | |||
| UPDATE_NOTE, | |||
| DELETE_NOTE, | |||
| SOFT_DELETE_NOTE, | |||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Associated issues
Closes cityofaustin/atd-data-tech#18788
Users cant delete recommendations only edit, so we dont need to implement soft-deletes for them.
Testing
URL to test:
Local
Steps to test:
crash_notestable but the is_deleted column will say true.Ship list