chore: deprecate edx-sphinx-theme

.github/workflows/ci.yml

@@ -16,7 +16,7 @@ jobs:
       fail-fast: false
       matrix:
         os: [ubuntu-20.04]
-        python-version: ['3.8']
+        python-version: ['3.12']
         toxenv: [django42, quality, pii_check]
 
     steps:
@@ -38,7 +38,7 @@ jobs:
       run: tox
 
     - name: Run coverage
-      if: matrix.python-version == '3.8' && matrix.toxenv == 'django42'
+      if: matrix.python-version == '3.12' && matrix.toxenv == 'django42'
       uses: py-cov-action/python-coverage-comment-action@v3
       with:
         GITHUB_TOKEN: ${{ github.token }}

.github/workflows/upgrade-python-requirements.yml

@@ -15,6 +15,7 @@ jobs:
     uses: openedx/.github/.github/workflows/upgrade-python-requirements.yml@master
     with:
       branch: ${{ github.event.inputs.branch || 'main' }}
+      python_version: "3.12"
       # optional parameters below; fill in if you'd like github or email notifications
       # user_reviewers: ""
       # team_reviewers: ""

Dockerfile

@@ -24,25 +24,50 @@ MAINTAINER sre@edx.org
 
 # make; necessary to provision the container
 
+# ENV variables for Python 3.12 support
+ARG PYTHON_VERSION=3.12
+ENV TZ=UTC
+ENV TERM=xterm-256color
+ENV DEBIAN_FRONTEND=noninteractive
+
+# software-properties-common is needed to setup Python 3.12 env
+RUN apt-get update && \
+  apt-get install -y software-properties-common && \
+  apt-add-repository -y ppa:deadsnakes/ppa
+
 # If you add a package here please include a comment above describing what it is used for
 RUN apt-get update && apt-get -qy install --no-install-recommends \
+ build-essential \
  language-pack-en \
  locales \
- python3.8 \
- python3-pip \
  libmysqlclient-dev \
  pkg-config \
  libssl-dev \
- python3-dev \
  gcc \
  make \
- git
+ git \
+ curl \
+ python3-pip \
+ python${PYTHON_VERSION} \
+ python${PYTHON_VERSION}-dev \
+ python${PYTHON_VERSION}-distutils 
 
+RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone
 
 RUN pip install --upgrade pip setuptools
 # delete apt package lists because we do not need them inflating our image
 RUN rm -rf /var/lib/apt/lists/*
 
+# need to use virtualenv pypi package with Python 3.12
+RUN curl -sS https://bootstrap.pypa.io/get-pip.py | python${PYTHON_VERSION}
+RUN pip install virtualenv
+
+# Create virtual environment with Python 3.12
+ENV VIRTUAL_ENV=/edx/venvs/edx-exams
+RUN virtualenv -p python${PYTHON_VERSION} $VIRTUAL_ENV
+ENV PATH="$VIRTUAL_ENV/bin:$PATH"
+
+# python is python3
 RUN ln -s /usr/bin/python3 /usr/bin/python
 
 RUN locale-gen en_US.UTF-8

docs/conf.py

@@ -15,8 +15,9 @@
 import re
 import sys
 from subprocess import check_call
+import datetime
 
-import edx_theme
+import sphinx_book_theme
 
 
 def get_version(*file_paths):
@@ -59,7 +60,7 @@ def get_version(*file_paths):
 # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom
 # ones.
 extensions = [
-    'edx_theme',
+    'sphinx_book_theme',
     'sphinx.ext.autodoc',
     'sphinx.ext.doctest',
     'sphinx.ext.intersphinx',
@@ -90,8 +91,8 @@ def get_version(*file_paths):
 
 # General information about the project.
 project = 'edx_exams'
-copyright = edx_theme.COPYRIGHT  # pylint: disable=redefined-builtin
-author = edx_theme.AUTHOR
+copyright = '{year}, edX Inc.'.format(year=datetime.datetime.now().year)  # pylint: disable=redefined-builtin
+author = 'edx-org'
 project_title = 'edx_exams'
 documentation_title = f"{project_title}"
 
@@ -172,17 +173,14 @@ def get_version(*file_paths):
 # The theme to use for HTML and HTML Help pages.  See the documentation for
 # a list of builtin themes.
 
-html_theme = 'edx_theme'
+html_theme = 'sphinx_book_theme'
 
 # Theme options are theme-specific and customize the look and feel of a theme
 # further.  For a list of options available for each theme, see the
 # documentation.
 #
 # html_theme_options = {}
 
-# Add any paths that contain custom themes here, relative to this directory.
-html_theme_path = [edx_theme.get_html_theme_path()]
-
 # The name for this set of Sphinx documents.
 # "<project> v<release> documentation" by default.
 #

edx_exams/apps/api/v1/tests/test_views.py

@@ -1789,16 +1789,18 @@ def setUp(self):
             course_id=self.course_id,
         )
 
-    def request_api(self, method, user, course_id, data=None):
+    def request_api(self, method, user, course_id, data=None, allowance_id=None):
         """
         Helper function to make API request
         """
-        assert method in ['get', 'post']
+        assert method in ['get', 'post', 'delete']
         headers = self.build_jwt_headers(user)
         url = reverse(
             'api:v1:course-allowances',
             kwargs={'course_id': course_id}
         )
+        if allowance_id:
+            url = f'{url}/{allowance_id}'
 
         if data:
             return getattr(self.client, method)(url, json.dumps(data), **headers, content_type='application/json')
@@ -1878,6 +1880,38 @@ def test_get_empty_response(self):
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.data, [])
 
+    def test_delete(self):
+        """
+        Test that the endpoint deletes an allowance
+        """
+        allowance = StudentAllowanceFactory.create(
+            exam=self.exam,
+            user=self.user,
+        )
+
+        response = self.request_api('delete', self.user, self.exam.course_id, allowance_id=allowance.id)
+        self.assertEqual(response.status_code, 204)
+        self.assertFalse(StudentAllowance.objects.filter(id=allowance.id).exists())
+
+    def test_delete_not_found(self):
+        """
+        Test that 404 is returned if allowance does not exist
+        """
+        response = self.request_api('delete', self.user, self.exam.course_id, allowance_id=19)
+        self.assertEqual(response.status_code, 404)
+
+    def test_delete_not_allowed(self):
+        """
+        Test that 404 is returned if the allowance is not for the authorized course
+        """
+        other_course_id = 'course-v1:edx+another+course'
+        allowance = StudentAllowanceFactory.create(
+            exam=ExamFactory.create(course_id=other_course_id),
+            user=self.user,
+        )
+        response = self.request_api('delete', self.user, self.exam.course_id, allowance_id=allowance.id)
+        self.assertEqual(response.status_code, 404)
+
     def test_post_allowances(self):
         """
         Test that the endpoint creates allowances for the given request data
@@ -1927,7 +1961,7 @@ def test_post_invalid_field_value(self):
         response = self.request_api('post', self.user, self.exam.course_id, data=request_data)
         self.assertEqual(response.status_code, 400)
 
-    def test_post_invalid_missing_user(self):
+    def test_post_missing_user(self):
         """
         Test that 400 response is returned if serializer is invalid due to missing required field
         """
@@ -1936,3 +1970,28 @@ def test_post_invalid_missing_user(self):
         ]
         response = self.request_api('post', self.user, self.exam.course_id, data=request_data)
         self.assertEqual(response.status_code, 400)
+
+    def test_post_invalid_user(self):
+        """
+        Test that 400 response is returned if a username/email does not exist
+        """
+        request_data = [
+            {'exam_id': self.exam.id, 'username': 'junk', 'extra_time_mins': 45},
+        ]
+        response = self.request_api('post', self.user, self.exam.course_id, data=request_data)
+        self.assertEqual(response.status_code, 400)
+
+    def test_post_unauthorized_course(self):
+        """
+        Test that an error is returned when atttempting to create an allowance for
+        an exam in a different course.
+        """
+        other_course_id = 'course-v1:edx+another+course'
+        other_course_exam = ExamFactory.create(course_id=other_course_id)
+
+        request_data = [
+            {'exam_id': self.exam.id, 'username': self.user.username, 'extra_time_mins': 45},
+            {'exam_id': other_course_exam.id, 'username': self.user.username, 'extra_time_mins': 45},
+        ]
+        response = self.request_api('post', self.user, self.exam.course_id, data=request_data)
+        self.assertEqual(response.status_code, 400)

edx_exams/apps/api/v1/urls.py

@@ -19,6 +19,11 @@
 app_name = 'v1'
 
 urlpatterns = [
+    re_path(
+        fr'exams/course_id/{COURSE_ID_PATTERN}/allowances/(?P<allowance_id>\d+)',
+        AllowanceView.as_view(),
+        name='course-allowance'
+    ),
     re_path(
         fr'exams/course_id/{COURSE_ID_PATTERN}/allowances',
         AllowanceView.as_view(),

edx_exams/apps/api/v1/views.py

@@ -828,7 +828,21 @@ def get(self, request, course_id):
         allowances = StudentAllowance.get_allowances_for_course(course_id)
         return Response(AllowanceSerializer(allowances, many=True).data)
 
-    def post(self, request, course_id):  # pylint: disable=unused-argument
+    def delete(self, request, course_id, allowance_id):
+        """
+        HTTP DELETE handler. Deletes all allowances for a course.
+        """
+        try:
+            StudentAllowance.objects.get(id=allowance_id, exam__course_id=course_id).delete()
+        except StudentAllowance.DoesNotExist:
+            return Response(
+                status=status.HTTP_404_NOT_FOUND,
+                data={'detail': f'Allowance with id={allowance_id} does not exist.'}
+            )
+
+        return Response(status=status.HTTP_204_NO_CONTENT)
+
+    def post(self, request, course_id):
         """
         HTTP POST handler. Creates allowances based on the given list.
         """
@@ -840,24 +854,31 @@ def post(self, request, course_id):  # pylint: disable=unused-argument
             # We expect the number of allowances in each request to be small. Should they increase,
             # we should not query within the loop, and instead refactor this to optimize
             # the DB calls.
-            allowance_objects = [
-                StudentAllowance(
-                    user=(
-                        User.objects.get(username=allowance['username'])
-                        if allowance.get('username')
-                        else User.objects.get(email=allowance['email'])
-                    ),
-                    exam=Exam.objects.get(id=allowance['exam_id']),
-                    extra_time_mins=allowance['extra_time_mins']
+            try:
+                allowance_objects = [
+                    StudentAllowance(
+                        user=(
+                            User.objects.get(username=allowance['username'])
+                            if allowance.get('username')
+                            else User.objects.get(email=allowance['email'])
+                        ),
+                        exam=Exam.objects.get(id=allowance['exam_id'], course_id=course_id),
+                        extra_time_mins=allowance['extra_time_mins']
+                    )
+                    for allowance in allowances
+                ]
+            except Exam.DoesNotExist:
+                return Response(
+                    status=status.HTTP_400_BAD_REQUEST,
+                    data={'detail': 'Exam does not exist'}
                 )
-                for allowance in allowances
-            ]
-            StudentAllowance.objects.bulk_create(
-                allowance_objects,
-                update_conflicts=True,
-                unique_fields=['user', 'exam'],
-                update_fields=['extra_time_mins']
-            )
+            except User.DoesNotExist:
+                return Response(
+                    status=status.HTTP_400_BAD_REQUEST,
+                    data={'detail': 'Learner with username/email not found'}
+                )
+
+            StudentAllowance.bulk_create_or_update(allowance_objects)
 
             return Response(status=status.HTTP_200_OK)
         else:

edx_exams/apps/core/api.py

@@ -99,8 +99,11 @@ def update_attempt_status(attempt_id, to_status):
         if not allowed_to_start:
             raise ExamIllegalStatusTransition(error_msg)
 
-        attempt_obj.start_time = datetime.now(pytz.UTC)
-        attempt_obj.allowed_time_limit_mins = _calculate_allowed_mins(attempt_obj.user, attempt_obj.exam)
+        # Once start time, and end time by extension, has been set further transitions to started
+        # must not update this value
+        if not attempt_obj.start_time:
+            attempt_obj.start_time = datetime.now(pytz.UTC)
+            attempt_obj.allowed_time_limit_mins = _calculate_allowed_mins(attempt_obj.user, attempt_obj.exam)
 
     course_key = CourseKey.from_string(attempt_obj.exam.course_id)
     usage_key = UsageKey.from_string(attempt_obj.exam.content_id)

edx_exams/apps/core/constants.py

@@ -18,3 +18,5 @@ class Status:
 USAGE_KEY_PATTERN = r'(?P<content_id>(?:i4x://?[^/]+/[^/]+/[^/]+/[^@]+(?:@[^/]+)?)|(?:[^/]+))'
 
 EXAM_ID_PATTERN = r'(?P<exam_id>\d+)'
+
+ATTEMPT_ID_PATTERN = r'(?P<attempt_id>\d+)'

edx_exams/apps/core/management/commands/bulk_add_course_staff.py

@@ -4,7 +4,7 @@
 import time
 
 from django.core.management.base import BaseCommand
-from django.db import transaction
+from django.db import IntegrityError, transaction
 
 from edx_exams.apps.core.models import CourseStaffRole, User
 
@@ -63,32 +63,31 @@ def add_course_staff_from_csv(self, csv_file, batch_size, batch_delay):
         Add the given set of course staff provided in csv
         """
         reader = list(csv.DictReader(csv_file))
-        users_to_create = []
-        users_existing = {u.username for u in User.objects.filter(username__in=[r.get('username') for r in reader])}
-        for row in reader:
-            if row.get('username') not in users_existing:
-                users_to_create.append(row)
-                users_existing.add(row.get('username'))
+        users = {}
 
-        # bulk create users
-        for i in range(0, len(users_to_create), batch_size):
-            User.objects.bulk_create(
-                User(
-                    username=user.get('username'),
-                    email=user.get('email'),
-                )
-                for user in users_to_create[i:i + batch_size]
-            )
-            time.sleep(batch_delay)
-
-        # bulk create course staff
         for i in range(0, len(reader), batch_size):
-            CourseStaffRole.objects.bulk_create(
-                CourseStaffRole(
-                    user=User.objects.get(username=row.get('username')),
-                    course_id=row.get('course_id'),
-                    role=row.get('role'),
-                )
-                for row in reader[i:i + batch_size]
-            )
+            users_list = []
+            for row in reader[i:i + batch_size]:
+                username = row.get('username')
+                email = row.get('email')
+                try:
+                    users_list.append(User.objects.get_or_create(username=row.get('username'), email=row.get('email')))
+                except IntegrityError:
+                    logger.warning(
+                        f'User with username={username} and email={email} was not created due to an existing duplicate '
+                        f'user with username.'
+                    )
+                    continue
+            users_dict = {(u.username, u) for (u, c) in users_list}
+            users.update(users_dict)
             time.sleep(batch_delay)
+
+        CourseStaffRole.objects.bulk_create(
+            (CourseStaffRole(
+                user=users.get(row.get('username')),
+                course_id=row.get('course_id'),
+                role=row.get('role'),
+            ) for row in reader),
+            ignore_conflicts=True,
+            batch_size=batch_size,
+        )