Fix: assign BasicMember role in members API so members can access draft project details

ami/users/api/views.py

@@ -15,7 +15,7 @@
     UserProjectMembershipListSerializer,
     UserProjectMembershipSerializer,
 )
-from ami.users.roles import Role
+from ami.users.roles import BasicMember, Role
 from ami.users.signals import manage_project_membership
 
 logger = logging.getLogger(__name__)
@@ -50,7 +50,6 @@ def get_serializer_context(self):
 
     def perform_create(self, serializer):
         project = self.get_active_project()
-        # user = serializer._validated_user
         role_cls = serializer._validated_role_cls
         with transaction.atomic():
             membership = serializer.save(project=project)
@@ -60,19 +59,20 @@ def perform_create(self, serializer):
             # The membership is already created above, so we don't need the signal to modify it
             m2m_changed.disconnect(manage_project_membership, sender=Group.user_set.through)
             try:
-                # unassign all existing roles for this project
+                # Unassign all roles, assign the chosen role, then BasicMember
                 for r in Role.__subclasses__():
                     r.unassign_user(user, project)
-
-                # assign new role
                 role_cls.assign_user(user, project)
+                if role_cls is not BasicMember:
+                    BasicMember.assign_user(user, project)
             finally:
                 # Reconnect signal
                 m2m_changed.connect(manage_project_membership, sender=Group.user_set.through)
 
     def perform_update(self, serializer):
         membership = self.get_object()
         project = membership.project
+        old_user = membership.user
         user = getattr(serializer, "_validated_user", None) or membership.user
         role_cls = getattr(serializer, "_validated_role_cls", None)
         if not role_cls:
@@ -86,10 +86,17 @@ def perform_update(self, serializer):
                 membership.user = user
                 membership.save()
 
+                # If user changed, revoke all roles from the old user
+                if old_user != user:
+                    for r in Role.__subclasses__():
+                        r.unassign_user(old_user, project)
+
+                # Unassign all roles, assign the chosen role, then BasicMember
                 for r in Role.__subclasses__():
                     r.unassign_user(user, project)
-
                 role_cls.assign_user(user, project)
+                if role_cls is not BasicMember:
+                    BasicMember.assign_user(user, project)
         finally:
             # Reconnect signal
             m2m_changed.connect(manage_project_membership, sender=Group.user_set.through)
@@ -103,7 +110,7 @@ def perform_destroy(self, instance):
         m2m_changed.disconnect(manage_project_membership, sender=Group.user_set.through)
         try:
             with transaction.atomic():
-                # remove roles for this project
+                # Revoke all roles (including BasicMember) before deleting membership
                 for r in Role.__subclasses__():
                     r.unassign_user(user, project)
 

ami/users/tests/test_membership_management_api.py

@@ -4,7 +4,7 @@
 from ami.main.models import Project, UserProjectMembership
 from ami.tests.fixtures.main import setup_test_project
 from ami.users.models import User
-from ami.users.roles import BasicMember, ProjectManager, Role
+from ami.users.roles import BasicMember, Identifier, ProjectManager, Researcher, Role, create_roles_for_project
 
 
 class TestUserProjectMembershipAPI(APITestCase):
@@ -243,3 +243,95 @@ def test_create_membership_missing_role_id(self):
         payload = {"email": self.user2.email}
         resp = self.client.post(self.members_url, payload, format="json")
         self.assertEqual(resp.status_code, 400)
+
+
+class TestMembersApiDraftProjectAccess(APITestCase):
+    """
+    Verify that members added via the Members API can access draft project details.
+    Regression tests for the BasicMember manual-assign fix.
+    """
+
+    def setUp(self):
+        self.project, _ = setup_test_project()
+        self.project.draft = True
+        self.project.save()
+        create_roles_for_project(self.project)
+
+        self.superuser = User.objects.create_superuser(email="super@insectai.org", password="x")
+        self.user_basic = User.objects.create_user(email="basic@insectai.org")
+        self.user_identifier = User.objects.create_user(email="identifier@insectai.org")
+        self.user_researcher = User.objects.create_user(email="researcher@insectai.org")
+        self.user_project_manager = User.objects.create_user(email="manager@insectai.org")
+        self.outsider = User.objects.create_user(email="outsider@insectai.org")
+
+        self.members_url = f"/api/v2/projects/{self.project.pk}/members/"
+        self.detail_url = f"/api/v2/projects/{self.project.pk}/"
+
+    def _add_member_and_assert_can_access_draft(self, user, role_id: str) -> None:
+        """Add user as role via API, then assert they can GET draft project details."""
+        self.client.force_authenticate(self.superuser)
+        resp = self.client.post(
+            self.members_url,
+            {"email": user.email, "role_id": role_id},
+            format="json",
+        )
+        self.assertEqual(resp.status_code, 201, f"Failed to add {role_id}: {resp.json()}")
+        self.client.force_authenticate(user)
+        detail_resp = self.client.get(self.detail_url)
+        self.assertEqual(
+            detail_resp.status_code,
+            200,
+            f"{role_id} member should access draft project, got {detail_resp.status_code}",
+        )
+
+    def test_member_added_via_api_can_access_draft_project_basic_member(self):
+        self._add_member_and_assert_can_access_draft(self.user_basic, BasicMember.__name__)
+
+    def test_member_added_via_api_can_access_draft_project_identifier(self):
+        self._add_member_and_assert_can_access_draft(self.user_identifier, Identifier.__name__)
+
+    def test_member_added_via_api_can_access_draft_project_researcher(self):
+        self._add_member_and_assert_can_access_draft(self.user_researcher, Researcher.__name__)
+
+    def test_member_added_via_api_can_access_draft_project_manager(self):
+        self._add_member_and_assert_can_access_draft(self.user_project_manager, ProjectManager.__name__)
+
+    def test_member_role_update_retains_draft_access(self):
+        """After a role change via API, member should still access draft project."""
+        self._add_member_and_assert_can_access_draft(self.user_basic, BasicMember.__name__)
+
+        self.client.force_authenticate(self.superuser)
+        membership = UserProjectMembership.objects.get(project=self.project, user=self.user_basic)
+        update_url = f"{self.members_url}{membership.pk}/"
+        resp = self.client.patch(update_url, {"role_id": Researcher.__name__}, format="json")
+        self.assertEqual(resp.status_code, 200)
+
+        self.client.force_authenticate(self.user_basic)
+        detail_resp = self.client.get(self.detail_url)
+        self.assertEqual(detail_resp.status_code, 200, "Member should retain draft access after role update")
+
+    def test_deleted_member_cannot_access_draft_project(self):
+        """After membership deletion, former member should lose draft access."""
+        self._add_member_and_assert_can_access_draft(self.user_basic, BasicMember.__name__)
+
+        self.client.force_authenticate(self.superuser)
+        membership = UserProjectMembership.objects.get(project=self.project, user=self.user_basic)
+        delete_url = f"{self.members_url}{membership.pk}/"
+        self.client.delete(delete_url)
+
+        self.client.force_authenticate(self.user_basic)
+        detail_resp = self.client.get(self.detail_url)
+        self.assertIn(
+            detail_resp.status_code,
+            (403, 404),
+            "Removed member should not access draft project",
+        )
+
+    def test_non_member_cannot_access_draft_project(self):
+        self.client.force_authenticate(self.outsider)
+        detail_resp = self.client.get(self.detail_url)
+        self.assertIn(
+            detail_resp.status_code,
+            (403, 404),
+            "Non-member should not access draft project",
+        )