From 32edb666692eb40b3a98fd794f0c591a333cd302 Mon Sep 17 00:00:00 2001 From: Ajay Nair Date: Mon, 13 Jul 2026 21:37:51 -0400 Subject: [PATCH 01/10] feat: Added organization API layer --- .../tests/test_organization.py | 500 ++++++++++++++++++ contentcuration/contentcuration/urls.py | 3 + .../contentcuration/viewsets/organization.py | 351 ++++++++++++ 3 files changed, 854 insertions(+) create mode 100644 contentcuration/contentcuration/tests/test_organization.py create mode 100644 contentcuration/contentcuration/viewsets/organization.py diff --git a/contentcuration/contentcuration/tests/test_organization.py b/contentcuration/contentcuration/tests/test_organization.py new file mode 100644 index 0000000000..ae2f3869fc --- /dev/null +++ b/contentcuration/contentcuration/tests/test_organization.py @@ -0,0 +1,500 @@ +""" +Tests for Organization API endpoints. +""" +import json + +from django.urls import reverse +from rest_framework import status +from rest_framework.test import APITestCase, APIClient + +from contentcuration.constants.organization_roles import ( + ORGANIZATION_ADMIN, + ORGANIZATION_EDITOR, + ORGANIZATION_VIEWER, + ORGANIZATION_ROLE_STATUS_ACTIVE, +) +from contentcuration.models import Organization, OrganizationRole, User +from contentcuration.tests.base import BaseAPITestCase +from contentcuration.tests import testdata + + +class OrganizationAPITestCase(BaseAPITestCase): + """Base test case for Organization API tests.""" + + def setUp(self): + super().setUp() + # Create additional test users + self.admin_user = testdata.user(email="admin@test.com") + self.editor_user = testdata.user(email="editor@test.com") + self.viewer_user = testdata.user(email="viewer@test.com") + self.other_user = testdata.user(email="other@test.com") + + # Create test organization + self.organization = Organization.objects.create( + name="Test Organization", + description="A test organization", + public=False, + ) + + # Add admin user to organization + OrganizationRole.objects.create( + user=self.admin_user, + organization=self.organization, + role=ORGANIZATION_ADMIN, + status=ORGANIZATION_ROLE_STATUS_ACTIVE, + ) + + # Add editor user to organization + OrganizationRole.objects.create( + user=self.editor_user, + organization=self.organization, + role=ORGANIZATION_EDITOR, + status=ORGANIZATION_ROLE_STATUS_ACTIVE, + ) + + # Add viewer user to organization + OrganizationRole.objects.create( + user=self.viewer_user, + organization=self.organization, + role=ORGANIZATION_VIEWER, + status=ORGANIZATION_ROLE_STATUS_ACTIVE, + ) + + def authenticate_as(self, user): + """Switch authentication to a different user.""" + self.client = APIClient() + self.client.force_authenticate(user) + + +class OrganizationListCreateTestCase(OrganizationAPITestCase): + """Tests for creating and listing organizations.""" + + def test_list_organizations_user_can_see_their_organizations(self): + """Authenticated users can list organizations they belong to.""" + self.client.force_authenticate(self.admin_user) + response = self.client.get("/api/organization/") + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(len(response.data["results"]), 1) + self.assertEqual(response.data["results"][0]["name"], "Test Organization") + + def test_list_organizations_user_cannot_see_orgs_they_dont_belong_to(self): + """Users should not see organizations they are not members of.""" + self.client.force_authenticate(self.other_user) + response = self.client.get("/api/organization/") + + self.assertEqual(response.status_code, status.HTTP_200_OK) + # other_user should not see the organization + self.assertEqual(len(response.data["results"]), 0) + + def test_create_organization_creates_user_as_admin(self): + """Creating an organization should make the creator an admin.""" + self.client.force_authenticate(self.other_user) + data = { + "name": "New Organization", + "description": "A new organization", + "public": False, + } + response = self.client.post("/api/organization/", data, format="json") + + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertEqual(response.data["name"], "New Organization") + + # Verify creator is admin + org = Organization.objects.get(id=response.data["id"]) + role = org.user_roles.get(user=self.other_user) + self.assertEqual(role.role, ORGANIZATION_ADMIN) + + def test_create_organization_requires_authentication(self): + """Creating an organization requires authentication.""" + client = APIClient() + data = { + "name": "New Organization", + "description": "A new organization", + "public": False, + } + response = client.post("/api/organization/", data, format="json") + + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + def test_list_organizations_requires_authentication(self): + """Listing organizations requires authentication.""" + client = APIClient() + response = client.get("/api/organization/") + + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + + +class OrganizationRetrieveUpdateDeleteTestCase(OrganizationAPITestCase): + """Tests for retrieving, updating, and deleting organizations.""" + + def test_retrieve_organization_member_can_access(self): + """Organization members can retrieve organization details.""" + self.client.force_authenticate(self.admin_user) + response = self.client.get(f"/api/organization/{self.organization.id}/") + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["name"], "Test Organization") + + def test_retrieve_organization_non_member_cannot_access(self): + """Non-members cannot retrieve organization details.""" + self.client.force_authenticate(self.other_user) + response = self.client.get(f"/api/organization/{self.organization.id}/") + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + def test_update_organization_admin_can_update(self): + """Organization admins can update organization details.""" + self.client.force_authenticate(self.admin_user) + data = {"name": "Updated Organization", "description": "Updated description"} + response = self.client.patch(f"/api/organization/{self.organization.id}/", data, format="json") + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.organization.refresh_from_db() + self.assertEqual(self.organization.name, "Updated Organization") + + def test_update_organization_editor_cannot_update(self): + """Organization editors cannot update organization settings.""" + self.client.force_authenticate(self.editor_user) + data = {"name": "Updated Organization"} + response = self.client.patch(f"/api/organization/{self.organization.id}/", data, format="json") + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + def test_update_organization_viewer_cannot_update(self): + """Organization viewers cannot update organization settings.""" + self.client.force_authenticate(self.viewer_user) + data = {"name": "Updated Organization"} + response = self.client.patch(f"/api/organization/{self.organization.id}/", data, format="json") + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + def test_delete_organization_admin_can_delete(self): + """Organization admins can delete organizations (soft delete).""" + self.client.force_authenticate(self.admin_user) + response = self.client.delete(f"/api/organization/{self.organization.id}/") + + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + self.organization.refresh_from_db() + self.assertTrue(self.organization.deleted) + + def test_delete_organization_editor_cannot_delete(self): + """Organization editors cannot delete organizations.""" + self.client.force_authenticate(self.editor_user) + response = self.client.delete(f"/api/organization/{self.organization.id}/") + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + +class OrganizationMemberListTestCase(OrganizationAPITestCase): + """Tests for listing organization members.""" + + def test_list_members_member_can_view(self): + """Organization members can view the member list.""" + self.client.force_authenticate(self.admin_user) + response = self.client.get(f"/api/organization/{self.organization.id}/members/") + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(len(response.data["results"]), 3) + + def test_list_members_non_member_cannot_view(self): + """Non-members cannot view the member list.""" + self.client.force_authenticate(self.other_user) + response = self.client.get(f"/api/organization/{self.organization.id}/members/") + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_list_members_returns_user_details(self): + """Member list includes user email and name.""" + self.client.force_authenticate(self.admin_user) + response = self.client.get(f"/api/organization/{self.organization.id}/members/") + + self.assertEqual(response.status_code, status.HTTP_200_OK) + members = response.data["results"] + + # Check that user details are included + admin_member = next((m for m in members if m["user"] == str(self.admin_user.id)), None) + self.assertIsNotNone(admin_member) + self.assertEqual(admin_member["user_email"], self.admin_user.email) + + +class OrganizationAddMemberTestCase(OrganizationAPITestCase): + """Tests for adding members to organization.""" + + def test_add_member_admin_can_add(self): + """Organization admins can add new members.""" + self.client.force_authenticate(self.admin_user) + data = { + "user_id": str(self.other_user.id), + "role": ORGANIZATION_VIEWER, + } + response = self.client.post(f"/api/organization/{self.organization.id}/add_member/", data, format="json") + + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertEqual(response.data["role"], ORGANIZATION_VIEWER) + + # Verify member was added + role = OrganizationRole.objects.get(user=self.other_user, organization=self.organization) + self.assertEqual(role.role, ORGANIZATION_VIEWER) + + def test_add_member_editor_cannot_add(self): + """Organization editors cannot add members.""" + self.client.force_authenticate(self.editor_user) + data = { + "user_id": str(self.other_user.id), + "role": ORGANIZATION_VIEWER, + } + response = self.client.post(f"/api/organization/{self.organization.id}/add_member/", data, format="json") + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_add_member_requires_user_id(self): + """Adding a member requires a user_id.""" + self.client.force_authenticate(self.admin_user) + data = {"role": ORGANIZATION_VIEWER} + response = self.client.post(f"/api/organization/{self.organization.id}/add_member/", data, format="json") + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + def test_add_member_defaults_to_viewer_role(self): + """If no role is specified, default is VIEWER.""" + self.client.force_authenticate(self.admin_user) + data = {"user_id": str(self.other_user.id)} + response = self.client.post(f"/api/organization/{self.organization.id}/add_member/", data, format="json") + + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + self.assertEqual(response.data["role"], ORGANIZATION_VIEWER) + + def test_add_member_invalid_role_rejected(self): + """Adding a member with an invalid role is rejected.""" + self.client.force_authenticate(self.admin_user) + data = { + "user_id": str(self.other_user.id), + "role": "invalid_role", + } + response = self.client.post(f"/api/organization/{self.organization.id}/add_member/", data, format="json") + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + def test_add_nonexistent_user_fails(self): + """Adding a non-existent user fails.""" + self.client.force_authenticate(self.admin_user) + data = { + "user_id": "00000000-0000-0000-0000-000000000000", + "role": ORGANIZATION_VIEWER, + } + response = self.client.post(f"/api/organization/{self.organization.id}/add_member/", data, format="json") + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + def test_update_existing_member_role(self): + """Adding a member that already exists updates their role.""" + # other_user is already a viewer + OrganizationRole.objects.create( + user=self.other_user, + organization=self.organization, + role=ORGANIZATION_VIEWER, + status=ORGANIZATION_ROLE_STATUS_ACTIVE, + ) + + self.client.force_authenticate(self.admin_user) + data = { + "user_id": str(self.other_user.id), + "role": ORGANIZATION_EDITOR, + } + response = self.client.post(f"/api/organization/{self.organization.id}/add_member/", data, format="json") + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["role"], ORGANIZATION_EDITOR) + + +class OrganizationUpdateMemberTestCase(OrganizationAPITestCase): + """Tests for updating member roles.""" + + def test_update_member_admin_can_update_role(self): + """Organization admins can update member roles.""" + membership = OrganizationRole.objects.get(user=self.viewer_user, organization=self.organization) + + self.client.force_authenticate(self.admin_user) + data = {"role": ORGANIZATION_EDITOR} + response = self.client.patch( + f"/api/organization/{self.organization.id}/update_member/?member_id={membership.id}", + data, + format="json" + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + membership.refresh_from_db() + self.assertEqual(membership.role, ORGANIZATION_EDITOR) + + def test_update_member_editor_cannot_update(self): + """Organization editors cannot update member roles.""" + membership = OrganizationRole.objects.get(user=self.viewer_user, organization=self.organization) + + self.client.force_authenticate(self.editor_user) + data = {"role": ORGANIZATION_ADMIN} + response = self.client.patch( + f"/api/organization/{self.organization.id}/update_member/?member_id={membership.id}", + data, + format="json" + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_update_member_requires_member_id(self): + """Updating a member requires member_id parameter.""" + self.client.force_authenticate(self.admin_user) + data = {"role": ORGANIZATION_EDITOR} + response = self.client.patch( + f"/api/organization/{self.organization.id}/update_member/", + data, + format="json" + ) + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + def test_update_member_invalid_member_id_fails(self): + """Updating with an invalid member_id fails.""" + self.client.force_authenticate(self.admin_user) + data = {"role": ORGANIZATION_EDITOR} + response = self.client.patch( + f"/api/organization/{self.organization.id}/update_member/?member_id=00000000-0000-0000-0000-000000000000", + data, + format="json" + ) + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + +class OrganizationRemoveMemberTestCase(OrganizationAPITestCase): + """Tests for removing members from organization.""" + + def test_remove_member_admin_can_remove(self): + """Organization admins can remove members.""" + membership = OrganizationRole.objects.get(user=self.viewer_user, organization=self.organization) + + self.client.force_authenticate(self.admin_user) + response = self.client.delete( + f"/api/organization/{self.organization.id}/update_member/?member_id={membership.id}" + ) + + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + self.assertFalse( + OrganizationRole.objects.filter( + user=self.viewer_user, organization=self.organization + ).exists() + ) + + def test_remove_member_editor_cannot_remove(self): + """Organization editors cannot remove members.""" + membership = OrganizationRole.objects.get(user=self.viewer_user, organization=self.organization) + + self.client.force_authenticate(self.editor_user) + response = self.client.delete( + f"/api/organization/{self.organization.id}/update_member/?member_id={membership.id}" + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_remove_member_non_member_cannot_remove(self): + """Non-members cannot remove members.""" + membership = OrganizationRole.objects.get(user=self.viewer_user, organization=self.organization) + + self.client.force_authenticate(self.other_user) + response = self.client.delete( + f"/api/organization/{self.organization.id}/update_member/?member_id={membership.id}" + ) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + +class OrganizationPermissionEnforcementTestCase(OrganizationAPITestCase): + """Tests for permission enforcement across different roles.""" + + def test_admin_can_manage_settings_members_and_roles(self): + """Admins have full management access.""" + self.client.force_authenticate(self.admin_user) + + # Can update organization + data = {"name": "Updated"} + response = self.client.patch(f"/api/organization/{self.organization.id}/", data, format="json") + self.assertEqual(response.status_code, status.HTTP_200_OK) + + # Can add members + data = {"user_id": str(self.other_user.id), "role": ORGANIZATION_VIEWER} + response = self.client.post(f"/api/organization/{self.organization.id}/add_member/", data, format="json") + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + def test_editor_cannot_manage_settings_or_members(self): + """Editors cannot manage settings or members.""" + self.client.force_authenticate(self.editor_user) + + # Cannot update organization + data = {"name": "Updated"} + response = self.client.patch(f"/api/organization/{self.organization.id}/", data, format="json") + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + # Cannot add members + data = {"user_id": str(self.other_user.id), "role": ORGANIZATION_VIEWER} + response = self.client.post(f"/api/organization/{self.organization.id}/add_member/", data, format="json") + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_viewer_has_read_only_access(self): + """Viewers have read-only access.""" + self.client.force_authenticate(self.viewer_user) + + # Can view organization + response = self.client.get(f"/api/organization/{self.organization.id}/") + self.assertEqual(response.status_code, status.HTTP_200_OK) + + # Can view members + response = self.client.get(f"/api/organization/{self.organization.id}/members/") + self.assertEqual(response.status_code, status.HTTP_200_OK) + + # Cannot update organization + data = {"name": "Updated"} + response = self.client.patch(f"/api/organization/{self.organization.id}/", data, format="json") + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + +class OrganizationPaginationTestCase(OrganizationAPITestCase): + """Tests for pagination in organization endpoints.""" + + def test_organization_list_pagination(self): + """Organization list should be paginated.""" + # Create multiple organizations + for i in range(25): + org = Organization.objects.create(name=f"Org {i}") + OrganizationRole.objects.create( + user=self.admin_user, + organization=org, + role=ORGANIZATION_ADMIN, + status=ORGANIZATION_ROLE_STATUS_ACTIVE, + ) + + self.client.force_authenticate(self.admin_user) + response = self.client.get("/api/organization/") + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertIn("results", response.data) + self.assertIn("count", response.data) + self.assertEqual(len(response.data["results"]), 20) # Default page size + + def test_member_list_pagination(self): + """Member list should be paginated.""" + # Add many members + for i in range(25): + user = testdata.user(email=f"user{i}@test.com") + OrganizationRole.objects.create( + user=user, + organization=self.organization, + role=ORGANIZATION_VIEWER, + status=ORGANIZATION_ROLE_STATUS_ACTIVE, + ) + + self.client.force_authenticate(self.admin_user) + response = self.client.get(f"/api/organization/{self.organization.id}/members/") + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertIn("results", response.data) + self.assertIn("count", response.data) diff --git a/contentcuration/contentcuration/urls.py b/contentcuration/contentcuration/urls.py index 6f36a5ac68..94588c3dab 100644 --- a/contentcuration/contentcuration/urls.py +++ b/contentcuration/contentcuration/urls.py @@ -57,6 +57,7 @@ from contentcuration.viewsets.feedback import RecommendationsInteractionEventViewSet from contentcuration.viewsets.file import FileViewSet from contentcuration.viewsets.invitation import InvitationViewSet +from contentcuration.viewsets.organization import OrganizationViewSet, OrganizationMemberViewSet from contentcuration.viewsets.recommendation import RecommendationView from contentcuration.viewsets.sync.endpoint import SyncView from contentcuration.viewsets.user import AdminUserViewSet @@ -83,6 +84,8 @@ def get_redirect_url(self, *args, **kwargs): router.register(r"channeluser", ChannelUserViewSet, basename="channeluser") router.register(r"user", UserViewSet) router.register(r"invitation", InvitationViewSet) +router.register(r"organization", OrganizationViewSet, basename="organization") +router.register(r"organization-members", OrganizationMemberViewSet, basename="organization-members") router.register(r"contentnode", ContentNodeViewSet) router.register(r"assessmentitem", AssessmentItemViewSet) router.register(r"admin-users", AdminUserViewSet, basename="admin-users") diff --git a/contentcuration/contentcuration/viewsets/organization.py b/contentcuration/contentcuration/viewsets/organization.py new file mode 100644 index 0000000000..90b6a4737e --- /dev/null +++ b/contentcuration/contentcuration/viewsets/organization.py @@ -0,0 +1,351 @@ +from django.db.models import Prefetch +from django_filters.rest_framework import CharFilter, FilterSet +from rest_framework import serializers +from rest_framework.decorators import action +from rest_framework.permissions import IsAuthenticated +from rest_framework.response import Response +from rest_framework.status import HTTP_403_FORBIDDEN + +from contentcuration.constants.organization_roles import ( + ORGANIZATION_ADMIN, + ORGANIZATION_EDITOR, + ORGANIZATION_VIEWER, +) +from contentcuration.models import Organization, OrganizationRole, User +from contentcuration.utils.pagination import ValuesViewsetPageNumberPagination +from contentcuration.viewsets.base import ( + BulkListSerializer, + BulkModelSerializer, + ValuesViewset, + RESTCreateModelMixin, +) + + + +class OrganizationSerializer(BulkModelSerializer): + """ + Serializer for Organization model. + Includes basic organization details (name, description, public status). + """ + + class Meta: + model = Organization + fields = ("id", "name", "description", "thumbnail", "public", "created_at", "updated_at") + read_only_fields = ("created_at", "updated_at") + list_serializer_class = BulkListSerializer + + +class OrganizationMemberSerializer(BulkModelSerializer): + """ + Serializer for OrganizationRole (membership). + Represents user membership in an organization with their role. + """ + + user_email = serializers.CharField(source="user.email", read_only=True) + user_name = serializers.CharField(source="user.get_full_name", read_only=True) + + class Meta: + model = OrganizationRole + fields = ("id", "user", "user_email", "user_name", "role", "status", "joined_at", "description") + read_only_fields = ("joined_at",) + list_serializer_class = BulkListSerializer + + +class OrganizationRoleSerializer(BulkModelSerializer): + """ + Serializer for reading organization roles. + Returns available roles for an organization. + """ + + class Meta: + model = OrganizationRole + fields = ("id", "role", "description") + list_serializer_class = BulkListSerializer + + +class OrganizationFilter(FilterSet): + """Filter for organization listing.""" + name = CharFilter(field_name="name", lookup_expr="icontains") + + class Meta: + model = Organization + fields = ("name", "public") + + +class OrganizationPagination(ValuesViewsetPageNumberPagination): + """Pagination for organization endpoints.""" + page_size = 20 + page_size_query_param = "page_size" + max_page_size = 100 + + +class OrganizationViewSet(ValuesViewset, RESTCreateModelMixin): + """ + ViewSet for Organization CRUD and membership management. + + Endpoints: + - GET /organizations/ - List organizations + - POST /organizations/ - Create organization + - GET /organizations/{id}/ - Retrieve organization + - PUT /organizations/{id}/ - Update organization (admin only) + - PATCH /organizations/{id}/ - Partial update (admin only) + - DELETE /organizations/{id}/ - Delete organization (admin only) + - GET /organizations/{id}/members/ - List members + - POST /organizations/{id}/members/ - Add member (admin only) + - PATCH /organizations/{id}/members/{member_id}/ - Update member role (admin only) + - DELETE /organizations/{id}/members/{member_id}/ - Remove member (admin only) + """ + + queryset = Organization.objects.filter(deleted=False) + serializer_class = OrganizationSerializer + permission_classes = [IsAuthenticated] + filterset_class = OrganizationFilter + pagination_class = OrganizationPagination + values = ("id", "name", "description", "thumbnail", "public", "created_at", "updated_at") + + def get_queryset(self): + """Filter organizations by user membership.""" + queryset = super().get_queryset() + user = self.request.user + + # Users can see organizations they are members of + if user.is_authenticated: + queryset = queryset.filter(user_roles__user=user).distinct() + else: + queryset = queryset.filter(public=True) + + return queryset + + def perform_create(self, serializer): + """Create organization and set creator as admin.""" + instance = serializer.save() + # Add the creating user as admin + OrganizationRole.objects.create( + user=self.request.user, + organization=instance, + role=ORGANIZATION_ADMIN, + status="active", + ) + return instance + + def perform_update(self, serializer): + """Update organization - only admin can do this.""" + org = self.get_object() + if not self._is_admin(org, self.request.user): + raise serializers.ValidationError("Only admins can update organization.") + serializer.save() + + def perform_destroy(self, instance): + """Delete organization - only admin can do this.""" + if not self._is_admin(instance, self.request.user): + raise serializers.ValidationError("Only admins can delete organization.") + instance.deleted = True + instance.save() + + def _is_admin(self, organization, user): + """Check if user is admin in organization.""" + try: + role = organization.user_roles.get(user=user) + return role.role == ORGANIZATION_ADMIN + except OrganizationRole.DoesNotExist: + return False + + def _get_user_role(self, organization, user): + """Get the user's role in the organization.""" + try: + return organization.user_roles.get(user=user) + except OrganizationRole.DoesNotExist: + return None + + @action(detail=True, methods=["get"], permission_classes=[IsAuthenticated]) + def members(self, request, pk=None): + """ + List members of an organization. + Permissions: Members can view. + """ + organization = self.get_object() + user_role = self._get_user_role(organization, request.user) + + if not user_role: + return Response( + {"detail": "You are not a member of this organization."}, + status=HTTP_403_FORBIDDEN, + ) + + members = organization.user_roles.all() + page = self.paginate_queryset(members) + if page is not None: + serializer = OrganizationMemberSerializer(page, many=True, context={"request": request}) + return self.get_paginated_response(serializer.data) + + serializer = OrganizationMemberSerializer(members, many=True, context={"request": request}) + return Response(serializer.data) + + @action(detail=True, methods=["post"], permission_classes=[IsAuthenticated]) + def add_member(self, request, pk=None): + """ + Add a member to organization. + Permissions: ORGANIZATION_ADMIN only. + Body: {user_id, role} + """ + organization = self.get_object() + + if not self._is_admin(organization, request.user): + return Response( + {"detail": "Only admins can add members."}, + status=HTTP_403_FORBIDDEN, + ) + + user_id = request.data.get("user_id") + role = request.data.get("role", ORGANIZATION_VIEWER) + + if not user_id: + return Response( + {"detail": "user_id is required."}, + status=400, + ) + + valid_roles = [ORGANIZATION_ADMIN, ORGANIZATION_EDITOR, ORGANIZATION_VIEWER] + if role not in valid_roles: + return Response( + {"detail": f"Invalid role. Must be one of: {', '.join(valid_roles)}"}, + status=400, + ) + + try: + user = User.objects.get(pk=user_id) + except User.DoesNotExist: + return Response( + {"detail": "User not found."}, + status=404, + ) + + membership, created = OrganizationRole.objects.get_or_create( + user=user, + organization=organization, + defaults={"role": role, "status": "active"}, + ) + + if not created: + membership.role = role + membership.save() + + serializer = OrganizationMemberSerializer(membership, context={"request": request}) + return Response(serializer.data, status=201 if created else 200) + + @action(detail=True, methods=["patch", "delete"], permission_classes=[IsAuthenticated]) + def update_member(self, request, pk=None): + """ + Update or remove a member's role in organization. + Permissions: ORGANIZATION_ADMIN only. + """ + organization = self.get_object() + + if not self._is_admin(organization, request.user): + return Response( + {"detail": "Only admins can manage members."}, + status=HTTP_403_FORBIDDEN, + ) + + member_id = request.query_params.get("member_id") + if not member_id: + return Response( + {"detail": "member_id query parameter is required."}, + status=400, + ) + + try: + membership = organization.user_roles.get(id=member_id) + except OrganizationRole.DoesNotExist: + return Response( + {"detail": "Member not found."}, + status=404, + ) + + if request.method == "PATCH": + role = request.data.get("role") + if not role: + return Response( + {"detail": "role is required."}, + status=400, + ) + + valid_roles = [ORGANIZATION_ADMIN, ORGANIZATION_EDITOR, ORGANIZATION_VIEWER] + if role not in valid_roles: + return Response( + {"detail": f"Invalid role. Must be one of: {', '.join(valid_roles)}"}, + status=400, + ) + + membership.role = role + membership.save() + + serializer = OrganizationMemberSerializer(membership, context={"request": request}) + return Response(serializer.data) + + elif request.method == "DELETE": + membership.delete() + return Response(status=204) + + +class OrganizationMemberViewSet(ValuesViewset): + """ + ViewSet for managing organization members. + This is an alternative to the nested /organizations/{id}/members/ endpoints. + """ + + queryset = OrganizationRole.objects.all() + serializer_class = OrganizationMemberSerializer + permission_classes = [IsAuthenticated] + pagination_class = OrganizationPagination + + def get_queryset(self): + """Filter members by organization.""" + queryset = super().get_queryset() + organization_id = self.request.query_params.get("organization") + + if organization_id: + queryset = queryset.filter(organization_id=organization_id) + + return queryset + + def perform_create(self, serializer): + """Add member to organization.""" + organization_id = self.request.data.get("organization") + if not organization_id: + raise serializers.ValidationError("organization is required.") + + try: + organization = Organization.objects.get(pk=organization_id) + except Organization.DoesNotExist: + raise serializers.ValidationError("Organization not found.") + + # Check if user is admin + user_role = organization.user_roles.filter(user=self.request.user).first() + if not user_role or user_role.role != ORGANIZATION_ADMIN: + raise serializers.ValidationError("Only admins can add members.") + + serializer.save() + + def perform_update(self, serializer): + """Update member role.""" + membership = self.get_object() + organization = membership.organization + + # Check if user is admin + user_role = organization.user_roles.filter(user=self.request.user).first() + if not user_role or user_role.role != ORGANIZATION_ADMIN: + raise serializers.ValidationError("Only admins can update member roles.") + + serializer.save() + + def perform_destroy(self, instance): + """Remove member from organization.""" + organization = instance.organization + + # Check if user is admin + user_role = organization.user_roles.filter(user=self.request.user).first() + if not user_role or user_role.role != ORGANIZATION_ADMIN: + raise serializers.ValidationError("Only admins can remove members.") + + instance.delete() From f78878239f2270892fd0605ffddbdd7adb29959d Mon Sep 17 00:00:00 2001 From: Ajay Nair Date: Mon, 20 Jul 2026 23:01:53 -0400 Subject: [PATCH 02/10] Feat: Edited the org model --- .../contentcuration/viewsets/organization.py | 588 ++++++++++-------- 1 file changed, 318 insertions(+), 270 deletions(-) diff --git a/contentcuration/contentcuration/viewsets/organization.py b/contentcuration/contentcuration/viewsets/organization.py index 90b6a4737e..acfdeb639b 100644 --- a/contentcuration/contentcuration/viewsets/organization.py +++ b/contentcuration/contentcuration/viewsets/organization.py @@ -1,70 +1,87 @@ -from django.db.models import Prefetch +from django.db import transaction +from django.db.models import Q from django_filters.rest_framework import CharFilter, FilterSet from rest_framework import serializers -from rest_framework.decorators import action +from rest_framework.exceptions import PermissionDenied, ValidationError from rest_framework.permissions import IsAuthenticated -from rest_framework.response import Response -from rest_framework.status import HTTP_403_FORBIDDEN from contentcuration.constants.organization_roles import ( ORGANIZATION_ADMIN, - ORGANIZATION_EDITOR, + ORGANIZATION_ROLE_STATUS_ACTIVE, ORGANIZATION_VIEWER, + organization_role_status_choices, ) -from contentcuration.models import Organization, OrganizationRole, User +from contentcuration.models import Organization, OrganizationRole from contentcuration.utils.pagination import ValuesViewsetPageNumberPagination from contentcuration.viewsets.base import ( BulkListSerializer, BulkModelSerializer, - ValuesViewset, RESTCreateModelMixin, + RESTDestroyModelMixin, + RESTUpdateModelMixin, + ValuesViewset, ) - class OrganizationSerializer(BulkModelSerializer): """ - Serializer for Organization model. - Includes basic organization details (name, description, public status). + Write serializer for organizations. + + Read operations are handled by OrganizationViewSet.values, following the + ValuesViewset pattern used elsewhere in Studio. """ class Meta: model = Organization - fields = ("id", "name", "description", "thumbnail", "public", "created_at", "updated_at") - read_only_fields = ("created_at", "updated_at") + fields = ( + "id", + "name", + "description", + "thumbnail", + "thumbnail_encoding", + "public", + ) list_serializer_class = BulkListSerializer class OrganizationMemberSerializer(BulkModelSerializer): """ - Serializer for OrganizationRole (membership). - Represents user membership in an organization with their role. + Write serializer for OrganizationRole membership records. + + Organization and user may be set when creating a membership, but cannot be + changed afterwards. Read operations are handled by the viewset values map. """ - user_email = serializers.CharField(source="user.email", read_only=True) - user_name = serializers.CharField(source="user.get_full_name", read_only=True) + status = serializers.ChoiceField( + choices=organization_role_status_choices, + default=ORGANIZATION_ROLE_STATUS_ACTIVE, + ) class Meta: model = OrganizationRole - fields = ("id", "user", "user_email", "user_name", "role", "status", "joined_at", "description") - read_only_fields = ("joined_at",) + fields = ( + "id", + "organization", + "user", + "role", + "description", + "status", + ) list_serializer_class = BulkListSerializer + def get_fields(self): + fields = super().get_fields() -class OrganizationRoleSerializer(BulkModelSerializer): - """ - Serializer for reading organization roles. - Returns available roles for an organization. - """ + # A membership may move between statuses and roles, but it must never be + # reassigned to a different organization or user. + if self.instance is not None: + fields["organization"].read_only = True + fields["user"].read_only = True - class Meta: - model = OrganizationRole - fields = ("id", "role", "description") - list_serializer_class = BulkListSerializer + return fields class OrganizationFilter(FilterSet): - """Filter for organization listing.""" name = CharFilter(field_name="name", lookup_expr="icontains") class Meta: @@ -72,280 +89,311 @@ class Meta: fields = ("name", "public") +class OrganizationMemberFilter(FilterSet): + organization = CharFilter(field_name="organization_id") + user = CharFilter(field_name="user_id") + + class Meta: + model = OrganizationRole + fields = ("organization", "user", "role", "status") + + class OrganizationPagination(ValuesViewsetPageNumberPagination): - """Pagination for organization endpoints.""" page_size = 20 page_size_query_param = "page_size" max_page_size = 100 -class OrganizationViewSet(ValuesViewset, RESTCreateModelMixin): +def _is_site_admin(user): + return bool(getattr(user, "is_admin", False)) + + +def _get_member_name(item): + first_name = item.pop("user__first_name", "") or "" + last_name = item.pop("user__last_name", "") or "" + return "{} {}".format(first_name, last_name).strip() + + +class OrganizationViewSet( + ValuesViewset, + RESTCreateModelMixin, + RESTUpdateModelMixin, + RESTDestroyModelMixin, +): """ - ViewSet for Organization CRUD and membership management. - - Endpoints: - - GET /organizations/ - List organizations - - POST /organizations/ - Create organization - - GET /organizations/{id}/ - Retrieve organization - - PUT /organizations/{id}/ - Update organization (admin only) - - PATCH /organizations/{id}/ - Partial update (admin only) - - DELETE /organizations/{id}/ - Delete organization (admin only) - - GET /organizations/{id}/members/ - List members - - POST /organizations/{id}/members/ - Add member (admin only) - - PATCH /organizations/{id}/members/{member_id}/ - Update member role (admin only) - - DELETE /organizations/{id}/members/{member_id}/ - Remove member (admin only) + Organization CRUD API. + + Active organization admins may update or delete an organization. Any + authenticated user may create an organization and becomes its first active + administrator. Site administrators may manage every organization. """ - queryset = Organization.objects.filter(deleted=False) + queryset = Organization.objects.all() serializer_class = OrganizationSerializer permission_classes = [IsAuthenticated] filterset_class = OrganizationFilter pagination_class = OrganizationPagination - values = ("id", "name", "description", "thumbnail", "public", "created_at", "updated_at") + ordering_fields = ("name", "created_at", "updated_at") + ordering = "name" + + values = ( + "id", + "name", + "description", + "thumbnail", + "thumbnail_encoding", + "public", + "created_at", + "updated_at", + ) def get_queryset(self): - """Filter organizations by user membership.""" - queryset = super().get_queryset() + """ + Return organizations visible to the current user. + + Public organizations are visible to authenticated users. Private + organizations require an active membership. Non-active memberships do + not grant access. + """ + queryset = Organization.objects.filter(deleted=False) user = self.request.user - - # Users can see organizations they are members of - if user.is_authenticated: - queryset = queryset.filter(user_roles__user=user).distinct() - else: - queryset = queryset.filter(public=True) - - return queryset - - def perform_create(self, serializer): - """Create organization and set creator as admin.""" - instance = serializer.save() - # Add the creating user as admin - OrganizationRole.objects.create( - user=self.request.user, - organization=instance, - role=ORGANIZATION_ADMIN, - status="active", - ) - return instance - def perform_update(self, serializer): - """Update organization - only admin can do this.""" - org = self.get_object() - if not self._is_admin(org, self.request.user): - raise serializers.ValidationError("Only admins can update organization.") - serializer.save() + if _is_site_admin(user): + return queryset - def perform_destroy(self, instance): - """Delete organization - only admin can do this.""" - if not self._is_admin(instance, self.request.user): - raise serializers.ValidationError("Only admins can delete organization.") - instance.deleted = True - instance.save() - - def _is_admin(self, organization, user): - """Check if user is admin in organization.""" - try: - role = organization.user_roles.get(user=user) - return role.role == ORGANIZATION_ADMIN - except OrganizationRole.DoesNotExist: - return False - - def _get_user_role(self, organization, user): - """Get the user's role in the organization.""" - try: - return organization.user_roles.get(user=user) - except OrganizationRole.DoesNotExist: - return None - - @action(detail=True, methods=["get"], permission_classes=[IsAuthenticated]) - def members(self, request, pk=None): - """ - List members of an organization. - Permissions: Members can view. - """ - organization = self.get_object() - user_role = self._get_user_role(organization, request.user) - - if not user_role: - return Response( - {"detail": "You are not a member of this organization."}, - status=HTTP_403_FORBIDDEN, - ) - - members = organization.user_roles.all() - page = self.paginate_queryset(members) - if page is not None: - serializer = OrganizationMemberSerializer(page, many=True, context={"request": request}) - return self.get_paginated_response(serializer.data) - - serializer = OrganizationMemberSerializer(members, many=True, context={"request": request}) - return Response(serializer.data) - - @action(detail=True, methods=["post"], permission_classes=[IsAuthenticated]) - def add_member(self, request, pk=None): - """ - Add a member to organization. - Permissions: ORGANIZATION_ADMIN only. - Body: {user_id, role} - """ - organization = self.get_object() - - if not self._is_admin(organization, request.user): - return Response( - {"detail": "Only admins can add members."}, - status=HTTP_403_FORBIDDEN, - ) - - user_id = request.data.get("user_id") - role = request.data.get("role", ORGANIZATION_VIEWER) - - if not user_id: - return Response( - {"detail": "user_id is required."}, - status=400, - ) - - valid_roles = [ORGANIZATION_ADMIN, ORGANIZATION_EDITOR, ORGANIZATION_VIEWER] - if role not in valid_roles: - return Response( - {"detail": f"Invalid role. Must be one of: {', '.join(valid_roles)}"}, - status=400, - ) - - try: - user = User.objects.get(pk=user_id) - except User.DoesNotExist: - return Response( - {"detail": "User not found."}, - status=404, - ) - - membership, created = OrganizationRole.objects.get_or_create( - user=user, - organization=organization, - defaults={"role": role, "status": "active"}, - ) - - if not created: - membership.role = role - membership.save() - - serializer = OrganizationMemberSerializer(membership, context={"request": request}) - return Response(serializer.data, status=201 if created else 200) - - @action(detail=True, methods=["patch", "delete"], permission_classes=[IsAuthenticated]) - def update_member(self, request, pk=None): - """ - Update or remove a member's role in organization. - Permissions: ORGANIZATION_ADMIN only. - """ - organization = self.get_object() - - if not self._is_admin(organization, request.user): - return Response( - {"detail": "Only admins can manage members."}, - status=HTTP_403_FORBIDDEN, - ) - - member_id = request.query_params.get("member_id") - if not member_id: - return Response( - {"detail": "member_id query parameter is required."}, - status=400, + if not user.is_authenticated: + return queryset.filter(public=True) + + return queryset.filter( + Q(public=True) + | Q( + user_roles__user=user, + user_roles__status=ORGANIZATION_ROLE_STATUS_ACTIVE, ) - - try: - membership = organization.user_roles.get(id=member_id) - except OrganizationRole.DoesNotExist: - return Response( - {"detail": "Member not found."}, - status=404, + ).distinct() + + def get_edit_queryset(self): + """Return organizations that the current user may modify.""" + queryset = Organization.objects.filter(deleted=False) + user = self.request.user + + if _is_site_admin(user): + return queryset + + if not user.is_authenticated: + return queryset.none() + + return queryset.filter( + user_roles__user=user, + user_roles__role=ORGANIZATION_ADMIN, + user_roles__status=ORGANIZATION_ROLE_STATUS_ACTIVE, + ).distinct() + + def perform_create(self, serializer, change=None): + """Create the organization and its initial administrator atomically.""" + with transaction.atomic(): + organization = serializer.save() + OrganizationRole.objects.create( + organization=organization, + user=self.request.user, + role=ORGANIZATION_ADMIN, + status=ORGANIZATION_ROLE_STATUS_ACTIVE, ) - - if request.method == "PATCH": - role = request.data.get("role") - if not role: - return Response( - {"detail": "role is required."}, - status=400, - ) - - valid_roles = [ORGANIZATION_ADMIN, ORGANIZATION_EDITOR, ORGANIZATION_VIEWER] - if role not in valid_roles: - return Response( - {"detail": f"Invalid role. Must be one of: {', '.join(valid_roles)}"}, - status=400, - ) - - membership.role = role - membership.save() - - serializer = OrganizationMemberSerializer(membership, context={"request": request}) - return Response(serializer.data) - - elif request.method == "DELETE": - membership.delete() - return Response(status=204) + + def perform_destroy(self, instance): + """Soft-delete an organization.""" + instance.deleted = True + instance.save(update_fields=["deleted", "updated_at"]) -class OrganizationMemberViewSet(ValuesViewset): +class OrganizationMemberViewSet( + ValuesViewset, + RESTCreateModelMixin, + RESTUpdateModelMixin, + RESTDestroyModelMixin, +): """ - ViewSet for managing organization members. - This is an alternative to the nested /organizations/{id}/members/ endpoints. + Organization membership and role API. + + Active organization members may read the membership list. Only active + organization admins may create, update, or remove memberships. Site admins + may manage all memberships. """ queryset = OrganizationRole.objects.all() serializer_class = OrganizationMemberSerializer permission_classes = [IsAuthenticated] + filterset_class = OrganizationMemberFilter pagination_class = OrganizationPagination + ordering_fields = ("joined_at", "updated_at", "role", "status") + ordering = "-joined_at" + + values = ( + "id", + "organization_id", + "organization__name", + "user_id", + "user__email", + "user__first_name", + "user__last_name", + "role", + "description", + "status", + "joined_at", + "updated_at", + ) + + field_map = { + "organization": "organization_id", + "organization_name": "organization__name", + "user": "user_id", + "user_email": "user__email", + "user_name": _get_member_name, + } def get_queryset(self): - """Filter members by organization.""" - queryset = super().get_queryset() - organization_id = self.request.query_params.get("organization") - - if organization_id: - queryset = queryset.filter(organization_id=organization_id) - - return queryset - - def perform_create(self, serializer): - """Add member to organization.""" - organization_id = self.request.data.get("organization") - if not organization_id: - raise serializers.ValidationError("organization is required.") - - try: - organization = Organization.objects.get(pk=organization_id) - except Organization.DoesNotExist: - raise serializers.ValidationError("Organization not found.") - - # Check if user is admin - user_role = organization.user_roles.filter(user=self.request.user).first() - if not user_role or user_role.role != ORGANIZATION_ADMIN: - raise serializers.ValidationError("Only admins can add members.") - - serializer.save() + """ + Return memberships belonging to organizations the user may inspect. + + A public organization does not expose its membership list to the + public; an active membership is required. + """ + queryset = OrganizationRole.objects.select_related( + "organization", "user" + ).filter(organization__deleted=False) + user = self.request.user + + if _is_site_admin(user): + return queryset + + if not user.is_authenticated: + return queryset.none() + + return queryset.filter( + organization__user_roles__user=user, + organization__user_roles__status=ORGANIZATION_ROLE_STATUS_ACTIVE, + ).distinct() + + def get_edit_queryset(self): + """Return memberships managed by organizations where the user is admin.""" + queryset = OrganizationRole.objects.select_related( + "organization", "user" + ).filter(organization__deleted=False) + user = self.request.user + + if _is_site_admin(user): + return queryset + + if not user.is_authenticated: + return queryset.none() + + return queryset.filter( + organization__user_roles__user=user, + organization__user_roles__role=ORGANIZATION_ADMIN, + organization__user_roles__status=ORGANIZATION_ROLE_STATUS_ACTIVE, + ).distinct() + + def _require_admin(self, organization): + user = self.request.user + + if _is_site_admin(user): + return + + is_admin = OrganizationRole.objects.filter( + organization=organization, + user=user, + role=ORGANIZATION_ADMIN, + status=ORGANIZATION_ROLE_STATUS_ACTIVE, + ).exists() + + if not is_admin: + raise PermissionDenied( + "Only active organization admins may manage membership." + ) + + def _ensure_not_last_active_admin( + self, + membership, + new_role=None, + new_status=None, + ): + """Prevent an organization from being left without an active admin.""" + if ( + membership.role != ORGANIZATION_ADMIN + or membership.status != ORGANIZATION_ROLE_STATUS_ACTIVE + ): + return + + resulting_role = new_role if new_role is not None else membership.role + resulting_status = ( + new_status if new_status is not None else membership.status + ) + + if ( + resulting_role == ORGANIZATION_ADMIN + and resulting_status == ORGANIZATION_ROLE_STATUS_ACTIVE + ): + return + + active_admin_ids = list( + OrganizationRole.objects.select_for_update() + .filter( + organization=membership.organization, + role=ORGANIZATION_ADMIN, + status=ORGANIZATION_ROLE_STATUS_ACTIVE, + ) + .values_list("id", flat=True) + ) + + if len(active_admin_ids) <= 1: + raise ValidationError( + "An organization must have at least one active admin." + ) + + def perform_create(self, serializer, change=None): + organization = serializer.validated_data["organization"] + self._require_admin(organization) + + if organization.deleted: + raise ValidationError("Cannot add members to a deleted organization.") + + with transaction.atomic(): + serializer.save() def perform_update(self, serializer): - """Update member role.""" - membership = self.get_object() - organization = membership.organization - - # Check if user is admin - user_role = organization.user_roles.filter(user=self.request.user).first() - if not user_role or user_role.role != ORGANIZATION_ADMIN: - raise serializers.ValidationError("Only admins can update member roles.") - - serializer.save() + with transaction.atomic(): + membership = ( + OrganizationRole.objects.select_for_update() + .select_related("organization", "user") + .get(pk=serializer.instance.pk) + ) + self._require_admin(membership.organization) + + self._ensure_not_last_active_admin( + membership, + new_role=serializer.validated_data.get("role"), + new_status=serializer.validated_data.get("status"), + ) + + serializer.instance = membership + serializer.save() def perform_destroy(self, instance): - """Remove member from organization.""" - organization = instance.organization - - # Check if user is admin - user_role = organization.user_roles.filter(user=self.request.user).first() - if not user_role or user_role.role != ORGANIZATION_ADMIN: - raise serializers.ValidationError("Only admins can remove members.") - - instance.delete() + with transaction.atomic(): + membership = ( + OrganizationRole.objects.select_for_update() + .select_related("organization", "user") + .get(pk=instance.pk) + ) + self._require_admin(membership.organization) + self._ensure_not_last_active_admin( + membership, + new_role=ORGANIZATION_VIEWER, + new_status=membership.status, + ) + membership.delete() + + +# The model is named OrganizationRole, while existing work may already import +# OrganizationMemberViewSet. Keep this alias so either name can be registered. +OrganizationRoleViewSet = OrganizationMemberViewSet From 24138b02e79b947d08d8a84293b99edeb8bf50b0 Mon Sep 17 00:00:00 2001 From: Ajay Nair Date: Mon, 27 Jul 2026 10:01:48 -0400 Subject: [PATCH 03/10] Feat: Added first name and last name --- contentcuration/contentcuration/viewsets/organization.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contentcuration/contentcuration/viewsets/organization.py b/contentcuration/contentcuration/viewsets/organization.py index acfdeb639b..976caf38f6 100644 --- a/contentcuration/contentcuration/viewsets/organization.py +++ b/contentcuration/contentcuration/viewsets/organization.py @@ -248,6 +248,8 @@ class OrganizationMemberViewSet( "organization_name": "organization__name", "user": "user_id", "user_email": "user__email", + "user_first_name": "user__first_name", + "user_last_name": "user__last_name", "user_name": _get_member_name, } From c5090829d5af8346715cc4eba2231d76ca80c436 Mon Sep 17 00:00:00 2001 From: Ajay Nair Date: Tue, 28 Jul 2026 12:23:40 -0400 Subject: [PATCH 04/10] Remove direct organization membership creation --- .../contentcuration/viewsets/organization.py | 43 ++++++------------- 1 file changed, 13 insertions(+), 30 deletions(-) diff --git a/contentcuration/contentcuration/viewsets/organization.py b/contentcuration/contentcuration/viewsets/organization.py index 976caf38f6..d6ed3431a8 100644 --- a/contentcuration/contentcuration/viewsets/organization.py +++ b/contentcuration/contentcuration/viewsets/organization.py @@ -19,6 +19,7 @@ RESTCreateModelMixin, RESTDestroyModelMixin, RESTUpdateModelMixin, + ReadOnlyValuesViewset, ValuesViewset, ) @@ -46,15 +47,17 @@ class Meta: class OrganizationMemberSerializer(BulkModelSerializer): """ - Write serializer for OrganizationRole membership records. + Write serializer for updating OrganizationRole membership records. - Organization and user may be set when creating a membership, but cannot be - changed afterwards. Read operations are handled by the viewset values map. + Membership creation is handled by invitation acceptance. Organization and + user are immutable through this endpoint; admins may only update an existing + membership's role, description, or status. Read operations are handled by + the viewset values map. """ status = serializers.ChoiceField( choices=organization_role_status_choices, - default=ORGANIZATION_ROLE_STATUS_ACTIVE, + required=False, ) class Meta: @@ -67,19 +70,9 @@ class Meta: "description", "status", ) + read_only_fields = ("organization", "user") list_serializer_class = BulkListSerializer - def get_fields(self): - fields = super().get_fields() - - # A membership may move between statuses and roles, but it must never be - # reassigned to a different organization or user. - if self.instance is not None: - fields["organization"].read_only = True - fields["user"].read_only = True - - return fields - class OrganizationFilter(FilterSet): name = CharFilter(field_name="name", lookup_expr="icontains") @@ -207,17 +200,17 @@ def perform_destroy(self, instance): class OrganizationMemberViewSet( - ValuesViewset, - RESTCreateModelMixin, + ReadOnlyValuesViewset, RESTUpdateModelMixin, RESTDestroyModelMixin, ): """ Organization membership and role API. - Active organization members may read the membership list. Only active - organization admins may create, update, or remove memberships. Site admins - may manage all memberships. + Active organization members may read the membership list. New membership + records are created only when an invitation is accepted. Active organization + admins may update or remove existing memberships. Site admins may manage all + existing memberships. """ queryset = OrganizationRole.objects.all() @@ -352,16 +345,6 @@ def _ensure_not_last_active_admin( "An organization must have at least one active admin." ) - def perform_create(self, serializer, change=None): - organization = serializer.validated_data["organization"] - self._require_admin(organization) - - if organization.deleted: - raise ValidationError("Cannot add members to a deleted organization.") - - with transaction.atomic(): - serializer.save() - def perform_update(self, serializer): with transaction.atomic(): membership = ( From 7bea2e6ab8aafc1f8e93471b6c9c623ccb5b2102 Mon Sep 17 00:00:00 2001 From: Ajay Nair Date: Thu, 30 Jul 2026 08:01:46 -0400 Subject: [PATCH 05/10] Feat: added tests --- .../tests/test_organization.py | 791 ++++++++++-------- 1 file changed, 432 insertions(+), 359 deletions(-) diff --git a/contentcuration/contentcuration/tests/test_organization.py b/contentcuration/contentcuration/tests/test_organization.py index ae2f3869fc..29a38d2b95 100644 --- a/contentcuration/contentcuration/tests/test_organization.py +++ b/contentcuration/contentcuration/tests/test_organization.py @@ -1,500 +1,573 @@ -""" -Tests for Organization API endpoints. -""" -import json +"""Tests for organization and organization membership API endpoints.""" from django.urls import reverse from rest_framework import status -from rest_framework.test import APITestCase, APIClient +from rest_framework.test import APIClient +from contentcuration.constants.organization_roles import ORGANIZATION_ADMIN +from contentcuration.constants.organization_roles import ORGANIZATION_EDITOR from contentcuration.constants.organization_roles import ( - ORGANIZATION_ADMIN, - ORGANIZATION_EDITOR, - ORGANIZATION_VIEWER, ORGANIZATION_ROLE_STATUS_ACTIVE, ) -from contentcuration.models import Organization, OrganizationRole, User -from contentcuration.tests.base import BaseAPITestCase +from contentcuration.constants.organization_roles import ( + ORGANIZATION_ROLE_STATUS_INACTIVE, +) +from contentcuration.constants.organization_roles import ORGANIZATION_VIEWER +from contentcuration.models import Organization +from contentcuration.models import OrganizationRole from contentcuration.tests import testdata +from contentcuration.tests.base import BaseAPITestCase +from contentcuration.viewsets.organization import OrganizationMemberViewSet class OrganizationAPITestCase(BaseAPITestCase): - """Base test case for Organization API tests.""" + """Shared organization API fixtures and URL helpers.""" def setUp(self): super().setUp() - # Create additional test users - self.admin_user = testdata.user(email="admin@test.com") - self.editor_user = testdata.user(email="editor@test.com") - self.viewer_user = testdata.user(email="viewer@test.com") - self.other_user = testdata.user(email="other@test.com") - # Create test organization + self.organization_admin = testdata.user(email="org-admin@test.com") + self.organization_admin.first_name = "Admin" + self.organization_admin.last_name = "User" + self.organization_admin.save(update_fields=["first_name", "last_name"]) + + self.editor_user = testdata.user(email="org-editor@test.com") + self.viewer_user = testdata.user(email="org-viewer@test.com") + self.other_user = testdata.user(email="org-other@test.com") + self.inactive_user = testdata.user(email="org-inactive@test.com") + self.organization = Organization.objects.create( name="Test Organization", description="A test organization", public=False, ) - - # Add admin user to organization - OrganizationRole.objects.create( - user=self.admin_user, + + self.admin_membership = OrganizationRole.objects.create( + user=self.organization_admin, organization=self.organization, role=ORGANIZATION_ADMIN, status=ORGANIZATION_ROLE_STATUS_ACTIVE, ) - - # Add editor user to organization - OrganizationRole.objects.create( + self.editor_membership = OrganizationRole.objects.create( user=self.editor_user, organization=self.organization, role=ORGANIZATION_EDITOR, status=ORGANIZATION_ROLE_STATUS_ACTIVE, ) - - # Add viewer user to organization - OrganizationRole.objects.create( + self.viewer_membership = OrganizationRole.objects.create( user=self.viewer_user, organization=self.organization, role=ORGANIZATION_VIEWER, status=ORGANIZATION_ROLE_STATUS_ACTIVE, ) + self.inactive_membership = OrganizationRole.objects.create( + user=self.inactive_user, + organization=self.organization, + role=ORGANIZATION_VIEWER, + status=ORGANIZATION_ROLE_STATUS_INACTIVE, + ) + + @property + def organization_list_url(self): + return reverse("organization-list") + + def organization_detail_url(self, organization=None): + organization = organization or self.organization + return reverse("organization-detail", kwargs={"pk": organization.id}) + + @property + def membership_list_url(self): + return reverse("organization-members-list") + + def membership_detail_url(self, membership): + return reverse("organization-members-detail", kwargs={"pk": membership.id}) def authenticate_as(self, user): - """Switch authentication to a different user.""" - self.client = APIClient() self.client.force_authenticate(user) class OrganizationListCreateTestCase(OrganizationAPITestCase): - """Tests for creating and listing organizations.""" + def test_member_can_list_private_organization(self): + self.authenticate_as(self.organization_admin) + + response = self.client.get(self.organization_list_url) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["count"], 1) + self.assertEqual(response.data["results"][0]["name"], self.organization.name) + + def test_nonmember_cannot_list_private_organization(self): + self.authenticate_as(self.other_user) + + response = self.client.get(self.organization_list_url) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["results"], []) + + def test_authenticated_nonmember_can_list_public_organization(self): + self.organization.public = True + self.organization.save(update_fields=["public"]) + self.authenticate_as(self.other_user) + + response = self.client.get(self.organization_list_url) - def test_list_organizations_user_can_see_their_organizations(self): - """Authenticated users can list organizations they belong to.""" - self.client.force_authenticate(self.admin_user) - response = self.client.get("/api/organization/") - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(len(response.data["results"]), 1) - self.assertEqual(response.data["results"][0]["name"], "Test Organization") - - def test_list_organizations_user_cannot_see_orgs_they_dont_belong_to(self): - """Users should not see organizations they are not members of.""" - self.client.force_authenticate(self.other_user) - response = self.client.get("/api/organization/") - + self.assertEqual(response.data["count"], 1) + + def test_inactive_membership_does_not_grant_organization_access(self): + self.authenticate_as(self.inactive_user) + + response = self.client.get(self.organization_list_url) + self.assertEqual(response.status_code, status.HTTP_200_OK) - # other_user should not see the organization - self.assertEqual(len(response.data["results"]), 0) + self.assertEqual(response.data["results"], []) - def test_create_organization_creates_user_as_admin(self): - """Creating an organization should make the creator an admin.""" - self.client.force_authenticate(self.other_user) + def test_list_can_filter_by_name(self): + second_organization = Organization.objects.create(name="Another Group") + OrganizationRole.objects.create( + user=self.organization_admin, + organization=second_organization, + role=ORGANIZATION_ADMIN, + status=ORGANIZATION_ROLE_STATUS_ACTIVE, + ) + self.authenticate_as(self.organization_admin) + + response = self.client.get(self.organization_list_url, {"name": "Another"}) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["count"], 1) + self.assertEqual(response.data["results"][0]["name"], "Another Group") + + def test_create_organization_creates_active_admin_membership(self): + self.authenticate_as(self.other_user) data = { "name": "New Organization", "description": "A new organization", "public": False, } - response = self.client.post("/api/organization/", data, format="json") - + + response = self.client.post(self.organization_list_url, data, format="json") + self.assertEqual(response.status_code, status.HTTP_201_CREATED) - self.assertEqual(response.data["name"], "New Organization") - - # Verify creator is admin - org = Organization.objects.get(id=response.data["id"]) - role = org.user_roles.get(user=self.other_user) - self.assertEqual(role.role, ORGANIZATION_ADMIN) + organization = Organization.objects.get(id=response.data["id"]) + membership = OrganizationRole.objects.get( + organization=organization, + user=self.other_user, + ) + self.assertEqual(membership.role, ORGANIZATION_ADMIN) + self.assertEqual(membership.status, ORGANIZATION_ROLE_STATUS_ACTIVE) def test_create_organization_requires_authentication(self): - """Creating an organization requires authentication.""" client = APIClient() - data = { - "name": "New Organization", - "description": "A new organization", - "public": False, - } - response = client.post("/api/organization/", data, format="json") - + + response = client.post( + self.organization_list_url, + {"name": "New Organization"}, + format="json", + ) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) def test_list_organizations_requires_authentication(self): - """Listing organizations requires authentication.""" - client = APIClient() - response = client.get("/api/organization/") - + response = APIClient().get(self.organization_list_url) + self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) class OrganizationRetrieveUpdateDeleteTestCase(OrganizationAPITestCase): - """Tests for retrieving, updating, and deleting organizations.""" + def test_active_member_can_retrieve_private_organization(self): + self.authenticate_as(self.viewer_user) + + response = self.client.get(self.organization_detail_url()) - def test_retrieve_organization_member_can_access(self): - """Organization members can retrieve organization details.""" - self.client.force_authenticate(self.admin_user) - response = self.client.get(f"/api/organization/{self.organization.id}/") - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.data["name"], "Test Organization") + self.assertEqual(response.data["name"], self.organization.name) + + def test_nonmember_cannot_retrieve_private_organization(self): + self.authenticate_as(self.other_user) + + response = self.client.get(self.organization_detail_url()) - def test_retrieve_organization_non_member_cannot_access(self): - """Non-members cannot retrieve organization details.""" - self.client.force_authenticate(self.other_user) - response = self.client.get(f"/api/organization/{self.organization.id}/") - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_update_organization_admin_can_update(self): - """Organization admins can update organization details.""" - self.client.force_authenticate(self.admin_user) - data = {"name": "Updated Organization", "description": "Updated description"} - response = self.client.patch(f"/api/organization/{self.organization.id}/", data, format="json") - + def test_nonmember_can_retrieve_public_organization(self): + self.organization.public = True + self.organization.save(update_fields=["public"]) + self.authenticate_as(self.other_user) + + response = self.client.get(self.organization_detail_url()) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + + def test_active_admin_can_update_organization(self): + self.authenticate_as(self.organization_admin) + + response = self.client.patch( + self.organization_detail_url(), + {"name": "Updated Organization"}, + format="json", + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) self.organization.refresh_from_db() self.assertEqual(self.organization.name, "Updated Organization") - def test_update_organization_editor_cannot_update(self): - """Organization editors cannot update organization settings.""" - self.client.force_authenticate(self.editor_user) - data = {"name": "Updated Organization"} - response = self.client.patch(f"/api/organization/{self.organization.id}/", data, format="json") - - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + def test_editor_cannot_update_organization(self): + self.authenticate_as(self.editor_user) - def test_update_organization_viewer_cannot_update(self): - """Organization viewers cannot update organization settings.""" - self.client.force_authenticate(self.viewer_user) - data = {"name": "Updated Organization"} - response = self.client.patch(f"/api/organization/{self.organization.id}/", data, format="json") - - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + response = self.client.patch( + self.organization_detail_url(), + {"name": "Updated Organization"}, + format="json", + ) + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + def test_viewer_cannot_update_organization(self): + self.authenticate_as(self.viewer_user) + + response = self.client.patch( + self.organization_detail_url(), + {"name": "Updated Organization"}, + format="json", + ) + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + def test_inactive_admin_cannot_update_organization(self): + inactive_admin = testdata.user(email="inactive-admin@test.com") + OrganizationRole.objects.create( + user=inactive_admin, + organization=self.organization, + role=ORGANIZATION_ADMIN, + status=ORGANIZATION_ROLE_STATUS_INACTIVE, + ) + self.authenticate_as(inactive_admin) + + response = self.client.patch( + self.organization_detail_url(), + {"name": "Updated Organization"}, + format="json", + ) + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + def test_active_admin_can_soft_delete_organization(self): + self.authenticate_as(self.organization_admin) + + response = self.client.delete(self.organization_detail_url()) - def test_delete_organization_admin_can_delete(self): - """Organization admins can delete organizations (soft delete).""" - self.client.force_authenticate(self.admin_user) - response = self.client.delete(f"/api/organization/{self.organization.id}/") - self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) self.organization.refresh_from_db() self.assertTrue(self.organization.deleted) - def test_delete_organization_editor_cannot_delete(self): - """Organization editors cannot delete organizations.""" - self.client.force_authenticate(self.editor_user) - response = self.client.delete(f"/api/organization/{self.organization.id}/") - - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + def test_editor_cannot_delete_organization(self): + self.authenticate_as(self.editor_user) + + response = self.client.delete(self.organization_detail_url()) + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.organization.refresh_from_db() + self.assertFalse(self.organization.deleted) -class OrganizationMemberListTestCase(OrganizationAPITestCase): - """Tests for listing organization members.""" +class OrganizationMembershipListTestCase(OrganizationAPITestCase): + def test_active_member_can_list_memberships(self): + self.authenticate_as(self.viewer_user) + + response = self.client.get( + self.membership_list_url, + {"organization": str(self.organization.id)}, + ) - def test_list_members_member_can_view(self): - """Organization members can view the member list.""" - self.client.force_authenticate(self.admin_user) - response = self.client.get(f"/api/organization/{self.organization.id}/members/") - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(len(response.data["results"]), 3) - - def test_list_members_non_member_cannot_view(self): - """Non-members cannot view the member list.""" - self.client.force_authenticate(self.other_user) - response = self.client.get(f"/api/organization/{self.organization.id}/members/") - - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - - def test_list_members_returns_user_details(self): - """Member list includes user email and name.""" - self.client.force_authenticate(self.admin_user) - response = self.client.get(f"/api/organization/{self.organization.id}/members/") - + self.assertEqual(response.data["count"], 4) + + def test_nonmember_receives_empty_membership_list(self): + self.authenticate_as(self.other_user) + + response = self.client.get( + self.membership_list_url, + {"organization": str(self.organization.id)}, + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) - members = response.data["results"] - - # Check that user details are included - admin_member = next((m for m in members if m["user"] == str(self.admin_user.id)), None) - self.assertIsNotNone(admin_member) - self.assertEqual(admin_member["user_email"], self.admin_user.email) + self.assertEqual(response.data["results"], []) + def test_public_organization_does_not_expose_memberships_to_nonmember(self): + self.organization.public = True + self.organization.save(update_fields=["public"]) + self.authenticate_as(self.other_user) -class OrganizationAddMemberTestCase(OrganizationAPITestCase): - """Tests for adding members to organization.""" + response = self.client.get( + self.membership_list_url, + {"organization": str(self.organization.id)}, + ) - def test_add_member_admin_can_add(self): - """Organization admins can add new members.""" - self.client.force_authenticate(self.admin_user) - data = { - "user_id": str(self.other_user.id), - "role": ORGANIZATION_VIEWER, - } - response = self.client.post(f"/api/organization/{self.organization.id}/add_member/", data, format="json") - - self.assertEqual(response.status_code, status.HTTP_201_CREATED) - self.assertEqual(response.data["role"], ORGANIZATION_VIEWER) - - # Verify member was added - role = OrganizationRole.objects.get(user=self.other_user, organization=self.organization) - self.assertEqual(role.role, ORGANIZATION_VIEWER) - - def test_add_member_editor_cannot_add(self): - """Organization editors cannot add members.""" - self.client.force_authenticate(self.editor_user) - data = { - "user_id": str(self.other_user.id), - "role": ORGANIZATION_VIEWER, - } - response = self.client.post(f"/api/organization/{self.organization.id}/add_member/", data, format="json") - - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - - def test_add_member_requires_user_id(self): - """Adding a member requires a user_id.""" - self.client.force_authenticate(self.admin_user) - data = {"role": ORGANIZATION_VIEWER} - response = self.client.post(f"/api/organization/{self.organization.id}/add_member/", data, format="json") - - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["results"], []) - def test_add_member_defaults_to_viewer_role(self): - """If no role is specified, default is VIEWER.""" - self.client.force_authenticate(self.admin_user) - data = {"user_id": str(self.other_user.id)} - response = self.client.post(f"/api/organization/{self.organization.id}/add_member/", data, format="json") - - self.assertEqual(response.status_code, status.HTTP_201_CREATED) - self.assertEqual(response.data["role"], ORGANIZATION_VIEWER) + def test_inactive_member_cannot_list_memberships(self): + self.authenticate_as(self.inactive_user) - def test_add_member_invalid_role_rejected(self): - """Adding a member with an invalid role is rejected.""" - self.client.force_authenticate(self.admin_user) - data = { - "user_id": str(self.other_user.id), - "role": "invalid_role", - } - response = self.client.post(f"/api/organization/{self.organization.id}/add_member/", data, format="json") - - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + response = self.client.get( + self.membership_list_url, + {"organization": str(self.organization.id)}, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["results"], []) + + def test_membership_response_includes_user_name_fields(self): + self.authenticate_as(self.organization_admin) + + response = self.client.get( + self.membership_list_url, + {"user": str(self.organization_admin.id)}, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["count"], 1) + membership = response.data["results"][0] + self.assertEqual(membership["user_email"], self.organization_admin.email) + self.assertEqual(membership["user_first_name"], "Admin") + self.assertEqual(membership["user_last_name"], "User") + self.assertEqual(membership["user_name"], "Admin User") + + def test_member_can_retrieve_membership_in_same_organization(self): + self.authenticate_as(self.viewer_user) + + response = self.client.get(self.membership_detail_url(self.admin_membership)) + + self.assertEqual(response.status_code, status.HTTP_200_OK) - def test_add_nonexistent_user_fails(self): - """Adding a non-existent user fails.""" - self.client.force_authenticate(self.admin_user) + def test_nonmember_cannot_retrieve_membership(self): + self.authenticate_as(self.other_user) + + response = self.client.get(self.membership_detail_url(self.admin_membership)) + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + +class OrganizationMembershipCreationTestCase(OrganizationAPITestCase): + def test_direct_membership_creation_is_not_allowed(self): + self.authenticate_as(self.organization_admin) data = { - "user_id": "00000000-0000-0000-0000-000000000000", + "organization": str(self.organization.id), + "user": str(self.other_user.id), "role": ORGANIZATION_VIEWER, + "status": ORGANIZATION_ROLE_STATUS_ACTIVE, } - response = self.client.post(f"/api/organization/{self.organization.id}/add_member/", data, format="json") - - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_update_existing_member_role(self): - """Adding a member that already exists updates their role.""" - # other_user is already a viewer - OrganizationRole.objects.create( - user=self.other_user, - organization=self.organization, - role=ORGANIZATION_VIEWER, - status=ORGANIZATION_ROLE_STATUS_ACTIVE, + response = self.client.post(self.membership_list_url, data, format="json") + + self.assertEqual(response.status_code, status.HTTP_405_METHOD_NOT_ALLOWED) + self.assertFalse( + OrganizationRole.objects.filter( + organization=self.organization, + user=self.other_user, + ).exists() ) - - self.client.force_authenticate(self.admin_user) - data = { - "user_id": str(self.other_user.id), - "role": ORGANIZATION_EDITOR, - } - response = self.client.post(f"/api/organization/{self.organization.id}/add_member/", data, format="json") - + + def test_membership_viewset_has_no_sync_creation_handler(self): + self.assertFalse(hasattr(OrganizationMemberViewSet, "create_from_changes")) + + +class OrganizationMembershipUpdateTestCase(OrganizationAPITestCase): + def test_active_admin_can_update_member_role(self): + self.authenticate_as(self.organization_admin) + + response = self.client.patch( + self.membership_detail_url(self.viewer_membership), + {"role": ORGANIZATION_EDITOR}, + format="json", + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(response.data["role"], ORGANIZATION_EDITOR) + self.viewer_membership.refresh_from_db() + self.assertEqual(self.viewer_membership.role, ORGANIZATION_EDITOR) + + def test_editor_cannot_update_membership(self): + self.authenticate_as(self.editor_user) + + response = self.client.patch( + self.membership_detail_url(self.viewer_membership), + {"role": ORGANIZATION_EDITOR}, + format="json", + ) + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + def test_nonmember_cannot_update_membership(self): + self.authenticate_as(self.other_user) -class OrganizationUpdateMemberTestCase(OrganizationAPITestCase): - """Tests for updating member roles.""" + response = self.client.patch( + self.membership_detail_url(self.viewer_membership), + {"role": ORGANIZATION_EDITOR}, + format="json", + ) + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + def test_invalid_role_is_rejected(self): + self.authenticate_as(self.organization_admin) - def test_update_member_admin_can_update_role(self): - """Organization admins can update member roles.""" - membership = OrganizationRole.objects.get(user=self.viewer_user, organization=self.organization) - - self.client.force_authenticate(self.admin_user) - data = {"role": ORGANIZATION_EDITOR} response = self.client.patch( - f"/api/organization/{self.organization.id}/update_member/?member_id={membership.id}", - data, - format="json" + self.membership_detail_url(self.viewer_membership), + {"role": "invalid-role"}, + format="json", ) - + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + def test_membership_user_and_organization_cannot_be_reassigned(self): + other_organization = Organization.objects.create(name="Other Organization") + self.authenticate_as(self.organization_admin) + + response = self.client.patch( + self.membership_detail_url(self.viewer_membership), + { + "user": str(self.other_user.id), + "organization": str(other_organization.id), + "description": "Updated description", + }, + format="json", + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) - membership.refresh_from_db() - self.assertEqual(membership.role, ORGANIZATION_EDITOR) - - def test_update_member_editor_cannot_update(self): - """Organization editors cannot update member roles.""" - membership = OrganizationRole.objects.get(user=self.viewer_user, organization=self.organization) - - self.client.force_authenticate(self.editor_user) - data = {"role": ORGANIZATION_ADMIN} + self.viewer_membership.refresh_from_db() + self.assertEqual(self.viewer_membership.user, self.viewer_user) + self.assertEqual(self.viewer_membership.organization, self.organization) + self.assertEqual(self.viewer_membership.description, "Updated description") + + def test_last_active_admin_cannot_be_demoted(self): + self.authenticate_as(self.organization_admin) + response = self.client.patch( - f"/api/organization/{self.organization.id}/update_member/?member_id={membership.id}", - data, - format="json" + self.membership_detail_url(self.admin_membership), + {"role": ORGANIZATION_EDITOR}, + format="json", ) - - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - def test_update_member_requires_member_id(self): - """Updating a member requires member_id parameter.""" - self.client.force_authenticate(self.admin_user) - data = {"role": ORGANIZATION_EDITOR} + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.admin_membership.refresh_from_db() + self.assertEqual(self.admin_membership.role, ORGANIZATION_ADMIN) + + def test_last_active_admin_cannot_be_deactivated(self): + self.authenticate_as(self.organization_admin) + response = self.client.patch( - f"/api/organization/{self.organization.id}/update_member/", - data, - format="json" + self.membership_detail_url(self.admin_membership), + {"status": ORGANIZATION_ROLE_STATUS_INACTIVE}, + format="json", ) - + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.admin_membership.refresh_from_db() + self.assertEqual( + self.admin_membership.status, + ORGANIZATION_ROLE_STATUS_ACTIVE, + ) + + def test_admin_can_be_demoted_when_another_active_admin_exists(self): + second_admin = testdata.user(email="second-admin@test.com") + OrganizationRole.objects.create( + user=second_admin, + organization=self.organization, + role=ORGANIZATION_ADMIN, + status=ORGANIZATION_ROLE_STATUS_ACTIVE, + ) + self.authenticate_as(self.organization_admin) - def test_update_member_invalid_member_id_fails(self): - """Updating with an invalid member_id fails.""" - self.client.force_authenticate(self.admin_user) - data = {"role": ORGANIZATION_EDITOR} response = self.client.patch( - f"/api/organization/{self.organization.id}/update_member/?member_id=00000000-0000-0000-0000-000000000000", - data, - format="json" + self.membership_detail_url(self.admin_membership), + {"role": ORGANIZATION_EDITOR}, + format="json", ) - - self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.admin_membership.refresh_from_db() + self.assertEqual(self.admin_membership.role, ORGANIZATION_EDITOR) -class OrganizationRemoveMemberTestCase(OrganizationAPITestCase): - """Tests for removing members from organization.""" +class OrganizationMembershipDeleteTestCase(OrganizationAPITestCase): + def test_active_admin_can_remove_nonadmin_member(self): + self.authenticate_as(self.organization_admin) - def test_remove_member_admin_can_remove(self): - """Organization admins can remove members.""" - membership = OrganizationRole.objects.get(user=self.viewer_user, organization=self.organization) - - self.client.force_authenticate(self.admin_user) response = self.client.delete( - f"/api/organization/{self.organization.id}/update_member/?member_id={membership.id}" + self.membership_detail_url(self.viewer_membership) ) - + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) self.assertFalse( - OrganizationRole.objects.filter( - user=self.viewer_user, organization=self.organization - ).exists() + OrganizationRole.objects.filter(id=self.viewer_membership.id).exists() ) - def test_remove_member_editor_cannot_remove(self): - """Organization editors cannot remove members.""" - membership = OrganizationRole.objects.get(user=self.viewer_user, organization=self.organization) - - self.client.force_authenticate(self.editor_user) + def test_editor_cannot_remove_membership(self): + self.authenticate_as(self.editor_user) + response = self.client.delete( - f"/api/organization/{self.organization.id}/update_member/?member_id={membership.id}" + self.membership_detail_url(self.viewer_membership) ) - - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - def test_remove_member_non_member_cannot_remove(self): - """Non-members cannot remove members.""" - membership = OrganizationRole.objects.get(user=self.viewer_user, organization=self.organization) - - self.client.force_authenticate(self.other_user) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + def test_nonmember_cannot_remove_membership(self): + self.authenticate_as(self.other_user) + response = self.client.delete( - f"/api/organization/{self.organization.id}/update_member/?member_id={membership.id}" + self.membership_detail_url(self.viewer_membership) ) - - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) -class OrganizationPermissionEnforcementTestCase(OrganizationAPITestCase): - """Tests for permission enforcement across different roles.""" + def test_last_active_admin_cannot_be_removed(self): + self.authenticate_as(self.organization_admin) - def test_admin_can_manage_settings_members_and_roles(self): - """Admins have full management access.""" - self.client.force_authenticate(self.admin_user) - - # Can update organization - data = {"name": "Updated"} - response = self.client.patch(f"/api/organization/{self.organization.id}/", data, format="json") - self.assertEqual(response.status_code, status.HTTP_200_OK) - - # Can add members - data = {"user_id": str(self.other_user.id), "role": ORGANIZATION_VIEWER} - response = self.client.post(f"/api/organization/{self.organization.id}/add_member/", data, format="json") - self.assertEqual(response.status_code, status.HTTP_201_CREATED) + response = self.client.delete( + self.membership_detail_url(self.admin_membership) + ) - def test_editor_cannot_manage_settings_or_members(self): - """Editors cannot manage settings or members.""" - self.client.force_authenticate(self.editor_user) - - # Cannot update organization - data = {"name": "Updated"} - response = self.client.patch(f"/api/organization/{self.organization.id}/", data, format="json") - self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - - # Cannot add members - data = {"user_id": str(self.other_user.id), "role": ORGANIZATION_VIEWER} - response = self.client.post(f"/api/organization/{self.organization.id}/add_member/", data, format="json") - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - - def test_viewer_has_read_only_access(self): - """Viewers have read-only access.""" - self.client.force_authenticate(self.viewer_user) - - # Can view organization - response = self.client.get(f"/api/organization/{self.organization.id}/") - self.assertEqual(response.status_code, status.HTTP_200_OK) - - # Can view members - response = self.client.get(f"/api/organization/{self.organization.id}/members/") - self.assertEqual(response.status_code, status.HTTP_200_OK) - - # Cannot update organization - data = {"name": "Updated"} - response = self.client.patch(f"/api/organization/{self.organization.id}/", data, format="json") self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertTrue( + OrganizationRole.objects.filter(id=self.admin_membership.id).exists() + ) class OrganizationPaginationTestCase(OrganizationAPITestCase): - """Tests for pagination in organization endpoints.""" - - def test_organization_list_pagination(self): - """Organization list should be paginated.""" - # Create multiple organizations - for i in range(25): - org = Organization.objects.create(name=f"Org {i}") + def test_organization_list_is_paginated(self): + for index in range(25): + organization = Organization.objects.create(name="Org {}".format(index)) OrganizationRole.objects.create( - user=self.admin_user, - organization=org, + user=self.organization_admin, + organization=organization, role=ORGANIZATION_ADMIN, status=ORGANIZATION_ROLE_STATUS_ACTIVE, ) - - self.client.force_authenticate(self.admin_user) - response = self.client.get("/api/organization/") - + self.authenticate_as(self.organization_admin) + + response = self.client.get(self.organization_list_url) + self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertIn("results", response.data) - self.assertIn("count", response.data) - self.assertEqual(len(response.data["results"]), 20) # Default page size - - def test_member_list_pagination(self): - """Member list should be paginated.""" - # Add many members - for i in range(25): - user = testdata.user(email=f"user{i}@test.com") + self.assertEqual(response.data["count"], 26) + self.assertEqual(len(response.data["results"]), 20) + + def test_membership_list_is_paginated(self): + for index in range(25): + user = testdata.user(email="member{}@test.com".format(index)) OrganizationRole.objects.create( user=user, organization=self.organization, role=ORGANIZATION_VIEWER, status=ORGANIZATION_ROLE_STATUS_ACTIVE, ) - - self.client.force_authenticate(self.admin_user) - response = self.client.get(f"/api/organization/{self.organization.id}/members/") - + self.authenticate_as(self.organization_admin) + + response = self.client.get( + self.membership_list_url, + {"organization": str(self.organization.id)}, + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertIn("results", response.data) - self.assertIn("count", response.data) + self.assertEqual(response.data["count"], 29) + self.assertEqual(len(response.data["results"]), 20) From 05260f0dfbe7181337b7c08a290b3748e15f8955 Mon Sep 17 00:00:00 2001 From: Ajay Nair Date: Sat, 1 Aug 2026 10:04:52 -0400 Subject: [PATCH 06/10] Feat: fixed tests --- contentcuration/contentcuration/tests/test_organization.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contentcuration/contentcuration/tests/test_organization.py b/contentcuration/contentcuration/tests/test_organization.py index 29a38d2b95..1c6e1ff2bd 100644 --- a/contentcuration/contentcuration/tests/test_organization.py +++ b/contentcuration/contentcuration/tests/test_organization.py @@ -166,12 +166,12 @@ def test_create_organization_requires_authentication(self): format="json", ) - self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) def test_list_organizations_requires_authentication(self): response = APIClient().get(self.organization_list_url) - self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) class OrganizationRetrieveUpdateDeleteTestCase(OrganizationAPITestCase): From 0a942d220d58994af7660e2392d15d4532ac6d40 Mon Sep 17 00:00:00 2001 From: Ajay Nair Date: Mon, 3 Aug 2026 08:12:07 -0400 Subject: [PATCH 07/10] Feat: Fixed tests --- .../contentcuration/viewsets/organization.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/contentcuration/contentcuration/viewsets/organization.py b/contentcuration/contentcuration/viewsets/organization.py index d6ed3431a8..8a779a5f56 100644 --- a/contentcuration/contentcuration/viewsets/organization.py +++ b/contentcuration/contentcuration/viewsets/organization.py @@ -101,12 +101,6 @@ def _is_site_admin(user): return bool(getattr(user, "is_admin", False)) -def _get_member_name(item): - first_name = item.pop("user__first_name", "") or "" - last_name = item.pop("user__last_name", "") or "" - return "{} {}".format(first_name, last_name).strip() - - class OrganizationViewSet( ValuesViewset, RESTCreateModelMixin, @@ -243,9 +237,17 @@ class OrganizationMemberViewSet( "user_email": "user__email", "user_first_name": "user__first_name", "user_last_name": "user__last_name", - "user_name": _get_member_name, } + def consolidate(self, items, queryset): + """Add the display name after field mappings have been applied.""" + for item in items: + item["user_name"] = "{} {}".format( + item.get("user_first_name", "") or "", + item.get("user_last_name", "") or "", + ).strip() + return items + def get_queryset(self): """ Return memberships belonging to organizations the user may inspect. From 6a488b5852946e4607d3e7ce671f4f93e1ddceb5 Mon Sep 17 00:00:00 2001 From: Ajay Nair Date: Mon, 3 Aug 2026 08:24:41 -0400 Subject: [PATCH 08/10] Feat: Fixed tests --- contentcuration/contentcuration/tests/test_organization.py | 4 +--- contentcuration/contentcuration/viewsets/organization.py | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/contentcuration/contentcuration/tests/test_organization.py b/contentcuration/contentcuration/tests/test_organization.py index 1c6e1ff2bd..b64284bf0f 100644 --- a/contentcuration/contentcuration/tests/test_organization.py +++ b/contentcuration/contentcuration/tests/test_organization.py @@ -524,9 +524,7 @@ def test_nonmember_cannot_remove_membership(self): def test_last_active_admin_cannot_be_removed(self): self.authenticate_as(self.organization_admin) - response = self.client.delete( - self.membership_detail_url(self.admin_membership) - ) + response = self.client.delete(self.membership_detail_url(self.admin_membership)) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertTrue( diff --git a/contentcuration/contentcuration/viewsets/organization.py b/contentcuration/contentcuration/viewsets/organization.py index 8a779a5f56..18db54c077 100644 --- a/contentcuration/contentcuration/viewsets/organization.py +++ b/contentcuration/contentcuration/viewsets/organization.py @@ -322,9 +322,7 @@ def _ensure_not_last_active_admin( return resulting_role = new_role if new_role is not None else membership.role - resulting_status = ( - new_status if new_status is not None else membership.status - ) + resulting_status = new_status if new_status is not None else membership.status if ( resulting_role == ORGANIZATION_ADMIN From 3639f872b297d7f5d5e125126935f1f9b6adceb8 Mon Sep 17 00:00:00 2001 From: Ajay Nair Date: Mon, 3 Aug 2026 08:28:07 -0400 Subject: [PATCH 09/10] Feat: Fixed tests --- contentcuration/contentcuration/urls.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/contentcuration/contentcuration/urls.py b/contentcuration/contentcuration/urls.py index 94588c3dab..e885f88893 100644 --- a/contentcuration/contentcuration/urls.py +++ b/contentcuration/contentcuration/urls.py @@ -13,6 +13,7 @@ 1. Add an import: from blog import urls as blog_urls 2. Add a URL to urlpatterns: re_path(r'^blog/', include(blog_urls)) """ + import uuid import django_js_reverse.views as django_js_reverse_views @@ -57,7 +58,10 @@ from contentcuration.viewsets.feedback import RecommendationsInteractionEventViewSet from contentcuration.viewsets.file import FileViewSet from contentcuration.viewsets.invitation import InvitationViewSet -from contentcuration.viewsets.organization import OrganizationViewSet, OrganizationMemberViewSet +from contentcuration.viewsets.organization import ( + OrganizationViewSet, + OrganizationMemberViewSet, +) from contentcuration.viewsets.recommendation import RecommendationView from contentcuration.viewsets.sync.endpoint import SyncView from contentcuration.viewsets.user import AdminUserViewSet @@ -85,7 +89,9 @@ def get_redirect_url(self, *args, **kwargs): router.register(r"user", UserViewSet) router.register(r"invitation", InvitationViewSet) router.register(r"organization", OrganizationViewSet, basename="organization") -router.register(r"organization-members", OrganizationMemberViewSet, basename="organization-members") +router.register( + r"organization-members", OrganizationMemberViewSet, basename="organization-members" +) router.register(r"contentnode", ContentNodeViewSet) router.register(r"assessmentitem", AssessmentItemViewSet) router.register(r"admin-users", AdminUserViewSet, basename="admin-users") From bfe04dee13afae6df89008968c38d59979e2d091 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci-lite[bot]" <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Date: Tue, 4 Aug 2026 18:19:00 +0000 Subject: [PATCH 10/10] [pre-commit.ci lite] apply automatic fixes --- .../tests/test_organization.py | 1 - contentcuration/contentcuration/urls.py | 7 ++--- .../contentcuration/viewsets/organization.py | 31 ++++++++++--------- 3 files changed, 18 insertions(+), 21 deletions(-) diff --git a/contentcuration/contentcuration/tests/test_organization.py b/contentcuration/contentcuration/tests/test_organization.py index b64284bf0f..f3c5bf3bde 100644 --- a/contentcuration/contentcuration/tests/test_organization.py +++ b/contentcuration/contentcuration/tests/test_organization.py @@ -1,5 +1,4 @@ """Tests for organization and organization membership API endpoints.""" - from django.urls import reverse from rest_framework import status from rest_framework.test import APIClient diff --git a/contentcuration/contentcuration/urls.py b/contentcuration/contentcuration/urls.py index e885f88893..d013650272 100644 --- a/contentcuration/contentcuration/urls.py +++ b/contentcuration/contentcuration/urls.py @@ -13,7 +13,6 @@ 1. Add an import: from blog import urls as blog_urls 2. Add a URL to urlpatterns: re_path(r'^blog/', include(blog_urls)) """ - import uuid import django_js_reverse.views as django_js_reverse_views @@ -58,10 +57,8 @@ from contentcuration.viewsets.feedback import RecommendationsInteractionEventViewSet from contentcuration.viewsets.file import FileViewSet from contentcuration.viewsets.invitation import InvitationViewSet -from contentcuration.viewsets.organization import ( - OrganizationViewSet, - OrganizationMemberViewSet, -) +from contentcuration.viewsets.organization import OrganizationMemberViewSet +from contentcuration.viewsets.organization import OrganizationViewSet from contentcuration.viewsets.recommendation import RecommendationView from contentcuration.viewsets.sync.endpoint import SyncView from contentcuration.viewsets.user import AdminUserViewSet diff --git a/contentcuration/contentcuration/viewsets/organization.py b/contentcuration/contentcuration/viewsets/organization.py index 18db54c077..a7a594f258 100644 --- a/contentcuration/contentcuration/viewsets/organization.py +++ b/contentcuration/contentcuration/viewsets/organization.py @@ -1,27 +1,28 @@ from django.db import transaction from django.db.models import Q -from django_filters.rest_framework import CharFilter, FilterSet +from django_filters.rest_framework import CharFilter +from django_filters.rest_framework import FilterSet from rest_framework import serializers -from rest_framework.exceptions import PermissionDenied, ValidationError +from rest_framework.exceptions import PermissionDenied +from rest_framework.exceptions import ValidationError from rest_framework.permissions import IsAuthenticated +from contentcuration.constants.organization_roles import ORGANIZATION_ADMIN +from contentcuration.constants.organization_roles import ORGANIZATION_ROLE_STATUS_ACTIVE from contentcuration.constants.organization_roles import ( - ORGANIZATION_ADMIN, - ORGANIZATION_ROLE_STATUS_ACTIVE, - ORGANIZATION_VIEWER, organization_role_status_choices, ) -from contentcuration.models import Organization, OrganizationRole +from contentcuration.constants.organization_roles import ORGANIZATION_VIEWER +from contentcuration.models import Organization +from contentcuration.models import OrganizationRole from contentcuration.utils.pagination import ValuesViewsetPageNumberPagination -from contentcuration.viewsets.base import ( - BulkListSerializer, - BulkModelSerializer, - RESTCreateModelMixin, - RESTDestroyModelMixin, - RESTUpdateModelMixin, - ReadOnlyValuesViewset, - ValuesViewset, -) +from contentcuration.viewsets.base import BulkListSerializer +from contentcuration.viewsets.base import BulkModelSerializer +from contentcuration.viewsets.base import ReadOnlyValuesViewset +from contentcuration.viewsets.base import RESTCreateModelMixin +from contentcuration.viewsets.base import RESTDestroyModelMixin +from contentcuration.viewsets.base import RESTUpdateModelMixin +from contentcuration.viewsets.base import ValuesViewset class OrganizationSerializer(BulkModelSerializer):