Skip to content

Commit 8f16bd2

Browse files
authored
Add rolespermissions module, refractor the group and delete submission to use it (#4151)
Updates group management: - **Centralized Group Definition:** Instead of creating groups within migration files, which can be cumbersome to maintain, group definitions are now managed through the AbstractRoles model. These roles are synchronized using the `python manage.py sync_roles` command. This ensures no existing groups or their associated permissions are deleted. - **Module Renaming**: The `hypha.apply.users.groups` module has been renamed to `hypha.apply.users.roles` to reflect the shift from group-based to role-based permissions. This aligns with upcoming changes utilizing the `rolepermissions` module. - **Streamlined Group Descriptions**: The GroupDesc model is removed. Instead, help text can be directly defined within the role itself. This simplifies management and allows for translation of group descriptions. **This is the first of a series of pull requests aimed at refactoring the permissions system.** As a sample implementation, converts the delete submission to use this role-permissions. See: 8239c21 ## Test Steps - make sure the migrations run fine. - groups are create correctly via `python manage.py sync_roles`, it should also keep the existing groups. - groups description is rendered in the admin. - ensure delete submission/application is working as expected
1 parent d340e87 commit 8f16bd2

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

60 files changed

+315
-479
lines changed

Procfile

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
release: python manage.py migrate --noinput && python manage.py clear_cache --cache=default
1+
release: python manage.py migrate --noinput && python manage.py clear_cache --cache=default && python manage.py sync_roles
22
web: gunicorn hypha.wsgi:application --log-file -

conftest.py

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
# This file is used to setup the django environment for pytest
2+
import pytest
3+
from django.core.management import call_command
4+
5+
6+
@pytest.fixture(scope="session")
7+
def django_db_setup(django_db_setup, django_db_blocker):
8+
"""Create initial groups before running tests."""
9+
with django_db_blocker.unblock():
10+
call_command("sync_roles")

docs/setup/deployment/development/docker.md

+3-2
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ pg_restore --verbose --clean --if-exists --no-acl --no-owner --dbname=hypha --us
134134
After restoring the sandbox db run the migrate command inside the py container.
135135

136136
```shell
137-
docker-compose exec py bash
138-
python3 manage.py migrate
137+
docker-compose exec py python3 manage.py migrate
138+
docker-compose exec py python3 manage.py sync_roles
139+
139140
```

docs/setup/deployment/development/stand-alone.md

+2
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,7 @@ There are two ways to about it, you can either load demo data from `/public/san
196196

197197
```shell
198198
python3 manage.py migrate
199+
python3 manage.py sync_roles
199200
```
200201

201202
=== "From Scratch"
@@ -209,6 +210,7 @@ There are two ways to about it, you can either load demo data from `/public/san
209210

210211
```text
211212
python3 manage.py migrate
213+
python3 manage.py sync_roles
212214
```
213215

214216
!!! tip "Tips"

docs/setup/deployment/production/heroku.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,13 @@ python3 -c "from django.core.management.utils import get_random_secret_key; prin
5151

5252
```shell
5353
heroku run python3 manage.py migrate -a [name-of-app]
54+
heroku run python3 manage.py sync_roles -a [name-of-app]
5455
heroku run python3 manage.py createcachetable -a [name-of-app]
5556
heroku run python3 manage.py createsuperuser -a [name-of-app]
5657
heroku run python3 manage.py wagtailsiteupdate [the-public-address] [the-apply-address] 443 -a [name-of-app]
5758
```
5859

59-
7. Now add the "release" step back to the "Procfile" and deploy again.
60+
7. Now add the "release" step back to the `Procfile` and deploy again.
6061

6162
You should now have a running site.
6263

docs/setup/deployment/production/stand-alone.md

+1
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ npm run build
152152
python manage.py collectstatic --noinput
153153
python manage.py createcachetable
154154
python manage.py migrate --noinput
155+
python manage.py sync_roles
155156
python manage.py clear_cache --cache=default
156157
python manage.py createsuperuser
157158
python manage.py wagtailsiteupdate apply.server.domain 80

hypha/apply/activity/adapters/emails.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,12 @@
1111
from hypha.apply.activity.models import ALL, APPLICANT_PARTNERS, PARTNER
1212
from hypha.apply.projects.models.payment import CHANGES_REQUESTED_BY_STAFF, DECLINED
1313
from hypha.apply.projects.templatetags.project_tags import display_project_status
14-
from hypha.apply.users.groups import (
14+
from hypha.apply.users.models import User
15+
from hypha.apply.users.roles import (
1516
CONTRACTING_GROUP_NAME,
1617
FINANCE_GROUP_NAME,
1718
STAFF_GROUP_NAME,
1819
)
19-
from hypha.apply.users.models import User
2020
from hypha.core.mail import (
2121
language,
2222
remove_extra_empty_lines,

hypha/apply/activity/adapters/utils.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@
1414
RESUBMITTED,
1515
SUBMITTED,
1616
)
17-
from hypha.apply.users.groups import (
17+
from hypha.apply.users.models import User
18+
from hypha.apply.users.roles import (
1819
CONTRACTING_GROUP_NAME,
1920
FINANCE_GROUP_NAME,
2021
STAFF_GROUP_NAME,
2122
)
22-
from hypha.apply.users.models import User
2323

2424

2525
def link_to(target, request):

hypha/apply/api/v1/serializers.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
)
1515
from hypha.apply.review.models import Review, ReviewOpinion
1616
from hypha.apply.review.options import RECOMMENDATION_CHOICES
17-
from hypha.apply.users.groups import PARTNER_GROUP_NAME, STAFF_GROUP_NAME
17+
from hypha.apply.users.roles import PARTNER_GROUP_NAME, STAFF_GROUP_NAME
1818

1919
User = get_user_model()
2020

hypha/apply/dashboard/templates/dashboard/partials/applicant_submissions.html

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
{% load i18n dashboard_statusbar_tags statusbar_tags workflow_tags heroicons submission_tags %}
2+
{% load can from permission_tags %}
23

34
{% for submission in page.object_list %}
45
<div class="wrapper wrapper--status-bar-outer">
@@ -28,7 +29,7 @@ <h4 class="heading mb-0 font-bold line-clamp-3 hover:line-clamp-none">
2829
{% endif %}
2930
</a>
3031
{% endif %}
31-
{% user_can_delete_submission submission request.user as can_delete_submission %}
32+
{% can "delete_submission" submission as can_delete_submission %}
3233
{% if can_delete_submission %}
3334
<a class="button button--white" href="{% url 'funds:submissions:delete' submission.id %}" hx-get="{% url 'funds:submissions:delete' submission.id %}" hx-target="#htmx-modal">
3435
{% heroicon_micro "trash" class="inline me-1 mt-1 fill-red-600" aria_hidden=true %}

hypha/apply/determinations/tests/test_views.py

+5-5
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ def test_can_access_form_if_lead(self):
6969
def test_cant_access_wrong_status(self):
7070
submission = ApplicationSubmissionFactory(status="rejected")
7171
response = self.get_page(submission, "form")
72-
self.assertRedirects(response, self.absolute_url(submission.get_absolute_url()))
72+
self.assertRedirects(response, submission.get_absolute_url())
7373

7474
def test_cant_resubmit_determination(self):
7575
submission = ApplicationSubmissionFactory(
@@ -81,7 +81,7 @@ def test_cant_resubmit_determination(self):
8181
response = self.post_page(
8282
submission, {"data": "value", "outcome": determination.outcome}, "form"
8383
)
84-
self.assertRedirects(response, self.absolute_url(submission.get_absolute_url()))
84+
self.assertRedirects(response, submission.get_absolute_url())
8585

8686
def test_can_edit_draft_determination(self):
8787
submission = ApplicationSubmissionFactory(
@@ -119,7 +119,7 @@ def test_can_edit_draft_determination_if_not_lead(self):
119119
submission, {"data": "value", "outcome": determination.outcome}, "form"
120120
)
121121
self.assertContains(response, "Approved")
122-
self.assertRedirects(response, self.absolute_url(submission.get_absolute_url()))
122+
self.assertRedirects(response, submission.get_absolute_url())
123123

124124
def test_can_edit_draft_determination_if_not_lead_with_projects(self):
125125
submission = ApplicationSubmissionFactory(status="in_discussion")
@@ -130,7 +130,7 @@ def test_can_edit_draft_determination_if_not_lead_with_projects(self):
130130
submission, {"data": "value", "outcome": determination.outcome}, "form"
131131
)
132132
self.assertContains(response, "Approved")
133-
self.assertRedirects(response, self.absolute_url(submission.get_absolute_url()))
133+
self.assertRedirects(response, submission.get_absolute_url())
134134

135135
def test_sends_message_if_requires_more_info(self):
136136
submission = ApplicationSubmissionFactory(
@@ -172,7 +172,7 @@ def test_can_progress_stage_via_determination(self):
172172
# Cannot use self.url() as that uses a different base.
173173
url = submission_next.get_absolute_url()
174174
self.assertRedirects(
175-
response, self.factory.get(url, secure=True).build_absolute_uri(url)
175+
response, self.factory.get(url, secure=False).get_full_path()
176176
)
177177
self.assertEqual(submission_original.status, "invited_to_proposal")
178178
self.assertEqual(submission_next.status, "draft_proposal")

hypha/apply/funds/management/commands/seed_community_lab_application.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
from hypha.apply.funds.models import ApplicationForm, LabType
88
from hypha.apply.funds.models.forms import LabBaseForm, LabBaseReviewForm
99
from hypha.apply.review.models import ReviewForm
10-
from hypha.apply.users.groups import STAFF_GROUP_NAME
10+
from hypha.apply.users.roles import STAFF_GROUP_NAME
1111
from hypha.home.models import ApplyHomePage
1212

1313
CL_FUND_TITLE = "Community lab (archive fund)"

hypha/apply/funds/management/commands/seed_concept_note.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
ApplicationBaseReviewForm,
1313
)
1414
from hypha.apply.review.models import ReviewForm
15-
from hypha.apply.users.groups import STAFF_GROUP_NAME
15+
from hypha.apply.users.roles import STAFF_GROUP_NAME
1616
from hypha.home.models import ApplyHomePage
1717

1818
CN_ROUND_TITLE = "Internet Freedom Fund (archive round)"

hypha/apply/funds/management/commands/seed_fellowship.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
ApplicationBaseReviewForm,
1313
)
1414
from hypha.apply.review.models import ReviewForm
15-
from hypha.apply.users.groups import STAFF_GROUP_NAME
15+
from hypha.apply.users.roles import STAFF_GROUP_NAME
1616
from hypha.home.models import ApplyHomePage
1717

1818
FS_ROUND_TITLE = "Fellowship (archive round)"

hypha/apply/funds/management/commands/seed_rapid_response.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
ApplicationBaseReviewForm,
1313
)
1414
from hypha.apply.review.models import ReviewForm
15-
from hypha.apply.users.groups import STAFF_GROUP_NAME
15+
from hypha.apply.users.roles import STAFF_GROUP_NAME
1616
from hypha.home.models import ApplyHomePage
1717

1818
RR_ROUND_TITLE = "Rapid Response (archive round)"

hypha/apply/funds/models/submissions.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
from hypha.apply.stream_forms.models import BaseStreamForm
4949
from hypha.apply.todo.options import SUBMISSION_DRAFT
5050
from hypha.apply.todo.views import remove_tasks_for_user
51-
from hypha.apply.users.groups import APPLICANT_GROUP_NAME
51+
from hypha.apply.users.roles import APPLICANT_GROUP_NAME
5252

5353
from ..blocks import NAMED_BLOCKS, ApplicationCustomFormFieldsBlock
5454
from ..workflow import (

hypha/apply/funds/models/utils.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
from hypha.apply.stream_forms.models import AbstractStreamForm
1818
from hypha.apply.todo.options import SUBMISSION_DRAFT
1919
from hypha.apply.todo.views import add_task_to_user
20-
from hypha.apply.users.groups import (
20+
from hypha.apply.users.roles import (
2121
COMMUNITY_REVIEWER_GROUP_NAME,
2222
PARTNER_GROUP_NAME,
2323
REVIEWER_GROUP_NAME,

hypha/apply/funds/permissions.py

+22-7
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
from django.conf import settings
22
from django.core.exceptions import PermissionDenied
3+
from rolepermissions.permissions import register_object_checker
34

45
from hypha.apply.funds.models.submissions import DRAFT_STATE
56

6-
from ..users.groups import STAFF_GROUP_NAME, SUPERADMIN, TEAMADMIN_GROUP_NAME
7+
from ..users.roles import STAFF_GROUP_NAME, SUPERADMIN, TEAMADMIN_GROUP_NAME, StaffAdmin
78

89

910
def has_permission(action, user, object=None, raise_exception=True):
@@ -26,12 +27,27 @@ def can_edit_submission(user, submission):
2627
return True, ""
2728

2829

29-
def can_delete_submission(user, submission):
30+
@register_object_checker()
31+
def delete_submission(role, user, submission) -> bool:
32+
"""
33+
Determines if a user has permission to delete a submission.
34+
35+
Permissions are granted if:
36+
- User is a Superuser, or StaffAdmin
37+
- User has explicit delete_applicationsubmission permission
38+
- User is the applicant of the submission and it is in draft state
39+
"""
40+
if role == StaffAdmin:
41+
return True
42+
3043
if user.has_perm("funds.delete_applicationsubmission"):
31-
return True, "User can delete submission"
32-
elif user == submission.user and submission.status == DRAFT_STATE:
33-
return True, "Applicant can delete draft submissions"
34-
return False, "Forbidden Error"
44+
return True
45+
46+
# Allow the user to delete their own draft submissions
47+
if user == submission.user and submission.status == DRAFT_STATE:
48+
return True
49+
50+
return False
3551

3652

3753
def can_bulk_delete_submissions(user) -> bool:
@@ -174,7 +190,6 @@ def can_view_submission_screening(user, submission):
174190
permissions_map = {
175191
"submission_view": is_user_has_access_to_view_submission,
176192
"submission_edit": can_edit_submission,
177-
"submission_delete": can_delete_submission,
178193
"can_view_submission_screening": can_view_submission_screening,
179194
"archive_alter": can_alter_archived_submissions,
180195
}

hypha/apply/funds/reviewers/services.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
from django.db.models import Q
33
from django.db.models.query import QuerySet
44

5-
from hypha.apply.users.groups import STAFF_GROUP_NAME
5+
from hypha.apply.users.roles import STAFF_GROUP_NAME
66

77
User = get_user_model()
88

hypha/apply/funds/templates/funds/applicationsubmission_detail.html

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
{% extends "base-apply.html" %}
22
{% load i18n static workflow_tags wagtailcore_tags statusbar_tags archive_tags submission_tags %}
33
{% load heroicons %}
4+
{% load can from permission_tags %}
45

56
{% block title %}{{ object.title_text_display }}{% endblock %}
67
{% block body_class %}{% endblock %}
@@ -122,7 +123,7 @@ <h5>{% blocktrans with stage=object.previous.stage %}Your {{ stage }} applicatio
122123
</strong>
123124
</span>
124125
<div class="flex gap-4 justify-end flex-1 items-center">
125-
{% user_can_delete_submission object request.user as can_delete_submission %}
126+
{% can "delete_submission" object as can_delete_submission %}
126127
{% if can_delete_submission %}
127128
<a
128129
class="flex items-center font-bold text-red-600 transition-opacity hover:opacity-70"

hypha/apply/funds/templatetags/submission_tags.py

-12
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
from django.utils.safestring import mark_safe
66

77
from hypha.apply.funds.models import ApplicationSubmission
8-
from hypha.apply.funds.permissions import has_permission
98

109
register = template.Library()
1110

@@ -30,14 +29,3 @@ def submission_links(value):
3029
value = re.sub(rf"(?<!\w){sid}(?!\w)", link, value)
3130

3231
return mark_safe(value)
33-
34-
35-
@register.simple_tag
36-
def user_can_delete_submission(submission, user):
37-
permission, _ = has_permission(
38-
"submission_delete",
39-
user=user,
40-
object=submission,
41-
raise_exception=False,
42-
)
43-
return permission

hypha/apply/funds/tests/factories/models.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
)
2929
from hypha.apply.funds.workflow import ConceptProposal, Request, RequestExternal
3030
from hypha.apply.stream_forms.testing.factories import FormDataFactory
31-
from hypha.apply.users.groups import REVIEWER_GROUP_NAME, STAFF_GROUP_NAME
31+
from hypha.apply.users.roles import REVIEWER_GROUP_NAME, STAFF_GROUP_NAME
3232
from hypha.apply.users.tests.factories import (
3333
ApplicantFactory,
3434
GroupFactory,

hypha/apply/funds/tests/test_admin_views.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from wagtail.test.utils import WagtailTestUtils
77

88
from hypha.apply.funds.models.forms import ApplicationForm
9-
from hypha.apply.users.groups import STAFF_GROUP_NAME
9+
from hypha.apply.users.roles import STAFF_GROUP_NAME
1010
from hypha.apply.users.tests.factories import SuperUserFactory
1111
from hypha.home.factories import ApplyHomePageFactory
1212

hypha/apply/funds/tests/test_views.py

+2-4
Original file line numberDiff line numberDiff line change
@@ -154,9 +154,7 @@ def test_redirected_to_determination(self):
154154
"funds:submissions:determinations:form",
155155
kwargs={"submission_pk": submission.id},
156156
)
157-
self.assertEqual(
158-
self.absolute_url(response.url), f"{url}?action=invited_to_proposal"
159-
)
157+
assert response.url == f"{url}?action=invited_to_proposal"
160158

161159
def test_new_form_after_progress(self):
162160
submission = ApplicationSubmissionFactory(
@@ -1760,7 +1758,7 @@ def test_anonymous_can_not_access(self):
17601758
submission = ApplicationSubmissionFactory()
17611759
response = self.get_page(submission)
17621760
self.assertEqual(response.status_code, 200)
1763-
self.assertEqual(len(response.redirect_chain), 2)
1761+
self.assertEqual(len(response.redirect_chain), 1)
17641762
for path, _ in response.redirect_chain:
17651763
self.assertIn(reverse(settings.LOGIN_URL), path)
17661764

0 commit comments

Comments
 (0)