From 85e0ff32791a28f61fc77c5791e9450204a72a46 Mon Sep 17 00:00:00 2001 From: Mikko Nieminen Date: Wed, 20 Nov 2024 13:55:30 +0100 Subject: [PATCH] fix role promoting crash with delegate limit reached (#1515) --- CHANGELOG.rst | 1 + docs/source/major_changes.rst | 1 + projectroles/forms.py | 2 +- projectroles/tests/test_views.py | 38 ++++++++++++++++++++++++++++++++ projectroles/views.py | 31 +++++++++++++++++++++----- 5 files changed, 66 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b5fa5f97..51ecf0dd 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -32,6 +32,7 @@ Fixed - Deprecated ``SODAR_API_*`` settings required in tests (#1495) - Add workaround to ``ProjectInviteCreateView`` returning 404 with category and query string (#1510) - Broken tour help attachments in ``ProjectRoleView`` (#1512) + - ``RoleAssignmentCreateView`` crash as delegate with promoting and delegate limit reached (#1515) v1.0.2 (2024-09-09) diff --git a/docs/source/major_changes.rst b/docs/source/major_changes.rst index b8023a3d..a1c8ce14 100644 --- a/docs/source/major_changes.rst +++ b/docs/source/major_changes.rst @@ -20,6 +20,7 @@ Release Highlights - Add user count in timeline siteinfo statistics - Add finder role info link in member list - Fix invite create view redirect failing in categories +- Fix role promoting crash as delegate with delegate limit reached - Fix requiring deprecated SODAR API settings in tests - General bug fixes and minor updates diff --git a/projectroles/forms.py b/projectroles/forms.py index e734076e..f40adce4 100644 --- a/projectroles/forms.py +++ b/projectroles/forms.py @@ -899,7 +899,7 @@ def __init__( self.fields['role'].initial = max( [c[0] for c in self.fields['role'].choices] ) - if not promote_as: + else: self.fields['role'].initial = Role.objects.get( name=PROJECT_ROLE_GUEST ).pk diff --git a/projectroles/tests/test_views.py b/projectroles/tests/test_views.py index 24b677ad..d6b4360b 100644 --- a/projectroles/tests/test_views.py +++ b/projectroles/tests/test_views.py @@ -2887,6 +2887,44 @@ def test_get_promote_owner(self): ) self.assertEqual(response.status_code, 302) + def test_get_promote_delegate_limit_reached(self): + """Test GET with inherited contributor and delegate limit reached""" + user_delegate = self.make_user('user_delegate') + self.make_assignment(self.project, user_delegate, self.role_delegate) + contrib_as_cat = self.make_assignment( + self.category, self.user_new, self.role_contributor + ) + with self.login(user_delegate): + response = self.client.get( + reverse( + 'projectroles:role_create_promote', + kwargs={ + 'project': self.project.sodar_uuid, + 'promote_as': contrib_as_cat.sodar_uuid, + }, + ) + ) + self.assertEqual(response.status_code, 302) + + def test_get_promote_delegate_limit_reached_superuser(self): + """Test GET with inherited contributor and delegate limit reached as superuser""" + user_delegate = self.make_user('user_delegate') + self.make_assignment(self.project, user_delegate, self.role_delegate) + contrib_as_cat = self.make_assignment( + self.category, self.user_new, self.role_contributor + ) + with self.login(self.user): + response = self.client.get( + reverse( + 'projectroles:role_create_promote', + kwargs={ + 'project': self.project.sodar_uuid, + 'promote_as': contrib_as_cat.sodar_uuid, + }, + ) + ) + self.assertEqual(response.status_code, 302) + def test_post(self): """Test RoleAssignmentCreateView POST""" self.assertEqual(RoleAssignment.objects.all().count(), 2) diff --git a/projectroles/views.py b/projectroles/views.py index f979949a..61bb0dfd 100644 --- a/projectroles/views.py +++ b/projectroles/views.py @@ -71,6 +71,7 @@ PROJECT_TYPE_CATEGORY = SODAR_CONSTANTS['PROJECT_TYPE_CATEGORY'] PROJECT_ROLE_OWNER = SODAR_CONSTANTS['PROJECT_ROLE_OWNER'] PROJECT_ROLE_DELEGATE = SODAR_CONSTANTS['PROJECT_ROLE_DELEGATE'] +PROJECT_ROLE_CONTRIBUTOR = SODAR_CONSTANTS['PROJECT_ROLE_CONTRIBUTOR'] PROJECT_ROLE_GUEST = SODAR_CONSTANTS['PROJECT_ROLE_GUEST'] PROJECT_ROLE_FINDER = SODAR_CONSTANTS['PROJECT_ROLE_FINDER'] SITE_MODE_TARGET = SODAR_CONSTANTS['SITE_MODE_TARGET'] @@ -2022,9 +2023,32 @@ def get(self, request, *args, **kwargs): # Validate inherited role promotion if set if self.kwargs.get('promote_as'): project = self.get_project() + redirect_url = reverse( + 'projectroles:roles', kwargs={'project': project.sodar_uuid} + ) self.promote_as = RoleAssignment.objects.filter( sodar_uuid=self.kwargs['promote_as'] ).first() + + # Check for reached delegate limit + del_count = RoleAssignment.objects.filter( + project=project, role__name=PROJECT_ROLE_DELEGATE + ).count() + del_limit = settings.PROJECTROLES_DELEGATE_LIMIT + if ( + self.promote_as + and self.promote_as.role.rank + >= ROLE_RANKING[PROJECT_ROLE_CONTRIBUTOR] + and del_count >= del_limit + ): + messages.warning( + request, + 'Local delegate limit ({}) reached, no available roles for ' + 'promotion.'.format(del_limit), + ) + return redirect(redirect_url) + + # Check for invalid roles if ( not self.promote_as or self.promote_as.role.rank @@ -2035,12 +2059,7 @@ def get(self, request, *args, **kwargs): messages.error( request, 'Invalid role assignment for promotion.' ) - return redirect( - reverse( - 'projectroles:roles', - kwargs={'project': project.sodar_uuid}, - ) - ) + return redirect(redirect_url) return super().get(request, *args, **kwargs)