Merge pull request #591 from taigaio/owner-must-be-a-valid-member-of-a-project

When leaving a project or removing a membership the project must allw…

taiga/projects/api.py

@@ -489,7 +489,7 @@ class MembershipViewSet(ModelCrudViewSet):
 
     def get_serializer_class(self):
         use_admin_serializer = False
-        
+
         if self.action == "create":
             use_admin_serializer = True
 
@@ -544,7 +544,7 @@ class MembershipViewSet(ModelCrudViewSet):
 
     def pre_delete(self, obj):
         if obj.user is not None and not services.can_user_leave_project(obj.user, obj.project):
-            raise exc.BadRequest(_("At least one of the user must be an active admin"))
+            raise exc.BadRequest(_("The project must have an owner and at least one of the users must be an active admin"))
 
     def pre_save(self, obj):
         if not obj.token:

taiga/projects/permissions.py

@@ -37,8 +37,7 @@ class CanLeaveProject(PermissionComponent):
 
         try:
             if not services.can_user_leave_project(request.user, obj):
-                raise exc.PermissionDenied(_("You can't leave the project if there are no "
-                                             "more owners"))
+                raise exc.PermissionDenied(_("You can't leave the project if you are the owner or there are no more admins"))
             return True
         except Membership.DoesNotExist:
             return False

taiga/projects/serializers.py

@@ -257,8 +257,8 @@ class MembershipSerializer(serializers.ModelSerializer):
             project = self.object.project
 
         if (self.object and
-                not services.project_has_valid_owners(project, exclude_user=self.object.user)):
-            raise serializers.ValidationError(_("At least one of the user must be an active admin"))
+                not services.project_has_valid_admins(project, exclude_user=self.object.user)):
+            raise serializers.ValidationError(_("The project must have an owner and at least one of the users must be an active admin"))
 
         return attrs
 

taiga/projects/services/__init__.py

@@ -37,7 +37,7 @@ from .logo import get_logo_big_thumbnail_url
 
 from .members import create_members_in_bulk
 from .members import get_members_from_bulk
-from .members import remove_user_from_project, project_has_valid_owners, can_user_leave_project
+from .members import remove_user_from_project, project_has_valid_admins, can_user_leave_project
 
 from .modules_config import get_modules_config
 

taiga/projects/services/members.py

@@ -36,15 +36,15 @@ def remove_user_from_project(user, project):
     models.Membership.objects.get(project=project, user=user).delete()
 
 
-def project_has_valid_owners(project, exclude_user=None):
+def project_has_valid_admins(project, exclude_user=None):
     """
     Checks if the project has any owner membership with a user different than the specified
     """
-    owner_memberships = project.memberships.filter(is_owner=True, user__is_active=True)
+    admin_memberships = project.memberships.filter(is_owner=True, user__is_active=True)
     if exclude_user:
-        owner_memberships = owner_memberships.exclude(user=exclude_user)
+        admin_memberships = admin_memberships.exclude(user=exclude_user)
 
-    return owner_memberships.count() > 0
+    return admin_memberships.count() > 0
 
 
 def can_user_leave_project(user, project):
@@ -52,7 +52,11 @@ def can_user_leave_project(user, project):
     if not membership.is_owner:
          return True
 
-    if not project_has_valid_owners(project, exclude_user=user):
+    #The user can't leave if is the real owner of the project
+    if project.owner == user:
+        return False
+
+    if not project_has_valid_admins(project, exclude_user=user):
         return False
 
     return True

tests/integration/test_projects.py

@@ -249,7 +249,22 @@ def test_leave_project_valid_membership_only_owner(client):
     url = reverse("projects-leave", args=(project.id,))
     response = client.post(url)
     assert response.status_code == 403
-    assert response.data["_error_message"] == "You can't leave the project if there are no more owners"
+    assert response.data["_error_message"] == "You can't leave the project if you are the owner or there are no more admins"
+
+
+def test_leave_project_valid_membership_real_owner(client):
+    owner_user = f.UserFactory.create()
+    member_user = f.UserFactory.create()
+    project = f.ProjectFactory.create(owner=owner_user)
+    role = f.RoleFactory.create(project=project, permissions=["view_project"])
+    f.MembershipFactory.create(project=project, user=owner_user, role=role, is_owner=True)
+    f.MembershipFactory.create(project=project, user=member_user, role=role, is_owner=True)
+
+    client.login(owner_user)
+    url = reverse("projects-leave", args=(project.id,))
+    response = client.post(url)
+    assert response.status_code == 403
+    assert response.data["_error_message"] == "You can't leave the project if you are the owner or there are no more admins"
 
 
 def test_leave_project_invalid_membership(client):
@@ -286,7 +301,22 @@ def test_delete_membership_only_owner(client):
     url = reverse("memberships-detail", args=(membership.id,))
     response = client.delete(url)
     assert response.status_code == 400
-    assert response.data["_error_message"] == "At least one of the user must be an active admin"
+    assert response.data["_error_message"] == "The project must have an owner and at least one of the users must be an active admin"
+
+
+def test_delete_membership_real_owner(client):
+    owner_user = f.UserFactory.create()
+    member_user = f.UserFactory.create()
+    project = f.ProjectFactory.create(owner=owner_user)
+    role = f.RoleFactory.create(project=project, permissions=["view_project"])
+    owner_membership = f.MembershipFactory.create(project=project, user=owner_user, role=role, is_owner=True)
+    f.MembershipFactory.create(project=project, user=member_user, role=role, is_owner=True)
+
+    client.login(owner_user)
+    url = reverse("memberships-detail", args=(owner_membership.id,))
+    response = client.delete(url)
+    assert response.status_code == 400
+    assert response.data["_error_message"] == "The project must have an owner and at least one of the users must be an active admin"
 
 
 def test_edit_membership_only_owner(client):
@@ -301,7 +331,7 @@ def test_edit_membership_only_owner(client):
     url = reverse("memberships-detail", args=(membership.id,))
     response = client.json.patch(url, json.dumps(data))
     assert response.status_code == 400
-    assert response.data["is_owner"][0] == "At least one of the user must be an active admin"
+    assert response.data["is_owner"][0] == "The project must have an owner and at least one of the users must be an active admin"
 
 
 def test_anon_permissions_generation_when_making_project_public(client):