(models) impose uniqueness on display name, to match ox's constraint

OpenXchange's primary key is display name (= first name + last name).
It must be unique in the domain's context. We don't have context info
but we can impose uniqueness by domain.
This commit is contained in:
Marie PUPO JEAMMET
2025-09-08 16:38:41 +02:00
committed by Marie
parent 608f8c6988
commit b24cb23a83
3 changed files with 111 additions and 4 deletions

View File

@@ -0,0 +1,25 @@
# Generated by Django 5.2.5 on 2025-09-08 12:41
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('mailbox_manager', '0025_alter_mailbox_secondary_email'),
]
operations = [
migrations.AlterUniqueTogether(
name='mailbox',
unique_together=set(),
),
migrations.AddConstraint(
model_name='mailbox',
constraint=models.UniqueConstraint(fields=('local_part', 'domain'), name='unique_username'),
),
migrations.AddConstraint(
model_name='mailbox',
constraint=models.UniqueConstraint(fields=('first_name', 'last_name', 'domain'), name='unique_ox_display_name'),
),
]

View File

@@ -303,7 +303,15 @@ class Mailbox(AbstractBaseUser, BaseModel):
db_table = "people_mail_box" db_table = "people_mail_box"
verbose_name = _("Mailbox") verbose_name = _("Mailbox")
verbose_name_plural = _("Mailboxes") verbose_name_plural = _("Mailboxes")
unique_together = ("local_part", "domain") constraints = [
models.UniqueConstraint(
fields=["local_part", "domain"], name="unique_username"
),
models.UniqueConstraint(
fields=["first_name", "last_name", "domain"],
name="unique_ox_display_name",
),
]
ordering = ["-created_at"] ordering = ["-created_at"]
def __str__(self): def __str__(self):

View File

@@ -86,6 +86,79 @@ def test_api_mailboxes__create_viewer_failure():
assert not models.Mailbox.objects.exists() assert not models.Mailbox.objects.exists()
def test_api_mailboxes__create_display_name_must_be_unique():
"""Primary id on OpenXchange is display name (first_name + last_name).
It needs to be unique on each context. We don't track context info for now
but can impose uniqueness by domain."""
access = factories.MailDomainAccessFactory(role=enums.MailDomainRoleChoices.OWNER)
existing_mailbox = factories.MailboxFactory(domain=access.domain)
client = APIClient()
client.force_login(access.user)
new_mailbox_data = {
"local_part": "something_else",
"first_name": existing_mailbox.first_name,
"last_name": existing_mailbox.last_name,
}
response = client.post(
f"/api/v1.0/mail-domains/{access.domain.slug}/mailboxes/",
new_mailbox_data,
format="json",
)
assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.json() == {
"__all__": [
"Mailbox with this First name, Last name and Domain already exists.",
]
}
assert models.Mailbox.objects.count() == 1
@responses.activate
def test_api_mailboxes__create_display_name_no_constraint_on_different_domains():
"""Should still be allowed to user same display name on another domain"""
existing_mailbox = factories.MailboxFactory()
access = factories.MailDomainAccessFactory(role=enums.MailDomainRoleChoices.ADMIN)
new_mailbox_data = {
"local_part": "something_else",
"first_name": existing_mailbox.first_name,
"last_name": existing_mailbox.last_name,
}
# ensure response
responses.add(
responses.GET,
re.compile(r".*/token/"),
body=TOKEN_OK,
status=status.HTTP_200_OK,
content_type="application/json",
)
responses.add(
responses.POST,
re.compile(rf".*/domains/{access.domain.name}/mailboxes/"),
body=response_mailbox_created(
f"{new_mailbox_data['local_part']}@{access.domain.name}"
),
status=status.HTTP_201_CREATED,
content_type="application/json",
)
client = APIClient()
client.force_login(access.user)
response = client.post(
f"/api/v1.0/mail-domains/{access.domain.slug}/mailboxes/",
new_mailbox_data,
format="json",
)
assert response.status_code == status.HTTP_201_CREATED
assert response.json()["first_name"] == existing_mailbox.first_name
assert response.json()["last_name"] == existing_mailbox.last_name
assert response.json()["status"] == enums.MailboxStatusChoices.ENABLED
assert models.Mailbox.objects.count() == 2
@pytest.mark.parametrize( @pytest.mark.parametrize(
"role", "role",
[enums.MailDomainRoleChoices.OWNER, enums.MailDomainRoleChoices.ADMIN], [enums.MailDomainRoleChoices.OWNER, enums.MailDomainRoleChoices.ADMIN],
@@ -772,7 +845,7 @@ def test_api_mailboxes__user_unrelated_to_domain():
def test_api_mailboxes__duplicate_display_name(): def test_api_mailboxes__duplicate_display_name():
""" """
In OpenXchange, the display name (first + last name) must be unique. In OpenXchange, the display name (first + last name) must be unique.
In absence of clear duplicate message from dimail (yet), we catch errors 500 and In absence of a specific response from dimail (yet), we catch errors 500 and
check if it's not due to the display name already existing in the context check if it's not due to the display name already existing in the context
""" """
# creating all needed objects # creating all needed objects
@@ -898,7 +971,7 @@ def test_api_mailboxes__handling_dimail_unexpected_error(caplog):
@responses.activate @responses.activate
def test_api_mailboxes__display_name_duplicate_error(caplog): def test_api_mailboxes__display_name_duplicate_error():
""" """
API should raise a clear error when display_name is already used on context. API should raise a clear error when display_name is already used on context.
""" """
@@ -955,7 +1028,8 @@ def test_api_mailboxes__display_name_duplicate_error(caplog):
assert response.status_code == status.HTTP_400_BAD_REQUEST assert response.status_code == status.HTTP_400_BAD_REQUEST
assert response.json() == { assert response.json() == {
"NON_FIELD_ERRORS": [ "NON_FIELD_ERRORS": [
f"First name + last name combination already in use in this context : {mailbox_data['local_part']}@primary.domain.com." f"First name + last name combination already in use in this \
context : {mailbox_data['local_part']}@primary.domain.com."
] ]
} }