-
-
Notifications
You must be signed in to change notification settings - Fork 511
Bug fix: Validation failed: Case contact has already been taken #6654
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a production bug where updating case contact contact types was causing a "Validation failed: Case contact has already been taken" uniqueness validation error. The root cause was the remove_unwanted_contact_types method calling destroy_all on the join table associations before the update, creating a timing issue where Rails would attempt to create duplicate join records that violated the uniqueness constraint in CaseContactContactType.
Key changes:
- Removed the
remove_unwanted_contact_typesmethod that was manually destroying associations - Removed the call to this method in the update action, allowing Rails to handle association updates atomically
- Added comprehensive test coverage for various contact type update scenarios
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| app/controllers/case_contacts/form_controller.rb | Removed the manual destroy_all approach for contact type associations, allowing Rails to manage the has_many :through relationship updates properly |
| spec/requests/case_contacts/form_spec.rb | Added six new test cases covering overlapping updates, consecutive updates, reducing/expanding contact types, and complete replacements to prevent regression of the uniqueness validation error |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| end | ||
|
|
||
| it "handles overlapping contact types (mix of existing and new)" do | ||
| # Prevent regression: Rails should update associations without destroy_all race condition |
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment "Prevent regression" is misleading here. This test verifies the fixed behavior after removing the destroy_all call, rather than preventing future regressions. Consider rephrasing to something like "Verifies that overlapping contact types can be updated without uniqueness errors" to more accurately describe what the test does.
| # Prevent regression: Rails should update associations without destroy_all race condition | |
| # Verifies that overlapping contact types can be updated without destroy_all-related race conditions |
|
|
||
| case_contact.reload | ||
| expect(case_contact.contact_type_ids).to contain_exactly(contact_types.first.id) | ||
| expect(case_contact.contact_type_ids).not_to include(contact_types.second.id) |
Copilot
AI
Jan 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This expectation is redundant. The previous line already asserts that contact_type_ids contains exactly the first contact type ID, which implicitly means it does not include the second contact type ID. Consider removing this line to reduce test verbosity.
| expect(case_contact.contact_type_ids).not_to include(contact_types.second.id) |
Uh oh!
There was an error while loading. Please reload this page.