From ba5c5e0913e1286935364aa04a5c3e4a81412b5e Mon Sep 17 00:00:00 2001
From: Guruprasad Lakshmi Narayanan <guruprasad@opencraft.com>
Date: Thu, 5 Sep 2019 02:07:56 +0530
Subject: [PATCH] Fix the automatic enrollment issue for inactive user

When a user registers an account and is enrolled in a course by the
instructor before completing the email validation, the course shows up
in the dashboard of the logged-in user. This change adds a check for
the email validation before enrolling the user.
---
 lms/djangoapps/instructor/enrollment.py       |  2 +-
 .../instructor/tests/test_enrollment.py       | 70 ++++++++++++++++++-
 2 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py
index fb40415fdb..19f22900f3 100644
--- a/lms/djangoapps/instructor/enrollment.py
+++ b/lms/djangoapps/instructor/enrollment.py
@@ -137,7 +137,7 @@ def enroll_email(course_id, student_email, auto_enroll=False, email_students=Fal
     """
     previous_state = EmailEnrollmentState(course_id, student_email)
     enrollment_obj = None
-    if previous_state.user:
+    if previous_state.user and User.objects.get(email=student_email).is_active:
         # if the student is currently unenrolled, don't enroll them in their
         # previous mode
 
diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py
index 0bcfd4240e..86a1eda941 100644
--- a/lms/djangoapps/instructor/tests/test_enrollment.py
+++ b/lms/djangoapps/instructor/tests/test_enrollment.py
@@ -106,6 +106,7 @@ class TestEnrollmentChangeBase(six.with_metaclass(ABCMeta, CacheIsolationTestCas
         self.assertEqual(after, after_ideal)
 
 
+@ddt.ddt
 class TestInstructorEnrollDB(TestEnrollmentChangeBase):
     """ Test instructor.enrollment.enroll_email """
     def test_enroll(self):
@@ -222,6 +223,71 @@ class TestInstructorEnrollDB(TestEnrollmentChangeBase):
 
         return self._run_state_change_test(before_ideal, after_ideal, action)
 
+    @ddt.data(True, False)
+    def test_enroll_inactive_user(self, auto_enroll):
+        before_ideal = SettableEnrollmentState(
+            user=True,
+            enrollment=False,
+            allowed=False,
+            auto_enroll=False,
+        )
+        print("checking initialization...")
+        eobjs = before_ideal.create_user(self.course_key, is_active=False)
+        before = EmailEnrollmentState(self.course_key, eobjs.email)
+        self.assertEqual(before, before_ideal)
+
+        print('running action...')
+        enroll_email(self.course_key, eobjs.email, auto_enroll=auto_enroll)
+
+        print('checking effects...')
+
+        after_ideal = SettableEnrollmentState(
+            user=True,
+            enrollment=False,
+            allowed=True,
+            auto_enroll=auto_enroll,
+        )
+        after = EmailEnrollmentState(self.course_key, eobjs.email)
+        self.assertEqual(after, after_ideal)
+
+    @ddt.data(True, False)
+    def test_enroll_inactive_user_again(self, auto_enroll):
+        course_key = CourseLocator('Robot', 'fAKE', 'C--se--ID')
+        before_ideal = SettableEnrollmentState(
+            user=True,
+            enrollment=False,
+            allowed=True,
+            auto_enroll=auto_enroll,
+        )
+        print("checking initialization...")
+        user = UserFactory()
+        user.is_active = False
+        user.save()
+        eobjs = EnrollmentObjects(
+            user.email,
+            None,
+            None,
+            CourseEnrollmentAllowed.objects.create(
+                email=user.email, course_id=course_key, auto_enroll=auto_enroll
+            )
+        )
+        before = EmailEnrollmentState(course_key, eobjs.email)
+        self.assertEqual(before, before_ideal)
+
+        print('running action...')
+        enroll_email(self.course_key, eobjs.email, auto_enroll=auto_enroll)
+
+        print('checking effects...')
+
+        after_ideal = SettableEnrollmentState(
+            user=True,
+            enrollment=False,
+            allowed=True,
+            auto_enroll=auto_enroll,
+        )
+        after = EmailEnrollmentState(self.course_key, eobjs.email)
+        self.assertEqual(after, after_ideal)
+
 
 class TestInstructorUnenrollDB(TestEnrollmentChangeBase):
     """ Test instructor.enrollment.unenroll_email """
@@ -756,7 +822,7 @@ class SettableEnrollmentState(EmailEnrollmentState):
     def __neq__(self, other):
         return not self == other
 
-    def create_user(self, course_id=None):
+    def create_user(self, course_id=None, is_active=True):
         """
         Utility method to possibly create and possibly enroll a user.
         Creates a state matching the SettableEnrollmentState properties.
@@ -770,7 +836,7 @@ class SettableEnrollmentState(EmailEnrollmentState):
         # if self.user=False, then this will just be used to generate an email.
         email = "robot_no_user_exists_with_this_email@edx.org"
         if self.user:
-            user = UserFactory()
+            user = UserFactory(is_active=is_active)
             email = user.email
             if self.enrollment:
                 cenr = CourseEnrollment.enroll(user, course_id)