From 32edb666692eb40b3a98fd794f0c591a333cd302 Mon Sep 17 00:00:00 2001 From: Ajay Nair Date: Mon, 13 Jul 2026 21:37:51 -0400 Subject: [PATCH 01/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] 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/14] [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): From e6e48471667aac71baece5adb225e811afacb14f Mon Sep 17 00:00:00 2001 From: Ajay Nair Date: Tue, 11 Aug 2026 13:35:18 -0400 Subject: [PATCH 11/14] Feat: Addressed comments --- contentcuration/contentcuration/models.py | 146 ++++++++++++- .../tests/{ => viewsets}/test_organization.py | 81 ++++++- .../contentcuration/viewsets/organization.py | 199 +++++++++--------- 3 files changed, 309 insertions(+), 117 deletions(-) rename contentcuration/contentcuration/tests/{ => viewsets}/test_organization.py (88%) diff --git a/contentcuration/contentcuration/models.py b/contentcuration/contentcuration/models.py index 8c7cee3919..6a452e6769 100644 --- a/contentcuration/contentcuration/models.py +++ b/contentcuration/contentcuration/models.py @@ -79,6 +79,10 @@ from contentcuration.constants import feedback from contentcuration.constants import user_history from contentcuration.constants.contentnode import kind_activity_map +from contentcuration.constants.organization_roles import ORGANIZATION_ADMIN +from contentcuration.constants.organization_roles import ORGANIZATION_EDITOR +from contentcuration.constants.organization_roles import ORGANIZATION_ROLE_STATUS_ACTIVE +from contentcuration.constants.organization_roles import ORGANIZATION_VIEWER from contentcuration.constants.organization_roles import organization_role_choices from contentcuration.constants.organization_roles import ( organization_role_status_choices, @@ -685,13 +689,39 @@ def filter_view_queryset(cls, queryset, user): @classmethod def filter_edit_queryset(cls, queryset, user): - if user.is_anonymous: + user_id = not user.is_anonymous and user.id + + # it won't return anything + if not user_id: return queryset.none() + edit = Exists( + User.editable_channels.through.objects.filter( + user_id=user_id, channel_id=OuterRef("id") + ) + ) + + organization_edit = Exists( + OrganizationRole.objects.filter( + user_id=user_id, + organization_id=OuterRef("organization_id"), + status=ORGANIZATION_ROLE_STATUS_ACTIVE, + role__in=( + ORGANIZATION_ADMIN, + ORGANIZATION_EDITOR, + ), + ) + ) + + queryset = queryset.annotate( + edit=edit, + organization_edit=organization_edit, + ) + if user.is_admin: return queryset - return queryset.filter(pk=user.pk) + return queryset.filter(Q(edit=True) | Q(organization_edit=True)) @classmethod def get_for_email(cls, email, deleted=False, **filters): @@ -818,7 +848,7 @@ def file_on_disk_name(instance, filename): def generate_file_on_disk_name(checksum, filename): - """ Separated from file_on_disk_name to allow for simple way to check if has already exists """ + """Separated from file_on_disk_name to allow for simple way to check if has already exists""" h = checksum basename, ext = os.path.splitext(filename) directory = os.path.join(settings.STORAGE_ROOT, h[0], h[1]) @@ -845,7 +875,7 @@ def object_storage_name(instance, filename): def generate_object_storage_name(checksum, filename, default_ext=""): - """ Separated from file_on_disk_name to allow for simple way to check if has already exists """ + """Separated from file_on_disk_name to allow for simple way to check if has already exists""" h = checksum basename, actual_ext = os.path.splitext(filename) ext = actual_ext if actual_ext else default_ext @@ -1061,7 +1091,7 @@ class ChannelModelManager(models.Manager.from_queryset(ChannelModelQuerySet)): class Channel(models.Model): - """ Permissions come from association with organizations """ + """Permissions come from association with organizations""" id = UUIDField(primary_key=True, default=uuid.uuid4) name = models.CharField(max_length=200, blank=True) @@ -1238,35 +1268,60 @@ def filter_view_queryset(cls, queryset, user): if user_id: filters = dict(user_id=user_id, channel_id=OuterRef("id")) + edit = Exists( User.editable_channels.through.objects.filter(**filters).values( "user_id" ) ) + view = Exists( User.view_only_channels.through.objects.filter(**filters).values( "user_id" ) ) + + organization_view = Exists( + OrganizationRole.objects.filter( + user_id=user_id, + organization_id=OuterRef("organization_id"), + status=ORGANIZATION_ROLE_STATUS_ACTIVE, + role__in=( + ORGANIZATION_ADMIN, + ORGANIZATION_EDITOR, + ORGANIZATION_VIEWER, + ), + ) + ) else: edit = boolean_val(False) view = boolean_val(False) + organization_view = boolean_val(False) queryset = queryset.annotate( edit=edit, view=view, + organization_view=organization_view, ) if user_id and user.is_admin: return queryset permission_filter = Q() + if user_id: pending_channels = Invitation.objects.filter( - email=user_email, revoked=False, declined=False, accepted=False + email=user_email, + revoked=False, + declined=False, + accepted=False, ).values_list("channel_id", flat=True) + permission_filter = ( - Q(view=True) | Q(edit=True) | Q(deleted=False, id__in=pending_channels) + Q(view=True) + | Q(edit=True) + | Q(organization_view=True) + | Q(deleted=False, id__in=pending_channels) ) return queryset.filter(permission_filter | Q(deleted=False, public=True)) @@ -1881,6 +1936,40 @@ class Organization(models.Model): objects = CustomManager() + @classmethod + def filter_view_queryset(cls, queryset, user): + queryset = queryset.filter(deleted=False) + + if user.is_anonymous: + return queryset.filter(public=True) + + if user.is_admin: + return queryset + + return queryset.filter( + Q(public=True) + | Q( + user_roles__user=user, + user_roles__status=ORGANIZATION_ROLE_STATUS_ACTIVE, + ) + ).distinct() + + @classmethod + def filter_edit_queryset(cls, queryset, user): + queryset = queryset.filter(deleted=False) + + if user.is_anonymous: + return queryset.none() + + if user.is_admin: + return queryset + + return queryset.filter( + user_roles__user=user, + user_roles__role=ORGANIZATION_ADMIN, + user_roles__status=ORGANIZATION_ROLE_STATUS_ACTIVE, + ).distinct() + class Meta: verbose_name = "Organization" verbose_name_plural = "Organizations" @@ -1936,6 +2025,47 @@ class OrganizationRole(models.Model): ) updated_at = models.DateTimeField(auto_now=True, help_text="Last update timestamp") + @classmethod + def filter_view_queryset(cls, queryset, user): + queryset = queryset.filter( + organization__deleted=False, + ).select_related( + "organization", + "user", + ) + + if user.is_anonymous: + return queryset.none() + + if user.is_admin: + return queryset + + return queryset.filter( + organization__user_roles__user=user, + organization__user_roles__status=ORGANIZATION_ROLE_STATUS_ACTIVE, + ).distinct() + + @classmethod + def filter_edit_queryset(cls, queryset, user): + queryset = queryset.filter( + organization__deleted=False, + ).select_related( + "organization", + "user", + ) + + if user.is_anonymous: + return queryset.none() + + if user.is_admin: + return queryset + + return queryset.filter( + organization__user_roles__user=user, + organization__user_roles__role=ORGANIZATION_ADMIN, + organization__user_roles__status=ORGANIZATION_ROLE_STATUS_ACTIVE, + ).distinct() + class Meta: unique_together = ("user", "organization") verbose_name = "Organization Role" @@ -3705,7 +3835,7 @@ def save(self, *args, **kwargs): class Invitation(models.Model): - """ Invitation to edit channel """ + """Invitation to edit channel""" id = UUIDField(primary_key=True, default=uuid.uuid4) accepted = models.BooleanField(default=False) diff --git a/contentcuration/contentcuration/tests/test_organization.py b/contentcuration/contentcuration/tests/viewsets/test_organization.py similarity index 88% rename from contentcuration/contentcuration/tests/test_organization.py rename to contentcuration/contentcuration/tests/viewsets/test_organization.py index b64284bf0f..7a2eda7b3b 100644 --- a/contentcuration/contentcuration/tests/test_organization.py +++ b/contentcuration/contentcuration/tests/viewsets/test_organization.py @@ -16,11 +16,10 @@ 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 +from contentcuration.tests.base import StudioAPITestCase -class OrganizationAPITestCase(BaseAPITestCase): +class OrganizationAPITestCase(StudioAPITestCase): """Shared organization API fixtures and URL helpers.""" def setUp(self): @@ -350,7 +349,7 @@ def test_nonmember_cannot_retrieve_membership(self): class OrganizationMembershipCreationTestCase(OrganizationAPITestCase): - def test_direct_membership_creation_is_not_allowed(self): + def test_active_admin_can_create_membership(self): self.authenticate_as(self.organization_admin) data = { "organization": str(self.organization.id), @@ -361,7 +360,45 @@ def test_direct_membership_creation_is_not_allowed(self): response = self.client.post(self.membership_list_url, data, format="json") - self.assertEqual(response.status_code, status.HTTP_405_METHOD_NOT_ALLOWED) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + membership = OrganizationRole.objects.get( + organization=self.organization, + user=self.other_user, + ) + self.assertEqual(membership.role, ORGANIZATION_VIEWER) + self.assertEqual(membership.status, ORGANIZATION_ROLE_STATUS_ACTIVE) + + def test_editor_cannot_create_membership(self): + self.authenticate_as(self.editor_user) + data = { + "organization": str(self.organization.id), + "user": str(self.other_user.id), + "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_403_FORBIDDEN) + self.assertFalse( + OrganizationRole.objects.filter( + organization=self.organization, + user=self.other_user, + ).exists() + ) + + def test_nonmember_cannot_create_membership(self): + self.authenticate_as(self.other_user) + data = { + "organization": str(self.organization.id), + "user": str(self.other_user.id), + "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_403_FORBIDDEN) self.assertFalse( OrganizationRole.objects.filter( organization=self.organization, @@ -369,8 +406,38 @@ def test_direct_membership_creation_is_not_allowed(self): ).exists() ) - def test_membership_viewset_has_no_sync_creation_handler(self): - self.assertFalse(hasattr(OrganizationMemberViewSet, "create_from_changes")) + def test_invalid_role_is_rejected_on_create(self): + self.authenticate_as(self.organization_admin) + data = { + "organization": str(self.organization.id), + "user": str(self.other_user.id), + "role": "invalid-role", + "status": ORGANIZATION_ROLE_STATUS_ACTIVE, + } + + response = self.client.post(self.membership_list_url, data, format="json") + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + + def test_duplicate_membership_is_rejected(self): + self.authenticate_as(self.organization_admin) + data = { + "organization": str(self.organization.id), + "user": str(self.viewer_user.id), + "role": ORGANIZATION_EDITOR, + "status": ORGANIZATION_ROLE_STATUS_ACTIVE, + } + + response = self.client.post(self.membership_list_url, data, format="json") + + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual( + OrganizationRole.objects.filter( + organization=self.organization, + user=self.viewer_user, + ).count(), + 1, + ) class OrganizationMembershipUpdateTestCase(OrganizationAPITestCase): diff --git a/contentcuration/contentcuration/viewsets/organization.py b/contentcuration/contentcuration/viewsets/organization.py index 18db54c077..3dc8bcf4ae 100644 --- a/contentcuration/contentcuration/viewsets/organization.py +++ b/contentcuration/contentcuration/viewsets/organization.py @@ -1,10 +1,10 @@ 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, FilterSet, NumberFilter from rest_framework import serializers from rest_framework.exceptions import PermissionDenied, ValidationError from rest_framework.permissions import IsAuthenticated - +from rest_framework.response import Response from contentcuration.constants.organization_roles import ( ORGANIZATION_ADMIN, ORGANIZATION_ROLE_STATUS_ACTIVE, @@ -47,12 +47,11 @@ class Meta: class OrganizationMemberSerializer(BulkModelSerializer): """ - Write serializer for updating OrganizationRole membership records. + Write serializer for OrganizationRole membership records. - 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. + Organization and user are writable when creating a membership. They are + immutable once the membership exists; admins may update role, description, + or status. Read operations are handled by the viewset values map. """ status = serializers.ChoiceField( @@ -70,9 +69,15 @@ class Meta: "description", "status", ) - read_only_fields = ("organization", "user") list_serializer_class = BulkListSerializer + def update(self, instance, validated_data): + validated_data.pop("organization", None) + validated_data.pop("user", None) + return super(OrganizationMemberSerializer, self).update( + instance, validated_data + ) + class OrganizationFilter(FilterSet): name = CharFilter(field_name="name", lookup_expr="icontains") @@ -84,7 +89,7 @@ class Meta: class OrganizationMemberFilter(FilterSet): organization = CharFilter(field_name="organization_id") - user = CharFilter(field_name="user_id") + user = NumberFilter(field_name="user_id") class Meta: model = OrganizationRole @@ -134,47 +139,47 @@ class OrganizationViewSet( "updated_at", ) - def get_queryset(self): - """ - Return organizations visible to the current user. + # def get_queryset(self): + # """ + # 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 + # 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 - if _is_site_admin(user): - return queryset + # if _is_site_admin(user): + # return queryset - if not user.is_authenticated: - return queryset.filter(public=True) + # 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, - ) - ).distinct() + # return queryset.filter( + # Q(public=True) + # | Q( + # user_roles__user=user, + # user_roles__status=ORGANIZATION_ROLE_STATUS_ACTIVE, + # ) + # ).distinct() - def get_edit_queryset(self): - """Return organizations that the current user may modify.""" - queryset = Organization.objects.filter(deleted=False) - user = self.request.user + # 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 _is_site_admin(user): + # return queryset - if not user.is_authenticated: - return queryset.none() + # 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() + # 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.""" @@ -194,17 +199,17 @@ def perform_destroy(self, instance): class OrganizationMemberViewSet( - ReadOnlyValuesViewset, + ValuesViewset, + RESTCreateModelMixin, RESTUpdateModelMixin, RESTDestroyModelMixin, ): """ Organization membership and role API. - 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. + Active organization members may read the membership list. Active organization + admins may create, update, or remove memberships and assign roles. Site admins + may manage all memberships. """ queryset = OrganizationRole.objects.all() @@ -248,48 +253,6 @@ def consolidate(self, items, queryset): ).strip() return items - def get_queryset(self): - """ - 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 @@ -308,9 +271,15 @@ def _require_admin(self, organization): "Only active organization admins may manage membership." ) + def perform_create(self, serializer, change=None): + organization = serializer.validated_data["organization"] + self._require_admin(organization) + serializer.save() + def _ensure_not_last_active_admin( self, membership, + active_admin_count, new_role=None, new_status=None, ): @@ -330,32 +299,37 @@ def _ensure_not_last_active_admin( ): return - active_admin_ids = list( - OrganizationRole.objects.select_for_update() + if active_admin_count <= 1: + raise ValidationError( + "An organization must have at least one active admin." + ) + + def _lock_active_admins(self, organization_id): + """Lock active admin memberships in a deterministic order.""" + return list( + OrganizationRole.objects.select_for_update(of=("self",)) .filter( - organization=membership.organization, + organization_id=organization_id, role=ORGANIZATION_ADMIN, status=ORGANIZATION_ROLE_STATUS_ACTIVE, ) + .order_by("id") .values_list("id", flat=True) ) - if len(active_admin_ids) <= 1: - raise ValidationError( - "An organization must have at least one active admin." - ) - def perform_update(self, serializer): with transaction.atomic(): + active_admin_ids = self._lock_active_admins( + serializer.instance.organization_id + ) membership = ( - OrganizationRole.objects.select_for_update() + OrganizationRole.objects.select_for_update(of=("self",)) .select_related("organization", "user") .get(pk=serializer.instance.pk) ) - self._require_admin(membership.organization) - self._ensure_not_last_active_admin( membership, + active_admin_count=len(active_admin_ids), new_role=serializer.validated_data.get("role"), new_status=serializer.validated_data.get("status"), ) @@ -365,19 +339,40 @@ def perform_update(self, serializer): def perform_destroy(self, instance): with transaction.atomic(): + active_admin_ids = self._lock_active_admins(instance.organization_id) membership = ( - OrganizationRole.objects.select_for_update() + OrganizationRole.objects.select_for_update(of=("self",)) .select_related("organization", "user") .get(pk=instance.pk) ) - self._require_admin(membership.organization) self._ensure_not_last_active_admin( membership, + active_admin_count=len(active_admin_ids), new_role=ORGANIZATION_VIEWER, new_status=membership.status, ) membership.delete() + def update(self, request, *args, **kwargs): + partial = kwargs.pop("partial", False) + instance = self.get_edit_object() + + serializer = self.get_serializer( + instance, + data=request.data, + partial=partial, + ) + serializer.is_valid(raise_exception=True) + + self.perform_update(serializer) + + queryset = OrganizationRole.objects.select_related( + "organization", + "user", + ).filter(pk=serializer.instance.pk) + + return Response(self.serialize(queryset)[0]) + # The model is named OrganizationRole, while existing work may already import # OrganizationMemberViewSet. Keep this alias so either name can be registered. From 0e7f272cfb89923335a00abb69244433020947ca Mon Sep 17 00:00:00 2001 From: Ajay Nair Date: Tue, 11 Aug 2026 22:54:43 -0400 Subject: [PATCH 12/14] Feat: addressed commnets --- contentcuration/contentcuration/models.py | 48 +++++++------------ .../contentcuration/viewsets/organization.py | 9 ++-- 2 files changed, 23 insertions(+), 34 deletions(-) diff --git a/contentcuration/contentcuration/models.py b/contentcuration/contentcuration/models.py index 6a452e6769..1ce9c3b6ea 100644 --- a/contentcuration/contentcuration/models.py +++ b/contentcuration/contentcuration/models.py @@ -689,39 +689,13 @@ def filter_view_queryset(cls, queryset, user): @classmethod def filter_edit_queryset(cls, queryset, user): - user_id = not user.is_anonymous and user.id - - # it won't return anything - if not user_id: + if user.is_anonymous: return queryset.none() - edit = Exists( - User.editable_channels.through.objects.filter( - user_id=user_id, channel_id=OuterRef("id") - ) - ) - - organization_edit = Exists( - OrganizationRole.objects.filter( - user_id=user_id, - organization_id=OuterRef("organization_id"), - status=ORGANIZATION_ROLE_STATUS_ACTIVE, - role__in=( - ORGANIZATION_ADMIN, - ORGANIZATION_EDITOR, - ), - ) - ) - - queryset = queryset.annotate( - edit=edit, - organization_edit=organization_edit, - ) - if user.is_admin: return queryset - return queryset.filter(Q(edit=True) | Q(organization_edit=True)) + return queryset.filter(pk=user.pk) @classmethod def get_for_email(cls, email, deleted=False, **filters): @@ -1255,11 +1229,25 @@ def filter_edit_queryset(cls, queryset, user): user_id=user_id, channel_id=OuterRef("id") ) ) - queryset = queryset.annotate(edit=edit) + organization_edit = Exists( + OrganizationRole.objects.filter( + user_id=user_id, + organization_id=OuterRef("organization_id"), + status=ORGANIZATION_ROLE_STATUS_ACTIVE, + role__in=( + ORGANIZATION_ADMIN, + ORGANIZATION_EDITOR, + ), + ) + ) + queryset = queryset.annotate( + edit=edit, + organization_edit=organization_edit, + ) if user.is_admin: return queryset - return queryset.filter(edit=True) + return queryset.filter(Q(edit=True) | Q(organization_edit=True)) @classmethod def filter_view_queryset(cls, queryset, user): diff --git a/contentcuration/contentcuration/viewsets/organization.py b/contentcuration/contentcuration/viewsets/organization.py index e91ea9f131..891b5b2721 100644 --- a/contentcuration/contentcuration/viewsets/organization.py +++ b/contentcuration/contentcuration/viewsets/organization.py @@ -1,23 +1,24 @@ from django.db import transaction -from django.db.models import Q from django_filters.rest_framework import CharFilter from django_filters.rest_framework import FilterSet from django_filters.rest_framework import NumberFilter +from django_filters.rest_framework import UUIDFilter from rest_framework import serializers from rest_framework.exceptions import PermissionDenied from rest_framework.exceptions import ValidationError from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response +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_VIEWER from contentcuration.constants.organization_roles import ( organization_role_status_choices, ) -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 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 @@ -88,7 +89,7 @@ class Meta: class OrganizationMemberFilter(FilterSet): - organization = CharFilter(field_name="organization_id") + organization = UUIDFilter(field_name="organization_id") user = NumberFilter(field_name="user_id") class Meta: From 82f09900fd02f2420d27a7b8192664f806126505 Mon Sep 17 00:00:00 2001 From: Ajay Nair Date: Wed, 12 Aug 2026 18:46:09 -0400 Subject: [PATCH 13/14] Feat: addressed comments --- contentcuration/contentcuration/models.py | 30 +++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/contentcuration/contentcuration/models.py b/contentcuration/contentcuration/models.py index 1ce9c3b6ea..0ba9c418dd 100644 --- a/contentcuration/contentcuration/models.py +++ b/contentcuration/contentcuration/models.py @@ -1249,6 +1249,36 @@ def filter_edit_queryset(cls, queryset, user): return queryset.filter(Q(edit=True) | Q(organization_edit=True)) + @classmethod + def filter_delete_queryset(cls, queryset, user): + user_id = not user.is_anonymous and user.id + + if not user_id: + return queryset.none() + + edit = Exists( + User.editable_channels.through.objects.filter( + user_id=user_id, channel_id=OuterRef("id") + ) + ) + organization_delete = Exists( + OrganizationRole.objects.filter( + user_id=user_id, + organization_id=OuterRef("organization_id"), + status=ORGANIZATION_ROLE_STATUS_ACTIVE, + role=ORGANIZATION_ADMIN, + ) + ) + queryset = queryset.annotate( + edit=edit, + organization_delete=organization_delete, + ) + + if user.is_admin: + return queryset + + return queryset.filter(Q(edit=True) | Q(organization_delete=True)) + @classmethod def filter_view_queryset(cls, queryset, user): user_id = not user.is_anonymous and user.id From 86d6528d39440fbdbf604c5d79003e88bf6e8774 Mon Sep 17 00:00:00 2001 From: Ajay Nair Date: Thu, 13 Aug 2026 10:09:16 -0400 Subject: [PATCH 14/14] Feat: Addressed comments --- .../contentcuration/viewsets/base.py | 26 ++++++++++++++----- .../contentcuration/viewsets/channel.py | 16 ++++++++---- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/contentcuration/contentcuration/viewsets/base.py b/contentcuration/contentcuration/viewsets/base.py index 3a03c45a9b..678727944c 100644 --- a/contentcuration/contentcuration/viewsets/base.py +++ b/contentcuration/contentcuration/viewsets/base.py @@ -38,7 +38,6 @@ from contentcuration.viewsets.sync.utils import generate_update_event from contentcuration.viewsets.sync.utils import log_sync_exception - logger = logging.getLogger(__name__) @@ -188,9 +187,9 @@ def create(self, validated_data): validated_data[field_name], relation_info.related_model ): # Trying to set a foreign key but do not have the object, only the key - validated_data[ - relation_info.model_field.attname - ] = validated_data.pop(field_name) + validated_data[relation_info.model_field.attname] = ( + validated_data.pop(field_name) + ) instance = ModelClass(**validated_data) @@ -598,6 +597,16 @@ def get_edit_queryset(self): return queryset.model.filter_edit_queryset(queryset, self.request.user) return self.get_queryset() + def get_delete_queryset(self): + """ + Return a filtered copy of the queryset to only the objects + that a user is able to delete. + """ + queryset = super(BaseValuesViewset, self).get_queryset() + if hasattr(queryset.model, "filter_delete_queryset"): + return queryset.model.filter_delete_queryset(queryset, self.request.user) + return self.get_edit_queryset() + def _get_lookup_filter(self): lookup_url_kwarg = self.lookup_url_kwarg or self.lookup_field @@ -635,6 +644,9 @@ def get_object(self): def get_edit_object(self): return self._get_object_from_queryset(self.get_edit_queryset()) + def get_delete_object(self): + return self._get_object_from_queryset(self.get_delete_queryset()) + def annotate_queryset(self, queryset): return queryset @@ -747,7 +759,7 @@ def perform_destroy(self, instance): def delete_from_changes(self, changes): errors = [] - queryset = self.get_edit_queryset().order_by() + queryset = self.get_delete_queryset().order_by() for change in changes: try: instance = queryset.get(**dict(self.values_from_key(change["key"]))) @@ -766,7 +778,7 @@ def delete_from_changes(self, changes): class RESTDestroyModelMixin(DestroyModelMixin): def destroy(self, request, *args, **kwargs): - instance = self.get_edit_object() + instance = self.get_delete_object() self.perform_destroy(instance) return Response(status=HTTP_204_NO_CONTENT) @@ -928,7 +940,7 @@ class BulkDeleteMixin(DestroyModelMixin): def delete_from_changes(self, changes): keys = [change["key"] for change in changes] queryset = self.filter_queryset_from_keys( - self.get_edit_queryset(), keys + self.get_delete_queryset(), keys ).order_by() errors = [] try: diff --git a/contentcuration/contentcuration/viewsets/channel.py b/contentcuration/contentcuration/viewsets/channel.py index 3175e8180a..9b94fbf344 100644 --- a/contentcuration/contentcuration/viewsets/channel.py +++ b/contentcuration/contentcuration/viewsets/channel.py @@ -29,6 +29,7 @@ from le_utils.constants import roles from rest_framework import serializers from rest_framework.decorators import action +from rest_framework.exceptions import PermissionDenied from rest_framework.exceptions import ValidationError from rest_framework.permissions import AllowAny from rest_framework.permissions import IsAuthenticated @@ -483,6 +484,9 @@ def create(self, request, *args, **kwargs): def destroy(self, request, *args, **kwargs): instance = self.get_edit_object() + if not self.get_delete_queryset().filter(pk=instance.pk).exists(): + raise PermissionDenied("You do not have permission to delete this channel.") + self.perform_destroy(instance) Change.create_change( generate_update_event( @@ -686,9 +690,9 @@ def publish_next(self, pk, use_staging_tree=False): channel.id, CHANNEL, { - "draft_token": draft_token.token - if draft_token - else None, + "draft_token": ( + draft_token.token if draft_token else None + ), }, channel_id=channel.id, ), @@ -1171,7 +1175,9 @@ def filter_keywords(self, queryset, name, value): or_, (Q(editors__last_name__icontains=k) for k in keywords) ) editors_email = reduce(or_, (Q(editors__email__icontains=k) for k in keywords)) - return queryset.annotate(primary_token=primary_token_subquery,).filter( + return queryset.annotate( + primary_token=primary_token_subquery, + ).filter( Q(name__icontains=value) | Q(pk__istartswith=value) | Q(primary_token=value.replace("-", "")) @@ -1349,7 +1355,7 @@ def annotate_queryset(self, queryset): class SettingsChannelSerializer(BulkModelSerializer): - """ Used for displaying list of user's channels on settings page """ + """Used for displaying list of user's channels on settings page""" editor_count = serializers.SerializerMethodField()