From 73cebea6b0849c978337ce47441f01751c10debc Mon Sep 17 00:00:00 2001 From: FrankApiyo Date: Mon, 22 Jul 2024 15:50:24 +0300 Subject: [PATCH 1/6] Add email field to the organization serializer Co-authored-by: Kelvin Muchiri --- .../api/tests/viewsets/test_organization_profile_viewset.py | 4 +++- onadata/libs/serializers/organization_serializer.py | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) 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 3425b6fe23..a8e27e2c57 100644 --- a/onadata/apps/api/tests/viewsets/test_organization_profile_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_organization_profile_viewset.py @@ -214,8 +214,10 @@ def test_orgs_get_anon(self): self.assertTrue(isinstance(user["user"], text)) def test_orgs_create(self): - self._org_create() + org_email = "mail@mail-server.org" + self._org_create(org_data = {"email": org_email}) self.assertTrue(self.organization.user.is_active) + self.assertEqual(self.organization.user.email, org_email) def test_orgs_create_without_name(self): data = { diff --git a/onadata/libs/serializers/organization_serializer.py b/onadata/libs/serializers/organization_serializer.py index 0887c830a5..6f8e62332e 100644 --- a/onadata/libs/serializers/organization_serializer.py +++ b/onadata/libs/serializers/organization_serializer.py @@ -37,6 +37,7 @@ class OrganizationSerializer(serializers.HyperlinkedModelSerializer): user = serializers.HyperlinkedRelatedField( view_name="user-detail", lookup_field="username", read_only=True ) + email = serializers.EmailField(allow_blank=True) creator = serializers.HyperlinkedRelatedField( view_name="user-detail", lookup_field="username", read_only=True ) From 40fe56f74e90fcad1c9a08c96df889a27bfa1ea5 Mon Sep 17 00:00:00 2001 From: FrankApiyo Date: Thu, 25 Jul 2024 15:57:36 +0300 Subject: [PATCH 2/6] Fix failing tests --- onadata/apps/api/models/organization_profile.py | 4 ++++ onadata/apps/api/tests/viewsets/test_abstract_viewset.py | 1 + .../tests/viewsets/test_organization_profile_viewset.py | 7 ++++--- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/onadata/apps/api/models/organization_profile.py b/onadata/apps/api/models/organization_profile.py index cd6625bebf..3a7e115304 100644 --- a/onadata/apps/api/models/organization_profile.py +++ b/onadata/apps/api/models/organization_profile.py @@ -183,6 +183,10 @@ def __str__(self): def save(self, *args, **kwargs): # pylint: disable=arguments-differ super().save(*args, **kwargs) + @property + def email(self): + return self.user.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_abstract_viewset.py b/onadata/apps/api/tests/viewsets/test_abstract_viewset.py index 81ac952be4..fb836d8aaf 100644 --- a/onadata/apps/api/tests/viewsets/test_abstract_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_abstract_viewset.py @@ -201,6 +201,7 @@ def _org_create(self, org_data=None): response = view(request) self.assertEqual(response.status_code, 200) data = { + "email": "mail@mail-server.org", "org": "denoinc", "name": "Dennis", "city": "Denoville", 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 a8e27e2c57..1fac73fe9d 100644 --- a/onadata/apps/api/tests/viewsets/test_organization_profile_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_organization_profile_viewset.py @@ -214,16 +214,16 @@ def test_orgs_get_anon(self): self.assertTrue(isinstance(user["user"], text)) def test_orgs_create(self): - org_email = "mail@mail-server.org" - self._org_create(org_data = {"email": org_email}) + self._org_create() self.assertTrue(self.organization.user.is_active) - self.assertEqual(self.organization.user.email, org_email) + self.assertEqual(self.organization.user.email, "mail@mail-server.org") def test_orgs_create_without_name(self): data = { "org": "denoinc", "city": "Denoville", "country": "US", + "email": "user@mail.org", "home_page": "deno.com", "twitter": "denoinc", "description": "", @@ -563,6 +563,7 @@ def test_orgs_create_with_mixed_case(self): "home_page": "deno.com", "twitter": "denoinc", "description": "", + "email": "user@mail.com", "address": "", "phonenumber": "", "require_auth": False, From 9694aacdab84aa04c5b52818157c13333a6795bc Mon Sep 17 00:00:00 2001 From: FrankApiyo Date: Thu, 25 Jul 2024 16:06:55 +0300 Subject: [PATCH 3/6] Add docstring to organization profile email property --- onadata/apps/api/models/organization_profile.py | 1 + 1 file changed, 1 insertion(+) diff --git a/onadata/apps/api/models/organization_profile.py b/onadata/apps/api/models/organization_profile.py index 3a7e115304..24a81dbf1a 100644 --- a/onadata/apps/api/models/organization_profile.py +++ b/onadata/apps/api/models/organization_profile.py @@ -185,6 +185,7 @@ def save(self, *args, **kwargs): # pylint: disable=arguments-differ @property def email(self): + "organization email" return self.user.email def remove_user_from_organization(self, user): From 50cb235f5498693519430b1f1584b48ca64804ec Mon Sep 17 00:00:00 2001 From: FrankApiyo Date: Fri, 26 Jul 2024 11:36:45 +0300 Subject: [PATCH 4/6] Get organization profile cache value based on authenticated user --- .../api/viewsets/organization_profile_viewset.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/onadata/apps/api/viewsets/organization_profile_viewset.py b/onadata/apps/api/viewsets/organization_profile_viewset.py index d42a2501b5..6a65c9fabb 100644 --- a/onadata/apps/api/viewsets/organization_profile_viewset.py +++ b/onadata/apps/api/viewsets/organization_profile_viewset.py @@ -66,11 +66,11 @@ class OrganizationProfileViewSet( def retrieve(self, request, *args, **kwargs): """Get organization from cache or db""" username = kwargs.get("user") - cached_org = cache.get(f"{ORG_PROFILE_CACHE}{username}") + cached_org = cache.get(f"{ORG_PROFILE_CACHE}{username}{request.user.username}") if cached_org: return Response(cached_org) response = super().retrieve(request, *args, **kwargs) - cache.set(f"{ORG_PROFILE_CACHE}{username}", response.data) + cache.set(f"{ORG_PROFILE_CACHE}{username}{request.user.username}", response.data) return response def create(self, request, *args, **kwargs): @@ -78,20 +78,20 @@ def create(self, request, *args, **kwargs): response = super().create(request, *args, **kwargs) organization = response.data username = organization.get("org") - cache.set(f"{ORG_PROFILE_CACHE}{username}", organization) + cache.set(f"{ORG_PROFILE_CACHE}{username}{request.user.username}", organization) return response def destroy(self, request, *args, **kwargs): """Clear cache and destroy organization""" username = kwargs.get("user") - safe_delete(f"{ORG_PROFILE_CACHE}{username}") + safe_delete(f"{ORG_PROFILE_CACHE}{username}{request.user.username}") return super().destroy(request, *args, **kwargs) def update(self, request, *args, **kwargs): """Update org in cache and db""" username = kwargs.get("user") response = super().update(request, *args, **kwargs) - cache.set(f"{ORG_PROFILE_CACHE}{username}", response.data) + cache.set(f"{ORG_PROFILE_CACHE}{username}{request.user.username}", response.data) return response @action(methods=["DELETE", "GET", "POST", "PUT"], detail=True) @@ -120,7 +120,7 @@ def members(self, request, *args, **kwargs): data = OrganizationSerializer( organization, context={"request": request} ).data - cache.set(f"{ORG_PROFILE_CACHE}{username}", data) + cache.set(f"{ORG_PROFILE_CACHE}{username}{request.user.username}", data) else: return Response(data=serializer.errors, status=status.HTTP_400_BAD_REQUEST) From 91d521f432d413731d076826539a393b7b766fe0 Mon Sep 17 00:00:00 2001 From: FrankApiyo Date: Fri, 26 Jul 2024 11:37:37 +0300 Subject: [PATCH 5/6] Add organization email field to owners only fields --- onadata/libs/serializers/organization_serializer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/onadata/libs/serializers/organization_serializer.py b/onadata/libs/serializers/organization_serializer.py index 6f8e62332e..ca1f0f006f 100644 --- a/onadata/libs/serializers/organization_serializer.py +++ b/onadata/libs/serializers/organization_serializer.py @@ -48,7 +48,7 @@ class OrganizationSerializer(serializers.HyperlinkedModelSerializer): class Meta: model = OrganizationProfile exclude = ("created_by", "is_organization", "organization") - owner_only_fields = ("metadata",) + owner_only_fields = ("metadata", "email") def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) From a098af046bf7897903b24d65471a05548f728f3d Mon Sep 17 00:00:00 2001 From: FrankApiyo Date: Fri, 26 Jul 2024 11:38:05 +0300 Subject: [PATCH 6/6] Fix failing tests --- .../tests/viewsets/test_abstract_viewset.py | 1 + .../test_organization_profile_viewset.py | 49 ++++++++++++++++++- .../viewsets/organization_profile_viewset.py | 8 +-- 3 files changed, 54 insertions(+), 4 deletions(-) diff --git a/onadata/apps/api/tests/viewsets/test_abstract_viewset.py b/onadata/apps/api/tests/viewsets/test_abstract_viewset.py index fb836d8aaf..dc109421f2 100644 --- a/onadata/apps/api/tests/viewsets/test_abstract_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_abstract_viewset.py @@ -225,6 +225,7 @@ def _org_create(self, org_data=None): data["url"] = f"http://testserver/api/v1/orgs/{data['org']}" data["user"] = f"http://testserver/api/v1/users/{data['org']}" data["creator"] = "http://testserver/api/v1/users/bob" + data.pop("email") self.assertDictContainsSubset(data, response.data) # pylint: disable=attribute-defined-outside-init self.company_data = response.data 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 1fac73fe9d..2925786a03 100644 --- a/onadata/apps/api/tests/viewsets/test_organization_profile_viewset.py +++ b/onadata/apps/api/tests/viewsets/test_organization_profile_viewset.py @@ -6,7 +6,7 @@ from builtins import str as text from unittest.mock import patch -from django.contrib.auth.models import User, timezone +from django.contrib.auth.models import User, timezone, AnonymousUser from django.core.cache import cache from django.test.utils import override_settings @@ -89,6 +89,7 @@ def test_orgs_list(self): self.assertNotEqual(response.get("Cache-Control"), None) self.assertEqual(response.status_code, 200) del self.company_data["metadata"] + del self.company_data["email"] self.assertEqual(response.data, [self.company_data]) # inactive organization @@ -113,6 +114,7 @@ def test_orgs_list_for_authenticated_user(self): self.assertNotEqual(response.get("Cache-Control"), None) self.assertEqual(response.status_code, 200) del self.company_data["metadata"] + del self.company_data["email"] self.assertEqual(response.data, [self.company_data]) def test_orgs_list_shared_with_user(self): @@ -194,6 +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'] self.assertEqual(response.data, self.company_data) self.assertIn("users", list(response.data)) for user in response.data["users"]: @@ -207,6 +211,8 @@ def test_orgs_get_anon(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'] self.assertEqual(response.data, self.company_data) self.assertIn("users", list(response.data)) for user in response.data["users"]: @@ -237,6 +243,29 @@ def test_orgs_create_without_name(self): response = self.view(request) self.assertEqual(response.data, {"name": ["This field is required."]}) + def test_org_create_and_fetch_by_admin_user(self): + org_email = "mail@mail-sever.org" + data = { + "name": "denoinc", + "org": "denoinc", + "city": "Denoville", + "country": "US", + "home_page": "deno.com", + "twitter": "denoinc", + "email": org_email, + "description": "", + "address": "", + "phonenumber": "", + "require_auth": False, + } + request = self.factory.post( + "/", data=json.dumps(data), content_type="application/json" + ) + request.user = self.user + response = self.view(request) + self.assertEqual(response.status_code, 201) + self.assertEqual(response.data['email'], org_email) + def test_org_create_with_anonymous_user(self): data = { "name": "denoinc", @@ -391,6 +420,7 @@ def test_member_sees_orgs_added_to(self): } ) del expected_data["metadata"] + del expected_data['email'] request = self.factory.get("/", **self.extra) response = view(request) @@ -422,6 +452,8 @@ 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.assertIn("users", list(response.data)) for user in response.data["users"]: @@ -434,6 +466,21 @@ def test_role_for_org_non_owner(self): ) self.assertEqual(role, expected_role) + # getting profile as a member + request = self.factory.get("/", **self.extra) + request.user = User.objects.get(username="aboy") + response = view(request, user="denoinc") + self.assertEqual(response.status_code, 200) + + # get profile as a anonymous user + request = self.factory.get("/", **self.extra) + request.user = AnonymousUser() + request.headers = None + request.META['HTTP_AUTHORIZATION'] = "" + response = view(request, user="denoinc") + self.assertEqual(response.status_code, 200) + self.assertFalse('email' in response.data) + def test_add_members_to_org_with_anonymous_user(self): self._org_create() view = OrganizationProfileViewSet.as_view({"post": "members"}) diff --git a/onadata/apps/api/viewsets/organization_profile_viewset.py b/onadata/apps/api/viewsets/organization_profile_viewset.py index 6a65c9fabb..b9ab39040d 100644 --- a/onadata/apps/api/viewsets/organization_profile_viewset.py +++ b/onadata/apps/api/viewsets/organization_profile_viewset.py @@ -66,11 +66,12 @@ class OrganizationProfileViewSet( def retrieve(self, request, *args, **kwargs): """Get organization from cache or db""" username = kwargs.get("user") - cached_org = cache.get(f"{ORG_PROFILE_CACHE}{username}{request.user.username}") + cache_key = f"{ORG_PROFILE_CACHE}{username}{request.user.username}" + cached_org = cache.get(cache_key) if cached_org: return Response(cached_org) response = super().retrieve(request, *args, **kwargs) - cache.set(f"{ORG_PROFILE_CACHE}{username}{request.user.username}", response.data) + cache.set(cache_key, response.data) return response def create(self, request, *args, **kwargs): @@ -91,7 +92,8 @@ def update(self, request, *args, **kwargs): """Update org in cache and db""" username = kwargs.get("user") response = super().update(request, *args, **kwargs) - cache.set(f"{ORG_PROFILE_CACHE}{username}{request.user.username}", response.data) + cache_key = f"{ORG_PROFILE_CACHE}{username}{request.user.username}" + cache.set(cache_key, response.data) return response @action(methods=["DELETE", "GET", "POST", "PUT"], detail=True)