Warn (not block) on approximate email dupes; widen duplicate matching#1854
Conversation
There was a problem hiding this comment.
Pull request overview
Tightens the People duplicate-check flow so that an exact name plus a matching email (primary, secondary, or user/login email) is treated as an exact match that blocks creation, reducing the chance of creating true duplicate Person records.
Changes:
- Update
format_duplicate/sorting to collapse secondary/user email matches into the exact (blocked) tier. - Update the duplicate-check page badges and copy to remove the prior “approximate” tier and reflect “exact name and email”.
- Adjust request specs for
/people/check_duplicatesto assert blocked behavior for secondary and user-email matches.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
app/controllers/people_controller.rb |
Reworks duplicate classification/sorting and broadens “blocked” to any matching email on exact-name matches. |
app/views/people/check_duplicates.html.erb |
Updates badge logic and blocked-message copy; removes the old approximate warning. |
spec/requests/people_check_duplicates_spec.rb |
Updates expectations for secondary/user email scenarios to assert exact-match + blocked messaging. |
| labeled_emails << { label: "Secondary email", value: person.email_2 } if person.email_2.present? | ||
| labeled_emails << { label: "User email", value: person.user.email } if person.user&.email.present? | ||
| emails = labeled_emails.map { |e| e[:value] }.uniq | ||
| all_emails = [ person.email, person.email_2, person.user&.email ].compact |
| @@ -366,7 +361,7 @@ def format_duplicate(person, exact: true, entered_email: nil) | |||
| labeled_emails: labeled_emails, | |||
| match_type: match_type, | |||
| name_match: exact, | |||
| blocked: primary_email_match | |||
| blocked: exact && any_email_match | |||
| } | |||
| @@ -366,7 +361,7 @@ def format_duplicate(person, exact: true, entered_email: nil) | |||
| labeled_emails: labeled_emails, | |||
| match_type: match_type, | |||
| name_match: exact, | |||
| blocked: primary_email_match | |||
| blocked: exact && any_email_match | |||
| } | |||
e78be62 to
5ef8761
Compare
| match_type: match_type, | ||
| name_match: exact, | ||
| blocked: primary_email_match | ||
| blocked: primary_email_match || secondary_email_match |
There was a problem hiding this comment.
🤖 From Claude: Both primary and secondary/user email matches now block, but they stay separate match_types (exact vs approximate) purely so the views can show which email matched. blocked is the OR of the two.
Exact name plus any matching email (primary, secondary, or user) now blocks duplicate creation. A primary-email match still shows as an "exact" match; a secondary/user-email match shows a distinct "approximate" display so the reviewer can see which email matched, but it blocks all the same. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
5ef8761 to
cb439a2
Compare
| it "hides the Create anyway button" do | ||
| existing_person.update!(email_2: "jane.secondary@testmail.org") | ||
|
|
||
| get check_duplicates_people_path, params: { | ||
| first_name: "Jane", last_name: "Doe", email: "jane.secondary@testmail.org" | ||
| } | ||
|
|
||
| expect(response.body).not_to include("Create anyway") | ||
| end |
| it "hides the Create anyway button" do | ||
| get check_duplicates_people_path, params: { | ||
| first_name: "Jane", last_name: "Doe", email: existing_person.user.email | ||
| } | ||
|
|
||
| expect(response.body).not_to include("Create anyway") | ||
| end |
…dary email Only an exact name + primary email match should block creation. Exact name plus a secondary or user email is an approximate match: two people can legitimately share an email, so the admin is warned but can still "Create anyway". Also widen duplicate detection to the full set of identity fields the form captures — first name, legal first name, nickname variants, last name, primary email, and secondary email — so a match is caught regardless of which pair lines up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| .where("REPLACE(REPLACE(LOWER(first_name), '.', ''), ' ', '') IN (?)", first_variants) | ||
| .where( | ||
| "REPLACE(REPLACE(LOWER(first_name), '.', ''), ' ', '') IN (:variants) OR " \ | ||
| "REPLACE(REPLACE(LOWER(legal_first_name), '.', ''), ' ', '') IN (:variants)", |
There was a problem hiding this comment.
🤖 From Claude: Name matching now also checks the stored legal_first_name column, and first_variants above includes nickname variants of both the entered first name and legal name — so e.g. entering "Roberto" surfaces a record stored as first "Bob"/legal "Roberto".
| "(users.id IS NULL AND LOWER(people.email) = :email) OR LOWER(people.email_2) = :email OR LOWER(users.email) = :email", | ||
| email: email_lower | ||
| "(users.id IS NULL AND LOWER(people.email) IN (:emails)) OR " \ | ||
| "LOWER(people.email_2) IN (:emails) OR LOWER(users.email) IN (:emails)", |
There was a problem hiding this comment.
🤖 From Claude: The email lookup now searches both the entered primary and secondary email (query_emails) against each record's email/email_2/user email, so a collision on the new person's secondary email is caught too.
|
|
||
| # Exact name + primary email is an "exact" match and blocks creation. Exact | ||
| # name plus a secondary or user email is "approximate" — the admin is warned | ||
| # but can still create the person (they may legitimately share an email). |
There was a problem hiding this comment.
🤖 From Claude: Blocking now keys off primary_email_match only. Exact name + a secondary/user email is "approximate" — warned but still creatable, since two people can legitimately share an email.
🤖 suggested review level: 3 Read 📖 contained duplicate-check change — widened matching logic + reverted blocking, covered by request specs
What is the goal of this PR and why is this important?
How did you approach the change?
blocked:is nowprimary_email_matchonly. Exact name + secondary/user email is an"approximate"match: orange warning badge + note, with "Create anyway" available.find_duplicate_peopleto consider legal first name (entered and stored, plus nickname variants) and the entered secondary email — so a match is caught regardless of which name/email pair lines up. Newexact_first_name_match?helper decides exact-vs-nickname across first + legal forms.createandcheck_duplicates, and carried them on the standalone page's "View details"/shareable links.UI Testing Checklist
Anything else to add?
skip_duplicate_check); unchanged from before, and only the approximate case was relaxed.