Skip to content

Commit

Permalink
When leaving a project or removing a membership the project must allw…
Browse files Browse the repository at this point in the history
…ays have a valid owner
  • Loading branch information
superalex authored and jespino committed Feb 2, 2016
1 parent 835865f commit e15e003
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 15 deletions.
4 changes: 2 additions & 2 deletions taiga/projects/api.py
Expand Up @@ -489,7 +489,7 @@ class MembershipViewSet(ModelCrudViewSet):

def get_serializer_class(self):
use_admin_serializer = False

if self.action == "create":
use_admin_serializer = True

Expand Down Expand Up @@ -544,7 +544,7 @@ def resend_invitation(self, request, **kwargs):

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:
Expand Down
3 changes: 1 addition & 2 deletions taiga/projects/permissions.py
Expand Up @@ -37,8 +37,7 @@ def check_permissions(self, request, view, obj=None):

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
Expand Down
4 changes: 2 additions & 2 deletions taiga/projects/serializers.py
Expand Up @@ -257,8 +257,8 @@ def validate_is_owner(self, attrs, source):
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

Expand Down
2 changes: 1 addition & 1 deletion taiga/projects/services/__init__.py
Expand Up @@ -37,7 +37,7 @@

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

Expand Down
14 changes: 9 additions & 5 deletions taiga/projects/services/members.py
Expand Up @@ -36,23 +36,27 @@ 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):
membership = project.memberships.get(user=user)
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
36 changes: 33 additions & 3 deletions tests/integration/test_projects.py
Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand All @@ -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):
Expand Down

0 comments on commit e15e003

Please sign in to comment.