diff --git a/cms/djangoapps/course_creators/admin.py b/cms/djangoapps/course_creators/admin.py index 75cde2ebd9..f2d014a2c4 100644 --- a/cms/djangoapps/course_creators/admin.py +++ b/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 diff --git a/cms/djangoapps/course_creators/migrations/0001_initial.py b/cms/djangoapps/course_creators/migrations/0001_initial.py index dea4a160c7..dd36b79fb3 100644 --- a/cms/djangoapps/course_creators/migrations/0001_initial.py +++ b/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'}) } } diff --git a/cms/djangoapps/course_creators/models.py b/cms/djangoapps/course_creators/models.py index 7b75af35c8..e67c57b1be 100644 --- a/cms/djangoapps/course_creators/models.py +++ b/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() diff --git a/cms/djangoapps/course_creators/tests/test_admin.py b/cms/djangoapps/course_creators/tests/test_admin.py index 94886fbdb3..d06b9066c4 100644 --- a/cms/djangoapps/course_creators/tests/test_admin.py +++ b/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) diff --git a/cms/djangoapps/course_creators/tests/test_views.py b/cms/djangoapps/course_creators/tests/test_views.py index 0ea5656e21..71d3e1523b 100644 --- a/cms/djangoapps/course_creators/tests/test_views.py +++ b/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) diff --git a/cms/djangoapps/course_creators/views.py b/cms/djangoapps/course_creators/views.py index 550ed9f548..4cf1693a97 100644 --- a/cms/djangoapps/course_creators/views.py +++ b/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()