Skip to content

Sync member email from CiviCRM to UniFi Access#14

Open
RyanMorash wants to merge 14 commits into
mainfrom
sync-member-email
Open

Sync member email from CiviCRM to UniFi Access#14
RyanMorash wants to merge 14 commits into
mainfrom
sync-member-email

Conversation

@RyanMorash

Copy link
Copy Markdown
Member

Summary

Provisions and keeps a member's CiviCRM primary email synced onto the UniFi Access user_email field, so credential/PIN-invite delivery reaches the right address. Email is treated as functional in UniFi (not cosmetic), so differences reconcile every cycle.

An optional email field rides the existing CiviMember → ResolvedMember → Diff → UnifiUser flow — no new module, no new diff set, and the pure/impure boundary and safety guards are untouched.

  • Read (CiviCRM): selects email_primary.email on the existing Contact.get; empty/missing → None.
  • Pure passthrough: tier_mapping carries email through unchanged.
  • Reconcile: email changes route into the existing to_update_credential set (same PUT /users/{id} endpoint as display_name), compared case-insensitively so case-only differences don't churn. None and "" are treated as equal.
  • Read/Write (UniFi): parses user_email on read; writes it on create, reactivation, and credential update. A name+email change is sent as a single combined PUT. An empty string clears a removed email.
  • Warn-but-provision: the orchestrator logs one WARN per door-tier-resolved member that has no email, then provisions them normally (door access never depends on email).
  • Strict layering respected — the UniFi client uses a local case-insensitive comparison rather than importing the reconciler.

Design spec: docs/superpowers/specs/2026-06-17-sync-member-email-design.md
Implementation plan: docs/superpowers/plans/2026-06-17-sync-member-email.md
Architecture §6 data contracts updated.

Test Plan

  • uv run pytest — 307 passed
  • uv run pyrefly check — 0 errors
  • uv run ruff check . — clean
  • Reconciler: email-only change → to_update_credential; case-only diff → no-op; None vs "" → no-op; set-vs-None → update; idempotency canary exercises an email-triggered update and re-diffs empty
  • CiviCRM: maps email_primary.email; missing and empty-string both → None
  • UniFi: parses user_email; writes it on create / reactivate / credential-update; combined single PUT for name+email; clears to "" when removed; omits the key entirely when None
  • Orchestrator: tier member with no email → exactly one WARN and still provisioned; non-tier member with no email → no warning
  • Confirm against a live controller that the GET /users read-back field is user_email (assumed to match the write field) and that UniFi doesn't rewrite stored emails beyond case — a dry-run (uv run door-sync run --once --dry-run) surfaces any mismatch as a persistent would-update-credential ... email-change line

🤖 Generated with Claude Code

RyanMorash and others added 13 commits June 17, 2026 18:29
Email is functional in UniFi Access (credential/PIN delivery). Spec rides the
existing CiviMember → ResolvedMember → Diff → UnifiUser flow, reusing
to_update_credential (shared PUT /users endpoint), with case-insensitive
comparison and a warn-but-provision path for members missing an email.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Eight TDD tasks: models field, tier passthrough, reconciler case-insensitive
compare, CiviCRM read, UniFi read/write of user_email, orchestrator warn-but-
provision, and doc/verification. Default-None field keeps existing body
assertions green; new behavior is additive.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Pass member.email to all three ResolvedMember constructions in resolve
(no-memberships, unmapped, and tier-match branches). Pure passthrough —
no logic added. Five new tests cover each branch and the None case.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ry; doc fix

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Read the user_email field from UniFi API user rows and populate
UnifiUser.email; absent or empty values map to None.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…pdate

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add test_apply_update_credential_email_cleared_to_none to verify that when
resolved.email=None and the UniFi user has an existing email, exactly one PUT
to /users/:id is issued with {"user_email": ""} to clear it, and no
nfc_cards calls are made.

Update _prepare_reactivation docstring to mention that it also sets
user_email when present, and describe the stale-card deletion behaviour.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add email: str | None = None to CiviMember, ResolvedMember, and
UnifiUser in the §6 code block, and extend the Naming note to explain
that email rides to_update_credential (same PUT endpoint, compared
case-insensitively).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add test_apply_reactivate_includes_user_email_when_set to verify that
_prepare_reactivation includes user_email in the profile PUT body when
the resolved member has an email set.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 17, 2026 23:38
@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@RyanMorash, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 46 minutes and 49 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 90924393-6544-4dfc-be14-9ea2a8eae972

📥 Commits

Reviewing files that changed from the base of the PR and between dcc3d3a and e39a99f.

📒 Files selected for processing (15)
  • docs/architecture.md
  • docs/superpowers/plans/2026-06-17-sync-member-email.md
  • docs/superpowers/specs/2026-06-17-sync-member-email-design.md
  • src/door_sync/civicrm/client.py
  • src/door_sync/models.py
  • src/door_sync/orchestrator.py
  • src/door_sync/reconciler.py
  • src/door_sync/tier_mapping.py
  • src/door_sync/unifi/client.py
  • tests/test_civicrm_client.py
  • tests/test_models.py
  • tests/test_orchestrator.py
  • tests/test_reconciler.py
  • tests/test_tier_mapping.py
  • tests/test_unifi_client.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sync-member-email

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds end-to-end syncing of a member’s primary CiviCRM email into UniFi Access user_email, carrying the field through the existing CiviMember → ResolvedMember → Diff → UnifiUser pipeline and reconciling email differences via the existing credential-update path.

Changes:

  • Extend core models and pure pipeline (tier_mapping, reconciler) to carry/compare optional email (case-insensitive; None/"" treated as equal).
  • Update CiviCRM read to select/map email_primary.email, and UniFi read/write to parse/set user_email on fetch/create/reactivate/credential update.
  • Add orchestrator warning for door-tier members missing email, plus broad test coverage and supporting documentation/spec/plan updates.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/door_sync/models.py Add optional email field to core dataclasses and update docstrings.
src/door_sync/civicrm/client.py Select and map email_primary.email into CiviMember.email.
src/door_sync/tier_mapping.py Pure passthrough of email into ResolvedMember.
src/door_sync/reconciler.py Treat email changes as credential changes (case-insensitive compare).
src/door_sync/unifi/client.py Parse user_email on read; write it on create/reactivate/update; dry-run logging.
src/door_sync/orchestrator.py Warn when a tier-resolved member has no email, without halting provisioning.
tests/test_models.py Assert email defaults and assignment on all three models.
tests/test_civicrm_client.py Validate select list includes email and mapping/normalization behavior.
tests/test_tier_mapping.py Validate email passthrough across resolution branches.
tests/test_reconciler.py Validate email diff behavior and idempotency carry-through.
tests/test_unifi_client.py Validate UniFi email parsing and request bodies for create/reactivate/update.
tests/test_orchestrator.py Validate warn-but-provision behavior for tier members missing email.
docs/superpowers/specs/2026-06-17-sync-member-email-design.md Design spec documenting intended behavior and constraints.
docs/superpowers/plans/2026-06-17-sync-member-email.md Implementation plan detailing steps and verification.
docs/architecture.md Data contract update documenting the new email field and diff semantics.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/door_sync/unifi/client.py Outdated
Comment on lines +2013 to +2026
user_puts = [
r
for r in httpx_mock.get_requests()
if r.method == "PUT" and r.url.path == "/api/v1/developer/users/uuid-42"
]
assert len(user_puts) == 2
profile_put_body = _json.loads(
next(r for r in user_puts if "employee_number" in _json.loads(r.content)).content
)
assert profile_put_body["user_email"] == "react@example.com"
assert profile_put_body["employee_number"] == "42"
# The activate PUT must not contain user_email.
activate_put_body = _json.loads(user_puts[1].content)
assert activate_put_body == {"status": "ACTIVE"}
Address Copilot PR review:
- _prepare_reactivation now sends user_email unconditionally (empty string
  clears) so a removed CiviCRM email is cleared in the same reconcile cycle,
  matching the credential-update path. Prevents a stale address persisting —
  and potentially misdelivering invites — until the next cycle. True-create
  still omits user_email (a new record has nothing to clear).
- Select the activate PUT in the reactivation test by body (absence of
  employee_number) rather than by list index, so it doesn't depend on request
  ordering.
- Add a test asserting reactivation clears a stale UniFi email to "".

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants