diff --git a/coldfront/config/core.py b/coldfront/config/core.py index 34dc95f06..4d94def95 100644 --- a/coldfront/config/core.py +++ b/coldfront/config/core.py @@ -35,6 +35,11 @@ PENDING_ACTIVE_ALLOCATION_STATUSES = PENDING_ALLOCATION_STATUSES + ACTIVE_ALLOCATION_STATUSES INACTIVE_ALLOCATION_STATUSES = ['Denied', 'Expired', 'Inactive', 'Pending Deactivation'] +# Categorization of project manager permissions +MANAGERS = ['General Manager', 'Access Manager', 'Data Manager'] +ACCESS_MANAGERS = ['General Manager', 'Access Manager'] +DATA_MANAGERS = ['General Manager', 'Data Manager'] + #------------------------------------------------------------------------------ # DjangoQ settings #------------------------------------------------------------------------------ diff --git a/coldfront/core/allocation/models.py b/coldfront/core/allocation/models.py index f1530b173..68992463f 100644 --- a/coldfront/core/allocation/models.py +++ b/coldfront/core/allocation/models.py @@ -467,10 +467,12 @@ def user_permissions(self, user): project_perms = self.project.user_permissions(user) - if ProjectPermission.PI in project_perms or ProjectPermission.MANAGER in project_perms: + if ProjectPermission.PI in project_perms or ProjectPermission.DATA_MANAGER in project_perms: return [AllocationPermission.USER, AllocationPermission.MANAGER] - if self.allocationuser_set.filter(user=user, status__name__in=['Active', 'New', ]).exists(): + if self.project.projectuser_set.filter(user=user, status__name='Active').exists(): + return [AllocationPermission.USER] + if self.allocationuser_set.filter(user=user, status__name__in=['Active', 'New']).exists(): return [AllocationPermission.USER] return [] diff --git a/coldfront/core/allocation/templates/allocation/allocation_detail.html b/coldfront/core/allocation/templates/allocation/allocation_detail.html index 5178ddfb2..b79348917 100644 --- a/coldfront/core/allocation/templates/allocation/allocation_detail.html +++ b/coldfront/core/allocation/templates/allocation/allocation_detail.html @@ -50,13 +50,13 @@

Allocation Detail

Allocation Information

- {% if allocation.is_changeable and not allocation.is_locked and is_allowed_to_update_project and allocation.status.name in 'Active, Renewal Requested, Payment Pending, Payment Requested, Paid' %} + {% if allocation.is_changeable and not allocation.is_locked and is_allowed_to_update_project and allocation.status.name in 'Active, Renewal Requested, Payment Pending, Payment Requested, Paid' %}
Request Change
- {% endif %} + {% endif %} diff --git a/coldfront/core/allocation/tests/test_views.py b/coldfront/core/allocation/tests/test_views.py index d5822c256..1a8ff5881 100644 --- a/coldfront/core/allocation/tests/test_views.py +++ b/coldfront/core/allocation/tests/test_views.py @@ -80,10 +80,8 @@ def setUpTestData(cls): def test_allocation_list_access_admin(self): """Confirm that AllocationList access control works for admin""" self.allocation_access_tstbase('/allocation/') - # confirm that show_all_allocations=on enables admin to view all allocations response = self.client.get("/allocation/?show_all_allocations=on") - self.assertEqual(len(response.context['item_list']), Allocation.objects.all().count()) def test_allocation_list_access_pi(self): @@ -96,6 +94,20 @@ def test_allocation_list_access_pi(self): response = self.client.get("/allocation/?show_all_allocations=on") self.assertEqual(len(response.context['item_list']), 1) + def test_allocation_list_access_manager(self): + """Confirm that AllocationList access control works for managers + When show_all_allocations=on, manager still sees only allocations belonging + to the projects they are manager for. + """ + # confirm that show_all_allocations=on enables admin to view all allocations + self.client.force_login(self.proj_datamanager, backend=BACKEND) + response = self.client.get("/allocation/?show_all_allocations=on") + self.assertEqual(len(response.context['item_list']), 1) + # confirm that show_all_allocations=on enables admin to view all allocations + self.client.force_login(self.proj_accessmanager, backend=BACKEND) + response = self.client.get("/allocation/?show_all_allocations=on") + self.assertEqual(len(response.context['item_list']), 1) + def test_allocation_list_access_user(self): """Confirm that AllocationList access control works for non-pi users When show_all_allocations=on, users see only the allocations they @@ -116,13 +128,13 @@ def test_allocation_list_access_user(self): response = self.client.get("/allocation/?show_all_allocations=on") self.assertEqual(len(response.context['item_list']), 1) - # nonallocation user belonging to project can't see allocation - self.client.force_login(self.nonproj_nonallocation_user, backend=BACKEND) + # nonallocation user belonging to project can see allocation + self.client.force_login(self.proj_nonallocation_user, backend=BACKEND) response = self.client.get("/allocation/?show_all_allocations=on") - self.assertEqual(len(response.context['item_list']), 0) + self.assertEqual(len(response.context['item_list']), 1) - # nonallocation user belonging to project can't see allocation - self.client.force_login(self.proj_nonallocation_user, backend=BACKEND) + # nonallocation user not belonging to project can't see allocation + self.client.force_login(self.nonproj_nonallocation_user, backend=BACKEND) response = self.client.get("/allocation/?show_all_allocations=on") self.assertEqual(len(response.context['item_list']), 0) @@ -143,20 +155,17 @@ def setUp(self): """create an AllocationChangeRequest to test""" self.client.force_login(self.admin_user, backend=BACKEND) AllocationChangeRequestFactory(id=2, allocation=self.proj_allocation) + self.url = reverse('allocation-change-detail', kwargs={'pk': 2}) def test_allocationchangedetailview_access(self): - response = self.client.get( - reverse('allocation-change-detail', kwargs={'pk': 2}) - ) + response = self.client.get(self.url) self.assertEqual(response.status_code, 200) def test_allocationchangedetailview_post_permissions_admin(self): """Test post request""" param = {'action': 'deny'} self.client.force_login(self.admin_user, backend=BACKEND) - response = self.client.post( - reverse('allocation-change-detail', kwargs={'pk': 2}), param, follow=True - ) + response = self.client.post(self.url, param, follow=True) alloc_change_req = AllocationChangeRequest.objects.get(pk=2) self.assertEqual(alloc_change_req.status_id, 3) @@ -164,9 +173,7 @@ def test_allocationchangedetailview_post_permissions_pi(self): """pi can't post changes""" param = {'action': 'deny'} self.client.force_login(self.pi_user, backend=BACKEND) - self.client.post( - reverse('allocation-change-detail', kwargs={'pk': 2}), param, follow=True - ) + self.client.post(self.url, param, follow=True) alloc_change_req = AllocationChangeRequest.objects.get(pk=2) self.assertNotEqual(alloc_change_req.status_id, 3) @@ -174,18 +181,13 @@ def test_allocationchangedetailview_post_permissions_normaluser(self): """normal user can't post changes""" param = {'action': 'deny'} self.client.force_login(self.proj_allocation_user, backend=BACKEND) - self.client.post( - reverse('allocation-change-detail', kwargs={'pk': 2}), param, follow=True - ) + self.client.post(self.url, param, follow=True) alloc_change_req = AllocationChangeRequest.objects.get(pk=2) self.assertNotEqual(alloc_change_req.status_id, 3) - def test_allocationchangedetailview_post_deny(self): param = {'action': 'deny'} - response = self.client.post( - reverse('allocation-change-detail', kwargs={'pk': 2}), param, follow=True - ) + response = self.client.post(self.url, param, follow=True) self.assertEqual(response.status_code, 200) alloc_change_req = AllocationChangeRequest.objects.get(pk=2) self.assertEqual(alloc_change_req.status_id, 3) @@ -193,14 +195,10 @@ def test_allocationchangedetailview_post_deny(self): def test_allocationchangedetailview_post_approve(self): # with nothing changed, should get error message of "You must make a change to the allocation." param = {'action': 'approve'} - response = self.client.post( - reverse('allocation-change-detail', kwargs={'pk': 2}), param, follow=True - ) + response = self.client.post(self.url, param, follow=True) self.assertEqual(response.status_code, 200) messages = list(response.context['messages']) self.assertEqual(str(messages[0]), "You must make a change to the allocation.") - # alloc_change_req = AllocationChangeRequest.objects.get(pk=2) - # self.assertEqual(alloc_change_req.status_id, 2) class AllocationChangeViewTest(AllocationViewBaseTest): @@ -218,6 +216,7 @@ def setUp(self): 'end_date_extension': 0, } self.url = '/allocation/1/change-request' + self.success_msg = "Allocation change request successfully submitted." def test_allocationchangeview_access(self): """Test get request""" @@ -230,14 +229,11 @@ def test_allocationchangeview_post_permissions(self): self.post_data['attributeform-0-new_value'] = '1000' self.client.force_login(self.admin_user, backend=BACKEND) response = self.client.post(self.url, data=self.post_data, follow=True) - self.assertContains( - response, "Allocation change request successfully submitted." - ) + + self.assertContains(response, self.success_msg) self.client.force_login(self.pi_user, backend=BACKEND) response = self.client.post(self.url, data=self.post_data, follow=True) - self.assertContains( - response, "Allocation change request successfully submitted." - ) + self.assertContains(response, self.success_msg) self.client.force_login(self.proj_allocation_user, backend=BACKEND) response = self.client.post(self.url, data=self.post_data, follow=True) @@ -245,50 +241,33 @@ def test_allocationchangeview_post_permissions(self): def test_allocationchangeview_post_extension(self): """Test post request to extend end date""" - self.post_data['end_date_extension'] = 90 self.assertEqual(len(AllocationChangeRequest.objects.all()), 0) - response = self.client.post( - '/allocation/1/change-request', data=self.post_data, follow=True - ) + response = self.client.post(self.url, data=self.post_data, follow=True) self.assertEqual(response.status_code, 200) - self.assertContains( - response, "Allocation change request successfully submitted." - ) + self.assertContains(response, self.success_msg) self.assertEqual(len(AllocationChangeRequest.objects.all()), 1) def test_allocationchangeview_post_no_change(self): """Post request with no change should not go through""" - self.assertEqual(len(AllocationChangeRequest.objects.all()), 0) - - response = self.client.post( - '/allocation/1/change-request', data=self.post_data, follow=True - ) + response = self.client.post(self.url, data=self.post_data, follow=True) self.assertContains(response, "You must request a change") self.assertEqual(len(AllocationChangeRequest.objects.all()), 0) def test_allocationchangeview_post_more_tb(self): """Post request with more TB should go through""" - self.post_data['attributeform-0-new_value'] = '1000' self.assertEqual(len(AllocationChangeRequest.objects.all()), 0) - response = self.client.post( - '/allocation/1/change-request', data=self.post_data, follow=True - ) - self.assertContains( - response, "Allocation change request successfully submitted." - ) + response = self.client.post(self.url, data=self.post_data, follow=True) + self.assertContains(response, self.success_msg) self.assertEqual(len(AllocationChangeRequest.objects.all()), 1) def test_allocationchangeview_post_more_tb_decimal(self): """Post request for more TB with decimal should not go through""" - self.post_data['attributeform-0-new_value'] = '1000.1' self.assertEqual(len(AllocationChangeRequest.objects.all()), 0) - response = self.client.post( - '/allocation/1/change-request', data=self.post_data, follow=True - ) + response = self.client.post(self.url, data=self.post_data, follow=True) self.assertContains(response, "Value must be an integer.") self.assertEqual(len(AllocationChangeRequest.objects.all()), 0) @@ -302,7 +281,7 @@ def setUp(self): def test_allocation_detail_access(self): self.allocation_access_tstbase(self.url) utils.test_user_can_access(self, self.pi_user, self.url) # PI can access - utils.test_user_cannot_access(self, self.proj_nonallocation_user, self.url) + utils.test_user_can_access(self, self.proj_nonallocation_user, self.url) # check access for allocation user with "Removed" status def test_allocation_detail_template_value_render(self): @@ -319,41 +298,35 @@ def test_allocation_detail_template_value_render(self): def test_allocationdetail_requestchange_button(self): """Test visibility of "Request Change" button for different user types""" - utils.page_contains_for_user(self, self.admin_user, self.url, 'Request Change') - utils.page_contains_for_user(self, self.pi_user, self.url, 'Request Change') + search_text = 'Request Change' + utils.page_contains_for_user(self, self.admin_user, self.url, search_text) + utils.page_contains_for_user(self, self.pi_user, self.url, search_text) + utils.page_contains_for_user(self, self.proj_datamanager, self.url, search_text) utils.page_does_not_contain_for_user( - self, self.proj_allocation_user, self.url, 'Request Change' - ) + self, self.proj_accessmanager, self.url, search_text) + utils.page_does_not_contain_for_user( + self, self.proj_allocation_user, self.url, search_text) def test_allocationattribute_button_visibility(self): """Test visibility of "Add Attribute" button for different user types""" # admin - utils.page_contains_for_user( - self, self.admin_user, self.url, 'Add Allocation Attribute' - ) - utils.page_contains_for_user( - self, self.admin_user, self.url, 'Delete Allocation Attribute' - ) + add_text = 'Add Allocation Attribute' + delete_text = 'Delete Allocation Attribute' + utils.page_contains_for_user(self, self.admin_user, self.url, add_text) + utils.page_contains_for_user(self, self.admin_user, self.url, delete_text) # pi - utils.page_does_not_contain_for_user( - self, self.pi_user, self.url, 'Add Allocation Attribute' - ) - utils.page_does_not_contain_for_user( - self, self.pi_user, self.url, 'Delete Allocation Attribute' - ) + utils.page_does_not_contain_for_user(self, self.pi_user, self.url, add_text) + utils.page_does_not_contain_for_user(self, self.pi_user, self.url, delete_text) # allocation user utils.page_does_not_contain_for_user( - self, self.proj_allocation_user, self.url, 'Add Allocation Attribute' + self, self.proj_allocation_user, self.url, add_text ) utils.page_does_not_contain_for_user( - self, self.proj_allocation_user, self.url, 'Delete Allocation Attribute' + self, self.proj_allocation_user, self.url, delete_text ) def test_allocationuser_button_visibility(self): """Test visibility of "Add/Remove Users" buttons for different user types""" - # admin - # utils.page_contains_for_user(self, self.admin_user, self.url, 'Add Users') - # utils.page_contains_for_user(self, self.admin_user, self.url, 'Remove Users') # we're removing these buttons for everybody, to avoid confusion re: procedure for user addition/removal utils.page_does_not_contain_for_user( self, self.admin_user, self.url, 'Add Users' @@ -379,7 +352,7 @@ class AllocationCreateViewTest(AllocationViewBaseTest): """Tests for the AllocationCreateView""" def setUp(self): - self.url = f'/allocation/project/{self.project.pk}/create' # url for AllocationCreateView + self.url = f'/allocation/project/{self.project.pk}/create' # url for AllocationCreateView self.client.force_login(self.pi_user) tier_restype = ResourceTypeFactory(name='Storage Tier') storage_tier = ResourceFactory(resource_type=tier_restype) @@ -394,13 +367,14 @@ def test_allocationcreateview_access(self): """Test access to the AllocationCreateView""" self.allocation_access_tstbase(self.url) utils.test_user_can_access(self, self.pi_user, self.url) + utils.test_user_can_access(self, self.proj_datamanager, self.url) + utils.test_user_cannot_access(self, self.proj_accessmanager, self.url) utils.test_user_cannot_access(self, self.proj_nonallocation_user, self.url) def test_allocationcreateview_post(self): """Test POST to the AllocationCreateView""" self.assertEqual(len(self.project.allocation_set.all()), 1) response = self.client.post(self.url, data=self.post_data, follow=True) - self.assertContains(response, "Allocation requested.") self.assertEqual(len(self.project.allocation_set.all()), 2) @@ -428,10 +402,9 @@ def test_allocationcreateview_post_offerlettercode_valid(self): def test_allocationcreateview_post_bools(self): """ensure booleans are properly saved""" - self.post_data['additional_specifications'] = [ - 'Heavy IO', 'Mounted', 'External Sharing', 'High Security', 'DUA'] - response = self.client.post(self.url, data=self.post_data, follow=True) aa_names = ['Heavy IO', 'Mounted', 'High Security', 'DUA', 'External Sharing'] + self.post_data['additional_specifications'] = aa_names + response = self.client.post(self.url, data=self.post_data, follow=True) aa_objs = AllocationAttribute.objects.filter( allocation_attribute_type__name__in=aa_names ) @@ -443,7 +416,6 @@ def test_allocationcreateview_post_offerlettercode_multiplefield_invalid(self): response = self.client.post(self.url, data=self.post_data, follow=True) self.assertContains(response, "must either select an existing expense code or") - def test_allocationcreateview_post_hsph_offerlettercode(self): """Ensure that form goes through if existing_expense_codes is checked""" self.post_data['existing_expense_codes'] = '000-000-000-000-000-000-000-000-000-000-000' @@ -466,19 +438,7 @@ def setUp(self): def test_allocationaddusersview_access(self): self.allocation_access_tstbase(self.url) - no_permission = 'You do not have permission to add users to the allocation.' - - self.client.force_login(self.admin_user, backend=BACKEND) - admin_response = self.client.get(self.url) - self.assertTrue(no_permission not in str(admin_response.content)) - - self.client.force_login(self.pi_user, backend=BACKEND) - pi_response = self.client.get(self.url) - self.assertTrue(no_permission not in str(pi_response.content)) - - self.client.force_login(self.proj_allocation_user, backend=BACKEND) - user_response = self.client.get(self.url) - self.assertTrue(no_permission in str(user_response.content)) + utils.test_user_cannot_access(self, self.pi_user, self.url) class AllocationRemoveUsersViewTest(AllocationViewBaseTest): @@ -489,6 +449,7 @@ def setUp(self): def test_allocationremoveusersview_access(self): self.allocation_access_tstbase(self.url) + utils.test_user_cannot_access(self, self.pi_user, self.url) class AllocationRequestListViewTest(AllocationViewBaseTest): @@ -499,6 +460,7 @@ def setUp(self): def test_allocationrequestlistview_access(self): self.allocation_access_tstbase(self.url) + utils.test_user_cannot_access(self, self.pi_user, self.url) class AllocationChangeListViewTest(AllocationViewBaseTest): diff --git a/coldfront/core/allocation/views.py b/coldfront/core/allocation/views.py index baa5a3a0c..f78a11608 100644 --- a/coldfront/core/allocation/views.py +++ b/coldfront/core/allocation/views.py @@ -217,7 +217,7 @@ def get_context_data(self, **kwargs): # Can the user update the project? project_update_perm = allocation_obj.project.has_perm( - self.request.user, ProjectPermission.UPDATE + self.request.user, ProjectPermission.DATA_MANAGER ) context['is_allowed_to_update_project'] = project_update_perm context['allocation_users'] = allocation_users @@ -464,8 +464,7 @@ def get_queryset(self): # Q(project__projectuser__status__name='Active') & # Q(project__projectuser__user=self.request.user) & ( - (Q(project__projectuser__role__name='Manager') & - Q(project__projectuser__user=self.request.user) )| + Q(project__projectuser__user=self.request.user) | Q(project__pi=self.request.user) | ( Q(allocationuser__user=self.request.user) @@ -544,7 +543,7 @@ class AllocationCreateView(LoginRequiredMixin, UserPassesTestMixin, FormView): def test_func(self): """UserPassesTestMixin Tests""" project_obj = get_object_or_404(Project, pk=self.kwargs.get('project_pk')) - if project_obj.has_perm(self.request.user, ProjectPermission.UPDATE): + if project_obj.has_perm(self.request.user, ProjectPermission.DATA_MANAGER): return True err = 'You do not have permission to create a new allocation.' messages.error(self.request, err) @@ -766,8 +765,7 @@ class AllocationAddUsersView(LoginRequiredMixin, UserPassesTestMixin, TemplateVi def test_func(self): """UserPassesTestMixin Tests""" - allocation_obj = get_object_or_404(Allocation, pk=self.kwargs.get('pk')) - if allocation_obj.has_perm(self.request.user, AllocationPermission.MANAGER): + if self.request.user.is_superuser: return True err = 'You do not have permission to add users to the allocation.' messages.error(self.request, err) @@ -874,10 +872,8 @@ class AllocationRemoveUsersView(LoginRequiredMixin, UserPassesTestMixin, Templat def test_func(self): """UserPassesTestMixin Tests""" - allocation_obj = get_object_or_404(Allocation, pk=self.kwargs.get('pk')) - if allocation_obj.has_perm(self.request.user, AllocationPermission.MANAGER): + if self.request.user.is_superuser: return True - err = 'You do not have permission to remove users from allocation.' messages.error(self.request, err) return False @@ -1045,13 +1041,13 @@ def get_allocation_attrs_to_change(self, allocation_obj): return attributes_to_change def get(self, request, *args, **kwargs): - context = {} allocation_obj = get_object_or_404(Allocation, pk=self.kwargs.get('pk')) + attrs_to_change = self.get_allocation_attrs_to_change(allocation_obj) + context = {} form = AllocationChangeForm(**self.get_form_kwargs()) context['form'] = form - attrs_to_change = self.get_allocation_attrs_to_change(allocation_obj) if attrs_to_change: formset = formset_factory(self.formset_class, max_num=len(attrs_to_change)) formset = formset(initial=attrs_to_change, prefix='attributeform') @@ -1067,7 +1063,6 @@ def get(self, request, *args, **kwargs): return render(request, self.template_name, context) def post(self, request, *args, **kwargs): - attribute_changes_to_make = set({}) pk = self.kwargs.get('pk') allocation_obj = get_object_or_404(Allocation, pk=pk) @@ -1097,9 +1092,6 @@ def post(self, request, *args, **kwargs): messages.error(request, error) return HttpResponseRedirect(reverse('allocation-attribute-edit', kwargs={'pk': pk})) - # ID changes - change_requested = False - if attrs_to_change: for entry in formset: formset_data = entry.cleaned_data @@ -1284,7 +1276,6 @@ def post(self, request, *args, **kwargs): return HttpResponseRedirect(reverse('allocation-detail', kwargs={'pk': pk})) - class AllocationRenewView(LoginRequiredMixin, UserPassesTestMixin, TemplateView): template_name = 'allocation/allocation_renew.html' @@ -1536,7 +1527,7 @@ def get_context_data(self, **kwargs): # Can the user update the project? project_update_perm = allocation_obj.project.has_perm( - self.request.user, ProjectPermission.UPDATE + self.request.user, ProjectPermission.DATA_MANAGER ) context['is_allowed_to_update_project'] = project_update_perm context['allocation_users'] = allocation_users diff --git a/coldfront/core/grant/views.py b/coldfront/core/grant/views.py index 695504a0d..6e46c1c0c 100644 --- a/coldfront/core/grant/views.py +++ b/coldfront/core/grant/views.py @@ -30,7 +30,7 @@ def test_func(self): if project_obj.pi == self.request.user: return True - if project_obj.projectuser_set.filter(user=self.request.user, role__name='Manager', status__name='Active').exists(): + if project_obj.projectuser_set.filter(user=self.request.user, role__name__contains='Manager', status__name='Active').exists(): return True messages.error(self.request, 'You do not have permission to add a new grant to this project.') @@ -85,7 +85,9 @@ def test_func(self): if grant_obj.project.pi == self.request.user: return True - if grant_obj.project.projectuser_set.filter(user=self.request.user, role__name='Manager', status__name='Active').exists(): + if grant_obj.project.projectuser_set.filter( + user=self.request.user, role__name__contains='Manager', status__name='Active' + ).exists(): return True messages.error(self.request, 'You do not have permission to update grant from this project.') @@ -113,7 +115,9 @@ def test_func(self): if project_obj.pi == self.request.user: return True - if project_obj.projectuser_set.filter(user=self.request.user, role__name='Manager', status__name='Active').exists(): + if project_obj.projectuser_set.filter( + user=self.request.user, role__name__contains='Manager', status__name='Active' + ).exists(): return True messages.error(self.request, 'You do not have permission to delete grants from this project.') diff --git a/coldfront/core/portal/views.py b/coldfront/core/portal/views.py index fe8608aae..5e08a60ed 100644 --- a/coldfront/core/portal/views.py +++ b/coldfront/core/portal/views.py @@ -18,10 +18,12 @@ from coldfront.core.resource.models import Resource from coldfront.config.env import ENV from coldfront.core.department.models import Department, DepartmentMember +from coldfront.core.utils.common import import_from_settings if ENV.bool('PLUGIN_SFTOCF', default=False): from coldfront.plugins.sftocf.utils import StarFishRedash, STARFISH_SERVER +MANAGERS = import_from_settings('MANAGERS', ['Manager']) def home(request): context = {} @@ -41,7 +43,7 @@ def home(request): Q(project__status__name__in=['Active', 'New']) & Q(project__projectuser__user=request.user) & Q(project__projectuser__status__name__in=['Active', ]) & - (Q(project__projectuser__role__name='Manager') | + (Q(project__projectuser__role__name__in=MANAGERS) | Q(allocationuser__user=request.user) & Q(allocationuser__status__name='Active')) ).distinct().order_by('-created') @@ -51,7 +53,7 @@ def home(request): & Q(project__status__name__in=['Active', 'New']) & ( Q(project__pi=request.user) | ( Q(project__projectuser__user=request.user) - & Q(project__projectuser__role__name='Manager') + & Q(project__projectuser__role__name__in=MANAGERS) ) ) ) diff --git a/coldfront/core/project/management/commands/add_default_project_choices.py b/coldfront/core/project/management/commands/add_default_project_choices.py index d2ba4f8ae..9254c1681 100644 --- a/coldfront/core/project/management/commands/add_default_project_choices.py +++ b/coldfront/core/project/management/commands/add_default_project_choices.py @@ -18,7 +18,7 @@ def handle(self, *args, **options): for choice in ['Completed', 'Pending', ]: ProjectReviewStatusChoice.objects.get_or_create(name=choice) - for choice in ['User', 'Manager', ]: + for choice in ['User', 'General Manager', 'Data Manager', 'Access Manager']: ProjectUserRoleChoice.objects.get_or_create(name=choice) for choice in ['Active', 'Pending - Add', 'Pending - Remove', 'Denied', 'Removed', ]: diff --git a/coldfront/core/project/models.py b/coldfront/core/project/models.py index 653f6beec..2085da5ed 100644 --- a/coldfront/core/project/models.py +++ b/coldfront/core/project/models.py @@ -13,14 +13,17 @@ from coldfront.core.utils.common import import_from_settings PROJECT_ENABLE_PROJECT_REVIEW = import_from_settings('PROJECT_ENABLE_PROJECT_REVIEW', False) +DATA_MANAGERS = import_from_settings('DATA_MANAGERS', ['Manager']) +ACCESS_MANAGERS = import_from_settings('ACCESS_MANAGERS', ['Manager']) +MANAGERS = import_from_settings('MANAGERS', ['Manager']) class ProjectPermission(Enum): """ A project permission stores the user, manager, pi, and update fields of a project. """ USER = 'user' - MANAGER = 'manager' + DATA_MANAGER = 'data_manager' + ACCESS_MANAGER = 'access_manager' PI = 'pi' - UPDATE = 'update' class ProjectStatusChoice(TimeStampedModel): """ A project status choice indicates the status of the project. Examples include Active, Archived, and New. @@ -193,16 +196,20 @@ def user_permissions(self, user): permissions = [ProjectPermission.USER] - if self.projectuser_set.filter(user_conditions & models.Q(role__name='Manager')).exists(): - permissions.append(ProjectPermission.MANAGER) + if ( + self.projectuser_set.filter(user_conditions & models.Q(role__name__in=DATA_MANAGERS)).exists() + or self.pi.id == user.id + ): + permissions.append(ProjectPermission.DATA_MANAGER) + if ( + self.projectuser_set.filter(user_conditions & models.Q(role__name__in=ACCESS_MANAGERS)).exists() + or self.pi.id == user.id + ): + permissions.append(ProjectPermission.ACCESS_MANAGER) if self.pi.id == user.id: permissions.append(ProjectPermission.PI) - - if ProjectPermission.MANAGER in permissions or ProjectPermission.PI in permissions: - permissions.append(ProjectPermission.UPDATE) - return permissions def has_perm(self, user, perm): diff --git a/coldfront/core/project/templates/project/project_detail.html b/coldfront/core/project/templates/project/project_detail.html index a6afc46db..11c7dbdcf 100644 --- a/coldfront/core/project/templates/project/project_detail.html +++ b/coldfront/core/project/templates/project/project_detail.html @@ -354,7 +354,7 @@

name="email_notifications_checkbox" value="{{ user.enable_notifications }}" {% if user.enable_notifications %} checked {% endif %} - {% if user.role.name == "Manager" %} disabled {% endif %} + {% if user.role.name == "Access Manager" %} disabled {% endif %} {% if not request.user.is_superuser and user.role.name == "User" and request.user == user.user %} disabled {% endif %}> {% elif request.user == user.user %} {% endif %} - {% if is_allowed_to_update_project %} + {% if is_allowed_to_update_users %} Edit {% endif %} @@ -385,8 +385,8 @@

-{% comment %} +{% if request.user.is_superuser %}

Attributes

{{attributes.count}} @@ -451,9 +451,11 @@

{{attribute}}

{% endif %}
+{% endif %} +{% comment %}