can_log_in is stale, misnamed, and wrong — replace with a derived method

[mroderick]

Summary

members.can_log_in is a misnamed, denormalized boolean. It is never checked in the login path, its only real consumer (requires_additional_details? → finish_registration) runs only after a member has already logged in via an auth service, and #2783 showed its value has degraded to auth_services.count.odd? (parity garbage) across 116 members. The column is stale state that duplicates easily-derived truth. Proposal: replace it with a derived method and drop the column.

What can_log_in actually does

Complete app-code reference set:

1. app/models/member.rb:30,32 — presence validations for name/surname/email/about_you + strict email, gated if: :can_log_in?.
2. app/models/member.rb:128 — requires_additional_details? = can_log_in? && !valid?, consumed by finish_registration (app/controllers/application_controller.rb:95).
3. app/controllers/auth_services_controller.rb:49 — the toggle!(:can_log_in) that corrupts it (can_log_in is silently toggled off when a member links a second auth service — 115 members affected #2783).
4. db/schema.rb + a migration.
5. Specs: spec/models/member_spec.rb, spec/features/member_joining_spec.rb.

Login never consults it: current_user = Member.find(session[:member_id]) (app/controllers/application_controller.rb:31-36), with the session set from a matched AuthService. There is no can_log_in check anywhere in the authentication path.

Why the flag is redundant

requires_additional_details? is only ever evaluated after login, and login requires an auth service. So the can_log_in? && prefix is always true at the point of use — the flag contributes nothing, it is dead weight that is also corrupted.

Live DB check (codebar_dump):

stored can_log_in	has auth svc	members
false	no	112
true	no	27
false	yes	116
true	yes	29,111

The 112 and 27 members have no auth service, so they cannot log in regardless — treating them as inactive matches reality. The 116 (including the 5 codebar + 110 github-github cohort from #2783, plus 485) all have an auth service and can log in; deriving "active" from auth_services.exists? makes them active without any data repair.

Git history

can_log_in has been misnamed since its inception (2013-12-08, 54a7edbe "Added github auth, no tests"): the name, toggle!, requires_additional_details?, and the gated validations all landed together, and login never checked it even then. Hours later b3918f14 "Fix already created members" changed Member.new → Member.find_or_create_by_email, letting a new provider reuse an existing member — which turned toggle! into a flip and is the origin of #2783's parity corruption. No commit in git log -S'can_log_in' ever used it as a login/access gate. So both the misleading name and the bug predate codebar; the codebar rollout only added 5 victims and produced the report. (Note: 7a604b18, the recent codebar sign-in switch, preserved the toggle unchanged.)

Proposed change: derive, don't store

1. Replace the stored boolean with a derived method on Member:
   • def active? = auth_services.exists?
   • def requires_additional_details? = active? && !valid?
     (active? is equivalent to the old gate in the live post-login path, but is defensive if requires_additional_details? is ever called without a session.)
2. Update the consumers:
   • app/models/member.rb:30,32 — if: :active? in place of if: :can_log_in?
   • app/models/member.rb:128 — use active?
   • app/controllers/auth_services_controller.rb:49 — remove the toggle!; a member is active by having an auth service.
   • app/controllers/auth_services_controller.rb:52 — gate the otherwise-unconditional details redirect on requires_additional_details? so a complete member linking a second provider is not bounced to /member/details/edit:

     redirect_to edit_member_details_path(member_type: member_type) if member.requires_additional_details?

3. Drop the can_log_in column via migration. No data repair is needed — the 116 self-correct to active because they have auth services.
4. If genuine account lockout is ever required, add a real gate (e.g. disabled_at) that is checked at login — can_log_in never locked anyone out, so nothing to preserve.

Alternative (minimal): rename only

If the team prefers not to remove the column, rename it to something honest (e.g. active). Note this leaves a stale, re-corruptible column in place; it fixes the mislabel but not the redundant-stored-state root cause.

Relationship to #2783

This change fully resolves #2783. It removes the can_log_in corruption at the root (part 1), gates the unconditional details redirect (part 2), and makes the 115-row data repair unnecessary (the 116 self-correct to active). PR #2778 (already merged) changed the write site to set the flag to true instead of toggling; this issue builds on that by dropping the column entirely — the remaining part 2 (redirect gating) is still open and is addressed here.

Relationship to #2780 (not obsoleted)

PR #2780 (fix/auth-callback-race) fixes a different bug: a concurrent OAuth callback collides on the unique (uid, provider) index during member.save! and errors the request. It is needed regardless of this issue — #2784 does not touch the race handling. The only overlap is the can_log_in write in the same else-branch, which #2784 removes entirely.

Ordering: #2780 currently writes to the can_log_in column, so it should land first, then this issue drops the column on top (or #2780 is rebased on this issue to drop its now-dead can_log_in write, keeping only the race guard). Both must ship; the two fixes are orthogonal and complementary, not competing.

Out of scope / dependency

• Whether the 116 rows should be treated as active → answered implicitly by this change (they are), so no separate repair step.
• Whether the team wants a genuine hard-lockout mechanism → a future, separate concern (disabled_at), not introduced here.