Change user to foreign key (tracks deletions).

cms/djangoapps/course_creators/admin.py

@@ -6,25 +6,32 @@ from course_creators.models import CourseCreator
 from django.contrib import admin
 
 
+def get_email(obj):
+    """ Returns the email address for a user """
+    return obj.user.email
+
+get_email.short_description = 'email'
+
+
 class CourseCreatorAdmin(admin.ModelAdmin):
     """
     Admin for the course creator table.
     """
 
     # Fields to display on the overview page.
-    list_display = ['username', 'email', 'state', 'state_changed', 'note']
-    readonly_fields = ['username', 'email', 'state_changed']
+    list_display = ['user', get_email, 'state', 'state_changed', 'note']
+    readonly_fields = ['user', 'state_changed']
     # Controls the order on the edit form (without this, read-only fields appear at the end).
     fieldsets = (
         (None, {
-            'fields': list_display
+            'fields': ['user', 'state', 'state_changed', 'note']
         }),
     )
     # Fields that filtering support
     list_filter = ['state', 'state_changed']
     # Fields that search supports. Note that the search term for state has to be
     # its key (ie, 'g' instead of 'granted').
-    search_fields = ['username', 'email', 'state', 'note']
+    search_fields = ['user__username', 'user__email', 'state', 'note']
     # Turn off the action bar (we have no bulk actions)
     actions = None
 

cms/djangoapps/course_creators/migrations/0001_initial.py

@@ -10,8 +10,8 @@ class Migration(SchemaMigration):
     def forwards(self, orm):
         # Adding model 'CourseCreator'
         db.create_table('course_creators_coursecreator', (
-            ('username', self.gf('django.db.models.fields.CharField')(unique=True, max_length=64, primary_key=True)),
-            ('email', self.gf('django.db.models.fields.CharField')(max_length=128)),
+            ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)),
+            ('user', self.gf('django.db.models.fields.related.ForeignKey')(to=orm['auth.User'], unique=True)),
             ('state_changed', self.gf('django.db.models.fields.DateTimeField')(auto_now_add=True, blank=True)),
             ('state', self.gf('django.db.models.fields.CharField')(default='u', max_length=1)),
             ('note', self.gf('django.db.models.fields.CharField')(max_length=512, blank=True)),
@@ -25,13 +25,49 @@ class Migration(SchemaMigration):
 
 
     models = {
+        'auth.group': {
+            'Meta': {'object_name': 'Group'},
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}),
+            'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'})
+        },
+        'auth.permission': {
+            'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'},
+            'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '50'})
+        },
+        'auth.user': {
+            'Meta': {'object_name': 'User'},
+            'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}),
+            'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+            'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}),
+            'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'})
+        },
+        'contenttypes.contenttype': {
+            'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"},
+            'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '100'})
+        },
         'course_creators.coursecreator': {
             'Meta': {'object_name': 'CourseCreator'},
-            'email': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
             'note': ('django.db.models.fields.CharField', [], {'max_length': '512', 'blank': 'True'}),
             'state': ('django.db.models.fields.CharField', [], {'default': "'u'", 'max_length': '1'}),
             'state_changed': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
-            'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '64', 'primary_key': 'True'})
+            'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'unique': 'True'})
         }
     }
 

cms/djangoapps/course_creators/models.py

@@ -4,7 +4,8 @@ Table for storing information about whether or not Studio users have course crea
 from django.db import models
 from django.db.models.signals import post_init, post_save
 from django.dispatch import receiver
-from auth.authz import add_user_to_creator_group, remove_user_from_creator_group, get_user_by_email
+from django.contrib.auth.models import User
+from auth.authz import add_user_to_creator_group, remove_user_from_creator_group
 
 from django.utils import timezone
 
@@ -19,8 +20,7 @@ class CourseCreator(models.Model):
               (u'd', u'denied'),
              )
 
-    username = models.CharField(max_length=64, blank=False, help_text="Studio username", primary_key=True, unique=True)
-    email = models.CharField(max_length=128, blank=False, help_text="Registered e-mail address")
+    user = models.ForeignKey(User, help_text="Studio user", unique=True)
     state_changed = models.DateTimeField('state last updated', auto_now_add=True,
                                          help_text='The date when state was last updated')
     state = models.CharField(max_length=1, blank=False, choices=STATES, default='u',
@@ -29,45 +29,34 @@ class CourseCreator(models.Model):
                                                                   'why course creation access was denied)')
 
     def __unicode__(self):
-        s = "%str %str | %str [%str] | %str" % (self.username, self.email, self.state, self.state_changed, self.note)
+        s = "%str %str | %str [%str] | %str" % (self.user.username, self.user.email, self.state, self.state_changed, self.note)
         return s
 
 
 @receiver(post_init, sender=CourseCreator)
 def post_init_callback(sender, **kwargs):
     """
-    Extend to remove deleted users and store previous state.
+    Extend to store previous state.
     """
     instance = kwargs['instance']
-    user = get_user_by_email(instance.email)
-    if user is None:
-        # User has been removed, delete from this table.
-        instance.delete()
-    else:
-        instance.orig_state = instance.state
+    instance.orig_state = instance.state
 
 
 @receiver(post_save, sender=CourseCreator)
 def post_save_callback(sender, **kwargs):
     """
-    Extend to remove deleted users, update state_changed time,
-    and modify the course creator group in authz.py.
+    Extend to update state_changed time and modify the course creator group in authz.py.
     """
     instance = kwargs['instance']
     # We only wish to modify the state_changed time if the state has been modified. We don't wish to
     # modify it for changes to the notes field.
     if instance.state != instance.orig_state:
-        user = get_user_by_email(instance.email)
-        if user is None:
-            # User has been removed, delete from this table.
-            instance.delete()
+        instance.state_changed = timezone.now()
+        if instance.state == 'g':
+            # We have granted access, add to course group
+            add_user_to_creator_group(instance.admin, instance.user)
         else:
-            instance.state_changed = timezone.now()
-            if instance.state == 'g':
-                # We have granted access, add to course group
-                add_user_to_creator_group(instance.admin, user)
-            else:
-                remove_user_from_creator_group(instance.admin, user)
+            remove_user_from_creator_group(instance.admin, instance.user)
 
-            instance.orig_state = instance.state
-            instance.save()
+        instance.orig_state = instance.state
+        instance.save()

cms/djangoapps/course_creators/tests/test_admin.py

@@ -21,7 +21,7 @@ class CourseCreatorAdminTest(TestCase):
     def setUp(self):
         """ Test case setup """
         self.user = User.objects.create_user('test_user', 'test_user+courses@edx.org', 'foo')
-        self.table_entry = CourseCreator(username=self.user.username, email=self.user.email)
+        self.table_entry = CourseCreator(user=self.user)
         self.table_entry.save()
 
         self.admin = User.objects.create_user('Mark', 'admin+courses@edx.org', 'foo')
@@ -64,15 +64,3 @@ class CourseCreatorAdminTest(TestCase):
             # and change state back to 'u' (unrequested)
             change_state('u', False)
 
-
-    def test_delete_bad_user(self):
-        """
-        Tests that users who no longer exist are deleted from the table.
-        """
-        with mock.patch.dict('django.conf.settings.MITX_FEATURES', {"ENABLE_CREATOR_GROUP": True}):
-            self.assertEqual('test_user', self.table_entry.username)
-            self.user.delete()
-            # Go through the post-save update, which will delete users who no longer exist.
-            self.table_entry.state = 'g'
-            self.creator_admin.save_model(self.request, self.table_entry, None, True)
-            self.assertEqual(None, self.table_entry.username)

cms/djangoapps/course_creators/tests/test_views.py

@@ -35,8 +35,7 @@ class CourseCreatorView(TestCase):
             add_user_with_status_unrequested(self.user, self.user)
 
     def test_table_initially_empty(self):
-        with self.assertRaises(AssertionError):
-            get_course_creator_status(self.user)
+        self.assertIsNone(get_course_creator_status(self.user))
 
     def test_add_unrequested(self):
         add_user_with_status_unrequested(self.admin, self.user)

cms/djangoapps/course_creators/views.py

@@ -29,17 +29,21 @@ def add_user_with_status_granted(caller, user):
 
 def get_course_creator_status(user):
     """
-    Returns the status for a particular user.
+    Returns the status for a particular user, or None if user is not in the table.
 
     Possible return values are:
         'g' = 'granted'
         'u' = 'unrequested'
         'p' = 'pending'
         'd' = 'denied'
+        None = user does not exist in the table
     """
-    user = CourseCreator.objects.filter(username=user.username)
-    assert user.count() == 1, "The user does not exist in the table."
-    return user[0].state
+    user = CourseCreator.objects.filter(user=user)
+    if user.count() == 0:
+        return None
+    else:
+        # User is defined to be unique, can assume a single entry.
+        return user[0].state
 
 
 def _add_user(caller, user, state):
@@ -49,6 +53,6 @@ def _add_user(caller, user, state):
     if not caller.is_active or not caller.is_authenticated or not caller.is_staff:
         raise PermissionDenied
 
-    if CourseCreator.objects.filter(username=user.username).count() == 0:
-        entry = CourseCreator(username=user.username, email=user.email, state=state)
+    if CourseCreator.objects.filter(user=user).count() == 0:
+        entry = CourseCreator(user=user, state=state)
         entry.save()