From f1f6acb5c70a7e08baac3783cd8e272cb7762bff Mon Sep 17 00:00:00 2001 From: cahrens Date: Wed, 3 Jul 2013 17:07:06 -0400 Subject: [PATCH] Updates to admin table view. Also hooks up the granted and deny events to the course creator group. --- cms/djangoapps/auth/authz.py | 30 ++++++++++++++----- cms/djangoapps/auth/tests/test_authz.py | 18 +++++++++-- .../management/commands/populate_creators.py | 18 +++++++++-- cms/djangoapps/course_creators/admin.py | 22 +++++++++++++- .../migrations/0001_initial.py | 2 ++ cms/djangoapps/course_creators/models.py | 22 +++++++++----- cms/djangoapps/course_creators/views.py | 11 +++++-- 7 files changed, 102 insertions(+), 21 deletions(-) diff --git a/cms/djangoapps/auth/authz.py b/cms/djangoapps/auth/authz.py index 77e4d5037d..d5179da05e 100644 --- a/cms/djangoapps/auth/authz.py +++ b/cms/djangoapps/auth/authz.py @@ -3,7 +3,6 @@ from django.core.exceptions import PermissionDenied from django.conf import settings from xmodule.modulestore import Location -from course_creators import views ''' This code is somewhat duplicative of access.py in the LMS. We will unify the code as a separate story @@ -215,13 +214,30 @@ def _grant_instructors_creator_access(caller): This is to be called only by either a command line code path or through an app which has already asserted permissions to do this action. - Gives all users with instructor role course creator rights. + Gives all users with instructor role course creator rights, and returns the set of + such users. This is only intended to be run once on a given environment. """ + instructors = _get_users_with_role(INSTRUCTOR_ROLE_NAME) + for user in instructors: + add_user_to_creator_group(caller, user) + return instructors + + +def get_users_with_staff_role(): + """ + Returns all users with the role 'staff' + """ + return _get_users_with_role(STAFF_ROLE_NAME) + + +def _get_users_with_role(role): + """ + Returns all users with the specified role. + """ + users = set() for group in Group.objects.all(): - if group.name.startswith(INSTRUCTOR_ROLE_NAME + "_"): + if group.name.startswith(role + "_"): for user in group.user_set.all(): - add_user_to_creator_group(caller, user) - views.add_user_with_status_granted(user) - elif group.name.startswith(STAFF_ROLE_NAME + "_"): - views.add_user_with_status_unrequested(user) + users.add(user) + return users diff --git a/cms/djangoapps/auth/tests/test_authz.py b/cms/djangoapps/auth/tests/test_authz.py index 658c176498..dd04a7d41d 100644 --- a/cms/djangoapps/auth/tests/test_authz.py +++ b/cms/djangoapps/auth/tests/test_authz.py @@ -9,7 +9,8 @@ from django.core.exceptions import PermissionDenied from auth.authz import add_user_to_creator_group, remove_user_from_creator_group, is_user_in_creator_group,\ create_all_course_groups, add_user_to_course_group, STAFF_ROLE_NAME, INSTRUCTOR_ROLE_NAME,\ - is_user_in_course_group_role, remove_user_from_course_group, _grant_instructors_creator_access + is_user_in_course_group_role, remove_user_from_course_group, _grant_instructors_creator_access,\ + get_users_with_staff_role class CreatorGroupTest(TestCase): @@ -175,6 +176,18 @@ class CourseGroupTest(TestCase): with self.assertRaises(PermissionDenied): remove_user_from_course_group(self.staff, self.staff, self.location, STAFF_ROLE_NAME) + def test_get_staff(self): + # Do this test with staff in 2 different classes. + create_all_course_groups(self.creator, self.location) + add_user_to_course_group(self.creator, self.staff, self.location, STAFF_ROLE_NAME) + + location2 = 'i4x', 'mitX', '103', 'course2', 'test2' + staff2 = User.objects.create_user('teststaff2', 'teststaff+courses@edx.org', 'foo') + create_all_course_groups(self.creator, location2) + add_user_to_course_group(self.creator, staff2, location2, STAFF_ROLE_NAME) + + self.assertSetEqual({self.staff, staff2, self.creator}, get_users_with_staff_role()) + class GrantInstructorsCreatorAccessTest(TestCase): """ @@ -206,7 +219,8 @@ class GrantInstructorsCreatorAccessTest(TestCase): admin = User.objects.create_user('populate_creators_command', 'grant+creator+access@edx.org', 'foo') admin.is_staff = True - _grant_instructors_creator_access(admin) + instructors = _grant_instructors_creator_access(admin) + self.assertSetEqual({creator1, creator2}, instructors) # Now instructors only are creators. self.assertTrue(is_user_in_creator_group(creator1)) diff --git a/cms/djangoapps/contentstore/management/commands/populate_creators.py b/cms/djangoapps/contentstore/management/commands/populate_creators.py index f627df88f5..e20d418abb 100644 --- a/cms/djangoapps/contentstore/management/commands/populate_creators.py +++ b/cms/djangoapps/contentstore/management/commands/populate_creators.py @@ -3,7 +3,8 @@ Script for granting existing course instructors course creator privileges. This script is only intended to be run once on a given environment. """ -from auth.authz import _grant_instructors_creator_access +from auth.authz import _grant_instructors_creator_access, get_users_with_staff_role +from course_creators.views import add_user_with_status_granted, add_user_with_status_unrequested from django.core.management.base import BaseCommand from django.contrib.auth.models import User @@ -31,5 +32,18 @@ class Command(BaseCommand): # the admin user will already exist. admin = User.objects.get(username=username, email=email) - _grant_instructors_creator_access(admin) + creators = _grant_instructors_creator_access(admin) + for user in creators: + add_user_with_status_granted(user) + + # Some users will be both staff and instructors. Those folks have been + # added with status granted above, and add_user_with_status_unrequested + # will not try to add them again if they already exist in the course creator database. + for user in get_users_with_staff_role(): + add_user_with_status_unrequested(user) + + # There could be users who are not in either staff or instructor (they've + # never actually done anything in Studio). I plan to add those as unrequested + # when they first go to their dashboard. + admin.delete() diff --git a/cms/djangoapps/course_creators/admin.py b/cms/djangoapps/course_creators/admin.py index 6cea8c6235..e9bc6b33d2 100644 --- a/cms/djangoapps/course_creators/admin.py +++ b/cms/djangoapps/course_creators/admin.py @@ -7,7 +7,22 @@ from django.contrib import admin class CourseCreatorAdmin(admin.ModelAdmin): - list_display = ('username', 'state', 'state_changed', 'note') + # Fields to display on the overview page. + list_display = ('username', 'email', 'state', 'state_changed') + readonly_fields = ('username', 'email', 'state_changed') + # Controls the order on the edit form (without this, read-only fields appear at the end). + fieldsets = ( + (None, { + 'fields': list_display + }), + ) + # Fields that filtering support + list_filter = list_display + # 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'] + # Turn off the action bar (we have no bulk actions) + actions = None def has_add_permission(self, request): return False @@ -18,5 +33,10 @@ class CourseCreatorAdmin(admin.ModelAdmin): def has_change_permission(self, request, obj=None): return request.user.is_staff + def save_model(self, request, obj, form, change): + # Store who is making the request. + obj.admin = request.user + obj.save() + admin.site.register(CourseCreator, CourseCreatorAdmin) diff --git a/cms/djangoapps/course_creators/migrations/0001_initial.py b/cms/djangoapps/course_creators/migrations/0001_initial.py index 7e91e08261..dea4a160c7 100644 --- a/cms/djangoapps/course_creators/migrations/0001_initial.py +++ b/cms/djangoapps/course_creators/migrations/0001_initial.py @@ -11,6 +11,7 @@ class Migration(SchemaMigration): # 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)), ('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)), @@ -26,6 +27,7 @@ class Migration(SchemaMigration): models = { 'course_creators.coursecreator': { 'Meta': {'object_name': 'CourseCreator'}, + 'email': ('django.db.models.fields.CharField', [], {'max_length': '128'}), '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'}), diff --git a/cms/djangoapps/course_creators/models.py b/cms/djangoapps/course_creators/models.py index c8159b66b1..48e56c63d6 100644 --- a/cms/djangoapps/course_creators/models.py +++ b/cms/djangoapps/course_creators/models.py @@ -4,7 +4,7 @@ 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 +from auth.authz import add_user_to_creator_group, remove_user_from_creator_group, get_user_by_email import datetime @@ -21,6 +21,7 @@ class CourseCreator(models.Model): ) 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") 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', @@ -30,14 +31,20 @@ class CourseCreator(models.Model): def __unicode__(self): - s = "%s | %s [%s] | %s" % (self.username, self.state, self.state_changed, self.note) + s = "%s %s | %s [%s] | %s" % (self.username, self.email, self.state, self.state_changed, self.note) return s @receiver(post_init, sender=CourseCreator) def post_init_callback(sender, **kwargs): instance = kwargs['instance'] - instance.orig_state = instance.state + 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 + @receiver(post_save, sender=CourseCreator) def post_save_callback(sender, **kwargs): @@ -46,12 +53,13 @@ def post_save_callback(sender, **kwargs): # modify it for changes to the notes field. if instance.state != instance.orig_state: instance.state_changed = datetime.datetime.now() + # We removed bad users in post_init. + user = get_user_by_email(instance.email) if instance.state == 'g': - # add to course group - instance.state = 'g' + # We have granted access, add to course group + add_user_to_creator_group(instance.admin, user) else: - # remove from course group - instance.state = instance.state + remove_user_from_creator_group(instance.admin, user) instance.orig_state = instance.state instance.save() diff --git a/cms/djangoapps/course_creators/views.py b/cms/djangoapps/course_creators/views.py index acdd50a0ca..8d2769d440 100644 --- a/cms/djangoapps/course_creators/views.py +++ b/cms/djangoapps/course_creators/views.py @@ -1,3 +1,6 @@ +""" +Methods for interacting programmatically with the user creator table. +""" from course_creators.models import CourseCreator @@ -14,7 +17,11 @@ def add_user_with_status_granted(user): """ _add_user(user, 'g') + def _add_user(user, state): + """ + Adds a user to the course creator table with the specified state. + """ if CourseCreator.objects.filter(username=user.username).count() == 0: - entry = CourseCreator(username=user.username, state=state) - entry.save() \ No newline at end of file + entry = CourseCreator(username=user.username, email=user.email, state=state) + entry.save()