Refactor organization member API to eliminate redundancy and optimize query for user mfa details

Author: rajpatel24

kobo/apps/organizations/serializers.py

@@ -21,15 +21,17 @@ class OrganizationUserSerializer(serializers.ModelSerializer):
         view_name='user-kpi-detail',
     )
     role = serializers.CharField()
-    has_mfa_enabled = serializers.SerializerMethodField()
+    user__has_mfa_enabled = serializers.BooleanField(
+        source='has_mfa_enabled', read_only=True
+    )
     url = serializers.SerializerMethodField()
     date_joined = serializers.DateTimeField(
         source='user.date_joined', format='%Y-%m-%dT%H:%M:%SZ'
     )
     user__username = serializers.ReadOnlyField(source='user.username')
     user__name = serializers.ReadOnlyField(source='user.get_full_name')
     user__email = serializers.ReadOnlyField(source='user.email')
-    is_active = serializers.ReadOnlyField(source='user.is_active')
+    user__is_active = serializers.ReadOnlyField(source='user.is_active')
 
     class Meta:
         model = OrganizationUser
@@ -40,13 +42,15 @@ class Meta:
             'user__email',
             'user__name',
             'role',
-            'has_mfa_enabled',
+            'user__has_mfa_enabled',
             'date_joined',
-            'is_active'
+            'user__is_active'
         ]
 
-    def get_has_mfa_enabled(self, obj):
-        return config.MFA_ENABLED
+    def update(self, instance, validated_data):
+        if role := validated_data.get('role', None):
+            validated_data['is_admin'] = role == 'admin'
+        return super().update(instance, validated_data)
 
     def get_url(self, obj):
         request = self.context.get('request')

kobo/apps/organizations/tests/test_organization_members_api.py

@@ -13,22 +13,14 @@ class OrganizationMemberAPITestCase(BaseTestCase):
     URL_NAMESPACE = URL_NAMESPACE
 
     def setUp(self):
-        self.organization = baker.make(Organization, id='org_12345')
+        self.organization = baker.make(
+            Organization, id='org_12345', mmo_override=True
+        )
         self.owner_user = baker.make(User, username='owner')
         self.member_user = baker.make(User, username='member')
-        self.invited_user = baker.make(User, username='invited')
 
-        self.organization_user_owner = baker.make(
-            OrganizationUser,
-            organization=self.organization,
-            user=self.owner_user,
-            is_admin=True,
-        )
-        self.organization_user_member = baker.make(
-            OrganizationUser,
-            organization=self.organization,
-            user=self.member_user
-        )
+        self.organization.add_user(self.owner_user)
+        self.organization.add_user(self.member_user, is_admin=False)
 
         self.client.force_login(self.owner_user)
         self.list_url = reverse(

kobo/apps/organizations/views.py

@@ -19,7 +19,7 @@
 from kpi import filters
 from kpi.constants import ASSET_TYPE_SURVEY
 from kpi.models.asset import Asset
-from kpi.paginators import AssetUsagePagination, OrganizationPagination
+from kpi.paginators import AssetUsagePagination, OrganizationMemberPagination
 from kpi.permissions import IsAuthenticated
 from kpi.serializers.v2.service_usage import (
     CustomAssetUsageSerializer,
@@ -29,6 +29,7 @@
 from .models import Organization, OrganizationOwner, OrganizationUser
 from .permissions import IsOrgAdminOrReadOnly
 from .serializers import OrganizationSerializer, OrganizationUserSerializer
+from ..accounts.mfa.models import MfaMethod
 from ..stripe.constants import ACTIVE_STRIPE_STATUSES
 
 
@@ -336,12 +337,19 @@ class OrganizationMemberViewSet(viewsets.ModelViewSet):
     """
     serializer_class = OrganizationUserSerializer
     permission_classes = [IsAuthenticated]
-    pagination_class = OrganizationPagination
+    pagination_class = OrganizationMemberPagination
+    http_method_names = ['get', 'patch', 'delete']
     lookup_field = 'user__username'
 
     def get_queryset(self):
         organization_id = self.kwargs['organization_id']
 
+        # Subquery to check if the user has an active MFA method
+        mfa_subquery = MfaMethod.objects.filter(
+            user=OuterRef('user_id'),
+            is_active=True
+        ).values('pk')
+
         # Subquery to check if the user is the owner
         owner_subquery = OrganizationOwner.objects.filter(
             organization_id=organization_id,
@@ -357,7 +365,8 @@ def get_queryset(self):
                 When(is_admin=True, then=Value('admin')),
                 default=Value('member'),
                 output_field=CharField()
-            )
+            ),
+            has_mfa_enabled=Exists(mfa_subquery)
         )
         return queryset
 

kpi/paginators.py

@@ -144,7 +144,7 @@ class TinyPaginated(PageNumberPagination):
     page_size = 50
 
 
-class OrganizationPagination(PageNumberPagination):
+class OrganizationMemberPagination(PageNumberPagination):
     """
     Pagination class for Organization
     """