Found in code review of PR #284 (merged).
PR #284 added a transaction-scoped advisory-lock block to add_update_identity (code/DHService/db.py) that inlines a primary-email lookup:
SELECT id FROM member
WHERE EXISTS (
SELECT 1 FROM jsonb_array_elements(identity->'emails') AS e
WHERE LOWER(e->>'email_address') = LOWER(%s)
AND e->>'type' = 'primary')This duplicates the case-insensitive email-resolution logic already in get_member_id_from_email() (same file). The inline copy was a deliberate choice — the lookup needs to run inside the locked transaction, and get_member_id_from_email() opens its own connection — but it leaves two copies of the same email-matching SQL that must be kept in sync if the email JSONB schema ever changes (e.g. additional email types, normalization rules).
Suggested fix
Extract a helper that accepts an existing cursor, e.g. _resolve_member_id_by_primary_email(cur, email), and have both get_member_id_from_email() (passing a cursor from its own connection) and the advisory-lock block call it. Single source of truth for the primary-email match.
Low priority — no functional bug, just drift risk.
Context: PR #284.
Found in code review of PR #284 (merged).
PR #284 added a transaction-scoped advisory-lock block to
add_update_identity(code/DHService/db.py) that inlines a primary-email lookup:This duplicates the case-insensitive email-resolution logic already in
get_member_id_from_email()(same file). The inline copy was a deliberate choice — the lookup needs to run inside the locked transaction, andget_member_id_from_email()opens its own connection — but it leaves two copies of the same email-matching SQL that must be kept in sync if the email JSONB schema ever changes (e.g. additional email types, normalization rules).Suggested fix
Extract a helper that accepts an existing cursor, e.g.
_resolve_member_id_by_primary_email(cur, email), and have bothget_member_id_from_email()(passing a cursor from its own connection) and the advisory-lock block call it. Single source of truth for the primary-email match.Low priority — no functional bug, just drift risk.
Context: PR #284.