diff --git a/kobo/apps/organizations/models.py b/kobo/apps/organizations/models.py index 9b9277ed61..4c7ad5ef63 100644 --- a/kobo/apps/organizations/models.py +++ b/kobo/apps/organizations/models.py @@ -186,6 +186,7 @@ def is_admin(self, user: 'User') -> bool: return super().is_admin(user) @property + @cache_for_request def is_mmo(self): """ Determines if the multi-members feature is active for the organization diff --git a/kpi/serializers/v2/asset.py b/kpi/serializers/v2/asset.py index 74778aaffb..0e2a57d624 100644 --- a/kpi/serializers/v2/asset.py +++ b/kpi/serializers/v2/asset.py @@ -291,6 +291,7 @@ class AssetSerializer(serializers.HyperlinkedModelSerializer): owner = RelativePrefixHyperlinkedRelatedField( view_name='user-kpi-detail', lookup_field='username', read_only=True) owner__username = serializers.ReadOnlyField(source='owner.username') + owner_label = serializers.SerializerMethodField() url = HyperlinkedIdentityField( lookup_field='uid', view_name='asset-detail') asset_type = serializers.ChoiceField(choices=ASSET_TYPES) @@ -407,6 +408,7 @@ class Meta: 'data_sharing', 'paired_data', 'project_ownership', + 'owner_label', ) extra_kwargs = { 'parent': { @@ -827,6 +829,19 @@ def get_access_types(self, obj): return access_types + def get_owner_label(self, asset): + try: + organization = self.context['organization_by_asset'].get(asset.id) + except KeyError: + organization = asset.owner.organization + if ( + organization + and organization.is_owner(asset.owner) + and organization.is_mmo + ): + return organization.name + return asset.owner.username + def validate_data_sharing(self, data_sharing: dict) -> dict: """ Validates `data_sharing`. It is really basic. @@ -1010,7 +1025,8 @@ class Meta(AssetSerializer.Meta): 'status', 'access_types', 'children', - 'data_sharing' + 'data_sharing', + 'owner_label', ) def get_permissions(self, asset): diff --git a/kpi/tests/api/v1/test_api_assets.py b/kpi/tests/api/v1/test_api_assets.py index 5d2f4be619..7feda2ec07 100644 --- a/kpi/tests/api/v1/test_api_assets.py +++ b/kpi/tests/api/v1/test_api_assets.py @@ -43,6 +43,10 @@ def test_asset_list_matches_detail(self): self.assertIsNotNone(list_result_detail) self.assertDictEqual(expected_list_data, dict(list_result_detail)) + @unittest.skip(reason='`owner_label` field only exists in v2 endpoint') + def test_asset_owner_label(self): + pass + class AssetVersionApiTests(test_api_assets.AssetVersionApiTests): URL_NAMESPACE = None diff --git a/kpi/tests/api/v2/test_api_assets.py b/kpi/tests/api/v2/test_api_assets.py index f5fc584f6e..66ea1bdfb2 100644 --- a/kpi/tests/api/v2/test_api_assets.py +++ b/kpi/tests/api/v2/test_api_assets.py @@ -3,8 +3,10 @@ import os import dateutil.parser +from django.conf import settings from django.urls import reverse from django.utils import timezone +from model_bakery import baker from rest_framework import status from kobo.apps.kobo_auth.shortcuts import User @@ -85,6 +87,77 @@ def test_asset_list_matches_detail(self): self.assertIsNotNone(list_result_detail) self.assertDictEqual(expected_list_data, dict(list_result_detail)) + def test_asset_owner_label(self): + """ + Test the behavior of the owner_label field in the Asset API. + + - If the user is an organization owner and the organization is MMO, + then the `owner_label` should be the organization's name. + - If the user is not an organization owner and the organization is MMO, + then the `owner_label` should be the owner's username. + - If the user is an organization owner but the organization is not MMO, + then the `owner_label` should be the owner's username. + """ + detail_response = self.create_asset() + someuser_username = detail_response.data.get('owner__username') + someuser = User.objects.get(username=someuser_username) + + # Check the user is an owner of an organization + self.assertTrue(someuser.is_org_owner) + + # Fetch the assets list and verify the initial owner_label + list_response = self.client.get(self.list_url) + self.assertEqual( + list_response.data['results'][0]['owner_label'], + someuser_username + ) + + # Make the organization a MMO and verify the owner_label + self.organization = someuser.organization + self.organization.mmo_override = True + self.organization.save(update_fields=['mmo_override']) + + list_response = self.client.get(self.list_url) + self.assertEqual( + list_response.data['results'][0]['owner_label'], + self.organization.name + ) + + # Add another user to the organization and share the asset + anotheruser = User.objects.get(username='anotheruser') + self.organization.add_user(anotheruser) + someuser_asset = someuser.assets.last() + someuser_asset.assign_perm(anotheruser, PERM_VIEW_ASSET) + + # Create an external user's asset and share it with anotheruser + external_user = baker.make(settings.AUTH_USER_MODEL) + self.client.force_login(external_user) + detail_response = self.create_asset() + external_user_asset = external_user.assets.last() + external_user_asset.assign_perm(anotheruser, PERM_VIEW_ASSET) + + # Fetch the external user's asset and verify the owner_label + list_response = self.client.get(self.list_url) + self.assertEqual( + list_response.data['results'][0]['owner_label'], + external_user.username + ) + + # Verify owner_label for both assets when logged in as anotheruser + self.client.force_login(anotheruser) + list_response = self.client.get(self.list_url) + + anotheruser_assets = { + item['uid']: item['owner_label'] + for item in list_response.data['results'] + } + self.assertEqual( + anotheruser_assets[external_user_asset.uid], external_user.username + ) + self.assertEqual( + anotheruser_assets[someuser_asset.uid], self.organization.name + ) + def test_assets_hash(self): another_user = User.objects.get(username='anotheruser') user_asset = Asset.objects.get(pk=1) diff --git a/kpi/views/v2/asset.py b/kpi/views/v2/asset.py index 1af06ba083..b5d10e3b64 100644 --- a/kpi/views/v2/asset.py +++ b/kpi/views/v2/asset.py @@ -722,6 +722,17 @@ def get_serializer_context(self): context_['children_count_per_asset'] = children_count_per_asset + # 5) Get organization per asset + assets = ( + Asset.objects.filter(id__in=asset_ids) + .prefetch_related('owner__organizations_organization') + ) + organization_by_asset = defaultdict(dict) + for asset in assets: + organization = getattr(asset.owner, 'organization', None) + organization_by_asset[asset.id] = organization + context_['organization_by_asset'] = organization_by_asset + return context_ def list(self, request, *args, **kwargs):