✨(contact) allow to create more than one contact with same email

[sdemagny]

Summary by CodeRabbit

• New Features

  • Automatically assigns a contact name from the provided display name or the email’s local part when messages are received.

• Bug Fixes

  • More accurate recipient handling: supports distinct contacts for the same email when display names differ.
  • BCC recipients with identical emails but different names are no longer merged; all are preserved. TO/CC deduplication remains unchanged.

• Tests

  • Expanded import tests to cover multi-address BCC scenarios and updated assertions to reflect the refined deduplication behavior.

[coderabbitai]

Walkthrough

The Contact model’s uniqueness now includes name alongside email and mailbox. Inbound recipient handling now supplies a name (provided display name or the email local-part) when creating contacts. A migration updates the unique_together constraint. Tests adjust expectations to allow distinct BCC contacts with the same email but different names.

Changes

Cohort / File(s)	Summary
Inbound recipient handling src/backend/core/mda/inbound.py	Passes a name when creating/fetching recipient contacts; uses provided display name or derives from the email local-part; updates defaults accordingly.
Model constraint + migration src/backend/core/models.py, src/backend/core/migrations/0010_alter_contact_unique_together.py	Expands Contact unique_together to (email, mailbox, name); adds migration to enforce the new constraint.
Importer tests src/backend/core/tests/importer/test_imap_import.py	Updates tests: includes Bcc with multiple display names for the same email; asserts creation of distinct BCC contacts by name while maintaining TO/CC dedup assumptions.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant M as Mail Inbound
  participant RP as Recipient Parser
  participant CS as Contact Store (DB)

  M->>RP: Parse recipients (To/Cc/Bcc)
  loop for each recipient
    RP->>CS: get_or_create(email, mailbox, name)
    Note right of CS: name = provided or local-part of email
    CS-->>RP: Contact instance
  end
  RP-->>M: Recipient contacts list
  M-->>M: Proceed with message delivery

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• ✨(Message) avoid log error when there is duplicated email in recipien… #356 — Touches the same recipient-processing in inbound.py, focusing on recipient deduplication per type; closely related to adding name-aware contact creation.

Poem

I nudge the inbox, nose aglow ✉️
Names now tag along where emails go.
Three BCCs hop in a row—
Same burrow, different bows, you know.
A gentle twitch, constraints align,
Carrots counted, contacts fine.
Thump-thump—unique by name this time!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title clearly conveys the core change of permitting multiple contacts with the same email by adjusting the contact model’s uniqueness constraints, making it specific and directly related to the changeset. It succinctly highlights the new capability without introducing unrelated details.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch sdem/import_msg_recipients

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Pylint (3.3.8)

src/backend/core/mda/inbound.py

src/backend/core/migrations/0010_alter_contact_unique_together.py

src/backend/core/tests/importer/test_imap_import.py

• 1 others

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 935cc7b and 3a3832d.

📒 Files selected for processing (4)

• src/backend/core/mda/inbound.py (1 hunks)
• src/backend/core/migrations/0010_alter_contact_unique_together.py (1 hunks)
• src/backend/core/models.py (1 hunks)
• src/backend/core/tests/importer/test_imap_import.py (2 hunks)

🧰 Additional context used

📓 Path-based instructions (4)

src/backend/**/*.py

📄 CodeRabbit inference engine (.cursor/rules/django-python.mdc)

src/backend/**/*.py: Follow Django/PEP 8 style with a 100-character line limit
Use descriptive, snake_case names for variables and functions
Use Django ORM for database access; avoid raw SQL unless necessary for performance
Use Django’s built-in user model and authentication framework
Prefer try-except blocks to handle exceptions in business logic and views
Log expected and unexpected actions with appropriate log levels
Capture and report exceptions to Sentry; use capture_exception() for custom errors
Do not log sensitive information (tokens, passwords, financial/health data, PII)

Files:

• src/backend/core/tests/importer/test_imap_import.py
• src/backend/core/models.py
• src/backend/core/mda/inbound.py
• src/backend/core/migrations/0010_alter_contact_unique_together.py

src/backend/**/{tests.py,tests/**/*.py}

📄 CodeRabbit inference engine (.cursor/rules/django-python.mdc)

src/backend/**/{tests.py,tests/**/*.py}: Use Django’s testing tools (pytest-django) to ensure code quality and reliability
Unit tests should focus on a single use case, keep assertions minimal, and cover all possible cases

Files:

• src/backend/core/tests/importer/test_imap_import.py

src/backend/**/{models.py,forms.py,views.py}

📄 CodeRabbit inference engine (.cursor/rules/django-python.mdc)

Keep business logic in models and forms; keep views thin and focused on request handling

Files:

• src/backend/core/models.py

src/backend/**/{models.py,migrations/**/*.py}

📄 CodeRabbit inference engine (.cursor/rules/django-python.mdc)

Implement database indexing and query optimization (Model Meta indexes, constraints)

Files:

• src/backend/core/models.py
• src/backend/core/migrations/0010_alter_contact_unique_together.py

🧬 Code graph analysis (2)

src/backend/core/tests/importer/test_imap_import.py (1)

src/backend/core/enums.py (1)

• MessageRecipientTypeChoices (31-36)

src/backend/core/mda/inbound.py (1)

src/backend/core/models.py (1)

• Contact (1114-1137)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: docker-publish-mta-out / docker-build-push
• GitHub Check: docker-publish-socks-proxy / docker-build-push
• GitHub Check: build-front
• GitHub Check: test-back

🔇 Additional comments (4)

src/backend/core/tests/importer/test_imap_import.py (2)

70-70: LGTM: Test data properly simulates name-aware deduplication.

The BCC header now includes three recipients with the same email but different display names ("Jean BCC", "Marie BCC", and no name), which correctly tests the new behavior where contacts are deduplicated by (email, mailbox, name).

---

457-463: LGTM: Assertions correctly validate the new deduplication behavior.

The test properly verifies that:

• TO and CC recipients are still deduplicated (1 each)
• BCC recipients with different names for the same email create distinct contacts (3 total)

This validates the updated unique constraint on Contact model.

src/backend/core/mda/inbound.py (1)

634-639: LGTM: Consistent name handling for recipient contacts.

The code now ensures recipient contacts always have a name by using the provided display name or falling back to the email local-part. This aligns with:

• The updated unique_together constraint in the Contact model
• The sender contact creation logic (lines 489-496)

The email validation at lines 630-632 ensures the email format is valid before splitting, preventing errors.

src/backend/core/migrations/0010_alter_contact_unique_together.py (1)

1-17: Verify migration against existing data.

The migration correctly adds name to the unique constraint. Before applying, ensure:

1. No existing duplicate (email, mailbox) pairs:

python src/backend/manage.py shell << 'EOF'
from django.db.models import Count
from core.models import Contact

duplicates = Contact.objects.values('email', 'mailbox') \
    .annotate(count=Count('id')).filter(count__gt=1)
if duplicates:
    print("Duplicates found:", list(duplicates))
else:
    print("OK: No duplicate (email, mailbox) pairs")
EOF

2. No records with name = NULL:

python src/backend/manage.py shell << 'EOF'
from core.models import Contact
null_count = Contact.objects.filter(name__isnull=True).count()
print(f"NULL name count: {null_count}")
EOF

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Risk: nullable field in unique constraint.

Adding name to unique_together when the field is nullable (line 1117) can lead to unexpected behavior. In most SQL databases (including PostgreSQL), NULL != NULL, so multiple contacts with the same email and mailbox but NULL names would be allowed, potentially creating duplicates.

Consider one of these solutions:

Solution 1: Make name required

-    name = models.CharField(_("name"), max_length=255, null=True, blank=True)
+    name = models.CharField(_("name"), max_length=255)

Solution 2: Use application-level default

If name must remain nullable in the database, ensure the application always provides a default value (like the email local-part) before saving, and document this requirement clearly.

Solution 3: Add a database constraint

Add a CheckConstraint to ensure name is never NULL:

class Meta:
    db_table = "messages_contact"
    verbose_name = _("contact")
    verbose_name_plural = _("contacts")
    unique_together = ("email", "mailbox", "name")
    constraints = [
        models.CheckConstraint(
            check=~models.Q(name__isnull=True),
            name="contact_name_not_null",
        ),
    ]