Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing issue with error when deleting contact from appeal. #1201

Closed
wants to merge 1 commit into from

Conversation

dr-bizz
Copy link
Contributor

@dr-bizz dr-bizz commented Nov 18, 2024

Description

On appeals, when deleting a contact from an appeal, it would sometimes error. This PR fixes this bug.

The issue arose as I added functionality that tested the contact in question is on the appeal. However, since it meant we had to load all the contacts before we deleted it, if the user clicks the "Remove" button before it all loads, it would cause this error.
I could have prevented the "Remove" button from being selected until all contacts were loaded or removed the functionality and let the server check and produce the error. I opted to remove it as it adds a better UX, and as a bonus, there's less code to manage.

Checklist:

  • I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)"
  • I have applied the appropriate labels. (Add the label "On Staging" to get the branch automatically merged into staging.)
  • I have requested a review from another person on the project

@dr-bizz dr-bizz requested a review from canac November 18, 2024 16:37
@dr-bizz dr-bizz added the Preview Environment Add this label to create an Amplify Preview label Nov 18, 2024
Copy link
Contributor

Preview branch generated at https://hs-1256989.d3dytjb8adxkk5.amplifyapp.com

Copy link
Contributor

Bundle sizes [mpdx-react]

Compared against 9d1180c

No significant changes found

@canac
Copy link
Contributor

canac commented Nov 18, 2024

Do you mind putting a link to the HelpScout ticket in the description and putting the ticket number in the title?

@canac
Copy link
Contributor

canac commented Nov 18, 2024

@dr-bizz Does this work for you? I'm getting errors like AppealContact with id=2c10008b-784e-4540-8cb8-297022f9b473 not found in preview. I think it would be best if the API accepted a contact id, but it seems that it's looking for a contact-appeal id from the join table.

@dr-bizz
Copy link
Contributor Author

dr-bizz commented Nov 18, 2024

Yeah, I think you are correct. it's looking for an ID that joins the contacts for the appeal, rather than the contact ID. I will fix it. I might talk to the backend team about making the API accept a contact ID

@canac
Copy link
Contributor

canac commented Nov 18, 2024

I might talk to the backend team about making the API accept a contact ID

@dr-bizz I'd recommend that if they have capacity. In most cases, I don't think that the API should make clients deal with join ids.

@dr-bizz
Copy link
Contributor Author

dr-bizz commented Nov 18, 2024

The backend will be adding the contact ID to GraphQL. so I will be closing this PR and will open a new one when I need to update this mutation.

@dr-bizz dr-bizz closed this Nov 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Preview Environment Add this label to create an Amplify Preview
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants