Fix error when saving CourseEnrollment in admin

common/djangoapps/course_modes/admin.py

@@ -56,7 +56,7 @@ class CourseModeForm(forms.ModelForm):
     def __init__(self, *args, **kwargs):
         # If args is a QueryDict, then the ModelForm addition request came in as a POST with a course ID string.
         # Change the course ID string to a CourseLocator object by copying the QueryDict to make it mutable.
-        if len(args) > 0 and 'course' in args[0] and isinstance(args[0], QueryDict):
+        if args and 'course' in args[0] and isinstance(args[0], QueryDict):
             args_copy = args[0].copy()
             args_copy['course'] = CourseKey.from_string(args_copy['course'])
             args = [args_copy]

common/djangoapps/student/admin.py

@@ -7,6 +7,7 @@ from django.contrib.auth import get_user_model
 from django.contrib.auth.admin import UserAdmin as BaseUserAdmin
 from django.contrib.auth.forms import ReadOnlyPasswordHashField, UserChangeForm as BaseUserChangeForm
 from django.db import models
+from django.http.request import QueryDict
 from django.utils.translation import ugettext_lazy as _
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import CourseKey
@@ -149,13 +150,26 @@ class LinkedInAddToProfileConfigurationAdmin(admin.ModelAdmin):
 class CourseEnrollmentForm(forms.ModelForm):
 
     def __init__(self, *args, **kwargs):
+        # If args is a QueryDict, then the ModelForm addition request came in as a POST with a course ID string.
+        # Change the course ID string to a CourseLocator object by copying the QueryDict to make it mutable.
+        if args and 'course' in args[0] and isinstance(args[0], QueryDict):
+            args_copy = args[0].copy()
+            try:
+                args_copy['course'] = CourseKey.from_string(args_copy['course'])
+            except InvalidKeyError:
+                raise forms.ValidationError("Cannot make a valid CourseKey from id {}!".format(args_copy['course']))
+            args = [args_copy]
+
         super(CourseEnrollmentForm, self).__init__(*args, **kwargs)
 
         if self.data.get('course'):
             try:
                 self.data['course'] = CourseKey.from_string(self.data['course'])
-            except InvalidKeyError:
+            except AttributeError:
-                raise forms.ValidationError("Cannot make a valid CourseKey from id {}!".format(self.data['course']))
+                # Change the course ID string to a CourseLocator.
+                # On a POST request, self.data is a QueryDict and is immutable - so this code will fail.
+                # However, the args copy above before the super() call handles this case.
+                pass
 
     def clean_course_id(self):
         course_id = self.cleaned_data['course']

common/djangoapps/student/tests/test_admin_views.py

@@ -4,6 +4,7 @@ Tests student admin.py
 import ddt
 from django.contrib.admin.sites import AdminSite
 from django.contrib.auth.models import User
+from django.forms import ValidationError
 from django.urls import reverse
 from django.test import TestCase
 from mock import Mock
@@ -209,7 +210,7 @@ class CourseEnrollmentAdminTest(SharedModuleStoreTestCase):
         super(CourseEnrollmentAdminTest, self).setUp()
         self.user = UserFactory.create(is_staff=True, is_superuser=True)
         self.course = CourseFactory()
-        CourseEnrollmentFactory(
+        self.course_enrollment = CourseEnrollmentFactory(
             user=self.user,
             course_id=self.course.id,  # pylint: disable=no-member
         )
@@ -254,3 +255,46 @@ class CourseEnrollmentAdminTest(SharedModuleStoreTestCase):
             # Locate the <td> column containing the username
             user_field = next(col for col in response.context['results'][idx] if "field-user" in col)
             self.assertIn(username, user_field)
+
+    def test_save_toggle_active(self):
+        """
+        Edit a CourseEnrollment to toggle its is_active checkbox, save it and verify that it was toggled.
+        When the form is saved, Django uses a QueryDict object which is immutable and needs special treatment.
+        This test implicitly verifies that the POST parameters are handled correctly.
+        """
+        # is_active will change from True to False
+        self.assertTrue(self.course_enrollment.is_active)
+        data = {
+            'user': unicode(self.course_enrollment.user.id),
+            'course': unicode(self.course_enrollment.course.id),
+            'is_active': 'false',
+            'mode': self.course_enrollment.mode,
+        }
+
+        with COURSE_ENROLLMENT_ADMIN_SWITCH.override(active=True):
+            response = self.client.post(
+                reverse('admin:student_courseenrollment_change', args=(self.course_enrollment.id, )),
+                data=data,
+            )
+        self.assertEqual(response.status_code, 302)
+
+        self.course_enrollment.refresh_from_db()
+        self.assertFalse(self.course_enrollment.is_active)
+
+    def test_save_invalid_course_id(self):
+        """
+        Send an invalid course ID instead of "org.0/course_0/Run_0" when saving, and verify that it fails.
+        """
+        data = {
+            'user': unicode(self.course_enrollment.user.id),
+            'course': 'invalid-course-id',
+            'is_active': 'true',
+            'mode': self.course_enrollment.mode,
+        }
+
+        with COURSE_ENROLLMENT_ADMIN_SWITCH.override(active=True):
+            with self.assertRaises(ValidationError):
+                self.client.post(
+                    reverse('admin:student_courseenrollment_change', args=(self.course_enrollment.id, )),
+                    data=data,
+                )