From e17f6a7e71aaf1e6b16276d03e4de38365062be7 Mon Sep 17 00:00:00 2001
From: rajpatel24 <rajpatel24998@gmail.com>
Date: Thu, 7 Nov 2024 01:04:42 +0530
Subject: [PATCH 01/29] Add endpoints to handle organization members

---
 kobo/apps/organizations/serializers.py        |  52 ++++-
 .../tests/test_organization_members_api.py    |  80 ++++++++
 kobo/apps/organizations/views.py              | 188 +++++++++++++++++-
 kpi/paginators.py                             |   9 +-
 kpi/urls/router_api_v2.py                     |   8 +-
 5 files changed, 329 insertions(+), 8 deletions(-)
 create mode 100644 kobo/apps/organizations/tests/test_organization_members_api.py

diff --git a/kobo/apps/organizations/serializers.py b/kobo/apps/organizations/serializers.py
index 21d73e131d..25dd405c30 100644
--- a/kobo/apps/organizations/serializers.py
+++ b/kobo/apps/organizations/serializers.py
@@ -1,4 +1,8 @@
+from constance import config
+from django.contrib.auth import get_user_model
+from django.utils.translation import gettext as t
 from rest_framework import serializers
+from rest_framework.reverse import reverse
 
 from kobo.apps.organizations.models import (
     create_organization,
@@ -11,10 +15,56 @@
 
 
 class OrganizationUserSerializer(serializers.ModelSerializer):
+    user = serializers.HyperlinkedRelatedField(
+        queryset=get_user_model().objects.all(),
+        lookup_field='username',
+        view_name='user-kpi-detail',
+    )
+    role = serializers.CharField()
+    has_mfa_enabled = serializers.SerializerMethodField()
+    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')
 
     class Meta:
         model = OrganizationUser
-        fields = ['user', 'organization']
+        fields = [
+            'url',
+            'user',
+            'user__username',
+            'user__email',
+            'user__name',
+            'role',
+            'has_mfa_enabled',
+            'date_joined',
+            'is_active'
+        ]
+
+    def get_has_mfa_enabled(self, obj):
+        return config.MFA_ENABLED
+
+    def get_url(self, obj):
+        request = self.context.get('request')
+        return reverse(
+            'organization-members-detail',
+            kwargs={
+                'organization_id': obj.organization.id,
+                'user__username': obj.user.username
+            },
+            request=request
+        )
+
+    def validate_role(self, role):
+        if role not in ['admin', 'member']:
+            raise serializers.ValidationError(
+                {'role': t("Invalid role. Only 'admin' or 'member' are allowed")}
+            )
+        return role
 
 
 class OrganizationOwnerSerializer(serializers.ModelSerializer):
diff --git a/kobo/apps/organizations/tests/test_organization_members_api.py b/kobo/apps/organizations/tests/test_organization_members_api.py
new file mode 100644
index 0000000000..7d0045be6a
--- /dev/null
+++ b/kobo/apps/organizations/tests/test_organization_members_api.py
@@ -0,0 +1,80 @@
+from django.urls import reverse
+from model_bakery import baker
+from rest_framework import status
+
+from kobo.apps.kobo_auth.shortcuts import User
+from kobo.apps.organizations.models import Organization, OrganizationUser
+from kpi.tests.kpi_test_case import BaseTestCase
+from kpi.urls.router_api_v2 import URL_NAMESPACE
+
+
+class OrganizationMemberAPITestCase(BaseTestCase):
+    fixtures = ['test_data']
+    URL_NAMESPACE = URL_NAMESPACE
+
+    def setUp(self):
+        self.organization = baker.make(Organization, id='org_12345')
+        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.client.force_login(self.owner_user)
+        self.list_url = reverse(
+            self._get_endpoint('organization-members-list'),
+            kwargs={'organization_id': self.organization.id},
+        )
+        self.detail_url = lambda username: reverse(
+            self._get_endpoint('organization-members-detail'),
+            kwargs={
+                'organization_id': self.organization.id,
+                'user__username': username
+            },
+        )
+
+    def test_list_members(self):
+        response = self.client.get(self.list_url)
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertIn(
+            'owner',
+            [member['user__username'] for member in response.data.get('results')]
+        )
+        self.assertIn(
+            'member',
+            [member['user__username'] for member in response.data.get('results')]
+        )
+
+    def test_retrieve_member_details(self):
+        response = self.client.get(self.detail_url('member'))
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(response.data['user__username'], 'member')
+        self.assertEqual(response.data['role'], 'member')
+
+    def test_update_member_role(self):
+        data = {'role': 'admin'}
+        response = self.client.patch(self.detail_url('member'), data)
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(response.data['role'], 'admin')
+
+    def test_delete_member(self):
+        response = self.client.delete(self.detail_url('member'))
+        self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
+        # Confirm deletion
+        response = self.client.get(self.detail_url('member'))
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+
+    def test_list_requires_authentication(self):
+        self.client.logout()
+        response = self.client.get(self.list_url)
+        self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED)
diff --git a/kobo/apps/organizations/views.py b/kobo/apps/organizations/views.py
index 7102f513bd..7792d1a1ee 100644
--- a/kobo/apps/organizations/views.py
+++ b/kobo/apps/organizations/views.py
@@ -1,26 +1,34 @@
 from django.conf import settings
 from django.contrib.postgres.aggregates import ArrayAgg
-from django.db.models import QuerySet
+from django.db.models import (
+    QuerySet,
+    Case,
+    When,
+    Value,
+    CharField,
+    OuterRef,
+)
+from django.db.models.expressions import Exists
 from django.utils.decorators import method_decorator
 from django.views.decorators.cache import cache_page
 from django_dont_vary_on.decorators import only_vary_on
-from kpi import filters
 from rest_framework import viewsets, status
 from rest_framework.decorators import action
 from rest_framework.response import Response
 
+from kpi import filters
 from kpi.constants import ASSET_TYPE_SURVEY
 from kpi.models.asset import Asset
-from kpi.paginators import AssetUsagePagination
+from kpi.paginators import AssetUsagePagination, OrganizationPagination
 from kpi.permissions import IsAuthenticated
 from kpi.serializers.v2.service_usage import (
     CustomAssetUsageSerializer,
     ServiceUsageSerializer,
 )
 from kpi.utils.object_permission import get_database_user
-from .models import Organization
+from .models import Organization, OrganizationOwner, OrganizationUser
 from .permissions import IsOrgAdminOrReadOnly
-from .serializers import OrganizationSerializer
+from .serializers import OrganizationSerializer, OrganizationUserSerializer
 from ..stripe.constants import ACTIVE_STRIPE_STATUSES
 
 
@@ -194,3 +202,173 @@ def asset_usage(self, request, pk=None, *args, **kwargs):
             page, many=True, context=context
         )
         return self.get_paginated_response(serializer.data)
+
+
+class OrganizationMemberViewSet(viewsets.ModelViewSet):
+    """
+    * Manage organization members and their roles within an organization.
+    * Run a partial update on an organization member to promote or demote.
+
+    ## Organization Members API
+
+    This API allows authorized users to view and manage the members of an
+    organization, including their roles. It handles existing members. It also
+    allows updating roles, such as promoting a member to an admin or assigning
+    a new owner.
+
+    ### List Members
+
+    Retrieves all members in the specified organization.
+
+    <pre class="prettyprint">
+    <b>GET</b> /api/v2/organizations/{organization_id}/members/
+    </pre>
+
+    > Example
+    >
+    >       curl -X GET https://[kpi]/api/v2/organizations/org_12345/members/
+
+    > Response 200
+
+    >       {
+    >           "count": 2,
+    >           "next": null,
+    >           "previous": null,
+    >           "results": [
+    >               {
+    >                   "url": "http://[kpi]/api/v2/organizations/org_12345/ \
+    >                   members/foo_bar/",
+    >                   "user": "http://[kpi]/api/v2/users/foo_bar/",
+    >                   "user__username": "foo_bar",
+    >                   "user__email": "foo_bar@example.com",
+    >                   "user__name": "Foo Bar",
+    >                   "role": "owner",
+    >                   "has_mfa_enabled": true,
+    >                   "date_joined": "2024-08-11T12:36:32Z",
+    >                   "is_active": true
+    >               },
+    >               {
+    >                   "url": "http://[kpi]/api/v2/organizations/org_12345/ \
+    >                   members/john_doe/",
+    >                   "user": "http://[kpi]/api/v2/users/john_doe/",
+    >                   "user__username": "john_doe",
+    >                   "user__email": "john_doe@example.com",
+    >                   "user__name": "John Doe",
+    >                   "role": "admin",
+    >                   "has_mfa_enabled": false,
+    >                   "date_joined": "2024-10-21T06:38:45Z",
+    >                   "is_active": true
+    >               }
+    >           ]
+    >       }
+
+    The response includes detailed information about each member, such as their
+    username, email, role (owner, admin, member), and account status.
+
+    ### Retrieve Member Details
+
+    Retrieves the details of a specific member within an organization by username.
+
+    <pre class="prettyprint">
+    <b>GET</b> /api/v2/organizations/{organization_id}/members/{username}/
+    </pre>
+
+    > Example
+    >
+    >       curl -X GET https://[kpi]/api/v2/organizations/org_12345/members/foo_bar/
+
+    > Response 200
+
+    >       {
+    >           "url": "http://[kpi]/api/v2/organizations/org_12345/members/foo_bar/",
+    >           "user": "http://[kpi]/api/v2/users/foo_bar/",
+    >           "user__username": "foo_bar",
+    >           "user__email": "foo_bar@example.com",
+    >           "user__name": "Foo Bar",
+    >           "role": "owner",
+    >           "has_mfa_enabled": true,
+    >           "date_joined": "2024-08-11T12:36:32Z",
+    >           "is_active": true
+    >       }
+
+    ### Update Member Role
+
+    Updates the role of a member within the organization to `owner`, `admin`, or
+     `member`.
+
+    <pre class="prettyprint">
+    <b>PATCH</b> /api/v2/organizations/{organization_id}/members/{username}/
+    </pre>
+
+    #### Payload
+    >       {
+    >           "role": "admin"
+    >       }
+
+    - **admin**: Grants the member admin privileges within the organization
+    - **member**: Revokes admin privileges, setting the member as a regular user
+
+    > Example
+    >
+    >       curl -X PATCH https://[kpi]/api/v2/organizations/org_12345/ \
+    >       members/demo_user/ -d '{"role": "admin"}'
+
+    ### Remove Member
+
+    Removes a member from the organization.
+
+    <pre class="prettyprint">
+    <b>DELETE</b> /api/v2/organizations/{organization_id}/members/{username}/
+    </pre>
+
+    > Example
+    >
+    >       curl -X DELETE https://[kpi]/api/v2/organizations/org_12345/members/foo_bar/
+
+    ## Permissions
+
+    - The user must be authenticated to perform these actions.
+
+    ## Notes
+
+    - **Role Validation**: Only valid roles ('admin', 'member') are accepted
+    in updates.
+    """
+    serializer_class = OrganizationUserSerializer
+    permission_classes = [IsAuthenticated]
+    pagination_class = OrganizationPagination
+    lookup_field = 'user__username'
+
+    def get_queryset(self):
+        organization_id = self.kwargs['organization_id']
+
+        # Subquery to check if the user is the owner
+        owner_subquery = OrganizationOwner.objects.filter(
+            organization_id=organization_id,
+            organization_user=OuterRef('pk')
+        ).values('pk')
+
+        # Annotate with role based on organization ownership and admin status
+        queryset = OrganizationUser.objects.filter(
+            organization_id=organization_id
+        ).annotate(
+            role=Case(
+                When(Exists(owner_subquery), then=Value('owner')),
+                When(is_admin=True, then=Value('admin')),
+                default=Value('member'),
+                output_field=CharField()
+            )
+        )
+        return queryset
+
+    def partial_update(self, request, *args, **kwargs):
+        instance = self.get_object()
+        serializer = self.get_serializer(
+            instance, data=request.data, partial=True
+        )
+        serializer.is_valid(raise_exception=True)
+        role = serializer.validated_data.get('role')
+        if role:
+            instance.is_admin = (role == 'admin')
+            instance.save()
+        return super().partial_update(request, *args, **kwargs)
diff --git a/kpi/paginators.py b/kpi/paginators.py
index 4b0b9ea667..33f29da90f 100644
--- a/kpi/paginators.py
+++ b/kpi/paginators.py
@@ -121,7 +121,7 @@ class DataPagination(LimitOffsetPagination):
     offset_query_param = 'start'
     max_limit = settings.SUBMISSION_LIST_LIMIT
 
-    
+
 class FastAssetPagination(Paginated):
     """
     Pagination class optimized for faster counting for DISTINCT queries on large tables.
@@ -142,3 +142,10 @@ class TinyPaginated(PageNumberPagination):
     Same as Paginated with a small page size
     """
     page_size = 50
+
+
+class OrganizationPagination(PageNumberPagination):
+    """
+    Pagination class for Organization
+    """
+    page_size = 10
diff --git a/kpi/urls/router_api_v2.py b/kpi/urls/router_api_v2.py
index 7c7dbd2575..411d77de2b 100644
--- a/kpi/urls/router_api_v2.py
+++ b/kpi/urls/router_api_v2.py
@@ -6,7 +6,7 @@
 from kobo.apps.hook.views.v2.hook import HookViewSet
 from kobo.apps.hook.views.v2.hook_log import HookLogViewSet
 from kobo.apps.languages.urls import router as language_router
-from kobo.apps.organizations.views import OrganizationViewSet
+from kobo.apps.organizations.views import OrganizationViewSet, OrganizationMemberViewSet
 from kobo.apps.project_ownership.urls import router as project_ownership_router
 from kobo.apps.project_views.views import ProjectViewViewSet
 from kpi.views.v2.asset import AssetViewSet
@@ -140,6 +140,12 @@ def get_urls(self, *args, **kwargs):
 router_api_v2.register(r'imports', ImportTaskViewSet)
 router_api_v2.register(r'organizations',
                        OrganizationViewSet, basename='organizations',)
+router_api_v2.register(
+    r'organizations/(?P<organization_id>[^/.]+)/members',
+    OrganizationMemberViewSet,
+    basename='organization-members',
+)
+
 router_api_v2.register(r'permissions', PermissionViewSet)
 router_api_v2.register(r'project-views', ProjectViewViewSet)
 router_api_v2.register(r'service_usage',

From 56cd7cf3ae5dbdca180300840f9485d5307a714c Mon Sep 17 00:00:00 2001
From: Leszek <smutnyleszek@gmail.com>
Date: Tue, 12 Nov 2024 15:23:18 +0100
Subject: [PATCH 02/29] add initial files for organization members route (WIP)

---
 .../js/account/organizations/MembersRoute.tsx | 78 +++++++++++++++++++
 .../js/account/organizations/membersQuery.ts  | 61 +++++++++++++++
 .../organizations/membersRoute.module.scss    | 26 +++++++
 jsapp/js/account/routes.constants.ts          |  3 +
 jsapp/js/account/routes.tsx                   |  3 +-
 jsapp/js/api.endpoints.ts                     |  1 +
 jsapp/js/query/queryKeys.ts                   |  1 +
 7 files changed, 172 insertions(+), 1 deletion(-)
 create mode 100644 jsapp/js/account/organizations/MembersRoute.tsx
 create mode 100644 jsapp/js/account/organizations/membersQuery.ts
 create mode 100644 jsapp/js/account/organizations/membersRoute.module.scss

diff --git a/jsapp/js/account/organizations/MembersRoute.tsx b/jsapp/js/account/organizations/MembersRoute.tsx
new file mode 100644
index 0000000000..0ca26099dc
--- /dev/null
+++ b/jsapp/js/account/organizations/MembersRoute.tsx
@@ -0,0 +1,78 @@
+// Libraries
+import React from 'react';
+
+// Partial components
+import PaginatedQueryUniversalTable from 'js/universalTable/paginatedQueryUniversalTable.component';
+
+// Stores, hooks and utilities
+import {formatTime} from 'js/utils';
+import useOrganiztionMembersQuery from './membersQuery';
+
+// Constants and types
+import type {OrganizationMember} from './membersQuery';
+
+// Styles
+import styles from './membersRoute.module.scss';
+
+export default function MembersRoute() {
+  return (
+    <div className={styles.membersRouteRoot}>
+      <header className={styles.header}>
+        <h2 className={styles.headerText}>{t('Members')}</h2>
+      </header>
+
+      <PaginatedQueryUniversalTable<OrganizationMember>
+        queryHook={useOrganiztionMembersQuery}
+        columns={[
+          {
+            key: 'user__username',
+            label: t('Name'),
+            cellFormatter: (member: OrganizationMember) => {
+              // TODO
+              return (
+                <>
+                  {member.user__name}
+                  @{member.user__username}
+                  {member.user__email}
+                </>
+              );
+            },
+          },
+          {
+            key: 'invite.status',
+            label: t('Status'),
+          },
+          {
+            key: 'date_joined',
+            label: t('Date added'),
+            cellFormatter: (member: OrganizationMember) => formatTime(member.date_joined),
+          },
+          {
+            key: 'role',
+            label: t('Role'),
+          },
+          {
+            key: 'user__has_mfa_enabled',
+            label: t('2FA'),
+            cellFormatter: (member: OrganizationMember) => {
+              if (member.user__has_mfa_enabled) {
+                return 'yes';
+              } else {
+                return null;
+              }
+            },
+          },
+          {
+            // We use `url` here, but the cell would contain interactive UI
+            // element
+            key: 'url',
+            label: '',
+            cellFormatter: () => {
+              return 'TBD';
+            },
+          },
+        ]}
+      />
+    </div>
+  );
+}
diff --git a/jsapp/js/account/organizations/membersQuery.ts b/jsapp/js/account/organizations/membersQuery.ts
new file mode 100644
index 0000000000..f7ea4b1a5c
--- /dev/null
+++ b/jsapp/js/account/organizations/membersQuery.ts
@@ -0,0 +1,61 @@
+import {keepPreviousData, useQuery} from '@tanstack/react-query';
+import {endpoints} from 'js/api.endpoints';
+import type {PaginatedResponse} from 'js/dataInterface';
+import {fetchGet} from 'js/api';
+import {QueryKeys} from 'js/query/queryKeys';
+
+export interface OrganizationMember {
+  /**
+   * The url to the member within the organization
+   * `/api/v2/organizations/<organization_uid>/members/<username>/`
+   */
+  url: string;
+  /** `/api/v2/users/<username>/` */
+  user: string;
+  user__username: string;
+  user__email: string;
+  user__name: string;
+  role: 'admin' | 'owner' | 'member' | 'external';
+  user__has_mfa_enabled: boolean;
+  /** yyyy-mm-dd HH:MM:SS */
+  date_joined: string;
+  user__is_active: boolean;
+  invite: {
+    /** '/api/v2/organizations/<organization_uid>/invites/<invite_uid>/' */
+    url: string;
+    /** yyyy-mm-dd HH:MM:SS */
+    date_created: string;
+    /** yyyy-mm-dd HH:MM:SS */
+    date_modified: string;
+    status: 'sent' | 'accepted' | 'expired' | 'declined';
+  };
+}
+
+async function getOrganizationMembers(limit: number, offset: number) {
+  const params = new URLSearchParams({
+    limit: limit.toString(),
+    offset: offset.toString(),
+  });
+  return fetchGet<PaginatedResponse<OrganizationMember>>(
+    endpoints.ORGANIZATION_MEMBERS_URL + '?' + params,
+    {
+      errorMessageDisplay: t('There was an error getting the list.'),
+    }
+  );
+}
+
+export default function useOrganizationMembersQuery(
+  itemLimit: number,
+  pageOffset: number
+) {
+  return useQuery({
+    queryKey: [QueryKeys.organizationMembers, itemLimit, pageOffset],
+    queryFn: () => getOrganizationMembers(itemLimit, pageOffset),
+    placeholderData: keepPreviousData,
+    // We might want to improve this in future, for now let's not retry
+    retry: false,
+    // The `refetchOnWindowFocus` option is `true` by default, I'm setting it
+    // here so we don't forget about it.
+    refetchOnWindowFocus: true,
+  });
+}
diff --git a/jsapp/js/account/organizations/membersRoute.module.scss b/jsapp/js/account/organizations/membersRoute.module.scss
new file mode 100644
index 0000000000..d100d36428
--- /dev/null
+++ b/jsapp/js/account/organizations/membersRoute.module.scss
@@ -0,0 +1,26 @@
+@use 'scss/colors';
+@use 'scss/breakpoints';
+
+.membersRouteRoot {
+  padding: 20px;
+  overflow-y: auto;
+  height: 100%;
+}
+
+.header {
+  margin-bottom: 20px;
+}
+
+h2.headerText {
+  color: colors.$kobo-storm;
+  text-transform: uppercase;
+  font-size: 18px;
+  font-weight: 700;
+  margin: 0;
+}
+
+@include breakpoints.breakpoint(mediumAndUp) {
+  .membersRouteRoot {
+    padding: 50px;
+  }
+}
diff --git a/jsapp/js/account/routes.constants.ts b/jsapp/js/account/routes.constants.ts
index a8ff1262bd..8f9e32fb2b 100644
--- a/jsapp/js/account/routes.constants.ts
+++ b/jsapp/js/account/routes.constants.ts
@@ -19,6 +19,9 @@ export const AccountSettings = React.lazy(
 export const DataStorage = React.lazy(
   () => import(/* webpackPrefetch: true */ './usage/usageTopTabs')
 );
+export const MembersRoute = React.lazy(
+  () => import(/* webpackPrefetch: true */ './organizations/MembersRoute')
+);
 export const ACCOUNT_ROUTES: {readonly [key: string]: string} = {
   ACCOUNT_SETTINGS: ROUTES.ACCOUNT_ROOT + '/settings',
   USAGE: ROUTES.ACCOUNT_ROOT + '/usage',
diff --git a/jsapp/js/account/routes.tsx b/jsapp/js/account/routes.tsx
index 1046d5ed85..0ebbf06301 100644
--- a/jsapp/js/account/routes.tsx
+++ b/jsapp/js/account/routes.tsx
@@ -11,6 +11,7 @@ import {
   DataStorage,
   PlansRoute,
   SecurityRoute,
+  MembersRoute,
 } from 'js/account/routes.constants';
 import {useFeatureFlag, FeatureFlag} from 'js/featureFlags';
 
@@ -109,7 +110,7 @@ export default function routes() {
             element={
               <RequireAuth>
                 <ValidateOrgPermissions mmoOnly>
-                  <div>Organization members view to be implemented</div>
+                  <MembersRoute />
                 </ValidateOrgPermissions>
               </RequireAuth>
             }
diff --git a/jsapp/js/api.endpoints.ts b/jsapp/js/api.endpoints.ts
index 5a4c6e899d..0f8a20dcdc 100644
--- a/jsapp/js/api.endpoints.ts
+++ b/jsapp/js/api.endpoints.ts
@@ -4,6 +4,7 @@ export const endpoints = {
   PRODUCTS_URL: '/api/v2/stripe/products/',
   SUBSCRIPTION_URL: '/api/v2/stripe/subscriptions/',
   ORGANIZATION_URL: '/api/v2/organizations/',
+  ORGANIZATION_MEMBERS_URL: '/api/v2/organizations/:organization_id/members/',
   /** Expected parameters: price_id and organization_id **/
   CHECKOUT_URL: '/api/v2/stripe/checkout-link',
   /** Expected parameter: organization_id  **/
diff --git a/jsapp/js/query/queryKeys.ts b/jsapp/js/query/queryKeys.ts
index 3cd3dc26e4..1ded3e7116 100644
--- a/jsapp/js/query/queryKeys.ts
+++ b/jsapp/js/query/queryKeys.ts
@@ -11,4 +11,5 @@ export enum QueryKeys {
   activityLogs = 'activityLogs',
   activityLogsFilter = 'activityLogsFilter',
   organization = 'organization',
+  organizationMembers = 'organizationMembers',
 }

From 95e48c48e38c92ece90aefd3f7d76118b212b217 Mon Sep 17 00:00:00 2001
From: Leszek <smutnyleszek@gmail.com>
Date: Tue, 12 Nov 2024 20:57:45 +0100
Subject: [PATCH 03/29] pass org id to hook (WIP)

---
 jsapp/js/account/organizations/MembersRoute.tsx | 16 ++++++++++++++--
 jsapp/js/account/organizations/membersQuery.ts  | 13 +++++++++----
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/jsapp/js/account/organizations/MembersRoute.tsx b/jsapp/js/account/organizations/MembersRoute.tsx
index 0ca26099dc..3acf57f588 100644
--- a/jsapp/js/account/organizations/MembersRoute.tsx
+++ b/jsapp/js/account/organizations/MembersRoute.tsx
@@ -3,10 +3,12 @@ import React from 'react';
 
 // Partial components
 import PaginatedQueryUniversalTable from 'js/universalTable/paginatedQueryUniversalTable.component';
+import LoadingSpinner from 'js/components/common/loadingSpinner';
 
 // Stores, hooks and utilities
 import {formatTime} from 'js/utils';
-import useOrganiztionMembersQuery from './membersQuery';
+import {useOrganizationQuery} from 'js/account/stripe.api';
+import useOrganizationMembersQuery from './membersQuery';
 
 // Constants and types
 import type {OrganizationMember} from './membersQuery';
@@ -15,6 +17,14 @@ import type {OrganizationMember} from './membersQuery';
 import styles from './membersRoute.module.scss';
 
 export default function MembersRoute() {
+  const orgQuery = useOrganizationQuery();
+
+  if (!orgQuery.data?.id) {
+    return (
+      <LoadingSpinner />
+    );
+  }
+
   return (
     <div className={styles.membersRouteRoot}>
       <header className={styles.header}>
@@ -22,7 +32,9 @@ export default function MembersRoute() {
       </header>
 
       <PaginatedQueryUniversalTable<OrganizationMember>
-        queryHook={useOrganiztionMembersQuery}
+        // TOOD: how to pass organization id to the query?
+        // Build something similar to PaginatedQueryUniversalTable here??
+        queryHook={useOrganizationMembersQuery}
         columns={[
           {
             key: 'user__username',
diff --git a/jsapp/js/account/organizations/membersQuery.ts b/jsapp/js/account/organizations/membersQuery.ts
index f7ea4b1a5c..8892e94320 100644
--- a/jsapp/js/account/organizations/membersQuery.ts
+++ b/jsapp/js/account/organizations/membersQuery.ts
@@ -31,13 +31,17 @@ export interface OrganizationMember {
   };
 }
 
-async function getOrganizationMembers(limit: number, offset: number) {
+async function getOrganizationMembers(
+  organizationId: string,
+  limit: number,
+  offset: number
+) {
   const params = new URLSearchParams({
     limit: limit.toString(),
     offset: offset.toString(),
   });
   return fetchGet<PaginatedResponse<OrganizationMember>>(
-    endpoints.ORGANIZATION_MEMBERS_URL + '?' + params,
+    endpoints.ORGANIZATION_MEMBERS_URL.replace(':organization_id', organizationId) + '?' + params,
     {
       errorMessageDisplay: t('There was an error getting the list.'),
     }
@@ -45,12 +49,13 @@ async function getOrganizationMembers(limit: number, offset: number) {
 }
 
 export default function useOrganizationMembersQuery(
+  organizationId: string,
   itemLimit: number,
   pageOffset: number
 ) {
   return useQuery({
-    queryKey: [QueryKeys.organizationMembers, itemLimit, pageOffset],
-    queryFn: () => getOrganizationMembers(itemLimit, pageOffset),
+    queryKey: [QueryKeys.organizationMembers, organizationId, itemLimit, pageOffset],
+    queryFn: () => getOrganizationMembers(organizationId, itemLimit, pageOffset),
     placeholderData: keepPreviousData,
     // We might want to improve this in future, for now let's not retry
     retry: false,

From 0401fc479ba3e678eb186b8ca0ca5f1809cf1c61 Mon Sep 17 00:00:00 2001
From: rajpatel24 <rajpatel24998@gmail.com>
Date: Wed, 13 Nov 2024 15:33:22 +0530
Subject: [PATCH 04/29] Refactor organization member API to eliminate
 redundancy and optimize query for user mfa details

---
 kobo/apps/organizations/serializers.py         | 16 ++++++++++------
 .../tests/test_organization_members_api.py     | 18 +++++-------------
 kobo/apps/organizations/views.py               | 15 ++++++++++++---
 kpi/paginators.py                              |  2 +-
 4 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/kobo/apps/organizations/serializers.py b/kobo/apps/organizations/serializers.py
index 25dd405c30..a68abeba2c 100644
--- a/kobo/apps/organizations/serializers.py
+++ b/kobo/apps/organizations/serializers.py
@@ -21,7 +21,9 @@ 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'
@@ -29,7 +31,7 @@ class OrganizationUserSerializer(serializers.ModelSerializer):
     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')
diff --git a/kobo/apps/organizations/tests/test_organization_members_api.py b/kobo/apps/organizations/tests/test_organization_members_api.py
index 7d0045be6a..d9cd447a2b 100644
--- a/kobo/apps/organizations/tests/test_organization_members_api.py
+++ b/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(
diff --git a/kobo/apps/organizations/views.py b/kobo/apps/organizations/views.py
index 7792d1a1ee..0bbcc84551 100644
--- a/kobo/apps/organizations/views.py
+++ b/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
 
diff --git a/kpi/paginators.py b/kpi/paginators.py
index 33f29da90f..34a7fe9ee5 100644
--- a/kpi/paginators.py
+++ b/kpi/paginators.py
@@ -144,7 +144,7 @@ class TinyPaginated(PageNumberPagination):
     page_size = 50
 
 
-class OrganizationPagination(PageNumberPagination):
+class OrganizationMemberPagination(PageNumberPagination):
     """
     Pagination class for Organization
     """

From 99eea0c79bf88400edf5fb3bd09007d3d6938ca3 Mon Sep 17 00:00:00 2001
From: Leszek <smutnyleszek@gmail.com>
Date: Wed, 13 Nov 2024 13:23:31 +0100
Subject: [PATCH 05/29] Add `queryHookOptions` to
 `PaginatedQueryUniversalTable` to make the hook more customizable

and then use it in `MembersRoute`
---
 .../js/account/organizations/MembersRoute.tsx |  3 +--
 .../js/account/organizations/membersQuery.ts  | 26 ++++++++++++++-----
 ...paginatedQueryUniversalTable.component.tsx | 21 +++++++++++++--
 3 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/jsapp/js/account/organizations/MembersRoute.tsx b/jsapp/js/account/organizations/MembersRoute.tsx
index 3acf57f588..4ae08d18cf 100644
--- a/jsapp/js/account/organizations/MembersRoute.tsx
+++ b/jsapp/js/account/organizations/MembersRoute.tsx
@@ -32,9 +32,8 @@ export default function MembersRoute() {
       </header>
 
       <PaginatedQueryUniversalTable<OrganizationMember>
-        // TOOD: how to pass organization id to the query?
-        // Build something similar to PaginatedQueryUniversalTable here??
         queryHook={useOrganizationMembersQuery}
+        queryHookOptions={{organizationId: orgQuery.data.id}}
         columns={[
           {
             key: 'user__username',
diff --git a/jsapp/js/account/organizations/membersQuery.ts b/jsapp/js/account/organizations/membersQuery.ts
index 8892e94320..066e84329b 100644
--- a/jsapp/js/account/organizations/membersQuery.ts
+++ b/jsapp/js/account/organizations/membersQuery.ts
@@ -3,6 +3,7 @@ import {endpoints} from 'js/api.endpoints';
 import type {PaginatedResponse} from 'js/dataInterface';
 import {fetchGet} from 'js/api';
 import {QueryKeys} from 'js/query/queryKeys';
+import type {PaginatedQueryHookOptions} from 'js/universalTable/paginatedQueryUniversalTable.component';
 
 export interface OrganizationMember {
   /**
@@ -32,16 +33,22 @@ export interface OrganizationMember {
 }
 
 async function getOrganizationMembers(
-  organizationId: string,
   limit: number,
-  offset: number
+  offset: number,
+  options?: PaginatedQueryHookOptions
 ) {
   const params = new URLSearchParams({
     limit: limit.toString(),
     offset: offset.toString(),
   });
+
+  let apiUrl = endpoints.ORGANIZATION_MEMBERS_URL;
+  if (options?.organizationId) {
+    apiUrl = apiUrl.replace(':organization_id', options.organizationId);
+  }
+
   return fetchGet<PaginatedResponse<OrganizationMember>>(
-    endpoints.ORGANIZATION_MEMBERS_URL.replace(':organization_id', organizationId) + '?' + params,
+    apiUrl + '?' + params,
     {
       errorMessageDisplay: t('There was an error getting the list.'),
     }
@@ -49,13 +56,18 @@ async function getOrganizationMembers(
 }
 
 export default function useOrganizationMembersQuery(
-  organizationId: string,
   itemLimit: number,
-  pageOffset: number
+  pageOffset: number,
+  /**
+   * This is optional only to satisfy TS in `PaginatedQueryUniversalTable`. In
+   * reality, data will not be fetched properly without `organizationId` passed
+   * in the `options` object - see `getOrganizationMembers()` for details.
+   */
+  options?: PaginatedQueryHookOptions
 ) {
   return useQuery({
-    queryKey: [QueryKeys.organizationMembers, organizationId, itemLimit, pageOffset],
-    queryFn: () => getOrganizationMembers(organizationId, itemLimit, pageOffset),
+    queryKey: [QueryKeys.organizationMembers, itemLimit, pageOffset, options],
+    queryFn: () => getOrganizationMembers(itemLimit, pageOffset, options),
     placeholderData: keepPreviousData,
     // We might want to improve this in future, for now let's not retry
     retry: false,
diff --git a/jsapp/js/universalTable/paginatedQueryUniversalTable.component.tsx b/jsapp/js/universalTable/paginatedQueryUniversalTable.component.tsx
index b5775af594..5c9b369699 100644
--- a/jsapp/js/universalTable/paginatedQueryUniversalTable.component.tsx
+++ b/jsapp/js/universalTable/paginatedQueryUniversalTable.component.tsx
@@ -9,12 +9,25 @@ import type {UseQueryResult} from '@tanstack/react-query';
 import type {PaginatedResponse} from 'js/dataInterface';
 import type {UniversalTableColumn} from './universalTable.component';
 
+/**
+ * A way of passing custom options, e.g. a parameter that needs to be included
+ * in the API url.
+ */
+export interface PaginatedQueryHookOptions {
+  [optionName: string]: string;
+}
+
 interface PaginatedQueryHook<DataItem> extends Function {
-  (limit: number, offset: number): UseQueryResult<PaginatedResponse<DataItem>>;
+  (
+    limit: number,
+    offset: number,
+    options?: PaginatedQueryHookOptions
+  ): UseQueryResult<PaginatedResponse<DataItem>>;
 }
 
 interface PaginatedQueryUniversalTableProps<DataItem> {
   queryHook: PaginatedQueryHook<DataItem>;
+  queryHookOptions?: PaginatedQueryHookOptions;
   // Below are props from `UniversalTable` that should come from the parent
   // component (these are kind of "configuration" props). The other
   // `UniversalTable` props are being handled here internally.
@@ -39,7 +52,11 @@ export default function PaginatedQueryUniversalTable<DataItem>(
     offset: 0,
   });
 
-  const paginatedQuery = props.queryHook(pagination.limit, pagination.offset);
+  const paginatedQuery = props.queryHook(
+    pagination.limit,
+    pagination.offset,
+    props.queryHookOptions
+  );
 
   const availablePages = useMemo(
     () => Math.ceil((paginatedQuery.data?.count ?? 0) / pagination.limit),

From a75ca3b5698facc93c84c1c1552c98d46f807078 Mon Sep 17 00:00:00 2001
From: Leszek <smutnyleszek@gmail.com>
Date: Tue, 12 Nov 2024 15:23:18 +0100
Subject: [PATCH 06/29] add initial files for organization members route (WIP)

---
 .../js/account/organizations/MembersRoute.tsx | 78 +++++++++++++++++++
 .../js/account/organizations/membersQuery.ts  | 61 +++++++++++++++
 .../organizations/membersRoute.module.scss    | 26 +++++++
 jsapp/js/account/routes.constants.ts          |  3 +
 jsapp/js/account/routes.tsx                   |  3 +-
 jsapp/js/api.endpoints.ts                     |  1 +
 jsapp/js/query/queryKeys.ts                   |  1 +
 7 files changed, 172 insertions(+), 1 deletion(-)
 create mode 100644 jsapp/js/account/organizations/MembersRoute.tsx
 create mode 100644 jsapp/js/account/organizations/membersQuery.ts
 create mode 100644 jsapp/js/account/organizations/membersRoute.module.scss

diff --git a/jsapp/js/account/organizations/MembersRoute.tsx b/jsapp/js/account/organizations/MembersRoute.tsx
new file mode 100644
index 0000000000..0ca26099dc
--- /dev/null
+++ b/jsapp/js/account/organizations/MembersRoute.tsx
@@ -0,0 +1,78 @@
+// Libraries
+import React from 'react';
+
+// Partial components
+import PaginatedQueryUniversalTable from 'js/universalTable/paginatedQueryUniversalTable.component';
+
+// Stores, hooks and utilities
+import {formatTime} from 'js/utils';
+import useOrganiztionMembersQuery from './membersQuery';
+
+// Constants and types
+import type {OrganizationMember} from './membersQuery';
+
+// Styles
+import styles from './membersRoute.module.scss';
+
+export default function MembersRoute() {
+  return (
+    <div className={styles.membersRouteRoot}>
+      <header className={styles.header}>
+        <h2 className={styles.headerText}>{t('Members')}</h2>
+      </header>
+
+      <PaginatedQueryUniversalTable<OrganizationMember>
+        queryHook={useOrganiztionMembersQuery}
+        columns={[
+          {
+            key: 'user__username',
+            label: t('Name'),
+            cellFormatter: (member: OrganizationMember) => {
+              // TODO
+              return (
+                <>
+                  {member.user__name}
+                  @{member.user__username}
+                  {member.user__email}
+                </>
+              );
+            },
+          },
+          {
+            key: 'invite.status',
+            label: t('Status'),
+          },
+          {
+            key: 'date_joined',
+            label: t('Date added'),
+            cellFormatter: (member: OrganizationMember) => formatTime(member.date_joined),
+          },
+          {
+            key: 'role',
+            label: t('Role'),
+          },
+          {
+            key: 'user__has_mfa_enabled',
+            label: t('2FA'),
+            cellFormatter: (member: OrganizationMember) => {
+              if (member.user__has_mfa_enabled) {
+                return 'yes';
+              } else {
+                return null;
+              }
+            },
+          },
+          {
+            // We use `url` here, but the cell would contain interactive UI
+            // element
+            key: 'url',
+            label: '',
+            cellFormatter: () => {
+              return 'TBD';
+            },
+          },
+        ]}
+      />
+    </div>
+  );
+}
diff --git a/jsapp/js/account/organizations/membersQuery.ts b/jsapp/js/account/organizations/membersQuery.ts
new file mode 100644
index 0000000000..f7ea4b1a5c
--- /dev/null
+++ b/jsapp/js/account/organizations/membersQuery.ts
@@ -0,0 +1,61 @@
+import {keepPreviousData, useQuery} from '@tanstack/react-query';
+import {endpoints} from 'js/api.endpoints';
+import type {PaginatedResponse} from 'js/dataInterface';
+import {fetchGet} from 'js/api';
+import {QueryKeys} from 'js/query/queryKeys';
+
+export interface OrganizationMember {
+  /**
+   * The url to the member within the organization
+   * `/api/v2/organizations/<organization_uid>/members/<username>/`
+   */
+  url: string;
+  /** `/api/v2/users/<username>/` */
+  user: string;
+  user__username: string;
+  user__email: string;
+  user__name: string;
+  role: 'admin' | 'owner' | 'member' | 'external';
+  user__has_mfa_enabled: boolean;
+  /** yyyy-mm-dd HH:MM:SS */
+  date_joined: string;
+  user__is_active: boolean;
+  invite: {
+    /** '/api/v2/organizations/<organization_uid>/invites/<invite_uid>/' */
+    url: string;
+    /** yyyy-mm-dd HH:MM:SS */
+    date_created: string;
+    /** yyyy-mm-dd HH:MM:SS */
+    date_modified: string;
+    status: 'sent' | 'accepted' | 'expired' | 'declined';
+  };
+}
+
+async function getOrganizationMembers(limit: number, offset: number) {
+  const params = new URLSearchParams({
+    limit: limit.toString(),
+    offset: offset.toString(),
+  });
+  return fetchGet<PaginatedResponse<OrganizationMember>>(
+    endpoints.ORGANIZATION_MEMBERS_URL + '?' + params,
+    {
+      errorMessageDisplay: t('There was an error getting the list.'),
+    }
+  );
+}
+
+export default function useOrganizationMembersQuery(
+  itemLimit: number,
+  pageOffset: number
+) {
+  return useQuery({
+    queryKey: [QueryKeys.organizationMembers, itemLimit, pageOffset],
+    queryFn: () => getOrganizationMembers(itemLimit, pageOffset),
+    placeholderData: keepPreviousData,
+    // We might want to improve this in future, for now let's not retry
+    retry: false,
+    // The `refetchOnWindowFocus` option is `true` by default, I'm setting it
+    // here so we don't forget about it.
+    refetchOnWindowFocus: true,
+  });
+}
diff --git a/jsapp/js/account/organizations/membersRoute.module.scss b/jsapp/js/account/organizations/membersRoute.module.scss
new file mode 100644
index 0000000000..d100d36428
--- /dev/null
+++ b/jsapp/js/account/organizations/membersRoute.module.scss
@@ -0,0 +1,26 @@
+@use 'scss/colors';
+@use 'scss/breakpoints';
+
+.membersRouteRoot {
+  padding: 20px;
+  overflow-y: auto;
+  height: 100%;
+}
+
+.header {
+  margin-bottom: 20px;
+}
+
+h2.headerText {
+  color: colors.$kobo-storm;
+  text-transform: uppercase;
+  font-size: 18px;
+  font-weight: 700;
+  margin: 0;
+}
+
+@include breakpoints.breakpoint(mediumAndUp) {
+  .membersRouteRoot {
+    padding: 50px;
+  }
+}
diff --git a/jsapp/js/account/routes.constants.ts b/jsapp/js/account/routes.constants.ts
index a8ff1262bd..8f9e32fb2b 100644
--- a/jsapp/js/account/routes.constants.ts
+++ b/jsapp/js/account/routes.constants.ts
@@ -19,6 +19,9 @@ export const AccountSettings = React.lazy(
 export const DataStorage = React.lazy(
   () => import(/* webpackPrefetch: true */ './usage/usageTopTabs')
 );
+export const MembersRoute = React.lazy(
+  () => import(/* webpackPrefetch: true */ './organizations/MembersRoute')
+);
 export const ACCOUNT_ROUTES: {readonly [key: string]: string} = {
   ACCOUNT_SETTINGS: ROUTES.ACCOUNT_ROOT + '/settings',
   USAGE: ROUTES.ACCOUNT_ROOT + '/usage',
diff --git a/jsapp/js/account/routes.tsx b/jsapp/js/account/routes.tsx
index 1046d5ed85..0ebbf06301 100644
--- a/jsapp/js/account/routes.tsx
+++ b/jsapp/js/account/routes.tsx
@@ -11,6 +11,7 @@ import {
   DataStorage,
   PlansRoute,
   SecurityRoute,
+  MembersRoute,
 } from 'js/account/routes.constants';
 import {useFeatureFlag, FeatureFlag} from 'js/featureFlags';
 
@@ -109,7 +110,7 @@ export default function routes() {
             element={
               <RequireAuth>
                 <ValidateOrgPermissions mmoOnly>
-                  <div>Organization members view to be implemented</div>
+                  <MembersRoute />
                 </ValidateOrgPermissions>
               </RequireAuth>
             }
diff --git a/jsapp/js/api.endpoints.ts b/jsapp/js/api.endpoints.ts
index 5a4c6e899d..0f8a20dcdc 100644
--- a/jsapp/js/api.endpoints.ts
+++ b/jsapp/js/api.endpoints.ts
@@ -4,6 +4,7 @@ export const endpoints = {
   PRODUCTS_URL: '/api/v2/stripe/products/',
   SUBSCRIPTION_URL: '/api/v2/stripe/subscriptions/',
   ORGANIZATION_URL: '/api/v2/organizations/',
+  ORGANIZATION_MEMBERS_URL: '/api/v2/organizations/:organization_id/members/',
   /** Expected parameters: price_id and organization_id **/
   CHECKOUT_URL: '/api/v2/stripe/checkout-link',
   /** Expected parameter: organization_id  **/
diff --git a/jsapp/js/query/queryKeys.ts b/jsapp/js/query/queryKeys.ts
index 3cd3dc26e4..1ded3e7116 100644
--- a/jsapp/js/query/queryKeys.ts
+++ b/jsapp/js/query/queryKeys.ts
@@ -11,4 +11,5 @@ export enum QueryKeys {
   activityLogs = 'activityLogs',
   activityLogsFilter = 'activityLogsFilter',
   organization = 'organization',
+  organizationMembers = 'organizationMembers',
 }

From f2301b7ce72b54038e4c75e66d2b9784269e6df7 Mon Sep 17 00:00:00 2001
From: Leszek <smutnyleszek@gmail.com>
Date: Tue, 12 Nov 2024 20:57:45 +0100
Subject: [PATCH 07/29] pass org id to hook (WIP)

---
 jsapp/js/account/organizations/MembersRoute.tsx | 16 ++++++++++++++--
 jsapp/js/account/organizations/membersQuery.ts  | 13 +++++++++----
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/jsapp/js/account/organizations/MembersRoute.tsx b/jsapp/js/account/organizations/MembersRoute.tsx
index 0ca26099dc..3acf57f588 100644
--- a/jsapp/js/account/organizations/MembersRoute.tsx
+++ b/jsapp/js/account/organizations/MembersRoute.tsx
@@ -3,10 +3,12 @@ import React from 'react';
 
 // Partial components
 import PaginatedQueryUniversalTable from 'js/universalTable/paginatedQueryUniversalTable.component';
+import LoadingSpinner from 'js/components/common/loadingSpinner';
 
 // Stores, hooks and utilities
 import {formatTime} from 'js/utils';
-import useOrganiztionMembersQuery from './membersQuery';
+import {useOrganizationQuery} from 'js/account/stripe.api';
+import useOrganizationMembersQuery from './membersQuery';
 
 // Constants and types
 import type {OrganizationMember} from './membersQuery';
@@ -15,6 +17,14 @@ import type {OrganizationMember} from './membersQuery';
 import styles from './membersRoute.module.scss';
 
 export default function MembersRoute() {
+  const orgQuery = useOrganizationQuery();
+
+  if (!orgQuery.data?.id) {
+    return (
+      <LoadingSpinner />
+    );
+  }
+
   return (
     <div className={styles.membersRouteRoot}>
       <header className={styles.header}>
@@ -22,7 +32,9 @@ export default function MembersRoute() {
       </header>
 
       <PaginatedQueryUniversalTable<OrganizationMember>
-        queryHook={useOrganiztionMembersQuery}
+        // TOOD: how to pass organization id to the query?
+        // Build something similar to PaginatedQueryUniversalTable here??
+        queryHook={useOrganizationMembersQuery}
         columns={[
           {
             key: 'user__username',
diff --git a/jsapp/js/account/organizations/membersQuery.ts b/jsapp/js/account/organizations/membersQuery.ts
index f7ea4b1a5c..8892e94320 100644
--- a/jsapp/js/account/organizations/membersQuery.ts
+++ b/jsapp/js/account/organizations/membersQuery.ts
@@ -31,13 +31,17 @@ export interface OrganizationMember {
   };
 }
 
-async function getOrganizationMembers(limit: number, offset: number) {
+async function getOrganizationMembers(
+  organizationId: string,
+  limit: number,
+  offset: number
+) {
   const params = new URLSearchParams({
     limit: limit.toString(),
     offset: offset.toString(),
   });
   return fetchGet<PaginatedResponse<OrganizationMember>>(
-    endpoints.ORGANIZATION_MEMBERS_URL + '?' + params,
+    endpoints.ORGANIZATION_MEMBERS_URL.replace(':organization_id', organizationId) + '?' + params,
     {
       errorMessageDisplay: t('There was an error getting the list.'),
     }
@@ -45,12 +49,13 @@ async function getOrganizationMembers(limit: number, offset: number) {
 }
 
 export default function useOrganizationMembersQuery(
+  organizationId: string,
   itemLimit: number,
   pageOffset: number
 ) {
   return useQuery({
-    queryKey: [QueryKeys.organizationMembers, itemLimit, pageOffset],
-    queryFn: () => getOrganizationMembers(itemLimit, pageOffset),
+    queryKey: [QueryKeys.organizationMembers, organizationId, itemLimit, pageOffset],
+    queryFn: () => getOrganizationMembers(organizationId, itemLimit, pageOffset),
     placeholderData: keepPreviousData,
     // We might want to improve this in future, for now let's not retry
     retry: false,

From 2066a3b627af7e987d7c2034c92aebed68858ef0 Mon Sep 17 00:00:00 2001
From: Leszek <smutnyleszek@gmail.com>
Date: Wed, 13 Nov 2024 13:23:31 +0100
Subject: [PATCH 08/29] Add `queryHookOptions` to
 `PaginatedQueryUniversalTable` to make the hook more customizable

and then use it in `MembersRoute`
---
 .../js/account/organizations/MembersRoute.tsx |  3 +--
 .../js/account/organizations/membersQuery.ts  | 26 ++++++++++++++-----
 ...paginatedQueryUniversalTable.component.tsx | 21 +++++++++++++--
 3 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/jsapp/js/account/organizations/MembersRoute.tsx b/jsapp/js/account/organizations/MembersRoute.tsx
index 3acf57f588..4ae08d18cf 100644
--- a/jsapp/js/account/organizations/MembersRoute.tsx
+++ b/jsapp/js/account/organizations/MembersRoute.tsx
@@ -32,9 +32,8 @@ export default function MembersRoute() {
       </header>
 
       <PaginatedQueryUniversalTable<OrganizationMember>
-        // TOOD: how to pass organization id to the query?
-        // Build something similar to PaginatedQueryUniversalTable here??
         queryHook={useOrganizationMembersQuery}
+        queryHookOptions={{organizationId: orgQuery.data.id}}
         columns={[
           {
             key: 'user__username',
diff --git a/jsapp/js/account/organizations/membersQuery.ts b/jsapp/js/account/organizations/membersQuery.ts
index 8892e94320..066e84329b 100644
--- a/jsapp/js/account/organizations/membersQuery.ts
+++ b/jsapp/js/account/organizations/membersQuery.ts
@@ -3,6 +3,7 @@ import {endpoints} from 'js/api.endpoints';
 import type {PaginatedResponse} from 'js/dataInterface';
 import {fetchGet} from 'js/api';
 import {QueryKeys} from 'js/query/queryKeys';
+import type {PaginatedQueryHookOptions} from 'js/universalTable/paginatedQueryUniversalTable.component';
 
 export interface OrganizationMember {
   /**
@@ -32,16 +33,22 @@ export interface OrganizationMember {
 }
 
 async function getOrganizationMembers(
-  organizationId: string,
   limit: number,
-  offset: number
+  offset: number,
+  options?: PaginatedQueryHookOptions
 ) {
   const params = new URLSearchParams({
     limit: limit.toString(),
     offset: offset.toString(),
   });
+
+  let apiUrl = endpoints.ORGANIZATION_MEMBERS_URL;
+  if (options?.organizationId) {
+    apiUrl = apiUrl.replace(':organization_id', options.organizationId);
+  }
+
   return fetchGet<PaginatedResponse<OrganizationMember>>(
-    endpoints.ORGANIZATION_MEMBERS_URL.replace(':organization_id', organizationId) + '?' + params,
+    apiUrl + '?' + params,
     {
       errorMessageDisplay: t('There was an error getting the list.'),
     }
@@ -49,13 +56,18 @@ async function getOrganizationMembers(
 }
 
 export default function useOrganizationMembersQuery(
-  organizationId: string,
   itemLimit: number,
-  pageOffset: number
+  pageOffset: number,
+  /**
+   * This is optional only to satisfy TS in `PaginatedQueryUniversalTable`. In
+   * reality, data will not be fetched properly without `organizationId` passed
+   * in the `options` object - see `getOrganizationMembers()` for details.
+   */
+  options?: PaginatedQueryHookOptions
 ) {
   return useQuery({
-    queryKey: [QueryKeys.organizationMembers, organizationId, itemLimit, pageOffset],
-    queryFn: () => getOrganizationMembers(organizationId, itemLimit, pageOffset),
+    queryKey: [QueryKeys.organizationMembers, itemLimit, pageOffset, options],
+    queryFn: () => getOrganizationMembers(itemLimit, pageOffset, options),
     placeholderData: keepPreviousData,
     // We might want to improve this in future, for now let's not retry
     retry: false,
diff --git a/jsapp/js/universalTable/paginatedQueryUniversalTable.component.tsx b/jsapp/js/universalTable/paginatedQueryUniversalTable.component.tsx
index b5775af594..5c9b369699 100644
--- a/jsapp/js/universalTable/paginatedQueryUniversalTable.component.tsx
+++ b/jsapp/js/universalTable/paginatedQueryUniversalTable.component.tsx
@@ -9,12 +9,25 @@ import type {UseQueryResult} from '@tanstack/react-query';
 import type {PaginatedResponse} from 'js/dataInterface';
 import type {UniversalTableColumn} from './universalTable.component';
 
+/**
+ * A way of passing custom options, e.g. a parameter that needs to be included
+ * in the API url.
+ */
+export interface PaginatedQueryHookOptions {
+  [optionName: string]: string;
+}
+
 interface PaginatedQueryHook<DataItem> extends Function {
-  (limit: number, offset: number): UseQueryResult<PaginatedResponse<DataItem>>;
+  (
+    limit: number,
+    offset: number,
+    options?: PaginatedQueryHookOptions
+  ): UseQueryResult<PaginatedResponse<DataItem>>;
 }
 
 interface PaginatedQueryUniversalTableProps<DataItem> {
   queryHook: PaginatedQueryHook<DataItem>;
+  queryHookOptions?: PaginatedQueryHookOptions;
   // Below are props from `UniversalTable` that should come from the parent
   // component (these are kind of "configuration" props). The other
   // `UniversalTable` props are being handled here internally.
@@ -39,7 +52,11 @@ export default function PaginatedQueryUniversalTable<DataItem>(
     offset: 0,
   });
 
-  const paginatedQuery = props.queryHook(pagination.limit, pagination.offset);
+  const paginatedQuery = props.queryHook(
+    pagination.limit,
+    pagination.offset,
+    props.queryHookOptions
+  );
 
   const availablePages = useMemo(
     () => Math.ceil((paginatedQuery.data?.count ?? 0) / pagination.limit),

From 864e2446d7bd31287d092f06ab2567559c274ece Mon Sep 17 00:00:00 2001
From: Leszek Pietrzak <leszek@magicznyleszek.xyz>
Date: Wed, 13 Nov 2024 14:02:02 +0100
Subject: [PATCH 09/29] style(universalTable) use Array<T> for columns prop
 (#5260)

---
 .../universalTable/paginatedQueryUniversalTable.component.tsx   | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/jsapp/js/universalTable/paginatedQueryUniversalTable.component.tsx b/jsapp/js/universalTable/paginatedQueryUniversalTable.component.tsx
index 5c9b369699..efc145f22b 100644
--- a/jsapp/js/universalTable/paginatedQueryUniversalTable.component.tsx
+++ b/jsapp/js/universalTable/paginatedQueryUniversalTable.component.tsx
@@ -31,7 +31,7 @@ interface PaginatedQueryUniversalTableProps<DataItem> {
   // Below are props from `UniversalTable` that should come from the parent
   // component (these are kind of "configuration" props). The other
   // `UniversalTable` props are being handled here internally.
-  columns: UniversalTableColumn<DataItem>[];
+  columns: Array<UniversalTableColumn<DataItem>>;
 }
 
 const PAGE_SIZES = [10, 30, 50, 100];

From 1f1cc123141b0a004e6e1c066619d5783fb5b196 Mon Sep 17 00:00:00 2001
From: Leszek <smutnyleszek@gmail.com>
Date: Wed, 13 Nov 2024 14:31:24 +0100
Subject: [PATCH 10/29] small fixes (WIP)

---
 jsapp/js/account/organizations/MembersRoute.tsx | 11 ++++++++---
 jsapp/js/account/organizations/membersQuery.ts  | 10 ++++++----
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/jsapp/js/account/organizations/MembersRoute.tsx b/jsapp/js/account/organizations/MembersRoute.tsx
index 4ae08d18cf..ce979addba 100644
--- a/jsapp/js/account/organizations/MembersRoute.tsx
+++ b/jsapp/js/account/organizations/MembersRoute.tsx
@@ -50,8 +50,14 @@ export default function MembersRoute() {
             },
           },
           {
-            key: 'invite.status',
+            key: 'invite',
             label: t('Status'),
+            cellFormatter: (member: OrganizationMember) => {
+              if (member.invite?.status) {
+                return member.invite.status;
+              }
+              return null;
+            },
           },
           {
             key: 'date_joined',
@@ -68,9 +74,8 @@ export default function MembersRoute() {
             cellFormatter: (member: OrganizationMember) => {
               if (member.user__has_mfa_enabled) {
                 return 'yes';
-              } else {
-                return null;
               }
+              return null;
             },
           },
           {
diff --git a/jsapp/js/account/organizations/membersQuery.ts b/jsapp/js/account/organizations/membersQuery.ts
index 066e84329b..1cfc09d6ee 100644
--- a/jsapp/js/account/organizations/membersQuery.ts
+++ b/jsapp/js/account/organizations/membersQuery.ts
@@ -14,14 +14,16 @@ export interface OrganizationMember {
   /** `/api/v2/users/<username>/` */
   user: string;
   user__username: string;
-  user__email: string;
-  user__name: string;
+  /** can be empty an string in some edge cases */
+  user__email: string | '';
+  /** can be empty an string in some edge cases */
+  user__name: string | '';
   role: 'admin' | 'owner' | 'member' | 'external';
   user__has_mfa_enabled: boolean;
+  user__is_active: boolean;
   /** yyyy-mm-dd HH:MM:SS */
   date_joined: string;
-  user__is_active: boolean;
-  invite: {
+  invite?: {
     /** '/api/v2/organizations/<organization_uid>/invites/<invite_uid>/' */
     url: string;
     /** yyyy-mm-dd HH:MM:SS */

From 3c174df4d80e2a0e0edb9bcaee047fcbc4c7ca79 Mon Sep 17 00:00:00 2001
From: rajpatel24 <rajpatel24998@gmail.com>
Date: Thu, 14 Nov 2024 21:14:39 +0530
Subject: [PATCH 11/29] Add role-based validation tests for organization member
 permissions

---
 kobo/apps/organizations/permissions.py        |  31 ++++-
 kobo/apps/organizations/serializers.py        |   1 -
 .../tests/test_organization_members_api.py    | 118 ++++++++++++------
 kobo/apps/organizations/views.py              |  20 +--
 4 files changed, 114 insertions(+), 56 deletions(-)

diff --git a/kobo/apps/organizations/permissions.py b/kobo/apps/organizations/permissions.py
index 7564a57eda..a29a71f37a 100644
--- a/kobo/apps/organizations/permissions.py
+++ b/kobo/apps/organizations/permissions.py
@@ -41,4 +41,33 @@ def has_object_permission(self, request, view, obj):
             return True
 
         # Instance must have an attribute named `is_admin`
-        return obj.is_admin(request.user)
+        return obj.organization.is_admin(request.user)
+
+
+class IsOrgOwnerOrAdminOrMember(permissions.BasePermission):
+    def has_permission(self, request, view):
+        # Ensure the user is authenticated
+        if not request.user.is_authenticated:
+            return False
+        return True
+
+    def has_object_permission(self, request, view, obj):
+        user_role = obj.organization.get_user_role(request.user)
+
+        # Allow owners to view, update, and delete members
+        if user_role == 'owner':
+            return True
+
+        # Allow admins to view and update, but not delete members
+        if user_role == 'admin':
+            return request.method in permissions.SAFE_METHODS or request.method == 'PATCH'
+
+        # Allow members to only view other members
+        if user_role == 'member':
+            return request.method in permissions.SAFE_METHODS
+
+        # Deny access to external users
+        if user_role == 'external':
+            raise Http404()
+
+        return False
diff --git a/kobo/apps/organizations/serializers.py b/kobo/apps/organizations/serializers.py
index 714b4ced63..7232ba1669 100644
--- a/kobo/apps/organizations/serializers.py
+++ b/kobo/apps/organizations/serializers.py
@@ -1,4 +1,3 @@
-from constance import config
 from django.contrib.auth import get_user_model
 from django.utils.translation import gettext as t
 from rest_framework import serializers
diff --git a/kobo/apps/organizations/tests/test_organization_members_api.py b/kobo/apps/organizations/tests/test_organization_members_api.py
index d9cd447a2b..9766600f92 100644
--- a/kobo/apps/organizations/tests/test_organization_members_api.py
+++ b/kobo/apps/organizations/tests/test_organization_members_api.py
@@ -1,28 +1,25 @@
+from ddt import ddt, data, unpack
 from django.urls import reverse
-from model_bakery import baker
 from rest_framework import status
 
-from kobo.apps.kobo_auth.shortcuts import User
-from kobo.apps.organizations.models import Organization, OrganizationUser
-from kpi.tests.kpi_test_case import BaseTestCase
+from kobo.apps.organizations.tests.test_organizations_api import (
+    BaseOrganizationAssetApiTestCase
+)
 from kpi.urls.router_api_v2 import URL_NAMESPACE
 
-
-class OrganizationMemberAPITestCase(BaseTestCase):
+@ddt
+class OrganizationMemberAPITestCase(BaseOrganizationAssetApiTestCase):
     fixtures = ['test_data']
     URL_NAMESPACE = URL_NAMESPACE
 
     def setUp(self):
-        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')
+        super().setUp()
+        self.organization = self.someuser.organization
+        self.owner_user = self.someuser
+        self.member_user = self.alice
+        self.admin_user = self.anotheruser
+        self.external_user = self.bob
 
-        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(
             self._get_endpoint('organization-members-list'),
             kwargs={'organization_id': self.organization.id},
@@ -35,36 +32,77 @@ def setUp(self):
             },
         )
 
-    def test_list_members(self):
+    @data(
+        ('owner', status.HTTP_200_OK),
+        ('admin', status.HTTP_200_OK),
+        ('member', status.HTTP_200_OK),
+        ('external', status.HTTP_200_OK),
+    )
+    @unpack
+    def test_list_members_with_different_roles(self, user_role, expected_status):
+        user = getattr(self, f"{user_role}_user")
+        self.client.force_login(user)
         response = self.client.get(self.list_url)
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.assertIn(
-            'owner',
-            [member['user__username'] for member in response.data.get('results')]
-        )
-        self.assertIn(
-            'member',
-            [member['user__username'] for member in response.data.get('results')]
-        )
+        self.assertEqual(response.status_code, expected_status)
+
+    @data(
+        ('owner', status.HTTP_200_OK),
+        ('admin', status.HTTP_200_OK),
+        ('member', status.HTTP_200_OK),
+        ('external', status.HTTP_404_NOT_FOUND),
+    )
+    @unpack
+    def test_retrieve_member_details_with_different_roles(self, user_role, expected_status):
+        user = getattr(self, f"{user_role}_user")
+        self.client.force_login(user)
+        response = self.client.get(self.detail_url(self.member_user))
+        self.assertEqual(response.status_code, expected_status)
+
+    @data(
+        ('owner', status.HTTP_200_OK),
+        ('admin', status.HTTP_200_OK),
+        ('member', status.HTTP_403_FORBIDDEN),
+        ('external', status.HTTP_404_NOT_FOUND),
+    )
+    @unpack
+    def test_update_member_role_with_different_roles(self, user_role, expected_status):
+        user = getattr(self, f"{user_role}_user")
+        data = {'role': 'admin'}
+        self.client.force_login(user)
+        response = self.client.patch(self.detail_url(self.member_user), data)
+        self.assertEqual(response.status_code, expected_status)
 
-    def test_retrieve_member_details(self):
-        response = self.client.get(self.detail_url('member'))
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.assertEqual(response.data['user__username'], 'member')
-        self.assertEqual(response.data['role'], 'member')
+    @data(
+        ('owner', status.HTTP_204_NO_CONTENT),
+        ('admin', status.HTTP_403_FORBIDDEN),
+        ('member', status.HTTP_403_FORBIDDEN),
+        ('external', status.HTTP_404_NOT_FOUND),
+    )
+    @unpack
+    def test_delete_member_with_different_roles(self, user_role, expected_status):
+        user = getattr(self, f"{user_role}_user")
+        self.client.force_login(user)
+        response = self.client.delete(self.detail_url(self.member_user))
+        self.assertEqual(response.status_code, expected_status)
+        if expected_status == status.HTTP_204_NO_CONTENT:
+            # Confirm deletion
+            response = self.client.get(self.detail_url(self.member_user))
+            self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
 
-    def test_update_member_role(self):
+    @data(
+        ('owner', status.HTTP_405_METHOD_NOT_ALLOWED),
+        ('admin', status.HTTP_405_METHOD_NOT_ALLOWED),
+        ('member', status.HTTP_405_METHOD_NOT_ALLOWED),
+        ('external', status.HTTP_405_METHOD_NOT_ALLOWED),
+    )
+    @unpack
+    def test_post_request_is_not_allowed(self, user_role, expected_status):
+        user = getattr(self, f"{user_role}_user")
         data = {'role': 'admin'}
-        response = self.client.patch(self.detail_url('member'), data)
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.assertEqual(response.data['role'], 'admin')
+        self.client.force_login(user)
+        response = self.client.post(self.list_url, data)
+        self.assertEqual(response.status_code, expected_status)
 
-    def test_delete_member(self):
-        response = self.client.delete(self.detail_url('member'))
-        self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
-        # Confirm deletion
-        response = self.client.get(self.detail_url('member'))
-        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
 
     def test_list_requires_authentication(self):
         self.client.logout()
diff --git a/kobo/apps/organizations/views.py b/kobo/apps/organizations/views.py
index 6230a16d7e..eff0fc2f73 100644
--- a/kobo/apps/organizations/views.py
+++ b/kobo/apps/organizations/views.py
@@ -30,7 +30,11 @@
 from kpi.utils.object_permission import get_database_user
 from kpi.views.v2.asset import AssetViewSet
 from .models import Organization, OrganizationOwner, OrganizationUser
-from .permissions import IsOrgAdmin, IsOrgAdminOrReadOnly
+from .permissions import (
+    IsOrgAdmin,
+    IsOrgAdminOrReadOnly,
+    IsOrgOwnerOrAdminOrMember
+)
 from .serializers import OrganizationSerializer, OrganizationUserSerializer
 from ..accounts.mfa.models import MfaMethod
 from ..stripe.constants import ACTIVE_STRIPE_STATUSES
@@ -399,7 +403,7 @@ class OrganizationMemberViewSet(viewsets.ModelViewSet):
     in updates.
     """
     serializer_class = OrganizationUserSerializer
-    permission_classes = [IsAuthenticated]
+    permission_classes = [IsOrgOwnerOrAdminOrMember]
     pagination_class = OrganizationMemberPagination
     http_method_names = ['get', 'patch', 'delete']
     lookup_field = 'user__username'
@@ -432,15 +436,3 @@ def get_queryset(self):
             has_mfa_enabled=Exists(mfa_subquery)
         )
         return queryset
-
-    def partial_update(self, request, *args, **kwargs):
-        instance = self.get_object()
-        serializer = self.get_serializer(
-            instance, data=request.data, partial=True
-        )
-        serializer.is_valid(raise_exception=True)
-        role = serializer.validated_data.get('role')
-        if role:
-            instance.is_admin = (role == 'admin')
-            instance.save()
-        return super().partial_update(request, *args, **kwargs)

From d876fbced21f4e10e78f91dfb09e0fdf250ead73 Mon Sep 17 00:00:00 2001
From: rajpatel24 <rajpatel24998@gmail.com>
Date: Fri, 15 Nov 2024 01:48:24 +0530
Subject: [PATCH 12/29] Revert unintended change and fix linting issue

---
 kobo/apps/organizations/permissions.py           |  7 +++++--
 .../tests/test_organization_members_api.py       | 16 +++++++++-------
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/kobo/apps/organizations/permissions.py b/kobo/apps/organizations/permissions.py
index af7d2f7b56..dbb895ac0e 100644
--- a/kobo/apps/organizations/permissions.py
+++ b/kobo/apps/organizations/permissions.py
@@ -40,7 +40,7 @@ def has_object_permission(self, request, view, obj):
             return True
 
         # Instance must have an attribute named `is_admin`
-        return obj.organization.is_admin(request.user)
+        return obj.is_admin(request.user)
 
 
 class IsOrgOwnerOrAdminOrMember(permissions.BasePermission):
@@ -59,7 +59,10 @@ def has_object_permission(self, request, view, obj):
 
         # Allow admins to view and update, but not delete members
         if user_role == 'admin':
-            return request.method in permissions.SAFE_METHODS or request.method == 'PATCH'
+            return (
+                request.method in permissions.SAFE_METHODS or
+                request.method == 'PATCH'
+            )
 
         # Allow members to only view other members
         if user_role == 'member':
diff --git a/kobo/apps/organizations/tests/test_organization_members_api.py b/kobo/apps/organizations/tests/test_organization_members_api.py
index 9766600f92..877d2dcb95 100644
--- a/kobo/apps/organizations/tests/test_organization_members_api.py
+++ b/kobo/apps/organizations/tests/test_organization_members_api.py
@@ -7,6 +7,7 @@
 )
 from kpi.urls.router_api_v2 import URL_NAMESPACE
 
+
 @ddt
 class OrganizationMemberAPITestCase(BaseOrganizationAssetApiTestCase):
     fixtures = ['test_data']
@@ -40,7 +41,7 @@ def setUp(self):
     )
     @unpack
     def test_list_members_with_different_roles(self, user_role, expected_status):
-        user = getattr(self, f"{user_role}_user")
+        user = getattr(self, f'{user_role}_user')
         self.client.force_login(user)
         response = self.client.get(self.list_url)
         self.assertEqual(response.status_code, expected_status)
@@ -52,8 +53,10 @@ def test_list_members_with_different_roles(self, user_role, expected_status):
         ('external', status.HTTP_404_NOT_FOUND),
     )
     @unpack
-    def test_retrieve_member_details_with_different_roles(self, user_role, expected_status):
-        user = getattr(self, f"{user_role}_user")
+    def test_retrieve_member_details_with_different_roles(
+            self, user_role, expected_status
+    ):
+        user = getattr(self, f'{user_role}_user')
         self.client.force_login(user)
         response = self.client.get(self.detail_url(self.member_user))
         self.assertEqual(response.status_code, expected_status)
@@ -66,7 +69,7 @@ def test_retrieve_member_details_with_different_roles(self, user_role, expected_
     )
     @unpack
     def test_update_member_role_with_different_roles(self, user_role, expected_status):
-        user = getattr(self, f"{user_role}_user")
+        user = getattr(self, f'{user_role}_user')
         data = {'role': 'admin'}
         self.client.force_login(user)
         response = self.client.patch(self.detail_url(self.member_user), data)
@@ -80,7 +83,7 @@ def test_update_member_role_with_different_roles(self, user_role, expected_statu
     )
     @unpack
     def test_delete_member_with_different_roles(self, user_role, expected_status):
-        user = getattr(self, f"{user_role}_user")
+        user = getattr(self, f'{user_role}_user')
         self.client.force_login(user)
         response = self.client.delete(self.detail_url(self.member_user))
         self.assertEqual(response.status_code, expected_status)
@@ -97,13 +100,12 @@ def test_delete_member_with_different_roles(self, user_role, expected_status):
     )
     @unpack
     def test_post_request_is_not_allowed(self, user_role, expected_status):
-        user = getattr(self, f"{user_role}_user")
+        user = getattr(self, f'{user_role}_user')
         data = {'role': 'admin'}
         self.client.force_login(user)
         response = self.client.post(self.list_url, data)
         self.assertEqual(response.status_code, expected_status)
 
-
     def test_list_requires_authentication(self):
         self.client.logout()
         response = self.client.get(self.list_url)

From 953b7161d14fd73709a7651ca2969dd59f1f3a46 Mon Sep 17 00:00:00 2001
From: rajpatel24 <rajpatel24998@gmail.com>
Date: Sat, 16 Nov 2024 00:10:57 +0530
Subject: [PATCH 13/29] Refactor organization members API with updated
 permission logic and comprehensive tests

---
 kobo/apps/organizations/permissions.py        | 56 +++----------------
 kobo/apps/organizations/serializers.py        | 10 ++--
 .../tests/test_organization_members_api.py    | 47 ++++++++++------
 kobo/apps/organizations/views.py              | 21 +++----
 4 files changed, 56 insertions(+), 78 deletions(-)

diff --git a/kobo/apps/organizations/permissions.py b/kobo/apps/organizations/permissions.py
index dbb895ac0e..c5aa6ebbd6 100644
--- a/kobo/apps/organizations/permissions.py
+++ b/kobo/apps/organizations/permissions.py
@@ -1,14 +1,17 @@
 from django.http import Http404
 from rest_framework import permissions
+from rest_framework.permissions import IsAuthenticated
 
 from kobo.apps.organizations.constants import ORG_EXTERNAL_ROLE
+from kobo.apps.organizations.models import Organization
 from kpi.mixins.validation_password_permission import ValidationPasswordPermissionMixin
 from kpi.utils.object_permission import get_database_user
 
 
-class IsOrgAdmin(ValidationPasswordPermissionMixin, permissions.BasePermission):
+class IsOrgAdminPermission(ValidationPasswordPermissionMixin, IsAuthenticated):
     """
-    Object-level permission to only allow admin members of an object to access it.
+    Object-level permission to only allow admin (and owner) members of an object
+    to access it.
     Assumes the model instance has an `is_admin` attribute.
     """
 
@@ -26,50 +29,9 @@ def has_object_permission(self, request, view, obj):
         return obj.is_admin(user)
 
 
-class IsOrgAdminOrReadOnly(IsOrgAdmin):
-    """
-    Object-level permission to only allow admin members of an object to edit it.
-    Assumes the model instance has an `is_admin` attribute.
-    """
-
-    def has_object_permission(self, request, view, obj):
-
-        # Read permissions are allowed to any request,
-        # so we'll always allow GET, HEAD or OPTIONS requests.
-        if request.method in permissions.SAFE_METHODS:
-            return True
-
-        # Instance must have an attribute named `is_admin`
-        return obj.is_admin(request.user)
-
-
-class IsOrgOwnerOrAdminOrMember(permissions.BasePermission):
-    def has_permission(self, request, view):
-        # Ensure the user is authenticated
-        if not request.user.is_authenticated:
-            return False
-        return True
-
+class HasOrgRolePermission(IsOrgAdminPermission):
     def has_object_permission(self, request, view, obj):
-        user_role = obj.organization.get_user_role(request.user)
-
-        # Allow owners to view, update, and delete members
-        if user_role == 'owner':
+        obj = obj if isinstance(obj, Organization) else obj.organization
+        if super().has_object_permission(request, view, obj):
             return True
-
-        # Allow admins to view and update, but not delete members
-        if user_role == 'admin':
-            return (
-                request.method in permissions.SAFE_METHODS or
-                request.method == 'PATCH'
-            )
-
-        # Allow members to only view other members
-        if user_role == 'member':
-            return request.method in permissions.SAFE_METHODS
-
-        # Deny access to external users
-        if user_role == 'external':
-            raise Http404()
-
-        return False
+        return request.method in permissions.SAFE_METHODS
diff --git a/kobo/apps/organizations/serializers.py b/kobo/apps/organizations/serializers.py
index 7b7db8df07..df855b011a 100644
--- a/kobo/apps/organizations/serializers.py
+++ b/kobo/apps/organizations/serializers.py
@@ -47,11 +47,6 @@ class Meta:
             'user__is_active'
         ]
 
-    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')
         return reverse(
@@ -63,6 +58,11 @@ def get_url(self, obj):
             request=request
         )
 
+    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 validate_role(self, role):
         if role not in ['admin', 'member']:
             raise serializers.ValidationError(
diff --git a/kobo/apps/organizations/tests/test_organization_members_api.py b/kobo/apps/organizations/tests/test_organization_members_api.py
index 877d2dcb95..b885f17187 100644
--- a/kobo/apps/organizations/tests/test_organization_members_api.py
+++ b/kobo/apps/organizations/tests/test_organization_members_api.py
@@ -38,11 +38,15 @@ def setUp(self):
         ('admin', status.HTTP_200_OK),
         ('member', status.HTTP_200_OK),
         ('external', status.HTTP_200_OK),
+        ('anonymous', status.HTTP_401_UNAUTHORIZED),
     )
     @unpack
     def test_list_members_with_different_roles(self, user_role, expected_status):
-        user = getattr(self, f'{user_role}_user')
-        self.client.force_login(user)
+        if user_role == 'anonymous':
+            self.client.logout()
+        else:
+            user = getattr(self, f'{user_role}_user')
+            self.client.force_login(user)
         response = self.client.get(self.list_url)
         self.assertEqual(response.status_code, expected_status)
 
@@ -51,13 +55,17 @@ def test_list_members_with_different_roles(self, user_role, expected_status):
         ('admin', status.HTTP_200_OK),
         ('member', status.HTTP_200_OK),
         ('external', status.HTTP_404_NOT_FOUND),
+        ('anonymous', status.HTTP_401_UNAUTHORIZED),
     )
     @unpack
     def test_retrieve_member_details_with_different_roles(
             self, user_role, expected_status
     ):
-        user = getattr(self, f'{user_role}_user')
-        self.client.force_login(user)
+        if user_role == 'anonymous':
+            self.client.logout()
+        else:
+            user = getattr(self, f'{user_role}_user')
+            self.client.force_login(user)
         response = self.client.get(self.detail_url(self.member_user))
         self.assertEqual(response.status_code, expected_status)
 
@@ -66,25 +74,33 @@ def test_retrieve_member_details_with_different_roles(
         ('admin', status.HTTP_200_OK),
         ('member', status.HTTP_403_FORBIDDEN),
         ('external', status.HTTP_404_NOT_FOUND),
+        ('anonymous', status.HTTP_401_UNAUTHORIZED),
     )
     @unpack
     def test_update_member_role_with_different_roles(self, user_role, expected_status):
-        user = getattr(self, f'{user_role}_user')
+        if user_role == 'anonymous':
+            self.client.logout()
+        else:
+            user = getattr(self, f'{user_role}_user')
+            self.client.force_login(user)
         data = {'role': 'admin'}
-        self.client.force_login(user)
         response = self.client.patch(self.detail_url(self.member_user), data)
         self.assertEqual(response.status_code, expected_status)
 
     @data(
         ('owner', status.HTTP_204_NO_CONTENT),
-        ('admin', status.HTTP_403_FORBIDDEN),
+        ('admin', status.HTTP_204_NO_CONTENT),
         ('member', status.HTTP_403_FORBIDDEN),
         ('external', status.HTTP_404_NOT_FOUND),
+        ('anonymous', status.HTTP_401_UNAUTHORIZED),
     )
     @unpack
     def test_delete_member_with_different_roles(self, user_role, expected_status):
-        user = getattr(self, f'{user_role}_user')
-        self.client.force_login(user)
+        if user_role == 'anonymous':
+            self.client.logout()
+        else:
+            user = getattr(self, f'{user_role}_user')
+            self.client.force_login(user)
         response = self.client.delete(self.detail_url(self.member_user))
         self.assertEqual(response.status_code, expected_status)
         if expected_status == status.HTTP_204_NO_CONTENT:
@@ -97,16 +113,15 @@ def test_delete_member_with_different_roles(self, user_role, expected_status):
         ('admin', status.HTTP_405_METHOD_NOT_ALLOWED),
         ('member', status.HTTP_405_METHOD_NOT_ALLOWED),
         ('external', status.HTTP_405_METHOD_NOT_ALLOWED),
+        ('anonymous', status.HTTP_401_UNAUTHORIZED),
     )
     @unpack
     def test_post_request_is_not_allowed(self, user_role, expected_status):
-        user = getattr(self, f'{user_role}_user')
+        if user_role == 'anonymous':
+            self.client.logout()
+        else:
+            user = getattr(self, f'{user_role}_user')
+            self.client.force_login(user)
         data = {'role': 'admin'}
-        self.client.force_login(user)
         response = self.client.post(self.list_url, data)
         self.assertEqual(response.status_code, expected_status)
-
-    def test_list_requires_authentication(self):
-        self.client.logout()
-        response = self.client.get(self.list_url)
-        self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED)
diff --git a/kobo/apps/organizations/views.py b/kobo/apps/organizations/views.py
index 65a9dc3426..8fd6147158 100644
--- a/kobo/apps/organizations/views.py
+++ b/kobo/apps/organizations/views.py
@@ -22,7 +22,7 @@
 from kpi.constants import ASSET_TYPE_SURVEY
 from kpi.filters import AssetOrderingFilter, SearchFilter
 from kpi.models.asset import Asset
-from kpi.paginators import AssetUsagePagination, OrganizationMemberPagination
+from kpi.paginators import AssetUsagePagination, OrganizationMembersPagination
 from kpi.permissions import IsAuthenticated
 from kpi.serializers.v2.service_usage import (
     CustomAssetUsageSerializer,
@@ -31,11 +31,7 @@
 from kpi.utils.object_permission import get_database_user
 from kpi.views.v2.asset import AssetViewSet
 from .models import Organization, OrganizationOwner, OrganizationUser
-from .permissions import (
-    IsOrgAdmin,
-    IsOrgAdminOrReadOnly,
-    IsOrgOwnerOrAdminOrMember
-)
+from .permissions import HasOrgRolePermission
 from .serializers import OrganizationSerializer, OrganizationUserSerializer
 from ..accounts.mfa.models import MfaMethod
 from ..stripe.constants import ACTIVE_STRIPE_STATUSES
@@ -91,10 +87,12 @@ class OrganizationViewSet(viewsets.ModelViewSet):
     queryset = Organization.objects.all()
     serializer_class = OrganizationSerializer
     lookup_field = 'id'
-    permission_classes = (IsAuthenticated, IsOrgAdminOrReadOnly)
+    permission_classes = [HasOrgRolePermission]
     pagination_class = AssetUsagePagination
 
-    @action(detail=True, methods=['GET'], permission_classes=[IsOrgAdmin])
+    @action(
+        detail=True, methods=['GET'], permission_classes=[HasOrgRolePermission]
+    )
     def assets(self, request: Request, *args, **kwargs):
         """
         ### Retrieve Organization Assets
@@ -279,6 +277,9 @@ def asset_usage(self, request, pk=None, *args, **kwargs):
 
 class OrganizationMemberViewSet(viewsets.ModelViewSet):
     """
+    Used `ModelViewSet` instead of `NestedViewSetMixin` to maintain explicit
+    control over the queryset.
+
     * Manage organization members and their roles within an organization.
     * Run a partial update on an organization member to promote or demote.
 
@@ -408,8 +409,8 @@ class OrganizationMemberViewSet(viewsets.ModelViewSet):
     in updates.
     """
     serializer_class = OrganizationUserSerializer
-    permission_classes = [IsOrgOwnerOrAdminOrMember]
-    pagination_class = OrganizationMemberPagination
+    permission_classes = [HasOrgRolePermission]
+    pagination_class = OrganizationMembersPagination
     http_method_names = ['get', 'patch', 'delete']
     lookup_field = 'user__username'
 

From f8cb95beb18301974fcfe68671644638436ac371 Mon Sep 17 00:00:00 2001
From: rajpatel24 <rajpatel24998@gmail.com>
Date: Sat, 16 Nov 2024 00:10:57 +0530
Subject: [PATCH 14/29] Refactor organization members API with updated
 permission logic and comprehensive tests

---
 kobo/apps/organizations/permissions.py        | 56 +++----------------
 kobo/apps/organizations/serializers.py        | 10 ++--
 .../tests/test_organization_members_api.py    | 47 ++++++++++------
 kobo/apps/organizations/views.py              | 22 ++++----
 kpi/paginators.py                             | 12 ++--
 5 files changed, 62 insertions(+), 85 deletions(-)

diff --git a/kobo/apps/organizations/permissions.py b/kobo/apps/organizations/permissions.py
index dbb895ac0e..c5aa6ebbd6 100644
--- a/kobo/apps/organizations/permissions.py
+++ b/kobo/apps/organizations/permissions.py
@@ -1,14 +1,17 @@
 from django.http import Http404
 from rest_framework import permissions
+from rest_framework.permissions import IsAuthenticated
 
 from kobo.apps.organizations.constants import ORG_EXTERNAL_ROLE
+from kobo.apps.organizations.models import Organization
 from kpi.mixins.validation_password_permission import ValidationPasswordPermissionMixin
 from kpi.utils.object_permission import get_database_user
 
 
-class IsOrgAdmin(ValidationPasswordPermissionMixin, permissions.BasePermission):
+class IsOrgAdminPermission(ValidationPasswordPermissionMixin, IsAuthenticated):
     """
-    Object-level permission to only allow admin members of an object to access it.
+    Object-level permission to only allow admin (and owner) members of an object
+    to access it.
     Assumes the model instance has an `is_admin` attribute.
     """
 
@@ -26,50 +29,9 @@ def has_object_permission(self, request, view, obj):
         return obj.is_admin(user)
 
 
-class IsOrgAdminOrReadOnly(IsOrgAdmin):
-    """
-    Object-level permission to only allow admin members of an object to edit it.
-    Assumes the model instance has an `is_admin` attribute.
-    """
-
-    def has_object_permission(self, request, view, obj):
-
-        # Read permissions are allowed to any request,
-        # so we'll always allow GET, HEAD or OPTIONS requests.
-        if request.method in permissions.SAFE_METHODS:
-            return True
-
-        # Instance must have an attribute named `is_admin`
-        return obj.is_admin(request.user)
-
-
-class IsOrgOwnerOrAdminOrMember(permissions.BasePermission):
-    def has_permission(self, request, view):
-        # Ensure the user is authenticated
-        if not request.user.is_authenticated:
-            return False
-        return True
-
+class HasOrgRolePermission(IsOrgAdminPermission):
     def has_object_permission(self, request, view, obj):
-        user_role = obj.organization.get_user_role(request.user)
-
-        # Allow owners to view, update, and delete members
-        if user_role == 'owner':
+        obj = obj if isinstance(obj, Organization) else obj.organization
+        if super().has_object_permission(request, view, obj):
             return True
-
-        # Allow admins to view and update, but not delete members
-        if user_role == 'admin':
-            return (
-                request.method in permissions.SAFE_METHODS or
-                request.method == 'PATCH'
-            )
-
-        # Allow members to only view other members
-        if user_role == 'member':
-            return request.method in permissions.SAFE_METHODS
-
-        # Deny access to external users
-        if user_role == 'external':
-            raise Http404()
-
-        return False
+        return request.method in permissions.SAFE_METHODS
diff --git a/kobo/apps/organizations/serializers.py b/kobo/apps/organizations/serializers.py
index 7b7db8df07..df855b011a 100644
--- a/kobo/apps/organizations/serializers.py
+++ b/kobo/apps/organizations/serializers.py
@@ -47,11 +47,6 @@ class Meta:
             'user__is_active'
         ]
 
-    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')
         return reverse(
@@ -63,6 +58,11 @@ def get_url(self, obj):
             request=request
         )
 
+    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 validate_role(self, role):
         if role not in ['admin', 'member']:
             raise serializers.ValidationError(
diff --git a/kobo/apps/organizations/tests/test_organization_members_api.py b/kobo/apps/organizations/tests/test_organization_members_api.py
index 877d2dcb95..b885f17187 100644
--- a/kobo/apps/organizations/tests/test_organization_members_api.py
+++ b/kobo/apps/organizations/tests/test_organization_members_api.py
@@ -38,11 +38,15 @@ def setUp(self):
         ('admin', status.HTTP_200_OK),
         ('member', status.HTTP_200_OK),
         ('external', status.HTTP_200_OK),
+        ('anonymous', status.HTTP_401_UNAUTHORIZED),
     )
     @unpack
     def test_list_members_with_different_roles(self, user_role, expected_status):
-        user = getattr(self, f'{user_role}_user')
-        self.client.force_login(user)
+        if user_role == 'anonymous':
+            self.client.logout()
+        else:
+            user = getattr(self, f'{user_role}_user')
+            self.client.force_login(user)
         response = self.client.get(self.list_url)
         self.assertEqual(response.status_code, expected_status)
 
@@ -51,13 +55,17 @@ def test_list_members_with_different_roles(self, user_role, expected_status):
         ('admin', status.HTTP_200_OK),
         ('member', status.HTTP_200_OK),
         ('external', status.HTTP_404_NOT_FOUND),
+        ('anonymous', status.HTTP_401_UNAUTHORIZED),
     )
     @unpack
     def test_retrieve_member_details_with_different_roles(
             self, user_role, expected_status
     ):
-        user = getattr(self, f'{user_role}_user')
-        self.client.force_login(user)
+        if user_role == 'anonymous':
+            self.client.logout()
+        else:
+            user = getattr(self, f'{user_role}_user')
+            self.client.force_login(user)
         response = self.client.get(self.detail_url(self.member_user))
         self.assertEqual(response.status_code, expected_status)
 
@@ -66,25 +74,33 @@ def test_retrieve_member_details_with_different_roles(
         ('admin', status.HTTP_200_OK),
         ('member', status.HTTP_403_FORBIDDEN),
         ('external', status.HTTP_404_NOT_FOUND),
+        ('anonymous', status.HTTP_401_UNAUTHORIZED),
     )
     @unpack
     def test_update_member_role_with_different_roles(self, user_role, expected_status):
-        user = getattr(self, f'{user_role}_user')
+        if user_role == 'anonymous':
+            self.client.logout()
+        else:
+            user = getattr(self, f'{user_role}_user')
+            self.client.force_login(user)
         data = {'role': 'admin'}
-        self.client.force_login(user)
         response = self.client.patch(self.detail_url(self.member_user), data)
         self.assertEqual(response.status_code, expected_status)
 
     @data(
         ('owner', status.HTTP_204_NO_CONTENT),
-        ('admin', status.HTTP_403_FORBIDDEN),
+        ('admin', status.HTTP_204_NO_CONTENT),
         ('member', status.HTTP_403_FORBIDDEN),
         ('external', status.HTTP_404_NOT_FOUND),
+        ('anonymous', status.HTTP_401_UNAUTHORIZED),
     )
     @unpack
     def test_delete_member_with_different_roles(self, user_role, expected_status):
-        user = getattr(self, f'{user_role}_user')
-        self.client.force_login(user)
+        if user_role == 'anonymous':
+            self.client.logout()
+        else:
+            user = getattr(self, f'{user_role}_user')
+            self.client.force_login(user)
         response = self.client.delete(self.detail_url(self.member_user))
         self.assertEqual(response.status_code, expected_status)
         if expected_status == status.HTTP_204_NO_CONTENT:
@@ -97,16 +113,15 @@ def test_delete_member_with_different_roles(self, user_role, expected_status):
         ('admin', status.HTTP_405_METHOD_NOT_ALLOWED),
         ('member', status.HTTP_405_METHOD_NOT_ALLOWED),
         ('external', status.HTTP_405_METHOD_NOT_ALLOWED),
+        ('anonymous', status.HTTP_401_UNAUTHORIZED),
     )
     @unpack
     def test_post_request_is_not_allowed(self, user_role, expected_status):
-        user = getattr(self, f'{user_role}_user')
+        if user_role == 'anonymous':
+            self.client.logout()
+        else:
+            user = getattr(self, f'{user_role}_user')
+            self.client.force_login(user)
         data = {'role': 'admin'}
-        self.client.force_login(user)
         response = self.client.post(self.list_url, data)
         self.assertEqual(response.status_code, expected_status)
-
-    def test_list_requires_authentication(self):
-        self.client.logout()
-        response = self.client.get(self.list_url)
-        self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED)
diff --git a/kobo/apps/organizations/views.py b/kobo/apps/organizations/views.py
index 65a9dc3426..c601086a69 100644
--- a/kobo/apps/organizations/views.py
+++ b/kobo/apps/organizations/views.py
@@ -22,8 +22,7 @@
 from kpi.constants import ASSET_TYPE_SURVEY
 from kpi.filters import AssetOrderingFilter, SearchFilter
 from kpi.models.asset import Asset
-from kpi.paginators import AssetUsagePagination, OrganizationMemberPagination
-from kpi.permissions import IsAuthenticated
+from kpi.paginators import AssetUsagePagination, OrganizationMembersPagination
 from kpi.serializers.v2.service_usage import (
     CustomAssetUsageSerializer,
     ServiceUsageSerializer,
@@ -31,11 +30,7 @@
 from kpi.utils.object_permission import get_database_user
 from kpi.views.v2.asset import AssetViewSet
 from .models import Organization, OrganizationOwner, OrganizationUser
-from .permissions import (
-    IsOrgAdmin,
-    IsOrgAdminOrReadOnly,
-    IsOrgOwnerOrAdminOrMember
-)
+from .permissions import HasOrgRolePermission
 from .serializers import OrganizationSerializer, OrganizationUserSerializer
 from ..accounts.mfa.models import MfaMethod
 from ..stripe.constants import ACTIVE_STRIPE_STATUSES
@@ -91,10 +86,12 @@ class OrganizationViewSet(viewsets.ModelViewSet):
     queryset = Organization.objects.all()
     serializer_class = OrganizationSerializer
     lookup_field = 'id'
-    permission_classes = (IsAuthenticated, IsOrgAdminOrReadOnly)
+    permission_classes = [HasOrgRolePermission]
     pagination_class = AssetUsagePagination
 
-    @action(detail=True, methods=['GET'], permission_classes=[IsOrgAdmin])
+    @action(
+        detail=True, methods=['GET'], permission_classes=[HasOrgRolePermission]
+    )
     def assets(self, request: Request, *args, **kwargs):
         """
         ### Retrieve Organization Assets
@@ -279,6 +276,9 @@ def asset_usage(self, request, pk=None, *args, **kwargs):
 
 class OrganizationMemberViewSet(viewsets.ModelViewSet):
     """
+    Used `ModelViewSet` instead of `NestedViewSetMixin` to maintain explicit
+    control over the queryset.
+
     * Manage organization members and their roles within an organization.
     * Run a partial update on an organization member to promote or demote.
 
@@ -408,8 +408,8 @@ class OrganizationMemberViewSet(viewsets.ModelViewSet):
     in updates.
     """
     serializer_class = OrganizationUserSerializer
-    permission_classes = [IsOrgOwnerOrAdminOrMember]
-    pagination_class = OrganizationMemberPagination
+    permission_classes = [HasOrgRolePermission]
+    pagination_class = OrganizationMembersPagination
     http_method_names = ['get', 'patch', 'delete']
     lookup_field = 'user__username'
 
diff --git a/kpi/paginators.py b/kpi/paginators.py
index 34a7fe9ee5..7aec997833 100644
--- a/kpi/paginators.py
+++ b/kpi/paginators.py
@@ -137,15 +137,15 @@ def get_count(self, queryset):
         return super().get_count(queryset)
 
 
-class TinyPaginated(PageNumberPagination):
+class OrganizationMembersPagination(PageNumberPagination):
     """
-    Same as Paginated with a small page size
+    Pagination class for Organization Members
     """
-    page_size = 50
+    page_size_query_param = 'page_size'
 
 
-class OrganizationMemberPagination(PageNumberPagination):
+class TinyPaginated(PageNumberPagination):
     """
-    Pagination class for Organization
+    Same as Paginated with a small page size
     """
-    page_size = 10
+    page_size = 50

From 906be3da208599811d25761624594acbf7ac98ee Mon Sep 17 00:00:00 2001
From: rajpatel24 <rajpatel24998@gmail.com>
Date: Sat, 16 Nov 2024 01:01:10 +0530
Subject: [PATCH 15/29] Fix failing tests

---
 kobo/apps/organizations/tests/test_organizations_api.py | 2 +-
 kobo/apps/organizations/views.py                        | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kobo/apps/organizations/tests/test_organizations_api.py b/kobo/apps/organizations/tests/test_organizations_api.py
index c97ebef66e..b1e467d5e9 100644
--- a/kobo/apps/organizations/tests/test_organizations_api.py
+++ b/kobo/apps/organizations/tests/test_organizations_api.py
@@ -349,7 +349,7 @@ def test_can_list(self, username, expected_status_code):
     def test_list_not_found_as_anonymous(self):
         self.client.logout()
         response = self.client.get(self.org_assets_list_url)
-        assert response.status_code == status.HTTP_404_NOT_FOUND
+        assert response.status_code == status.HTTP_401_UNAUTHORIZED
 
     def test_list_only_organization_assets(self):
         # The organization's assets endpoint only returns assets where the `owner`
diff --git a/kobo/apps/organizations/views.py b/kobo/apps/organizations/views.py
index c601086a69..364bf71d56 100644
--- a/kobo/apps/organizations/views.py
+++ b/kobo/apps/organizations/views.py
@@ -30,7 +30,7 @@
 from kpi.utils.object_permission import get_database_user
 from kpi.views.v2.asset import AssetViewSet
 from .models import Organization, OrganizationOwner, OrganizationUser
-from .permissions import HasOrgRolePermission
+from .permissions import HasOrgRolePermission, IsOrgAdminPermission
 from .serializers import OrganizationSerializer, OrganizationUserSerializer
 from ..accounts.mfa.models import MfaMethod
 from ..stripe.constants import ACTIVE_STRIPE_STATUSES
@@ -90,7 +90,7 @@ class OrganizationViewSet(viewsets.ModelViewSet):
     pagination_class = AssetUsagePagination
 
     @action(
-        detail=True, methods=['GET'], permission_classes=[HasOrgRolePermission]
+        detail=True, methods=['GET'], permission_classes=[IsOrgAdminPermission]
     )
     def assets(self, request: Request, *args, **kwargs):
         """

From 837c1ac757ed5a4db56ec37c8dcf2fb8a0cb56e2 Mon Sep 17 00:00:00 2001
From: Leszek <smutnyleszek@gmail.com>
Date: Mon, 18 Nov 2024 20:27:33 -0500
Subject: [PATCH 16/29] adjust MembersRoute table column sizes

---
 jsapp/js/account/organizations/MembersRoute.tsx      | 10 ++++++++++
 jsapp/js/universalTable/universalTable.component.tsx |  2 +-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/jsapp/js/account/organizations/MembersRoute.tsx b/jsapp/js/account/organizations/MembersRoute.tsx
index ce979addba..85eb36f581 100644
--- a/jsapp/js/account/organizations/MembersRoute.tsx
+++ b/jsapp/js/account/organizations/MembersRoute.tsx
@@ -4,6 +4,7 @@ import React from 'react';
 // Partial components
 import PaginatedQueryUniversalTable from 'js/universalTable/paginatedQueryUniversalTable.component';
 import LoadingSpinner from 'js/components/common/loadingSpinner';
+import Avatar from 'js/components/common/avatar';
 
 // Stores, hooks and utilities
 import {formatTime} from 'js/utils';
@@ -42,8 +43,12 @@ export default function MembersRoute() {
               // TODO
               return (
                 <>
+                  <Avatar size='s' username={member.user__username} isUsernameVisible/>
+                  &nbsp;
                   {member.user__name}
+                  &nbsp;
                   @{member.user__username}
+                  <br/>
                   {member.user__email}
                 </>
               );
@@ -52,6 +57,7 @@ export default function MembersRoute() {
           {
             key: 'invite',
             label: t('Status'),
+            size: 120,
             cellFormatter: (member: OrganizationMember) => {
               if (member.invite?.status) {
                 return member.invite.status;
@@ -62,15 +68,18 @@ export default function MembersRoute() {
           {
             key: 'date_joined',
             label: t('Date added'),
+            size: 130,
             cellFormatter: (member: OrganizationMember) => formatTime(member.date_joined),
           },
           {
             key: 'role',
             label: t('Role'),
+            size: 120,
           },
           {
             key: 'user__has_mfa_enabled',
             label: t('2FA'),
+            size: 90,
             cellFormatter: (member: OrganizationMember) => {
               if (member.user__has_mfa_enabled) {
                 return 'yes';
@@ -83,6 +92,7 @@ export default function MembersRoute() {
             // element
             key: 'url',
             label: '',
+            size: 64,
             cellFormatter: () => {
               return 'TBD';
             },
diff --git a/jsapp/js/universalTable/universalTable.component.tsx b/jsapp/js/universalTable/universalTable.component.tsx
index eaa29bacb2..8b3dc480da 100644
--- a/jsapp/js/universalTable/universalTable.component.tsx
+++ b/jsapp/js/universalTable/universalTable.component.tsx
@@ -86,7 +86,7 @@ interface UniversalTableProps<DataItem> {
 
 const DEFAULT_COLUMN_SIZE = {
   size: 200, // starting column size
-  minSize: 100, // enforced during column resizing
+  minSize: 60, // enforced during column resizing
   maxSize: 600, // enforced during column resizing
 };
 

From 08bc12ed677fa5687729101eda0af3c2b58917fa Mon Sep 17 00:00:00 2001
From: Leszek <smutnyleszek@gmail.com>
Date: Mon, 18 Nov 2024 20:52:54 -0500
Subject: [PATCH 17/29] display badges in MembersRoute table and remove some
 WIP code

---
 .../js/account/organizations/MembersRoute.tsx | 33 ++++++++-----------
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/jsapp/js/account/organizations/MembersRoute.tsx b/jsapp/js/account/organizations/MembersRoute.tsx
index 85eb36f581..bf62685b9d 100644
--- a/jsapp/js/account/organizations/MembersRoute.tsx
+++ b/jsapp/js/account/organizations/MembersRoute.tsx
@@ -5,6 +5,7 @@ import React from 'react';
 import PaginatedQueryUniversalTable from 'js/universalTable/paginatedQueryUniversalTable.component';
 import LoadingSpinner from 'js/components/common/loadingSpinner';
 import Avatar from 'js/components/common/avatar';
+import Badge from 'jsapp/js/components/common/badge';
 
 // Stores, hooks and utilities
 import {formatTime} from 'js/utils';
@@ -39,20 +40,11 @@ export default function MembersRoute() {
           {
             key: 'user__username',
             label: t('Name'),
-            cellFormatter: (member: OrganizationMember) => {
-              // TODO
-              return (
-                <>
-                  <Avatar size='s' username={member.user__username} isUsernameVisible/>
-                  &nbsp;
-                  {member.user__name}
-                  &nbsp;
-                  @{member.user__username}
-                  <br/>
-                  {member.user__email}
-                </>
-              );
-            },
+            cellFormatter: (member: OrganizationMember) => (
+              // TODO: when https://github.com/kobotoolbox/kpi/pull/5268 is merged
+              // update this to also display full name and email.
+              <Avatar size='s' username={member.user__username} isUsernameVisible/>
+            ),
           },
           {
             key: 'invite',
@@ -61,6 +53,8 @@ export default function MembersRoute() {
             cellFormatter: (member: OrganizationMember) => {
               if (member.invite?.status) {
                 return member.invite.status;
+              } else {
+                return <Badge color='light-green' size='s' label={t('Active')} />
               }
               return null;
             },
@@ -68,7 +62,7 @@ export default function MembersRoute() {
           {
             key: 'date_joined',
             label: t('Date added'),
-            size: 130,
+            size: 140,
             cellFormatter: (member: OrganizationMember) => formatTime(member.date_joined),
           },
           {
@@ -82,9 +76,9 @@ export default function MembersRoute() {
             size: 90,
             cellFormatter: (member: OrganizationMember) => {
               if (member.user__has_mfa_enabled) {
-                return 'yes';
+                return <Badge size='s' color='light-blue' icon='check' />;
               }
-              return null;
+              return <Badge size='s' color='light-storm' icon='minus' />;
             },
           },
           {
@@ -93,9 +87,8 @@ export default function MembersRoute() {
             key: 'url',
             label: '',
             size: 64,
-            cellFormatter: () => {
-              return 'TBD';
-            },
+            // TODO: this will be added soon
+            cellFormatter: () => (' '),
           },
         ]}
       />

From 01c189c49d47c16919e697e018446df5d901c3e3 Mon Sep 17 00:00:00 2001
From: rajpatel24 <rajpatel24998@gmail.com>
Date: Tue, 19 Nov 2024 17:09:35 +0530
Subject: [PATCH 18/29] Update delete logic to remove user from user table
 along with organization user table and enhance test coverage

---
 .../tests/test_organization_members_api.py    |  4 +
 kobo/apps/organizations/views.py              | 75 ++++++++++++-------
 2 files changed, 53 insertions(+), 26 deletions(-)

diff --git a/kobo/apps/organizations/tests/test_organization_members_api.py b/kobo/apps/organizations/tests/test_organization_members_api.py
index b885f17187..a86c494341 100644
--- a/kobo/apps/organizations/tests/test_organization_members_api.py
+++ b/kobo/apps/organizations/tests/test_organization_members_api.py
@@ -2,6 +2,7 @@
 from django.urls import reverse
 from rest_framework import status
 
+from kobo.apps.kobo_auth.shortcuts import User
 from kobo.apps.organizations.tests.test_organizations_api import (
     BaseOrganizationAssetApiTestCase
 )
@@ -107,6 +108,9 @@ def test_delete_member_with_different_roles(self, user_role, expected_status):
             # Confirm deletion
             response = self.client.get(self.detail_url(self.member_user))
             self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+            self.assertFalse(
+                User.objects.filter(username=f'{user_role}_user').exists()
+            )
 
     @data(
         ('owner', status.HTTP_405_METHOD_NOT_ALLOWED),
diff --git a/kobo/apps/organizations/views.py b/kobo/apps/organizations/views.py
index 364bf71d56..343682f39a 100644
--- a/kobo/apps/organizations/views.py
+++ b/kobo/apps/organizations/views.py
@@ -276,18 +276,16 @@ def asset_usage(self, request, pk=None, *args, **kwargs):
 
 class OrganizationMemberViewSet(viewsets.ModelViewSet):
     """
-    Used `ModelViewSet` instead of `NestedViewSetMixin` to maintain explicit
-    control over the queryset.
-
-    * Manage organization members and their roles within an organization.
-    * Run a partial update on an organization member to promote or demote.
+    The API uses `ModelViewSet` instead of `NestedViewSetMixin` to maintain
+    explicit control over the queryset.
 
     ## Organization Members API
 
-    This API allows authorized users to view and manage the members of an
-    organization, including their roles. It handles existing members. It also
-    allows updating roles, such as promoting a member to an admin or assigning
-    a new owner.
+    This API allows authorized users to view and manage organization members and
+    their roles, including promoting or demoting members (eg. to admin).
+
+    * Manage members and their roles within an organization.
+    * Update member roles (promote/demote).
 
     ### List Members
 
@@ -316,9 +314,9 @@ class OrganizationMemberViewSet(viewsets.ModelViewSet):
     >                   "user__email": "foo_bar@example.com",
     >                   "user__name": "Foo Bar",
     >                   "role": "owner",
-    >                   "has_mfa_enabled": true,
+    >                   "user__has_mfa_enabled": true,
     >                   "date_joined": "2024-08-11T12:36:32Z",
-    >                   "is_active": true
+    >                   "user__is_active": true
     >               },
     >               {
     >                   "url": "http://[kpi]/api/v2/organizations/org_12345/ \
@@ -328,15 +326,13 @@ class OrganizationMemberViewSet(viewsets.ModelViewSet):
     >                   "user__email": "john_doe@example.com",
     >                   "user__name": "John Doe",
     >                   "role": "admin",
-    >                   "has_mfa_enabled": false,
+    >                   "user__has_mfa_enabled": false,
     >                   "date_joined": "2024-10-21T06:38:45Z",
-    >                   "is_active": true
+    >                   "user__is_active": true
     >               }
     >           ]
     >       }
 
-    The response includes detailed information about each member, such as their
-    username, email, role (owner, admin, member), and account status.
 
     ### Retrieve Member Details
 
@@ -359,36 +355,51 @@ class OrganizationMemberViewSet(viewsets.ModelViewSet):
     >           "user__email": "foo_bar@example.com",
     >           "user__name": "Foo Bar",
     >           "role": "owner",
-    >           "has_mfa_enabled": true,
+    >           "user__has_mfa_enabled": true,
     >           "date_joined": "2024-08-11T12:36:32Z",
-    >           "is_active": true
+    >           "user__is_active": true
     >       }
 
     ### Update Member Role
 
-    Updates the role of a member within the organization to `owner`, `admin`, or
+    Updates the role of a member within the organization to `admin` or
      `member`.
 
+    - **admin**: Grants the member admin privileges within the organization
+    - **member**: Revokes admin privileges, setting the member as a regular user
+
     <pre class="prettyprint">
     <b>PATCH</b> /api/v2/organizations/{organization_id}/members/{username}/
     </pre>
 
-    #### Payload
+    > Example
+    >
+    >       curl -X PATCH https://[kpi]/api/v2/organizations/org_12345/members/foo_bar/
+
+    > Payload
+
     >       {
     >           "role": "admin"
     >       }
 
-    - **admin**: Grants the member admin privileges within the organization
-    - **member**: Revokes admin privileges, setting the member as a regular user
+    > Response 200
+
+    >       {
+    >           "url": "http://[kpi]/api/v2/organizations/org_12345/members/foo_bar/",
+    >           "user": "http://[kpi]/api/v2/users/foo_bar/",
+    >           "user__username": "foo_bar",
+    >           "user__email": "foo_bar@example.com",
+    >           "user__name": "Foo Bar",
+    >           "role": "admin",
+    >           "user__has_mfa_enabled": true,
+    >           "date_joined": "2024-08-11T12:36:32Z",
+    >           "user__is_active": true
+    >       }
 
-    > Example
-    >
-    >       curl -X PATCH https://[kpi]/api/v2/organizations/org_12345/ \
-    >       members/demo_user/ -d '{"role": "admin"}'
 
     ### Remove Member
 
-    Removes a member from the organization.
+    Delete an organization member and their associated user account.
 
     <pre class="prettyprint">
     <b>DELETE</b> /api/v2/organizations/{organization_id}/members/{username}/
@@ -401,6 +412,8 @@ class OrganizationMemberViewSet(viewsets.ModelViewSet):
     ## Permissions
 
     - The user must be authenticated to perform these actions.
+    - Owners and admins can manage members and roles.
+    - Members can view the list but cannot update roles or delete members.
 
     ## Notes
 
@@ -441,3 +454,13 @@ def get_queryset(self):
             has_mfa_enabled=Exists(mfa_subquery)
         )
         return queryset
+
+    def destroy(self, request, *args, **kwargs):
+        """
+        Delete an organization member and their associated user account
+        """
+        instance = self.get_object()
+        user = instance.user
+        instance.delete()
+        user.delete()
+        return Response(status=status.HTTP_204_NO_CONTENT)

From b5ccac797950411c37b7c7a5554e8fe9ae047075 Mon Sep 17 00:00:00 2001
From: Leszek <smutnyleszek@gmail.com>
Date: Tue, 19 Nov 2024 10:10:21 -0500
Subject: [PATCH 19/29] add some comments

---
 jsapp/js/account/organizations/membersQuery.ts | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/jsapp/js/account/organizations/membersQuery.ts b/jsapp/js/account/organizations/membersQuery.ts
index 1cfc09d6ee..f3aca0eed2 100644
--- a/jsapp/js/account/organizations/membersQuery.ts
+++ b/jsapp/js/account/organizations/membersQuery.ts
@@ -34,6 +34,12 @@ export interface OrganizationMember {
   };
 }
 
+/**
+ * Fetches paginated list of members for given organization. Requires
+ * `options.organizationId` to work.
+ * This is mainly needed for `useOrganizationMembersQuery`, so you most probably
+ * would use it through that hook rather than directly.
+ */
 async function getOrganizationMembers(
   limit: number,
   offset: number,
@@ -57,6 +63,10 @@ async function getOrganizationMembers(
   );
 }
 
+/**
+ * A hook that gives you paginated list of organization members. Requires
+ * `options.organizationId` to work.
+ */
 export default function useOrganizationMembersQuery(
   itemLimit: number,
   pageOffset: number,

From 3ea9420b7572effc98086989d0c34e447555d842 Mon Sep 17 00:00:00 2001
From: Leszek <smutnyleszek@gmail.com>
Date: Tue, 19 Nov 2024 10:50:45 -0500
Subject: [PATCH 20/29] add missing semicolon

---
 jsapp/js/account/organizations/MembersRoute.tsx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/jsapp/js/account/organizations/MembersRoute.tsx b/jsapp/js/account/organizations/MembersRoute.tsx
index bf62685b9d..7f9a69dcc9 100644
--- a/jsapp/js/account/organizations/MembersRoute.tsx
+++ b/jsapp/js/account/organizations/MembersRoute.tsx
@@ -54,7 +54,7 @@ export default function MembersRoute() {
               if (member.invite?.status) {
                 return member.invite.status;
               } else {
-                return <Badge color='light-green' size='s' label={t('Active')} />
+                return <Badge color='light-green' size='s' label={t('Active')} />;
               }
               return null;
             },

From ac809995ec2c83dc1bd8a53c9f5bef9a2521a216 Mon Sep 17 00:00:00 2001
From: Leszek <smutnyleszek@gmail.com>
Date: Tue, 19 Nov 2024 14:21:59 -0500
Subject: [PATCH 21/29] use latest Avatar version in the table

---
 jsapp/js/account/organizations/MembersRoute.tsx | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/jsapp/js/account/organizations/MembersRoute.tsx b/jsapp/js/account/organizations/MembersRoute.tsx
index 7f9a69dcc9..f35c025524 100644
--- a/jsapp/js/account/organizations/MembersRoute.tsx
+++ b/jsapp/js/account/organizations/MembersRoute.tsx
@@ -41,10 +41,16 @@ export default function MembersRoute() {
             key: 'user__username',
             label: t('Name'),
             cellFormatter: (member: OrganizationMember) => (
-              // TODO: when https://github.com/kobotoolbox/kpi/pull/5268 is merged
-              // update this to also display full name and email.
-              <Avatar size='s' username={member.user__username} isUsernameVisible/>
+              <Avatar
+                size='m'
+                username={member.user__username}
+                isUsernameVisible
+                email={member.user__email}
+                // We pass `undefined` for the case it's an empty string
+                fullName={member.user__name || undefined}
+              />
             ),
+            size: 360,
           },
           {
             key: 'invite',

From e5688a34e70cc562af504a2c874cd1c755ae9b64 Mon Sep 17 00:00:00 2001
From: Leszek <smutnyleszek@gmail.com>
Date: Tue, 19 Nov 2024 14:54:04 -0500
Subject: [PATCH 22/29] add functions for updating and removing organization
 member

---
 .../js/account/organizations/membersQuery.ts  | 43 ++++++++++++++++++-
 jsapp/js/api.endpoints.ts                     |  1 +
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/jsapp/js/account/organizations/membersQuery.ts b/jsapp/js/account/organizations/membersQuery.ts
index f3aca0eed2..3587c378ef 100644
--- a/jsapp/js/account/organizations/membersQuery.ts
+++ b/jsapp/js/account/organizations/membersQuery.ts
@@ -1,10 +1,21 @@
 import {keepPreviousData, useQuery} from '@tanstack/react-query';
 import {endpoints} from 'js/api.endpoints';
 import type {PaginatedResponse} from 'js/dataInterface';
-import {fetchGet} from 'js/api';
+import {fetchGet, fetchPatch, fetchDelete} from 'js/api';
 import {QueryKeys} from 'js/query/queryKeys';
 import type {PaginatedQueryHookOptions} from 'js/universalTable/paginatedQueryUniversalTable.component';
 
+/**
+ * Note that it's only possible to update the role via API to either `admin` or
+ * `member`.
+ */
+export enum OrganizationMemberRole {
+  admin = 'admin',
+  member = 'member',
+  owner = 'owner',
+  external = 'external',
+}
+
 export interface OrganizationMember {
   /**
    * The url to the member within the organization
@@ -18,7 +29,7 @@ export interface OrganizationMember {
   user__email: string | '';
   /** can be empty an string in some edge cases */
   user__name: string | '';
-  role: 'admin' | 'owner' | 'member' | 'external';
+  role: OrganizationMemberRole;
   user__has_mfa_enabled: boolean;
   user__is_active: boolean;
   /** yyyy-mm-dd HH:MM:SS */
@@ -34,6 +45,34 @@ export interface OrganizationMember {
   };
 }
 
+/**
+ * For updating member within given organization. Accepts partial properties
+ * of `OrganizationMember`.
+ */
+export async function patchOrganizationMember(
+  organizationId: string,
+  username: string,
+  newMemberData: Partial<OrganizationMember>
+) {
+  const apiUrl = endpoints.ORGANIZATION_MEMBER_URL
+    .replace(':organization_id', organizationId)
+    .replace(':username', username);
+  return fetchPatch<OrganizationMember>(apiUrl, newMemberData);
+}
+
+/**
+ * For removing member from given organization.
+ */
+export async function removeOrganizationMember(
+  organizationId: string,
+  username: string
+) {
+  const apiUrl = endpoints.ORGANIZATION_MEMBER_URL
+    .replace(':organization_id', organizationId)
+    .replace(':username', username);
+  return fetchDelete(apiUrl);
+}
+
 /**
  * Fetches paginated list of members for given organization. Requires
  * `options.organizationId` to work.
diff --git a/jsapp/js/api.endpoints.ts b/jsapp/js/api.endpoints.ts
index 65a26c28fa..0f2ae2e412 100644
--- a/jsapp/js/api.endpoints.ts
+++ b/jsapp/js/api.endpoints.ts
@@ -7,6 +7,7 @@ export const endpoints = {
   ADD_ONS_URL: '/api/v2/stripe/addons/',
   ORGANIZATION_URL: '/api/v2/organizations/',
   ORGANIZATION_MEMBERS_URL: '/api/v2/organizations/:organization_id/members/',
+  ORGANIZATION_MEMBER_URL: '/api/v2/organizations/:organization_id/members/:username/',
   /** Expected parameters: price_id and organization_id **/
   CHECKOUT_URL: '/api/v2/stripe/checkout-link',
   /** Expected parameter: organization_id  **/

From 76721434cc5f22cce1d57c301bceb857d81aebc4 Mon Sep 17 00:00:00 2001
From: James Kiger <james.kiger@gmail.com>
Date: Tue, 19 Nov 2024 17:28:09 -0500
Subject: [PATCH 23/29] Use useOrganizationQuery directly in
 useOrganizationMembersQuery

---
 .../js/account/organizations/membersQuery.ts  | 24 ++++++++-----------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/jsapp/js/account/organizations/membersQuery.ts b/jsapp/js/account/organizations/membersQuery.ts
index f3aca0eed2..d00a6c23d8 100644
--- a/jsapp/js/account/organizations/membersQuery.ts
+++ b/jsapp/js/account/organizations/membersQuery.ts
@@ -3,7 +3,7 @@ import {endpoints} from 'js/api.endpoints';
 import type {PaginatedResponse} from 'js/dataInterface';
 import {fetchGet} from 'js/api';
 import {QueryKeys} from 'js/query/queryKeys';
-import type {PaginatedQueryHookOptions} from 'js/universalTable/paginatedQueryUniversalTable.component';
+import {useOrganizationQuery} from '../stripe.api';
 
 export interface OrganizationMember {
   /**
@@ -43,7 +43,7 @@ export interface OrganizationMember {
 async function getOrganizationMembers(
   limit: number,
   offset: number,
-  options?: PaginatedQueryHookOptions
+  orgId: string
 ) {
   const params = new URLSearchParams({
     limit: limit.toString(),
@@ -51,9 +51,7 @@ async function getOrganizationMembers(
   });
 
   let apiUrl = endpoints.ORGANIZATION_MEMBERS_URL;
-  if (options?.organizationId) {
-    apiUrl = apiUrl.replace(':organization_id', options.organizationId);
-  }
+  apiUrl = apiUrl.replace(':organization_id', orgId);
 
   return fetchGet<PaginatedResponse<OrganizationMember>>(
     apiUrl + '?' + params,
@@ -69,18 +67,16 @@ async function getOrganizationMembers(
  */
 export default function useOrganizationMembersQuery(
   itemLimit: number,
-  pageOffset: number,
-  /**
-   * This is optional only to satisfy TS in `PaginatedQueryUniversalTable`. In
-   * reality, data will not be fetched properly without `organizationId` passed
-   * in the `options` object - see `getOrganizationMembers()` for details.
-   */
-  options?: PaginatedQueryHookOptions
+  pageOffset: number
 ) {
+  const orgQuery = useOrganizationQuery();
+  const orgId = orgQuery.data?.id;
+
   return useQuery({
-    queryKey: [QueryKeys.organizationMembers, itemLimit, pageOffset, options],
-    queryFn: () => getOrganizationMembers(itemLimit, pageOffset, options),
+    queryKey: [QueryKeys.organizationMembers, itemLimit, pageOffset, orgId],
+    queryFn: () => getOrganizationMembers(itemLimit, pageOffset, orgId!),
     placeholderData: keepPreviousData,
+    enabled: !!orgId,
     // We might want to improve this in future, for now let's not retry
     retry: false,
     // The `refetchOnWindowFocus` option is `true` by default, I'm setting it

From c20c0f4e4301b6691767924d532b39237e14fa3e Mon Sep 17 00:00:00 2001
From: Leszek <smutnyleszek@gmail.com>
Date: Tue, 19 Nov 2024 17:30:37 -0500
Subject: [PATCH 24/29] remove queryHookOptions from
 PaginatedQueryUniversalTable

---
 .../js/account/organizations/MembersRoute.tsx |  1 -
 ...paginatedQueryUniversalTable.component.tsx | 21 ++-----------------
 2 files changed, 2 insertions(+), 20 deletions(-)

diff --git a/jsapp/js/account/organizations/MembersRoute.tsx b/jsapp/js/account/organizations/MembersRoute.tsx
index f35c025524..05542f509a 100644
--- a/jsapp/js/account/organizations/MembersRoute.tsx
+++ b/jsapp/js/account/organizations/MembersRoute.tsx
@@ -35,7 +35,6 @@ export default function MembersRoute() {
 
       <PaginatedQueryUniversalTable<OrganizationMember>
         queryHook={useOrganizationMembersQuery}
-        queryHookOptions={{organizationId: orgQuery.data.id}}
         columns={[
           {
             key: 'user__username',
diff --git a/jsapp/js/universalTable/paginatedQueryUniversalTable.component.tsx b/jsapp/js/universalTable/paginatedQueryUniversalTable.component.tsx
index efc145f22b..e53245b3d5 100644
--- a/jsapp/js/universalTable/paginatedQueryUniversalTable.component.tsx
+++ b/jsapp/js/universalTable/paginatedQueryUniversalTable.component.tsx
@@ -9,25 +9,12 @@ import type {UseQueryResult} from '@tanstack/react-query';
 import type {PaginatedResponse} from 'js/dataInterface';
 import type {UniversalTableColumn} from './universalTable.component';
 
-/**
- * A way of passing custom options, e.g. a parameter that needs to be included
- * in the API url.
- */
-export interface PaginatedQueryHookOptions {
-  [optionName: string]: string;
-}
-
 interface PaginatedQueryHook<DataItem> extends Function {
-  (
-    limit: number,
-    offset: number,
-    options?: PaginatedQueryHookOptions
-  ): UseQueryResult<PaginatedResponse<DataItem>>;
+  (limit: number, offset: number): UseQueryResult<PaginatedResponse<DataItem>>;
 }
 
 interface PaginatedQueryUniversalTableProps<DataItem> {
   queryHook: PaginatedQueryHook<DataItem>;
-  queryHookOptions?: PaginatedQueryHookOptions;
   // Below are props from `UniversalTable` that should come from the parent
   // component (these are kind of "configuration" props). The other
   // `UniversalTable` props are being handled here internally.
@@ -52,11 +39,7 @@ export default function PaginatedQueryUniversalTable<DataItem>(
     offset: 0,
   });
 
-  const paginatedQuery = props.queryHook(
-    pagination.limit,
-    pagination.offset,
-    props.queryHookOptions
-  );
+  const paginatedQuery = props.queryHook(pagination.limit, pagination.offset);
 
   const availablePages = useMemo(
     () => Math.ceil((paginatedQuery.data?.count ?? 0) / pagination.limit),

From f250c27545f6f1f7f8cf2ef5361432fa3bb0b8ce Mon Sep 17 00:00:00 2001
From: Leszek <smutnyleszek@gmail.com>
Date: Wed, 20 Nov 2024 08:52:46 -0500
Subject: [PATCH 25/29] add helpful comment

---
 jsapp/js/account/organizations/membersQuery.ts | 1 +
 1 file changed, 1 insertion(+)

diff --git a/jsapp/js/account/organizations/membersQuery.ts b/jsapp/js/account/organizations/membersQuery.ts
index d00a6c23d8..3832b4d310 100644
--- a/jsapp/js/account/organizations/membersQuery.ts
+++ b/jsapp/js/account/organizations/membersQuery.ts
@@ -74,6 +74,7 @@ export default function useOrganizationMembersQuery(
 
   return useQuery({
     queryKey: [QueryKeys.organizationMembers, itemLimit, pageOffset, orgId],
+    // `orgId!` because it's ensured to be there in `enabled` property :ok:
     queryFn: () => getOrganizationMembers(itemLimit, pageOffset, orgId!),
     placeholderData: keepPreviousData,
     enabled: !!orgId,

From 4f0f55a93ae90ea0c1e8b0a48ccb9b71ad7c073b Mon Sep 17 00:00:00 2001
From: Leszek <smutnyleszek@gmail.com>
Date: Wed, 20 Nov 2024 10:12:32 -0500
Subject: [PATCH 26/29] update comments

---
 jsapp/js/account/organizations/membersQuery.ts | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/jsapp/js/account/organizations/membersQuery.ts b/jsapp/js/account/organizations/membersQuery.ts
index 3832b4d310..43808329ff 100644
--- a/jsapp/js/account/organizations/membersQuery.ts
+++ b/jsapp/js/account/organizations/membersQuery.ts
@@ -35,8 +35,7 @@ export interface OrganizationMember {
 }
 
 /**
- * Fetches paginated list of members for given organization. Requires
- * `options.organizationId` to work.
+ * Fetches paginated list of members for given organization.
  * This is mainly needed for `useOrganizationMembersQuery`, so you most probably
  * would use it through that hook rather than directly.
  */
@@ -62,8 +61,8 @@ async function getOrganizationMembers(
 }
 
 /**
- * A hook that gives you paginated list of organization members. Requires
- * `options.organizationId` to work.
+ * A hook that gives you paginated list of organization members. Uses
+ * `useOrganizationQuery` to get the id.
  */
 export default function useOrganizationMembersQuery(
   itemLimit: number,

From def1a7ae4ee4c1685b8ed6e42f9753df5ddd801b Mon Sep 17 00:00:00 2001
From: rajpatel24 <rajpatel24998@gmail.com>
Date: Thu, 21 Nov 2024 00:44:01 +0530
Subject: [PATCH 27/29] Refactor permissions to block external users from
 listing organization members

---
 kobo/apps/organizations/permissions.py              | 13 +++++++++++++
 .../tests/test_organization_members_api.py          |  6 +++---
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/kobo/apps/organizations/permissions.py b/kobo/apps/organizations/permissions.py
index c5aa6ebbd6..f37dc36757 100644
--- a/kobo/apps/organizations/permissions.py
+++ b/kobo/apps/organizations/permissions.py
@@ -30,6 +30,19 @@ def has_object_permission(self, request, view, obj):
 
 
 class HasOrgRolePermission(IsOrgAdminPermission):
+    def has_permission(self, request, view):
+        if not super().has_permission(request, view):
+            return False
+
+        organization = Organization.objects.filter(
+            id=view.kwargs.get('organization_id')
+        ).first()
+        if organization and not self.has_object_permission(
+            request, view, organization
+        ):
+            return False
+        return True
+
     def has_object_permission(self, request, view, obj):
         obj = obj if isinstance(obj, Organization) else obj.organization
         if super().has_object_permission(request, view, obj):
diff --git a/kobo/apps/organizations/tests/test_organization_members_api.py b/kobo/apps/organizations/tests/test_organization_members_api.py
index a86c494341..eb9e74b33b 100644
--- a/kobo/apps/organizations/tests/test_organization_members_api.py
+++ b/kobo/apps/organizations/tests/test_organization_members_api.py
@@ -38,7 +38,7 @@ def setUp(self):
         ('owner', status.HTTP_200_OK),
         ('admin', status.HTTP_200_OK),
         ('member', status.HTTP_200_OK),
-        ('external', status.HTTP_200_OK),
+        ('external', status.HTTP_404_NOT_FOUND),
         ('anonymous', status.HTTP_401_UNAUTHORIZED),
     )
     @unpack
@@ -115,8 +115,8 @@ def test_delete_member_with_different_roles(self, user_role, expected_status):
     @data(
         ('owner', status.HTTP_405_METHOD_NOT_ALLOWED),
         ('admin', status.HTTP_405_METHOD_NOT_ALLOWED),
-        ('member', status.HTTP_405_METHOD_NOT_ALLOWED),
-        ('external', status.HTTP_405_METHOD_NOT_ALLOWED),
+        ('member', status.HTTP_403_FORBIDDEN),
+        ('external', status.HTTP_404_NOT_FOUND),
         ('anonymous', status.HTTP_401_UNAUTHORIZED),
     )
     @unpack

From 40ca814419e0f92c3e8cdd9faa04bf8b7867a1f8 Mon Sep 17 00:00:00 2001
From: Leszek <smutnyleszek@gmail.com>
Date: Wed, 20 Nov 2024 17:14:42 -0500
Subject: [PATCH 28/29] use OrganizationUserRole in membersQuery

---
 jsapp/js/account/organization/membersQuery.ts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/jsapp/js/account/organization/membersQuery.ts b/jsapp/js/account/organization/membersQuery.ts
index a7e096f5ef..8a2e399c13 100644
--- a/jsapp/js/account/organization/membersQuery.ts
+++ b/jsapp/js/account/organization/membersQuery.ts
@@ -3,7 +3,7 @@ import {endpoints} from 'js/api.endpoints';
 import type {PaginatedResponse} from 'js/dataInterface';
 import {fetchGet} from 'js/api';
 import {QueryKeys} from 'js/query/queryKeys';
-import {useOrganizationQuery} from './organizationQuery';
+import {useOrganizationQuery, type OrganizationUserRole} from './organizationQuery';
 
 export interface OrganizationMember {
   /**
@@ -18,7 +18,7 @@ export interface OrganizationMember {
   user__email: string | '';
   /** can be empty an string in some edge cases */
   user__name: string | '';
-  role: 'admin' | 'owner' | 'member' | 'external';
+  role: OrganizationUserRole;
   user__has_mfa_enabled: boolean;
   user__is_active: boolean;
   /** yyyy-mm-dd HH:MM:SS */

From 3e9048e0c825c0ec0e6066ea224f871b5fb8d590 Mon Sep 17 00:00:00 2001
From: Leszek <smutnyleszek@gmail.com>
Date: Mon, 25 Nov 2024 17:49:08 +0100
Subject: [PATCH 29/29] post merge conflict fixes

---
 kpi/paginators.py | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/kpi/paginators.py b/kpi/paginators.py
index 7aec997833..29823ea1a4 100644
--- a/kpi/paginators.py
+++ b/kpi/paginators.py
@@ -137,13 +137,6 @@ def get_count(self, queryset):
         return super().get_count(queryset)
 
 
-class OrganizationMembersPagination(PageNumberPagination):
-    """
-    Pagination class for Organization Members
-    """
-    page_size_query_param = 'page_size'
-
-
 class TinyPaginated(PageNumberPagination):
     """
     Same as Paginated with a small page size