Allow all users to export jobs to CSV (#651)

* Add class that defines user access to jobs

* Add class for storing job search filters in session storage

* Refactor job views to use new utilities

* Unhide export button for all users

* Update tests

coldfront/core/statistics/templates/job_list.html

@@ -114,7 +114,6 @@ <h1>Job List</h1>
             </div>
         </div>
     {% endif %}
-  {% if user.is_superuser %}
     <div class="card mb-3 bg-light">
         <div class="card-header">
                 <a class="btn btn-primary" href="{% url 'export-job-list' %}" role="button" style="float: right;">
@@ -123,7 +122,6 @@ <h1>Job List</h1>
                 </a>
         </div>
     </div>
-  {% endif %}
     {% if job_list %}
         <div class="table-responsive">
             <table class="table table-sm">

coldfront/core/statistics/tests/test_job_views.py

@@ -1,17 +1,18 @@
 import copy
 
-from django.urls import reverse
 from http import HTTPStatus
+from urllib.parse import urlencode
+
+from django.contrib.auth.models import User
+from django.urls import reverse
+from django.utils import timezone
 
 from coldfront.core.project.models import *
 from coldfront.core.user.models import UserProfile
 from coldfront.core.utils.common import utc_now_offset_aware
 from coldfront.core.utils.tests.test_base import TestBase
 from coldfront.core.statistics.models import Job
 
-from django.contrib.auth.models import User
-from django.utils import timezone
-
 
 class TestJobBase(TestBase):
     """A base class for testing job views."""
@@ -69,6 +70,9 @@ def setUp(self):
         self.project2 = Project.objects.create(
             name='project2', status=active_project_status)
 
+        self.project3 = Project.objects.create(
+            name='project3', status=active_project_status)
+
         self.project1_user1 = ProjectUser.objects.create(
             user=self.user1,
             project=self.project1,
@@ -116,6 +120,22 @@ def setUp(self):
         self.staff.is_staff = True
         self.staff.save()
 
+        ProjectUser.objects.create(
+            user=self.admin,
+            project=self.project3,
+            role=user_project_role,
+            status=self.active_project_user_status)
+        ProjectUser.objects.create(
+            user=self.staff,
+            project=self.project3,
+            role=user_project_role,
+            status=self.active_project_user_status)
+        ProjectUser.objects.create(
+            user=self.manager,
+            project=self.project3,
+            role=user_project_role,
+            status=self.active_project_user_status)
+
         self.password = 'password'
 
         for user in User.objects.all():
@@ -242,6 +262,18 @@ def test_admin_contents(user):
         test_admin_contents(self.admin)
         test_admin_contents(self.staff)
 
+    def test_export_button_visible(self):
+        """Testing if 'Export Job List to CSV' button is visible."""
+        url = reverse('slurm-job-list')
+        response = self.get_response(self.user1, url)
+        self.assertNotContains(response, 'Export Job List to CSV')
+
+        response = self.get_response(self.staff, url)
+        self.assertNotContains(response, 'Export Job List to CSV')
+
+        response = self.get_response(self.admin, url)
+        self.assertContains(response, 'Export Job List to CSV')
+
     def test_pagination(self):
         """Testing pagination of list view"""
 
@@ -259,6 +291,16 @@ def test_pagination(self):
         self.assertContains(response, 'Next')
         self.assertContains(response, 'Previous')
 
+    def test_saves_filters_in_session(self):
+        """Testing if cleaned form data is saved in session"""
+        url = reverse('slurm-job-list') + '?show_all_jobs=on&username=user1&partition=test_partition'
+        self.get_response(self.admin, url)
+
+        self.assertEqual(self.client.session.get('job_search_filters')['username'],
+                         self.user1.username)
+        self.assertEqual(self.client.session.get('job_search_filters')['partition'],
+                         'test_partition')
+
     def test_status_colors(self):
         """Test different status colors"""
 
@@ -378,44 +420,63 @@ def test_admin_access(self):
         self.assert_has_access(self.staff, True, url)
 
 
-class ExportJobListView(TestJobBase):
-    """A class for testing SlurmJobDetailView"""
+class TestExportJobListView(TestJobBase):
+    """A class for testing ExportJobListView."""
 
     def setUp(self):
         """Set up test data."""
         super().setUp()
 
-    def test_access(self):
-        url = reverse('export-job-list')
+        self.admin_job = Job.objects.create(
+            jobslurmid='11111',
+            submitdate=self.current_time - datetime.timedelta(days=3),
+            startdate=self.current_time - datetime.timedelta(days=2),
+            enddate=self.current_time - datetime.timedelta(days=1),
+            userid=self.admin,
+            accountid=self.project3,
+            jobstatus='COMPLETING',
+            partition='test_partition1')
+        self.staff_job = Job.objects.create(
+            jobslurmid='22222',
+            submitdate=self.current_time - datetime.timedelta(days=3),
+            startdate=self.current_time - datetime.timedelta(days=2),
+            enddate=self.current_time - datetime.timedelta(days=1),
+            userid=self.staff,
+            accountid=self.project3,
+            jobstatus='COMPLETING',
+            partition='test_partition2')
+        self.manager_job_on_project3 = Job.objects.create(
+            jobslurmid='33333',
+            submitdate=self.current_time - datetime.timedelta(days=3),
+            startdate=self.current_time - datetime.timedelta(days=2),
+            enddate=self.current_time - datetime.timedelta(days=1),
+            userid=self.manager,
+            accountid=self.project3,
+            jobstatus='COMPLETING',
+            partition='test_partition2')
+
+    def _assert_csv_job_ids(self, user, query_params, expected_jobs):
+        """Assert that the CSV generated by the given User and query
+        parameters has exactly the given jobs, based on ID."""
+        self.client.login(username=user.username, password=self.password)
 
-        self.assert_has_access(self.user1, False, url)
-        self.assert_has_access(self.user2, False, url)
-        self.assert_has_access(self.admin, True, url)
-        self.assert_has_access(self.staff, False, url)
+        list_url = f'{reverse("slurm-job-list")}?{urlencode(query_params)}'
+        self.get_response(user, list_url)
 
-    def test_job_list_view(self):
-        """Testing if 'Export Job List to CSV' button appears"""
-        url = reverse('slurm-job-list')
-        response = self.get_response(self.user1, url)
-        self.assertNotContains(response, 'Export Job List to CSV')
+        export_url = reverse('export-job-list')
+        response = self.client.get(export_url)
+        csv_lines = copy.deepcopy(list(response.streaming_content))
 
-        response = self.get_response(self.staff, url)
-        self.assertNotContains(response, 'Export Job List to CSV')
+        actual_job_slurm_ids = set()
+        for line in csv_lines[1:]:
+            line = line.decode('utf-8')
+            actual_job_slurm_ids.add(line[:line.find(',')])
 
-        response = self.get_response(self.admin, url)
-        self.assertContains(response, 'Export Job List to CSV')
+        expected_job_slurm_ids = {job.jobslurmid for job in expected_jobs}
 
-    def test_job_list_saves_session(self):
-        """Testing if cleaned form data is saved in session"""
-        url = reverse('slurm-job-list') + '?show_all_jobs=on&username=user1&partition=testpartition'
-        self.get_response(self.admin, url)
+        self.assertEqual(actual_job_slurm_ids, expected_job_slurm_ids)
 
-        self.assertEqual(self.client.session.get('job_search_form_data')['username'],
-                         self.user1.username)
-        self.assertEqual(self.client.session.get('job_search_form_data')['partition'],
-                         'testpartition')
-
-    def create_job_csv_line(self, job):
+    def _create_job_csv_line(self, job):
         job_line = f'{job.jobslurmid},{job.userid.username},' \
                    f'{job.accountid.name},{job.partition},' \
                    f'{job.jobstatus},{job.submitdate},' \
@@ -424,14 +485,131 @@ def create_job_csv_line(self, job):
 
         return job_line
 
+    def test_view_access(self):
+        """Test that all users have access to the view."""
+        url = reverse('export-job-list')
+
+        self.assert_has_access(self.user1, True, url)
+        self.assert_has_access(self.user2, True, url)
+        self.assert_has_access(self.admin, True, url)
+        self.assert_has_access(self.staff, True, url)
+
+    def test_admin_job_accessibility(self):
+        """Test that admins can export all jobs across all users, as
+        well as their own jobs."""
+        # Superuser
+        query_params = {
+            'show_all_jobs': 'on',
+        }
+        expected_jobs = (
+            self.job1, self.job2, self.admin_job, self.staff_job,
+            self.manager_job_on_project3)
+        self._assert_csv_job_ids(self.admin, query_params, expected_jobs)
+
+        query_params = {}
+        expected_jobs = {self.admin_job}
+        self._assert_csv_job_ids(self.admin, query_params, expected_jobs)
+
+        # Staff
+        query_params = {
+            'show_all_jobs': 'on',
+        }
+        expected_jobs = (
+            self.job1, self.job2, self.admin_job, self.staff_job,
+            self.manager_job_on_project3)
+        self._assert_csv_job_ids(self.staff, query_params, expected_jobs)
+
+        query_params = {}
+        expected_jobs = {self.staff_job}
+        self._assert_csv_job_ids(self.staff, query_params, expected_jobs)
+
+    def test_pi_manager_accessibility(self):
+        """Test that project PIs and managers can export all jobs under
+        projects that they manage, as well as their own jobs."""
+        # PI of project1
+        query_params = {}
+        expected_jobs = (self.job1,)
+        self._assert_csv_job_ids(self.pi, query_params, expected_jobs)
+
+        # Manager of project1 (and regular user of project3)
+        query_params = {}
+        expected_jobs = (self.job1, self.manager_job_on_project3)
+        self._assert_csv_job_ids(self.manager, query_params, expected_jobs)
+
+    def test_user_accessibility(self):
+        """Test that regular users can only export their own jobs."""
+        # user1
+        query_params = {}
+        expected_jobs = (self.job1, self.job2)
+        self._assert_csv_job_ids(self.user1, query_params, expected_jobs)
+
+        # user2
+        query_params = {}
+        expected_jobs = ()
+        self._assert_csv_job_ids(self.user2, query_params, expected_jobs)
+
+    def test_show_all_jobs_ineffective_for_non_admins(self):
+        """Test that non-superuser/staff users may not view all jobs by
+        specifying 'show_all_jobs'."""
+        # user2
+        query_params = {
+            'show_all_jobs': 'on',
+        }
+        expected_jobs = ()
+        self._assert_csv_job_ids(self.user2, query_params, expected_jobs)
+
+        # PI of project1
+        query_params = {
+            'show_all_jobs': 'on',
+        }
+        expected_jobs = (self.job1,)
+        self._assert_csv_job_ids(self.pi, query_params, expected_jobs)
+
+    def test_filters_saved_in_session(self):
+        """Test that filters saved by the list view are accessible to
+        and considered by this view."""
+        self.client.login(username=self.user1.username, password=self.password)
+        export_url = reverse('export-job-list')
+
+        # Without visiting the list view first, no filters are set. All the
+        # user's jobs are exported.
+        response = self.client.get(export_url)
+        csv_lines = copy.deepcopy(list(response.streaming_content))
+        self.assertIn(f'jobslurmid,username,project_name,partition,'
+                      f'jobstatus,submitdate,startdate,enddate',
+                      csv_lines[0].decode('utf-8'))
+        job1_line = self._create_job_csv_line(self.job1)
+        job2_line = self._create_job_csv_line(self.job2)
+        self.assertIn(job1_line, csv_lines[1].decode('utf-8'))
+        self.assertIn(job2_line, csv_lines[2].decode('utf-8'))
+        self.assertEqual(len(csv_lines), 3)
+
+        # The user sets some filters on the list view.
+        query_params = {
+            'username': 'user1',
+            'partition': 'test_partition1',
+        }
+        list_url = f'{reverse("slurm-job-list")}?{urlencode(query_params)}'
+        self.get_response(self.user1, list_url)
+
+        # Only filtered jobs should be exported.
+        response = self.client.get(export_url)
+        csv_lines = copy.deepcopy(list(response.streaming_content))
+        self.assertIn(f'jobslurmid,username,project_name,partition,'
+                      f'jobstatus,submitdate,startdate,enddate',
+                      csv_lines[0].decode('utf-8'))
+        job1_line = self._create_job_csv_line(self.job1)
+        self.assertIn(job1_line, csv_lines[1].decode('utf-8'))
+        self.assertEqual(len(csv_lines), 2)
+
     def test_exported_csv_no_filtering(self):
         """Testing if exported CSV file has correct data"""
 
         url = reverse('export-job-list')
         self.client.login(username=self.admin.username, password=self.password)
 
         session = self.client.session
-        session['job_search_form_data'] = {'status': '',
+        session['job_search_filters'] = {'status': '',
                                            'jobslurmid': '',
                                            'project_name': '',
                                            'username': '',
@@ -453,12 +631,16 @@ def test_exported_csv_no_filtering(self):
                       f'jobstatus,submitdate,startdate,enddate',
                       csv_lines[0].decode('utf-8'))
 
-        job1_line = self.create_job_csv_line(self.job1)
-        job2_line = self.create_job_csv_line(self.job2)
+        job1_line = self._create_job_csv_line(self.job1)
+        job2_line = self._create_job_csv_line(self.job2)
+        admin_job_line = self._create_job_csv_line(self.admin_job)
+        staff_job_line = self._create_job_csv_line(self.staff_job)
 
         self.assertIn(job1_line, csv_lines[1].decode('utf-8'))
         self.assertIn(job2_line, csv_lines[2].decode('utf-8'))
-        self.assertEqual(len(csv_lines), 3)
+        self.assertIn(admin_job_line, csv_lines[3].decode('utf-8'))
+        self.assertIn(staff_job_line, csv_lines[4].decode('utf-8'))
+        self.assertEqual(len(csv_lines), 6)
 
     def test_exported_csv_with_filtering(self):
         """Testing if exported CSV file has correct data"""
@@ -467,7 +649,7 @@ def test_exported_csv_with_filtering(self):
         self.client.login(username=self.admin.username, password=self.password)
 
         session = self.client.session
-        session['job_search_form_data'] = {'status': '',
+        session['job_search_filters'] = {'status': '',
                                            'jobslurmid': '',
                                            'project_name': '',
                                            'username': '',
@@ -489,7 +671,9 @@ def test_exported_csv_with_filtering(self):
                       f'jobstatus,submitdate,startdate,enddate,service_units',
                       csv_lines[0].decode('utf-8'))
 
-        job1_line = self.create_job_csv_line(self.job1)
+        job1_line = self._create_job_csv_line(self.job1)
+        admin_job_line = self._create_job_csv_line(self.admin_job)
 
         self.assertIn(job1_line, csv_lines[1].decode('utf-8'))
-        self.assertEqual(len(csv_lines), 2)
+        self.assertIn(admin_job_line, csv_lines[2].decode('utf-8'))
+        self.assertEqual(len(csv_lines), 3)