🗃️(contacts) rename base to override

To improve code readability, I propose to rename
the contact field `override`. This comes along
with the fact a contact should not not always
override another (it's the case were I only want
to create some personal contacts).
This commit is contained in:
Quentin BEY
2024-12-02 11:14:19 +01:00
committed by BEY Quentin
parent 625f122ad5
commit 60ab61d125
9 changed files with 119 additions and 63 deletions

View File

@@ -194,7 +194,7 @@ class ContactAdmin(admin.ModelAdmin):
list_display = ( list_display = (
"full_name", "full_name",
"owner", "owner",
"base", "override",
) )

View File

@@ -14,7 +14,7 @@ class ContactSerializer(serializers.ModelSerializer):
model = models.Contact model = models.Contact
fields = [ fields = [
"id", "id",
"base", "override",
"data", "data",
"full_name", "full_name",
"notes", "notes",
@@ -23,12 +23,12 @@ class ContactSerializer(serializers.ModelSerializer):
] ]
read_only_fields = ["id", "owner"] read_only_fields = ["id", "owner"]
extra_kwargs = { extra_kwargs = {
"base": {"required": False}, "override": {"required": False},
} }
def update(self, instance, validated_data): def update(self, instance, validated_data):
"""Make "base" field readonly but only for update/patch.""" """Make "override" field readonly but only for update/patch."""
validated_data.pop("base", None) validated_data.pop("override", None)
return super().update(instance, validated_data) return super().update(instance, validated_data)

View File

@@ -152,7 +152,7 @@ class ContactViewSet(
# - are profile contacts for a user # - are profile contacts for a user
user__isnull=True, user__isnull=True,
# - are overriden base contacts # - are overriden base contacts
overriding_contacts__isnull=True, overridden_by__isnull=True,
) )
# Search by case-insensitive and accent-insensitive similarity # Search by case-insensitive and accent-insensitive similarity

View File

@@ -115,7 +115,16 @@ class ContactFactory(BaseContactFactory):
class Meta: class Meta:
model = models.Contact model = models.Contact
base = factory.SubFactory("core.factories.ContactFactory", base=None, owner=None) owner = factory.SubFactory("core.factories.UserFactory", profile_contact=None)
class OverrideContactFactory(BaseContactFactory):
"""A factory to create contacts for a user"""
class Meta:
model = models.Contact
override = factory.SubFactory("core.factories.ContactFactory", owner=None)
owner = factory.SubFactory("core.factories.UserFactory", profile_contact=None) owner = factory.SubFactory("core.factories.UserFactory", profile_contact=None)

View File

@@ -0,0 +1,47 @@
# Generated by Django 5.1.3 on 2024-12-02 10:04
import django.db.models.deletion
from django.db import migrations, models
class Migration(migrations.Migration):
dependencies = [
('core', '0006_contact_notes_alter_contact_full_name'),
]
operations = [
migrations.RemoveConstraint(
model_name='contact',
name='base_owner_constraint',
),
migrations.RemoveConstraint(
model_name='contact',
name='base_not_self',
),
migrations.AlterUniqueTogether(
name='contact',
unique_together=set(),
),
migrations.AddField(
model_name='contact',
name='override',
field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='overridden_by', to='core.contact'),
),
migrations.AddConstraint(
model_name='contact',
constraint=models.CheckConstraint(condition=models.Q(('override__isnull', False), ('owner__isnull', True), _negated=True), name='override_owner_constraint', violation_error_message='A contact overriding a base contact must be owned.'),
),
migrations.AddConstraint(
model_name='contact',
constraint=models.CheckConstraint(condition=models.Q(('override', models.F('id')), _negated=True), name='override_not_self', violation_error_message='A contact cannot override itself.'),
),
migrations.RemoveField(
model_name='contact',
name='base',
),
migrations.AlterUniqueTogether(
name='contact',
unique_together={('owner', 'override')},
),
]

View File

@@ -99,10 +99,10 @@ class BaseModel(models.Model):
class Contact(BaseModel): class Contact(BaseModel):
"""User contacts""" """User contacts"""
base = models.ForeignKey( override = models.ForeignKey(
"self", "self",
on_delete=models.SET_NULL, on_delete=models.SET_NULL,
related_name="overriding_contacts", related_name="overridden_by",
null=True, null=True,
blank=True, blank=True,
) )
@@ -136,17 +136,17 @@ class Contact(BaseModel):
ordering = ("full_name", "short_name") ordering = ("full_name", "short_name")
verbose_name = _("contact") verbose_name = _("contact")
verbose_name_plural = _("contacts") verbose_name_plural = _("contacts")
unique_together = ("owner", "base") unique_together = ("owner", "override")
constraints = [ constraints = [
models.CheckConstraint( models.CheckConstraint(
condition=~models.Q(base__isnull=False, owner__isnull=True), condition=~models.Q(override__isnull=False, owner__isnull=True),
name="base_owner_constraint", name="override_owner_constraint",
violation_error_message="A contact overriding a base contact must be owned.", violation_error_message="A contact overriding a base contact must be owned.",
), ),
models.CheckConstraint( models.CheckConstraint(
condition=~models.Q(base=models.F("id")), condition=~models.Q(override=models.F("id")),
name="base_not_self", name="override_not_self",
violation_error_message="A contact cannot be based on itself.", violation_error_message="A contact cannot override itself.",
), ),
] ]
@@ -157,10 +157,10 @@ class Contact(BaseModel):
"""Validate fields.""" """Validate fields."""
super().clean() super().clean()
# Check if the contact points to a base contact that itself points to another base contact # Check if the contact overrides a contact that itself overrides another base contact
if self.base_id and self.base.base_id: if self.override_id and self.override.override_id:
raise exceptions.ValidationError( raise exceptions.ValidationError(
"A contact cannot point to a base contact that itself points to a base contact." "A contact cannot override a contact that itself overrides a contact."
) )
# Validate the content of the "data" field against our jsonschema definition # Validate the content of the "data" field against our jsonschema definition

View File

@@ -67,7 +67,7 @@ def test_api_contacts_list_anonymous():
def test_api_contacts_list_authenticated_no_query(): def test_api_contacts_list_authenticated_no_query():
""" """
Authenticated users should be able to list contacts without applying a query. Authenticated users should be able to list contacts without applying a query.
Profile and base contacts should be excluded. Profile and overridden contacts should be excluded.
""" """
user = factories.UserFactory() user = factories.UserFactory()
contact = factories.ContactFactory(owner=user) contact = factories.ContactFactory(owner=user)
@@ -78,10 +78,10 @@ def test_api_contacts_list_authenticated_no_query():
assert user.profile_contact is not None # Excluded because profile contact assert user.profile_contact is not None # Excluded because profile contact
base_contact = factories.BaseContactFactory() # Excluded because overridden base_contact = factories.BaseContactFactory() # Excluded because overridden
factories.ContactFactory( factories.ContactFactory(
base=base_contact override=base_contact
) # Excluded because belongs to other user ) # Excluded because belongs to other user
contact2 = factories.ContactFactory( contact2 = factories.ContactFactory(
base=base_contact, owner=user, full_name="Bernard" override=base_contact, owner=user, full_name="Bernard"
) # Included ) # Included
client = APIClient() client = APIClient()
@@ -93,7 +93,7 @@ def test_api_contacts_list_authenticated_no_query():
assert response.json() == [ assert response.json() == [
{ {
"id": str(contact2.id), "id": str(contact2.id),
"base": str(base_contact.id), "override": str(base_contact.id),
"owner": str(contact2.owner.id), "owner": str(contact2.owner.id),
"data": contact2.data, "data": contact2.data,
"full_name": contact2.full_name, "full_name": contact2.full_name,
@@ -268,7 +268,7 @@ def test_api_contacts_retrieve_authenticated_owned():
assert response.status_code == 200 assert response.status_code == 200
assert response.json() == { assert response.json() == {
"id": str(contact.id), "id": str(contact.id),
"base": str(contact.base.id), "override": None,
"owner": str(contact.owner.id), "owner": str(contact.owner.id),
"data": contact.data, "data": contact.data,
"full_name": contact.full_name, "full_name": contact.full_name,
@@ -291,7 +291,7 @@ def test_api_contacts_retrieve_authenticated_public():
assert response.status_code == 200 assert response.status_code == 200
assert response.json() == { assert response.json() == {
"id": str(contact.id), "id": str(contact.id),
"base": None, "override": None,
"owner": None, "owner": None,
"data": contact.data, "data": contact.data,
"full_name": contact.full_name, "full_name": contact.full_name,
@@ -351,7 +351,7 @@ def test_api_contacts_create_authenticated_missing_base():
new_contact = models.Contact.objects.get(owner=user) new_contact = models.Contact.objects.get(owner=user)
assert response.json() == { assert response.json() == {
"base": None, "override": None,
"data": {}, "data": {},
"full_name": "David Bowman", "full_name": "David Bowman",
"id": str(new_contact.pk), "id": str(new_contact.pk),
@@ -370,12 +370,12 @@ def test_api_contacts_create_authenticated_successful():
client.force_login(user) client.force_login(user)
# Existing override for another user should not interfere # Existing override for another user should not interfere
factories.ContactFactory(base=base_contact) factories.ContactFactory(override=base_contact)
response = client.post( response = client.post(
"/api/v1.0/contacts/", "/api/v1.0/contacts/",
{ {
"base": str(base_contact.id), "override": str(base_contact.id),
"full_name": "David Bowman", "full_name": "David Bowman",
"short_name": "Dave", "short_name": "Dave",
"data": CONTACT_DATA, "data": CONTACT_DATA,
@@ -389,7 +389,7 @@ def test_api_contacts_create_authenticated_successful():
contact = models.Contact.objects.get(owner=user) contact = models.Contact.objects.get(owner=user)
assert response.json() == { assert response.json() == {
"id": str(contact.id), "id": str(contact.id),
"base": str(base_contact.id), "override": str(base_contact.id),
"data": CONTACT_DATA, "data": CONTACT_DATA,
"full_name": "David Bowman", "full_name": "David Bowman",
"notes": "", "notes": "",
@@ -400,19 +400,19 @@ def test_api_contacts_create_authenticated_successful():
assert contact.full_name == "David Bowman" assert contact.full_name == "David Bowman"
assert contact.short_name == "Dave" assert contact.short_name == "Dave"
assert contact.data == CONTACT_DATA assert contact.data == CONTACT_DATA
assert contact.base == base_contact assert contact.override == base_contact
assert contact.owner == user assert contact.owner == user
@override_settings(ALLOW_API_USER_CREATE=True) @override_settings(ALLOW_API_USER_CREATE=True)
def test_api_contacts_create_authenticated_existing_override(): def test_api_contacts_create_authenticated_existing_override():
""" """
Trying to create a contact for base contact that is already overridden by the user Trying to create a contact overriding a contact that is already overridden by the user
should receive a 400 error. should receive a 400 error.
""" """
user = factories.UserFactory(profile_contact=None) user = factories.UserFactory(profile_contact=None)
base_contact = factories.BaseContactFactory() base_contact = factories.BaseContactFactory()
factories.ContactFactory(base=base_contact, owner=user) factories.ContactFactory(override=base_contact, owner=user)
client = APIClient() client = APIClient()
client.force_login(user) client.force_login(user)
@@ -420,7 +420,7 @@ def test_api_contacts_create_authenticated_existing_override():
response = client.post( response = client.post(
"/api/v1.0/contacts/", "/api/v1.0/contacts/",
{ {
"base": str(base_contact.id), "override": str(base_contact.id),
"full_name": "David Bowman", "full_name": "David Bowman",
"notes": "", "notes": "",
"short_name": "Dave", "short_name": "Dave",
@@ -433,7 +433,7 @@ def test_api_contacts_create_authenticated_existing_override():
assert models.Contact.objects.count() == 2 assert models.Contact.objects.count() == 2
assert response.json() == { assert response.json() == {
"__all__": ["Contact with this Owner and Base already exists."] "__all__": ["Contact with this Owner and Override already exists."]
} }
@@ -461,7 +461,7 @@ def test_api_contacts_create_authenticated_successful_with_notes():
contact = models.Contact.objects.get(owner=user) contact = models.Contact.objects.get(owner=user)
assert response.json() == { assert response.json() == {
"id": str(contact.id), "id": str(contact.id),
"base": None, "override": None,
"data": CONTACT_DATA, "data": CONTACT_DATA,
"full_name": "David Bowman", "full_name": "David Bowman",
"notes": "This is a note", "notes": "This is a note",
@@ -484,7 +484,7 @@ def test_api_contacts_update_anonymous():
new_contact_values = serializers.ContactSerializer( new_contact_values = serializers.ContactSerializer(
instance=factories.ContactFactory() instance=factories.ContactFactory()
).data ).data
new_contact_values["base"] = str(factories.ContactFactory().id) new_contact_values["override"] = str(factories.ContactFactory().id)
response = APIClient().put( response = APIClient().put(
f"/api/v1.0/contacts/{contact.id!s}/", f"/api/v1.0/contacts/{contact.id!s}/",
new_contact_values, new_contact_values,
@@ -515,7 +515,7 @@ def test_api_contacts_update_authenticated_owned():
new_contact_values = serializers.ContactSerializer( new_contact_values = serializers.ContactSerializer(
instance=factories.ContactFactory() instance=factories.ContactFactory()
).data ).data
new_contact_values["base"] = str(factories.ContactFactory().id) new_contact_values["override"] = str(factories.ContactFactory().id)
response = client.put( response = client.put(
f"/api/v1.0/contacts/{contact.id!s}/", f"/api/v1.0/contacts/{contact.id!s}/",
@@ -528,7 +528,7 @@ def test_api_contacts_update_authenticated_owned():
contact.refresh_from_db() contact.refresh_from_db()
contact_values = serializers.ContactSerializer(instance=contact).data contact_values = serializers.ContactSerializer(instance=contact).data
for key, value in contact_values.items(): for key, value in contact_values.items():
if key in ["base", "owner", "id"]: if key in ["override", "owner", "id"]:
assert value == old_contact_values[key] assert value == old_contact_values[key]
else: else:
assert value == new_contact_values[key] assert value == new_contact_values[key]
@@ -551,7 +551,7 @@ def test_api_contacts_update_authenticated_profile():
new_contact_values = serializers.ContactSerializer( new_contact_values = serializers.ContactSerializer(
instance=factories.ContactFactory() instance=factories.ContactFactory()
).data ).data
new_contact_values["base"] = str(factories.ContactFactory().id) new_contact_values["override"] = str(factories.ContactFactory().id)
response = client.put( response = client.put(
f"/api/v1.0/contacts/{contact.id!s}/", f"/api/v1.0/contacts/{contact.id!s}/",
@@ -563,7 +563,7 @@ def test_api_contacts_update_authenticated_profile():
contact.refresh_from_db() contact.refresh_from_db()
contact_values = serializers.ContactSerializer(instance=contact).data contact_values = serializers.ContactSerializer(instance=contact).data
for key, value in contact_values.items(): for key, value in contact_values.items():
if key in ["base", "owner", "id"]: if key in ["override", "owner", "id"]:
assert value == old_contact_values[key] assert value == old_contact_values[key]
else: else:
assert value == new_contact_values[key] assert value == new_contact_values[key]
@@ -584,7 +584,7 @@ def test_api_contacts_update_authenticated_other():
new_contact_values = serializers.ContactSerializer( new_contact_values = serializers.ContactSerializer(
instance=factories.ContactFactory() instance=factories.ContactFactory()
).data ).data
new_contact_values["base"] = str(factories.ContactFactory().id) new_contact_values["override"] = str(factories.ContactFactory().id)
response = client.put( response = client.put(
f"/api/v1.0/contacts/{contact.id!s}/", f"/api/v1.0/contacts/{contact.id!s}/",
@@ -606,7 +606,7 @@ def test_api_contacts_delete_list_anonymous():
response = APIClient().delete("/api/v1.0/contacts/") response = APIClient().delete("/api/v1.0/contacts/")
assert response.status_code == 401 assert response.status_code == 401
assert models.Contact.objects.count() == 4 assert models.Contact.objects.count() == 2
def test_api_contacts_delete_list_authenticated(): def test_api_contacts_delete_list_authenticated():
@@ -621,7 +621,7 @@ def test_api_contacts_delete_list_authenticated():
response = client.delete("/api/v1.0/contacts/") response = client.delete("/api/v1.0/contacts/")
assert response.status_code == 405 assert response.status_code == 405
assert models.Contact.objects.count() == 4 assert models.Contact.objects.count() == 2
def test_api_contacts_delete_anonymous(): def test_api_contacts_delete_anonymous():
@@ -632,7 +632,7 @@ def test_api_contacts_delete_anonymous():
response = client.delete(f"/api/v1.0/contacts/{contact.id!s}/") response = client.delete(f"/api/v1.0/contacts/{contact.id!s}/")
assert response.status_code == 401 assert response.status_code == 401
assert models.Contact.objects.count() == 2 assert models.Contact.objects.count() == 1
def test_api_contacts_delete_authenticated_public(): def test_api_contacts_delete_authenticated_public():
@@ -669,7 +669,7 @@ def test_api_contacts_delete_authenticated_owner():
) )
assert response.status_code == 204 assert response.status_code == 204
assert models.Contact.objects.count() == 1 assert models.Contact.objects.count() == 0
assert models.Contact.objects.filter(id=contact.id).exists() is False assert models.Contact.objects.filter(id=contact.id).exists() is False
@@ -678,7 +678,7 @@ def test_api_contacts_delete_authenticated_profile():
Authenticated users should be allowed to delete their profile contact. Authenticated users should be allowed to delete their profile contact.
""" """
user = factories.UserFactory() user = factories.UserFactory()
contact = factories.ContactFactory(owner=user, base=None) contact = factories.ContactFactory(owner=user, override=None)
user.profile_contact = contact user.profile_contact = contact
user.save() user.save()
@@ -708,4 +708,4 @@ def test_api_contacts_delete_authenticated_other():
) )
assert response.status_code == 403 assert response.status_code == 403
assert models.Contact.objects.count() == 2 assert models.Contact.objects.count() == 1

View File

@@ -18,51 +18,51 @@ def test_models_contacts_str_full_name():
def test_models_contacts_base_self(): def test_models_contacts_base_self():
"""A contact should not point to itself as a base contact.""" """A contact should not override itself."""
contact = factories.ContactFactory() contact = factories.ContactFactory()
contact.base = contact contact.override = contact
with pytest.raises(ValidationError) as excinfo: with pytest.raises(ValidationError) as excinfo:
contact.save() contact.save()
error_message = ( error_message = (
"{'__all__': ['A contact cannot point to a base contact that itself points to a " "{'__all__': ['A contact cannot override a contact that itself overrides a contact.',"
"base contact.', 'A contact cannot be based on itself.']}" " 'A contact cannot override itself.']}"
) )
assert str(excinfo.value) == error_message assert str(excinfo.value) == error_message
def test_models_contacts_base_to_base(): def test_models_contacts_base_to_base():
"""A contact should not point to a base contact that is itself derived from a base contact.""" """A contact should not override a contact that is itself overrides contact."""
contact = factories.ContactFactory() contact = factories.OverrideContactFactory()
with pytest.raises(ValidationError) as excinfo: with pytest.raises(ValidationError) as excinfo:
factories.ContactFactory(base=contact) factories.OverrideContactFactory(override=contact)
error_message = ( error_message = (
"{'__all__': ['A contact cannot point to a base contact that itself points to a " "{'__all__': ['A contact cannot override a contact that "
"base contact.']}" "itself overrides a contact.']}"
) )
assert str(excinfo.value) == error_message assert str(excinfo.value) == error_message
def test_models_contacts_owner_base_unique(): def test_models_contacts_owner_base_unique():
"""There should be only one contact deriving from a given base contact for a given owner.""" """There should be only one contact overriding a contact for a given owner."""
contact = factories.ContactFactory() contact = factories.OverrideContactFactory()
with pytest.raises(ValidationError) as excinfo: with pytest.raises(ValidationError) as excinfo:
factories.ContactFactory(base=contact.base, owner=contact.owner) factories.OverrideContactFactory(override=contact.override, owner=contact.owner)
assert ( assert (
str(excinfo.value) str(excinfo.value)
== "{'__all__': ['Contact with this Owner and Base already exists.']}" == "{'__all__': ['Contact with this Owner and Override already exists.']}"
) )
def test_models_contacts_base_not_owned(): def test_models_contacts_base_not_owned():
"""A contact cannot have a base and not be owned.""" """A contact cannot override a contact without being owned."""
with pytest.raises(ValidationError) as excinfo: with pytest.raises(ValidationError) as excinfo:
factories.ContactFactory(owner=None) factories.OverrideContactFactory(owner=None)
assert ( assert (
str(excinfo.value) str(excinfo.value)
@@ -72,7 +72,7 @@ def test_models_contacts_base_not_owned():
def test_models_contacts_profile_not_owned(): def test_models_contacts_profile_not_owned():
"""A contact cannot be defined as profile for a user if is not owned.""" """A contact cannot be defined as profile for a user if is not owned."""
base_contact = factories.ContactFactory(owner=None, base=None) base_contact = factories.ContactFactory(owner=None)
with pytest.raises(ValidationError) as excinfo: with pytest.raises(ValidationError) as excinfo:
factories.UserFactory(profile_contact=base_contact) factories.UserFactory(profile_contact=base_contact)

View File

@@ -133,7 +133,7 @@ def test_models_users_email_not_unique():
def test_models_users_profile_not_owned(): def test_models_users_profile_not_owned():
"""A user cannot declare as profile a contact that not is owned.""" """A user cannot declare as profile a contact that not is owned."""
user = factories.UserFactory() user = factories.UserFactory()
contact = factories.ContactFactory(base=None, owner=None) contact = factories.ContactFactory(override=None, owner=None)
user.profile_contact = contact user.profile_contact = contact
with pytest.raises(ValidationError) as excinfo: with pytest.raises(ValidationError) as excinfo: