alumni_lookup

Phase 28.1 — Email Uniqueness

Status: 📋 Planned Effort Class: Small. One migration, one index, no new table, no new route. Surfaces: Shared model + Signup Admin. No held-surface writes. Ships first. 28.2 is unsound without it.

---

1. Why This Comes First

Google sign-in auto-links a verified identity to a record found by email. That is only meaningful if an email names at most one record.

Today it doesn’t, formally — champion_signups has no unique index on email (schema.rb:264-272) — and find_active_by_email resolves collisions with .order(:created_at).last.

That heuristic is fine under a mailed link: the link goes to one inbox, and picking the newest row is a reasonable guess about which record the person cares about. It is not fine under OAuth, where the credential is verified and the sign-in is immediate. 28.2 would confer verified-credential strength on a guess. Uniqueness retires the guess instead of hardening it.

2. The Table Is Already Nearly Unique

Active rows	200
Duplicate email groups among active rows	1 — chiphayner@gmail.com ×2, staff test data
All rows (incl. soft-deleted and merged)	244
Duplicate groups across all rows	15
Null emails	0

All 15 collisions live in soft-deleted and merged rows, which is exactly where merging puts them. The constraint therefore has to be partial, and its predicate has to match scope :active (champion_signup.rb:84) exactly — a constraint and a finder that disagree about which rows count is worse than neither.

3. The Migration

add_index :champion_signups,
          "LOWER(email)",
          unique: true,
          where: "deleted_at IS NULL",
          name: "index_champion_signups_on_lower_email_active",
          algorithm: :concurrently

Three things that are not cosmetic:

• LOWER(email), not email. find_active_by_email compares lowercased (champion_signup.rb:405). A plain index would admit Chip@x.com alongside chip@x.com while the finder still saw two rows — the constraint would be satisfied and the bug would survive it.
• where: "deleted_at IS NULL" — matches scope :active and nothing more. Merged rows are soft-deleted, so they fall out; the 15 historical collisions are unaffected and stay readable.
• algorithm: :concurrently with disable_ddl_transaction!. 244 rows makes this academic today, but a unique index build takes an exclusive lock and this table is on the public signup path.

Prerequisite, manual: resolve the chiphayner@gmail.com pair before the migration runs, using the existing Signup Admin merge tool. The migration must fail loudly rather than skip rows — see §7.

Also add the model-level validation, so the failure is a form error rather than a 500:

validates :email, uniqueness: { case_sensitive: false, conditions: -> { active } }

The database constraint is the authority; the validation is for the error message. Both, not either — a validation alone loses the race, and an index alone produces RecordNotUnique in front of an alum.

4. What Uniqueness Changes

find_active_by_email loses its tie-breaker. .order(:created_at).last becomes .first on a set that can hold at most one row. Keep the active scope and the LOWER comparison; delete the ordering, and say why in the comment, because a future reader will otherwise reintroduce it as defensive.

Merge becomes constrained rather than merely tidy. ChampionSignupMerger must soft-delete the absorbed row in the same transaction that rewrites the survivor, or it trips the index. The data suggests it already does — 6 merges, zero active collisions — but that has been an emergent property, and it is now a correctness requirement. It gets a test (§6).

The 27.1 duplicate nudge moves to the failure path.

Spec deviation, found during implementation (August 14, 2026). This section originally claimed the 27.1 flow was “unchanged”. It was wrong, and it was wrong because it conflated two different things: the client-side /sign-up/email-check notice, which is genuinely unaffected, and the server-side create path, which changes materially.

Before 28.1, submitting step 1 with a taken address succeeded. A duplicate row was created, and notify_existing_signup then mailed the original record’s owner a link back into it. That call sat inside if @signup.errors.none? && @signup.save.

With the unique index the save fails, so that branch never runs. Left alone, the nudge email would have disappeared and the submitter would have been stranded on a bare “Email is already registered” with no way forward — a worse outcome than the duplicate.

Resolved by moving the nudge to the failure path (#handle_who_you_are_failure), gated on the address being the only thing wrong:

if creating && @signup.errors.attribute_names == [:email]
  existing_signup = ChampionSignup.find_active_by_email(who_you_are_params[:email])
  ...

Three properties that gate carries:

• No duplicate row. The point of the sub-phase.
• An incomplete submission still sends nothing — 27.2’s property, and the reason the gate is explicit rather than implicit. Before 28.1 “the save succeeded” did that job; the save never succeeds on this path now, so a minimal payload would otherwise become an email trigger. A blank address also lands on :email and passes the attribute gate, but resolves to no record, which is what actually stops it.
• The person has a route forward. The notice renders server-side from the same markup the Stimulus check reveals, branched on one flag — not a second copy.

triggered_by_signup_id was dropped from the event metadata: it named the duplicate row the nudge accompanied, and there is no longer a duplicate row to name. Rows written before 28.1 keep the key, which is the honest record of a period when the second row existed.

Enumeration: this response now differs from the success case, which is a bit the POST did not previously leak. It is the same bit /sign-up/email-check answers by design (27.1, disclosed in the privacy policy), and POST /sign-up is throttled harder than that endpoint — 3 per 15 minutes per address against 10 per hour per IP.

One copy change fell out of it: “or keep going to start a fresh signup” was deleted from the notice. The unique index made it false in both the JS and server paths.

5. Finding: the Signup Admin email-duplicate queue goes permanently empty

status=duplicates in Signup Admin surfaces active rows duplicated under either key — BUID or email — which 27.1 added specifically to catch pairs that ChampionSignupMerger could never merge, the ones with no BUID on either row.

After this migration, the email half can never return a result again. Not “usually empty” — empty by construction.

The BUID half stays live and still matters: two different addresses on one BUID remains possible, and at 95% BUID coverage it is the likelier duplicate shape going forward. In fact it becomes the only shape, which makes the filter more useful rather than less.

Decided (August 14, 2026): deleted. What forced it was not the argument above but the tests. An active email duplicate is now unrepresentable — the validation refuses it and the index refuses it underneath — so every test for the subsystem described a state the database cannot hold. There was no version of “keep it” that left it covered.

Deploy ordering is what made deletion safe: staff merge the one production collision on current production, where merge_duplicates_for_email still exists, before 28.1 ships. By deploy time the data is clean and the tool has no remaining job.

Removed, with everything that fed it:

Layer	Gone
Model	scope :duplicates_by_email, has_duplicates_by_email?, duplicate_signups_by_email, the email half of find_all_with_duplicates
Service	ChampionSignupMerger.merge_duplicates_for_email, .conflicting_buids_for_email, ConflictingBuidsError
Controller	SignupAdmin::SignupsController#merge_duplicates_by_email, the @duplicate_emails / @unmergeable_emails preloads, the before_action entry
Route	post :merge_duplicates_by_email
Views	the “Same Email on Multiple Signups” banner on show, the amber row/card highlighting, the “Same email” and “Email” badges, and the amber Merge button on all
Helper	signup_email_duplicate?
Tests	signup_admin/email_merge_test.rb (whole file) and 8 others

Kept: everything BUID-keyed. Two addresses on one BUID is still ordinary, and at 95% BUID coverage it is now the only duplicate shape the app can produce — which makes duplicates_by_buid more load-bearing than it was, not less.

active_by_email also stays. It is still the one definition of “which rows does this address name”, still normalizes .strip and LOWER in one place, and now backs find_active_by_email, which used to hand-roll its own copy of that clause.

A ratchet was added in its place: champion_signup_merger_test.rb asserts that neither merge_duplicates_for_email nor merge_all_duplicates_by_email! responds, so an email-keyed entry point cannot grow back quietly.

6. Tests

Test	Asserts
champion_signup_test.rb	Two active rows with the same email raise; with differing case, still raise
champion_signup_test.rb	A soft-deleted row does not block reuse of its address
champion_signup_test.rb	find_active_by_email returns the single active row and ignores soft-deleted ones
champion_signup_merger_test.rb	Merge soft-deletes the absorbed row in the same transaction; the index is never violated mid-merge
champion_signup_merger_test.rb	A merge that would leave two active rows sharing an address raises rather than half-completing
signup/signups_controller_test.rb	Typed collision at step 1 still renders 27.1’s notice, still sends mode: "link", still issues no code
Migration test	Refuses to run with an unresolved active collision present

What shipped

bin/test: 5879 runs, 17000 assertions, 0 failures, 0 errors, 3 skips.

Sabotage check — run, and it caught a tautology first. The plan was “drop LOWER() from the index and confirm the differing-case test fails.” Every case-insensitivity test written up to that point ran through the validation’s case_sensitive: false and would have kept passing against an index on the raw column. Only one test pins LOWER() in the index:

test "the database refuses a case-different duplicate when validations are skipped" do
  ...
  assert_raises ActiveRecord::RecordNotUnique do
    second.update_column(:email, first.email.downcase)   # update_column skips validations
  end
end

Verified by swapping the index for one on the raw column: the test fails (RecordNotUnique expected but nothing was raised), and passes again once restored.

A second tautology nearly hid inside the sabotage itself. The first sabotage run passed, which looked like proof the test was worthless. It wasn’t — bin/rails test spawns 4 parallel workers, each with its own database rebuilt from schema.rb, so the sabotage never reached the database the test ran against. PARALLEL_WORKERS=1 was required to make the sabotage real. Worth knowing before trusting any schema-level sabotage in this repo.

One test-data fix worth naming

SignupsControllerTest#linked_signup built a second active signup for alumni(:john_doe) under the same address the completed_linked fixture already held, which the index rejected in four tests. The fix soft-deletes the fixture row first — not a workaround, but the constraint applied to test data: two active rows for one person is precisely what this sub-phase says is wrong, and the fixtures had been quietly doing it.

7. Rollback

DROP INDEX index_champion_signups_on_lower_email_active and revert the validation. No data is destroyed by the constraint itself — the only destructive step is the manual merge of the chiphayner@gmail.com pair, which happens before the migration and is a normal staff merge with its own signup_merged event.

The migration must fail rather than auto-resolve if it finds an unresolved collision. Silently merging rows during a migration would destroy a record without a staff review and without the event trail every other merge leaves.