diff --git a/loads/models.py b/loads/models.py index c0012cd..3f2b770 100644 --- a/loads/models.py +++ b/loads/models.py @@ -790,6 +790,29 @@ def get_all_packages(self, include_hidden=False): return packages + def get_all_staff_in_all_packages(self, include_hidden=False): + """ + Get all the staff in all the packages that are relevant for a staff member + + This is usually used to restrict a Staff queryset to sensible lists + """ + + #TODO: probably want to enable this after more testing if this function but it will just add orphans? + #if self.user.is_superuser: + # return Staff.objects.all() + + # Get all the packages the staff member has access first + all_packages = self.get_all_packages(include_hidden=include_hidden) + + # Get all the groups pointed to by all of those packages + target_groups = Group.objects.filter(workpackage__in=all_packages).distinct() + + # These are user objects + users_by_groups = User.objects.all().filter(groups__in=target_groups).distinct().order_by('last_name') + # Get the matching staff objects + staff = Staff.objects.all().filter(user__in=users_by_groups).distinct().order_by('user__last_name') + return staff + def get_examined_programmes(self): examined_programmes = Programme.objects.all().filter(examiners=self).distinct() return examined_programmes diff --git a/loads/tests_models.py b/loads/tests_models.py index b64bca3..e3d2e2a 100644 --- a/loads/tests_models.py +++ b/loads/tests_models.py @@ -756,3 +756,101 @@ def test_both_regex_user_creates_staff(self): # Check the invalid user is disabled self.assertFalse(invalid_user.is_active) + + +class StaffTestCase(TestCase): + """Tests for specific model creation functionality""" + + @override_settings(WAM_STAFF_REGEX=None) + @override_settings(WAM_EXTERNAL_REGEX=None) + def setUp(self): + # Logging is very noisy typically + logging.disable(logging.CRITICAL) + + # Create a workpackage + packageA = WorkPackage.objects.create(name="A", startdate="2017-09-01", enddate="2018-08-31") + packageB = WorkPackage.objects.create(name="B", startdate="2017-09-01", enddate="2018-08-31") + + # Create three groups + groupA = Group.objects.create(name="A") + groupB1 = Group.objects.create(name="B1") + groupB2 = Group.objects.create(name="B2") + groupC = Group.objects.create(name="C") + + # Package A has one group, B has 2, the last group is "orphaned" + packageA.groups.add(groupA) + packageB.groups.add(groupB1) + packageB.groups.add(groupB2) + + packageA.save() + packageB.save() + + # Create some Users + userA_1 = User.objects.create(username="A_1", password="test") + userA_2 = User.objects.create(username="A_2", password="test") + userB1_1 = User.objects.create(username="B1_1", password="test") + userB2_1 = User.objects.create(username="B2_1", password="test") + userC = User.objects.create(username="C", password="test") + + userA_1.groups.add(groupA) + userA_2.groups.add(groupA) + userB1_1.groups.add(groupB1) + userB2_1.groups.add(groupB2) + userC.groups.add(groupC) + + def tearDown(self): + # Put the logging back in place + logging.disable(logging.NOTSET) + + def test_staff_all_in_package(self): + userA_1 = User.objects.get(username="A_1") + userA_2 = User.objects.get(username="A_2") + userB1_1 = User.objects.get(username="B1_1") + userB2_1 = User.objects.get(username="B2_1") + userC = User.objects.get(username="C") + + staffA_1 = Staff.objects.get(user=userA_1) + staffA_2 = Staff.objects.get(user=userA_2) + staffB1_1 = Staff.objects.get(user=userB1_1) + staffB2_1 = Staff.objects.get(user=userB2_1) + staffC = Staff.objects.get(user=userC) + + # A_1 should see one package, and two people in it + self.assertEqual(len(staffA_1.get_all_packages()), 1) + self.assertEqual(len(staffA_1.get_all_staff_in_all_packages()), 2) + + # B_1 should see one package, and two people in it (in two separate groups) + self.assertEqual(len(staffB1_1.get_all_packages()), 1) + self.assertEqual(len(staffB1_1.get_all_staff_in_all_packages()), 2) + + # C should see nothing + self.assertEqual(len(staffC.get_all_packages()), 0) + self.assertEqual(len(staffC.get_all_staff_in_all_packages()), 0) + + # Now add A_1 into group B1 + groupB1 = Group.objects.get(name="B1") + userA_1.groups.add(groupB1) + # Now A_1 should see two package, and four people in them + self.assertEqual(len(staffA_1.get_all_packages()), 2) + self.assertEqual(len(staffA_1.get_all_staff_in_all_packages()), 4) + + # Now hide package B + packageB = WorkPackage.objects.get(name="B") + packageB.hidden = True + packageB.save() + + # Now A_1 should see one package, and two people in them if we don't include hidden + self.assertEqual(len(staffA_1.get_all_packages(include_hidden=False)), 1) + self.assertEqual(len(staffA_1.get_all_staff_in_all_packages(include_hidden=False)), 2) + # Now A_1 should see two package, and four people in them if we do + self.assertEqual(len(staffA_1.get_all_packages(include_hidden=True)), 2) + self.assertEqual(len(staffA_1.get_all_staff_in_all_packages(include_hidden=True)), 4) + + user = User.objects.create_superuser(username="superuser", password="") + # Try to get the matching staff object + staff = Staff.objects.get(user=user) + + # A Superuser should see all of these people (and doesn't see the orphan, or themselves) + self.assertEqual(len(staff.get_all_packages()), 2) + self.assertEqual(len(staff.get_all_staff_in_all_packages()), 4) + diff --git a/loads/views.py b/loads/views.py index 8dc3791..94b5425 100644 --- a/loads/views.py +++ b/loads/views.py @@ -1199,11 +1199,12 @@ def module_staff_allocation(request, module_id, package_id): if formset.is_valid(): formset.save(commit=False) for form in formset: - # Some fields are missing, so don't do a full save yet - allocation = form.save(commit=False) - # Fix the fields - allocation.module = module - allocation.package = package + if form not in formset.deleted_forms: + # Some fields are missing, so don't do a full save yet + allocation = form.save(commit=False) + # Fix the fields + allocation.module = module + allocation.package = package logger.info("[%s] allocation for %s processed", request.user, allocation.staff) # Now do a real save formset.save(commit=True) @@ -1725,11 +1726,12 @@ def staff_module_allocation(request, staff_id, package_id): if formset.is_valid(): formset.save(commit=False) for form in formset: - # Some fields are missing, so don't do a full save yet - allocation = form.save(commit=False) - # Fix the fields - allocation.staff = staff - allocation.package = package + if form not in formset.deleted_forms: + # Some fields are missing, so don't do a full save yet + allocation = form.save(commit=False) + # Fix the fields + allocation.staff = staff + allocation.package = package # Now do a real save formset.save(commit=True) logger.info("[%s] edited the module allocation for %s on package %s" % @@ -1824,16 +1826,17 @@ def projects_details(request, project_id): """Allows a Project and allocated staff to be edited""" # Fetch the staff user associated with the person requesting - user_staff = get_object_or_404(Staff, user=request.user) + staff = get_object_or_404(Staff, user=request.user) # And the project we are going to act on project = get_object_or_404(Project, pk=project_id) - package = user_staff.package + package = staff.package # We usually want to restrict the staff to select to the package, but best honour the possibility # that there are staff not in, or no longer in the package, so add them too. # Some Database layers can't combine the querysets, because of the existing ordering, so we need the ids - package_staff_qs = package.get_all_staff() + # TODO: maybe add this to the model layer, something like Staff::AugmentQuerySet(A, B) + package_staff_qs = staff.get_all_staff_in_all_packages(include_hidden="True") package_staff_pks = package_staff_qs.values_list('pk', flat=True) logger.debug("[%s] creating query set: package staff pks: %s" % (request.user, package_staff_pks)) project_staff_pks = ProjectStaff.objects.filter(project=project).values_list('staff_id', flat=True) @@ -1863,35 +1866,20 @@ def projects_details(request, project_id): if project_form.is_valid(): project_form.save() - # Restrain querysets as we did for GET for validation checks. - for form in formset: - # Get a sane list of staff to pick from. + if formset.is_valid(): formset.form.base_fields['staff'].queryset = combined_staff_qs - for form in formset: - form.fields['staff'].queryset = combined_staff_qs formset.empty_form.fields['staff'].queryset = combined_staff_qs - - if formset.is_valid(): formset.save(commit=False) - - logger.debug("[%s] (admin) formset processing" % (request.user,)) for form in formset: - # Some fields are missing, so don't do a full save yet - allocation = form.save(commit=False) - # Fix the fields - allocation.project = project - logger.debug("[%s] (admin) processing project allocation %s, staff %s" % (request.user, - project, - form.cleaned_data.get("staff"))) - - for allocation in formset.deleted_objects: - logger.debug("[%s] (admin) deleting project allocation %s, staff %s" % (request.user, - allocation.project, - allocation.staff)) - allocation.delete() - + # Get a sane list of staff to pick from. + form.fields['staff'].queryset = combined_staff_qs + # Some fields are missing, so don't do a full save yet, don't do this on delete items + if form not in formset.deleted_forms: + allocation = form.save(commit=False) + # Fix the fields + allocation.project = project # Now do a real save formset.save(commit=True) logger.info("[%s] (admin) edited the details for project %s" % (request.user, project),