From 1350876c26a28bfd631cd26972331c8896b8b1d2 Mon Sep 17 00:00:00 2001 From: FrankApiyo Date: Thu, 26 Sep 2024 16:43:30 +0300 Subject: [PATCH 1/5] Use email provided when creating an organization to set metadata --- .../test_organization_profile_viewset.py | 28 +++++++++++-------- onadata/apps/api/tools.py | 3 +- .../serializers/organization_serializer.py | 2 +- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/onadata/apps/api/tests/viewsets/test_organization_profile_viewset.py b/onadata/apps/api/tests/viewsets/test_organization_profile_viewset.py index 07fe2d4dd2..36b85fcd56 100644 --- a/onadata/apps/api/tests/viewsets/test_organization_profile_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_organization_profile_viewset.py @@ -196,8 +196,8 @@ def test_orgs_get_not_creator(self): response = view(request, user="denoinc") self.assertNotEqual(response.get("Cache-Control"), None) self.assertEqual(response.status_code, 200) - del self.company_data['email'] - del self.company_data['metadata'] + del self.company_data["email"] + del self.company_data["metadata"] self.assertEqual(response.data, self.company_data) self.assertIn("users", list(response.data)) for user in response.data["users"]: @@ -212,7 +212,7 @@ def test_orgs_get_anon(self): self.assertNotEqual(response.get("Cache-Control"), None) self.assertEqual(response.status_code, 200) del self.company_data["email"] - del self.company_data['metadata'] + del self.company_data["metadata"] self.assertEqual(response.data, self.company_data) self.assertIn("users", list(response.data)) for user in response.data["users"]: @@ -222,7 +222,10 @@ def test_orgs_get_anon(self): def test_orgs_create(self): self._org_create() self.assertTrue(self.organization.user.is_active) - self.assertEqual(self.organization.user.email, "mail@mail-server.org") + self.assertTrue("billing_email" in self.organization.metadata) + self.assertEqual( + self.organization.metadata["billing_email"], "mail@mail-server.org" + ) def test_orgs_create_without_name(self): data = { @@ -264,7 +267,8 @@ def test_org_create_and_fetch_by_admin_user(self): request.user = self.user response = self.view(request) self.assertEqual(response.status_code, 201) - self.assertEqual(response.data['email'], org_email) + self.assertTrue("billing_email" in response.data["metadata"]) + self.assertEqual(response.data["metadata"]["billing_email"], org_email) def test_org_create_with_anonymous_user(self): data = { @@ -420,7 +424,7 @@ def test_member_sees_orgs_added_to(self): } ) del expected_data["metadata"] - del expected_data['email'] + del expected_data["email"] request = self.factory.get("/", **self.extra) response = view(request) @@ -452,8 +456,10 @@ def test_role_for_org_non_owner(self): request = self.factory.get("/", **self.extra) response = view(request, user="denoinc") self.assertEqual(response.status_code, 200) - self.assertTrue('email' in response.data) - self.assertEqual(response.data['email'], 'mail@mail-server.org') + self.assertTrue("billing_email" in response.data["metadata"]) + self.assertEqual( + response.data["metadata"]["billing_email"], "mail@mail-server.org" + ) self.assertIn("users", list(response.data)) for user in response.data["users"]: @@ -476,10 +482,10 @@ def test_role_for_org_non_owner(self): request = self.factory.get("/", **self.extra) request.user = AnonymousUser() request.headers = None - request.META['HTTP_AUTHORIZATION'] = "" + request.META["HTTP_AUTHORIZATION"] = "" response = view(request, user="denoinc") self.assertEqual(response.status_code, 200) - self.assertFalse('email' in response.data) + self.assertFalse("email" in response.data) def test_add_members_to_org_with_anonymous_user(self): self._org_create() @@ -982,7 +988,7 @@ def test_member_added_to_org_with_correct_perms(self): }, { "is_org": True, - "metadata": {}, + "metadata": {"billing_email": "mail@mail-server.org"}, "first_name": "Dennis", "last_name": "", "user": "denoinc", diff --git a/onadata/apps/api/tools.py b/onadata/apps/api/tools.py index 164ef12a4d..28534e4818 100644 --- a/onadata/apps/api/tools.py +++ b/onadata/apps/api/tools.py @@ -172,7 +172,6 @@ def create_organization_object(org_name, creator, attrs=None): username=org_name, first_name=first_name, last_name=last_name, - email=email, is_active=getattr(settings, "ORG_ON_CREATE_IS_ACTIVE", True), ) new_user.save() @@ -191,6 +190,8 @@ def create_organization_object(org_name, creator, attrs=None): home_page=attrs.get("home_page", ""), twitter=attrs.get("twitter", ""), ) + profile.metadata["billing_email"] = email + profile.save() return profile diff --git a/onadata/libs/serializers/organization_serializer.py b/onadata/libs/serializers/organization_serializer.py index 0d52b64fa9..81ec36631d 100644 --- a/onadata/libs/serializers/organization_serializer.py +++ b/onadata/libs/serializers/organization_serializer.py @@ -73,7 +73,7 @@ def update(self, instance, validated_data): instance.user.last_name = last_name if "email" in validated_data: - instance.user.email = validated_data.pop("email") + instance.metadata["billing_email"] = validated_data.pop("email") instance.user.save() return super().update(instance, validated_data) From 7edc29c4b9860c9929b54c73ad183ebec3098834 Mon Sep 17 00:00:00 2001 From: FrankApiyo Date: Thu, 26 Sep 2024 16:47:18 +0300 Subject: [PATCH 2/5] Add metadata to organization profile in one step --- onadata/apps/api/tools.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/onadata/apps/api/tools.py b/onadata/apps/api/tools.py index 28534e4818..3877997336 100644 --- a/onadata/apps/api/tools.py +++ b/onadata/apps/api/tools.py @@ -189,9 +189,8 @@ def create_organization_object(org_name, creator, attrs=None): organization=attrs.get("organization", ""), home_page=attrs.get("home_page", ""), twitter=attrs.get("twitter", ""), + metadata={"billing_email": email}, ) - profile.metadata["billing_email"] = email - profile.save() return profile From a8c0a66f05ab9e0e713c2ead9f7f1e890e1ed967 Mon Sep 17 00:00:00 2001 From: FrankApiyo Date: Tue, 1 Oct 2024 11:15:33 +0300 Subject: [PATCH 3/5] Move organization email from metadata -> organization email field --- ...8_organizationprofile_organization_email.py | 18 ++++++++++++++++++ .../apps/api/models/organization_profile.py | 5 +++-- .../test_organization_profile_viewset.py | 15 ++++----------- onadata/apps/api/tools.py | 2 +- .../serializers/organization_serializer.py | 2 +- 5 files changed, 27 insertions(+), 15 deletions(-) create mode 100644 onadata/apps/api/migrations/0008_organizationprofile_organization_email.py diff --git a/onadata/apps/api/migrations/0008_organizationprofile_organization_email.py b/onadata/apps/api/migrations/0008_organizationprofile_organization_email.py new file mode 100644 index 0000000000..7815819932 --- /dev/null +++ b/onadata/apps/api/migrations/0008_organizationprofile_organization_email.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.14 on 2024-10-01 08:01 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('api', '0007_odktoken_expires'), + ] + + operations = [ + migrations.AddField( + model_name='organizationprofile', + name='organization_email', + field=models.EmailField(blank=True, max_length=254, verbose_name='email address'), + ), + ] diff --git a/onadata/apps/api/models/organization_profile.py b/onadata/apps/api/models/organization_profile.py index 24a81dbf1a..44e94c5ee0 100644 --- a/onadata/apps/api/models/organization_profile.py +++ b/onadata/apps/api/models/organization_profile.py @@ -7,6 +7,7 @@ from django.contrib.contenttypes.models import ContentType from django.db import models from django.db.models.signals import post_delete, post_save +from django.utils.translation import gettext_lazy as _ from guardian.models import GroupObjectPermissionBase, UserObjectPermissionBase from guardian.shortcuts import assign_perm, get_perms_for_model @@ -154,7 +155,6 @@ def _post_save_create_owner_team(sender, instance, created, **kwargs): class OrganizationProfile(UserProfile): - """Organization: Extends the user profile for organization specific info * What does this do? @@ -176,6 +176,7 @@ class Meta: is_organization = models.BooleanField(default=True) # Other fields here creator = models.ForeignKey(User, on_delete=models.CASCADE) + organization_email = models.EmailField(_("email address"), blank=True) def __str__(self): return f"{self.name}[{self.user.username}]" @@ -186,7 +187,7 @@ def save(self, *args, **kwargs): # pylint: disable=arguments-differ @property def email(self): "organization email" - return self.user.email + return self.organization_email def remove_user_from_organization(self, user): """Removes a user from all teams/groups in the organization. diff --git a/onadata/apps/api/tests/viewsets/test_organization_profile_viewset.py b/onadata/apps/api/tests/viewsets/test_organization_profile_viewset.py index 36b85fcd56..d86e2966f5 100644 --- a/onadata/apps/api/tests/viewsets/test_organization_profile_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_organization_profile_viewset.py @@ -222,10 +222,7 @@ def test_orgs_get_anon(self): def test_orgs_create(self): self._org_create() self.assertTrue(self.organization.user.is_active) - self.assertTrue("billing_email" in self.organization.metadata) - self.assertEqual( - self.organization.metadata["billing_email"], "mail@mail-server.org" - ) + self.assertEqual(self.organization.email, "mail@mail-server.org") def test_orgs_create_without_name(self): data = { @@ -267,8 +264,7 @@ def test_org_create_and_fetch_by_admin_user(self): request.user = self.user response = self.view(request) self.assertEqual(response.status_code, 201) - self.assertTrue("billing_email" in response.data["metadata"]) - self.assertEqual(response.data["metadata"]["billing_email"], org_email) + self.assertEqual(response.data["email"], org_email) def test_org_create_with_anonymous_user(self): data = { @@ -456,10 +452,7 @@ def test_role_for_org_non_owner(self): request = self.factory.get("/", **self.extra) response = view(request, user="denoinc") self.assertEqual(response.status_code, 200) - self.assertTrue("billing_email" in response.data["metadata"]) - self.assertEqual( - response.data["metadata"]["billing_email"], "mail@mail-server.org" - ) + self.assertEqual(response.data["email"], "mail@mail-server.org") self.assertIn("users", list(response.data)) for user in response.data["users"]: @@ -988,7 +981,7 @@ def test_member_added_to_org_with_correct_perms(self): }, { "is_org": True, - "metadata": {"billing_email": "mail@mail-server.org"}, + "metadata": {}, "first_name": "Dennis", "last_name": "", "user": "denoinc", diff --git a/onadata/apps/api/tools.py b/onadata/apps/api/tools.py index 3877997336..a1d435b098 100644 --- a/onadata/apps/api/tools.py +++ b/onadata/apps/api/tools.py @@ -189,7 +189,7 @@ def create_organization_object(org_name, creator, attrs=None): organization=attrs.get("organization", ""), home_page=attrs.get("home_page", ""), twitter=attrs.get("twitter", ""), - metadata={"billing_email": email}, + organization_email=email, ) return profile diff --git a/onadata/libs/serializers/organization_serializer.py b/onadata/libs/serializers/organization_serializer.py index 81ec36631d..2193190a0a 100644 --- a/onadata/libs/serializers/organization_serializer.py +++ b/onadata/libs/serializers/organization_serializer.py @@ -73,7 +73,7 @@ def update(self, instance, validated_data): instance.user.last_name = last_name if "email" in validated_data: - instance.metadata["billing_email"] = validated_data.pop("email") + instance.organization_email = validated_data.pop("email") instance.user.save() return super().update(instance, validated_data) From 1dfcd5de3b9b92201ec921e7596fbda863983f83 Mon Sep 17 00:00:00 2001 From: FrankApiyo Date: Tue, 1 Oct 2024 11:38:11 +0300 Subject: [PATCH 4/5] Fix linting errors --- .../api/migrations/0008_org_profile_email.py | 25 +++++++++++++++++++ ..._organizationprofile_organization_email.py | 18 ------------- 2 files changed, 25 insertions(+), 18 deletions(-) create mode 100644 onadata/apps/api/migrations/0008_org_profile_email.py delete mode 100644 onadata/apps/api/migrations/0008_organizationprofile_organization_email.py diff --git a/onadata/apps/api/migrations/0008_org_profile_email.py b/onadata/apps/api/migrations/0008_org_profile_email.py new file mode 100644 index 0000000000..3a7364f940 --- /dev/null +++ b/onadata/apps/api/migrations/0008_org_profile_email.py @@ -0,0 +1,25 @@ +# Generated by Django 4.2.14 on 2024-10-01 08:01 +""" +Add organization_email field to the organization profile model +""" +from django.db import migrations, models + + +class Migration(migrations.Migration): + """ + Add organization_email field to the organization profile model + """ + + dependencies = [ + ("api", "0007_odktoken_expires"), + ] + + operations = [ + migrations.AddField( + model_name="organizationprofile", + name="organization_email", + field=models.EmailField( + blank=True, max_length=254, verbose_name="email address" + ), + ), + ] diff --git a/onadata/apps/api/migrations/0008_organizationprofile_organization_email.py b/onadata/apps/api/migrations/0008_organizationprofile_organization_email.py deleted file mode 100644 index 7815819932..0000000000 --- a/onadata/apps/api/migrations/0008_organizationprofile_organization_email.py +++ /dev/null @@ -1,18 +0,0 @@ -# Generated by Django 4.2.14 on 2024-10-01 08:01 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('api', '0007_odktoken_expires'), - ] - - operations = [ - migrations.AddField( - model_name='organizationprofile', - name='organization_email', - field=models.EmailField(blank=True, max_length=254, verbose_name='email address'), - ), - ] From 1c80a5e2946b93b9445d6f999d5bbc3fad70a118 Mon Sep 17 00:00:00 2001 From: FrankApiyo Date: Wed, 2 Oct 2024 16:35:18 +0300 Subject: [PATCH 5/5] Rename organization_email -> email in organization profile --- onadata/apps/api/migrations/0008_org_profile_email.py | 6 +++--- onadata/apps/api/models/organization_profile.py | 7 +------ onadata/apps/api/tools.py | 2 +- onadata/libs/serializers/organization_serializer.py | 2 +- 4 files changed, 6 insertions(+), 11 deletions(-) diff --git a/onadata/apps/api/migrations/0008_org_profile_email.py b/onadata/apps/api/migrations/0008_org_profile_email.py index 3a7364f940..28f51b4bec 100644 --- a/onadata/apps/api/migrations/0008_org_profile_email.py +++ b/onadata/apps/api/migrations/0008_org_profile_email.py @@ -1,13 +1,13 @@ # Generated by Django 4.2.14 on 2024-10-01 08:01 """ -Add organization_email field to the organization profile model +Add email field to the organization profile model """ from django.db import migrations, models class Migration(migrations.Migration): """ - Add organization_email field to the organization profile model + Add email field to the organization profile model """ dependencies = [ @@ -17,7 +17,7 @@ class Migration(migrations.Migration): operations = [ migrations.AddField( model_name="organizationprofile", - name="organization_email", + name="email", field=models.EmailField( blank=True, max_length=254, verbose_name="email address" ), diff --git a/onadata/apps/api/models/organization_profile.py b/onadata/apps/api/models/organization_profile.py index 44e94c5ee0..146e112ea2 100644 --- a/onadata/apps/api/models/organization_profile.py +++ b/onadata/apps/api/models/organization_profile.py @@ -176,7 +176,7 @@ class Meta: is_organization = models.BooleanField(default=True) # Other fields here creator = models.ForeignKey(User, on_delete=models.CASCADE) - organization_email = models.EmailField(_("email address"), blank=True) + email = models.EmailField(_("email address"), blank=True) def __str__(self): return f"{self.name}[{self.user.username}]" @@ -184,11 +184,6 @@ def __str__(self): def save(self, *args, **kwargs): # pylint: disable=arguments-differ super().save(*args, **kwargs) - @property - def email(self): - "organization email" - return self.organization_email - def remove_user_from_organization(self, user): """Removes a user from all teams/groups in the organization. diff --git a/onadata/apps/api/tools.py b/onadata/apps/api/tools.py index a1d435b098..46835451c4 100644 --- a/onadata/apps/api/tools.py +++ b/onadata/apps/api/tools.py @@ -189,7 +189,7 @@ def create_organization_object(org_name, creator, attrs=None): organization=attrs.get("organization", ""), home_page=attrs.get("home_page", ""), twitter=attrs.get("twitter", ""), - organization_email=email, + email=email, ) return profile diff --git a/onadata/libs/serializers/organization_serializer.py b/onadata/libs/serializers/organization_serializer.py index 2193190a0a..69b9e6845c 100644 --- a/onadata/libs/serializers/organization_serializer.py +++ b/onadata/libs/serializers/organization_serializer.py @@ -73,7 +73,7 @@ def update(self, instance, validated_data): instance.user.last_name = last_name if "email" in validated_data: - instance.organization_email = validated_data.pop("email") + instance.email = validated_data.pop("email") instance.user.save() return super().update(instance, validated_data)