From f759d318b9f70d6aed5b910dc52d9cd276c67fd3 Mon Sep 17 00:00:00 2001 From: Quentin BEY Date: Mon, 2 Dec 2024 16:26:41 +0100 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F(contacts)=20move=20user=20pr?= =?UTF-8?q?ofile=20to=20contact?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the user <-> contact relation for "profile" to the contact model. Now the Contact model is the only one to point to User (and not backward). Contact: - FK to User for the owner - FK to User for the profile --- CHANGELOG.md | 4 ++ src/backend/core/authentication/backends.py | 20 +++++++- src/backend/core/factories.py | 4 +- .../0008_change_user_profile_to_contact.py | 47 +++++++++++++++++++ src/backend/core/models.py | 27 ++++++----- src/backend/core/tests/test_api_contacts.py | 22 ++++----- src/backend/core/tests/test_api_users.py | 8 +--- .../core/tests/test_models_contacts.py | 7 ++- src/backend/core/tests/test_models_users.py | 8 ++-- 9 files changed, 104 insertions(+), 43 deletions(-) create mode 100644 src/backend/core/migrations/0008_change_user_profile_to_contact.py diff --git a/CHANGELOG.md b/CHANGELOG.md index d4c2bf0..697e879 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ and this project adheres to - ✨(contacts) add notes & force full_name #565 +### Changed + +- ♻️(contacts) move user profile to contact #572 + ## [1.7.1] - 2024-11-28 ## [1.7.0] - 2024-11-28 diff --git a/src/backend/core/authentication/backends.py b/src/backend/core/authentication/backends.py index d9f2102..3a33e02 100644 --- a/src/backend/core/authentication/backends.py +++ b/src/backend/core/authentication/backends.py @@ -14,7 +14,12 @@ from mozilla_django_oidc.auth import ( OIDCAuthenticationBackend as MozillaOIDCAuthenticationBackend, ) -from core.models import Organization, OrganizationAccess, OrganizationRoleChoices +from core.models import ( + Contact, + Organization, + OrganizationAccess, + OrganizationRoleChoices, +) logger = logging.getLogger(__name__) @@ -196,6 +201,19 @@ class OIDCAuthenticationBackend(MozillaOIDCAuthenticationBackend): user=user, role=OrganizationRoleChoices.ADMIN, ) + + # Initiate the user's profile + Contact.objects.create( + owner=user, + user=user, + full_name=name or email, + data={ + "emails": [ + {"type": "Work", "value": email}, + ], + }, + ) + return user def compute_full_name(self, user_info): diff --git a/src/backend/core/factories.py b/src/backend/core/factories.py index a5c8a7e..54e46f3 100644 --- a/src/backend/core/factories.py +++ b/src/backend/core/factories.py @@ -115,7 +115,7 @@ class ContactFactory(BaseContactFactory): class Meta: model = models.Contact - owner = factory.SubFactory("core.factories.UserFactory", profile_contact=None) + owner = factory.SubFactory("core.factories.UserFactory") class OverrideContactFactory(BaseContactFactory): @@ -125,7 +125,7 @@ class OverrideContactFactory(BaseContactFactory): 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") class OrganizationFactory(factory.django.DjangoModelFactory): diff --git a/src/backend/core/migrations/0008_change_user_profile_to_contact.py b/src/backend/core/migrations/0008_change_user_profile_to_contact.py new file mode 100644 index 0000000..508cd40 --- /dev/null +++ b/src/backend/core/migrations/0008_change_user_profile_to_contact.py @@ -0,0 +1,47 @@ +# Generated by Django 5.1.3 on 2024-12-02 10:58 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +def init_profile_contact(apps, schema_editor): + """Create a Contact for each User to be used as their profile.""" + User = apps.get_model("core", "User") + Contact = apps.get_model("core", "Contact") + + for user in User.objects.all(): # This is a small table for now + Contact.objects.create( + owner=user, + user=user, + full_name=user.name or user.email, + data={ + "emails": [ + {"type": "Work", "value": user.email}, + ], + }, + ) + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0007_rename_contact_base_to_override'), + ] + + operations = [ + migrations.RemoveField( + model_name='user', + name='profile_contact', + ), + migrations.AddField( + model_name='contact', + name='user', + field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='profile_contact', to=settings.AUTH_USER_MODEL), + ), + migrations.AddConstraint( + model_name='contact', + constraint=models.CheckConstraint(condition=models.Q(('user__isnull', True), models.Q(('owner', models.F('user')), ('owner__isnull', False)), _connector='OR'), name='profile_contact_owner_constraint', violation_error_message='Users can only declare as profile a contact they own.'), + ), + migrations.RunPython(init_profile_contact, reverse_code=migrations.RunPython.noop), + ] diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 327d5e2..a272ec5 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -113,6 +113,14 @@ class Contact(BaseModel): null=True, blank=True, ) + user = models.OneToOneField( + settings.AUTH_USER_MODEL, + on_delete=models.SET_NULL, + related_name="profile_contact", + blank=True, + null=True, + ) + full_name = models.CharField(_("full name"), max_length=150) short_name = models.CharField(_("short name"), max_length=30, null=True, blank=True) @@ -148,6 +156,13 @@ class Contact(BaseModel): name="override_not_self", violation_error_message="A contact cannot override itself.", ), + # When a user is set, the owner must be the user + models.CheckConstraint( + condition=models.Q(user__isnull=True) + | models.Q(owner=models.F("user"), owner__isnull=False), + name="profile_contact_owner_constraint", + violation_error_message="Users can only declare as profile a contact they own.", + ), ] def __str__(self): @@ -392,13 +407,6 @@ class User(AbstractBaseUser, BaseModel, auth_models.PermissionsMixin): ) email = models.EmailField(_("email address"), null=True, blank=True) name = models.CharField(_("name"), max_length=100, null=True, blank=True) - profile_contact = models.OneToOneField( - Contact, - on_delete=models.SET_NULL, - related_name="user", - blank=True, - null=True, - ) language = models.CharField( max_length=10, choices=lazy(lambda: settings.LANGUAGES, tuple)(), @@ -467,11 +475,6 @@ class User(AbstractBaseUser, BaseModel, auth_models.PermissionsMixin): if self.email: self.email = User.objects.normalize_email(self.email) - if self.profile_contact_id and not self.profile_contact.owner == self: - raise exceptions.ValidationError( - "Users can only declare as profile a contact they own." - ) - def _convert_valid_invitations(self): """ Convert valid invitations to team accesses. diff --git a/src/backend/core/tests/test_api_contacts.py b/src/backend/core/tests/test_api_contacts.py index f5b94a3..e621075 100644 --- a/src/backend/core/tests/test_api_contacts.py +++ b/src/backend/core/tests/test_api_contacts.py @@ -70,9 +70,7 @@ def test_api_contacts_list_authenticated_no_query(): Profile and overridden contacts should be excluded. """ user = factories.UserFactory() - contact = factories.ContactFactory(owner=user) - user.profile_contact = contact - user.save() + factories.ContactFactory(owner=user, user=user) # Let's have 5 contacts in database: assert user.profile_contact is not None # Excluded because profile contact @@ -332,7 +330,7 @@ def test_api_contacts_create_anonymous_forbidden(): def test_api_contacts_create_authenticated_missing_base(): """Authenticated user should be able to create contact without override.""" - user = factories.UserFactory(profile_contact=None) + user = factories.UserFactory() client = APIClient() client.force_login(user) @@ -363,7 +361,7 @@ def test_api_contacts_create_authenticated_missing_base(): def test_api_contacts_create_authenticated_successful(): """Authenticated users should be able to create contacts.""" - user = factories.UserFactory(profile_contact=None) + user = factories.UserFactory() base_contact = factories.BaseContactFactory() client = APIClient() @@ -410,7 +408,7 @@ def test_api_contacts_create_authenticated_existing_override(): Trying to create a contact overriding a contact that is already overridden by the user should receive a 400 error. """ - user = factories.UserFactory(profile_contact=None) + user = factories.UserFactory() base_contact = factories.BaseContactFactory() factories.ContactFactory(override=base_contact, owner=user) @@ -439,7 +437,7 @@ def test_api_contacts_create_authenticated_existing_override(): def test_api_contacts_create_authenticated_successful_with_notes(): """Authenticated users should be able to create contacts with notes.""" - user = factories.UserFactory(profile_contact=None) + user = factories.UserFactory() client = APIClient() client.force_login(user) @@ -504,7 +502,7 @@ def test_api_contacts_update_authenticated_owned(): """ Authenticated users should be allowed to update their own contacts. """ - user = factories.UserFactory(profile_contact=None) + user = factories.UserFactory() client = APIClient() client.force_login(user) @@ -543,9 +541,7 @@ def test_api_contacts_update_authenticated_profile(): client = APIClient() client.force_login(user) - contact = factories.ContactFactory(owner=user) - user.profile_contact = contact - user.save() + contact = factories.ContactFactory(owner=user, user=user) old_contact_values = serializers.ContactSerializer(instance=contact).data new_contact_values = serializers.ContactSerializer( @@ -678,9 +674,7 @@ def test_api_contacts_delete_authenticated_profile(): Authenticated users should be allowed to delete their profile contact. """ user = factories.UserFactory() - contact = factories.ContactFactory(owner=user, override=None) - user.profile_contact = contact - user.save() + contact = factories.ContactFactory(owner=user, user=user) client = APIClient() client.force_login(user) diff --git a/src/backend/core/tests/test_api_users.py b/src/backend/core/tests/test_api_users.py index 2879431..ba64a1c 100644 --- a/src/backend/core/tests/test_api_users.py +++ b/src/backend/core/tests/test_api_users.py @@ -534,9 +534,7 @@ def test_api_users_retrieve_me_authenticated(): client.force_login(user) # Define profile contact - contact = factories.ContactFactory(owner=user) - user.profile_contact = contact - user.save() + factories.ContactFactory(owner=user, user=user) factories.UserFactory.create_batch(2) response = client.get( @@ -575,9 +573,7 @@ def test_api_users_retrieve_me_authenticated_abilities(): client.force_login(user) # Define profile contact - contact = factories.ContactFactory(owner=user) - user.profile_contact = contact - user.save() + factories.ContactFactory(owner=user, user=user) factories.UserFactory.create_batch(2) diff --git a/src/backend/core/tests/test_models_contacts.py b/src/backend/core/tests/test_models_contacts.py index b882d68..98057ea 100644 --- a/src/backend/core/tests/test_models_contacts.py +++ b/src/backend/core/tests/test_models_contacts.py @@ -72,10 +72,8 @@ def test_models_contacts_base_not_owned(): def test_models_contacts_profile_not_owned(): """A contact cannot be defined as profile for a user if is not owned.""" - base_contact = factories.ContactFactory(owner=None) - with pytest.raises(ValidationError) as excinfo: - factories.UserFactory(profile_contact=base_contact) + factories.ContactFactory(owner=None, user=factories.UserFactory()) assert ( str(excinfo.value) @@ -88,7 +86,8 @@ def test_models_contacts_profile_owned_by_other(): contact = factories.ContactFactory() with pytest.raises(ValidationError) as excinfo: - factories.UserFactory(profile_contact=contact) + contact.user = factories.UserFactory() + contact.save() assert ( str(excinfo.value) diff --git a/src/backend/core/tests/test_models_users.py b/src/backend/core/tests/test_models_users.py index d2a86da..128daa1 100644 --- a/src/backend/core/tests/test_models_users.py +++ b/src/backend/core/tests/test_models_users.py @@ -135,9 +135,9 @@ def test_models_users_profile_not_owned(): user = factories.UserFactory() contact = factories.ContactFactory(override=None, owner=None) - user.profile_contact = contact with pytest.raises(ValidationError) as excinfo: - user.save() + contact.user = user + contact.save() assert ( str(excinfo.value) @@ -150,9 +150,9 @@ def test_models_users_profile_owned_by_other(): user = factories.UserFactory() contact = factories.ContactFactory() - user.profile_contact = contact with pytest.raises(ValidationError) as excinfo: - user.save() + contact.user = user + contact.save() assert ( str(excinfo.value)