Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ These are notable changes in edx-platform. This is a rolling list of changes,
in roughly chronological order, most recent first. Add your entries at or near
the top. Include a label indicating the component affected.

Studio: Add table for tracking course creator permissions (not yet used).
Update rake django-admin[syncdb] and rake django-admin[migrate] so they
run for both LMS and CMS.

Common: Student information is now passed to the tracking log via POST instead of GET.

Common: Add tests for documentation generation to test suite
Expand Down
26 changes: 19 additions & 7 deletions cms/djangoapps/auth/authz.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,15 +209,27 @@ def is_user_in_creator_group(user):
return True


def _grant_instructors_creator_access(caller):
def get_users_with_instructor_role():
"""
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.
Returns all users with the role 'instructor'
"""
return _get_users_with_role(INSTRUCTOR_ROLE_NAME)


def get_users_with_staff_role():
"""
Returns all users with the role 'staff'
"""
return _get_users_with_role(STAFF_ROLE_NAME)

Gives all users with instructor role course creator rights.
This is only intended to be run once on a given environment.

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)
users.add(user)
return users
63 changes: 25 additions & 38 deletions cms/djangoapps/auth/tests/test_authz.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@

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, get_users_with_staff_role,\
get_users_with_instructor_role


class CreatorGroupTest(TestCase):
Expand Down Expand Up @@ -175,41 +176,27 @@ def test_remove_user_from_course_group_permission_denied(self):
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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need docstrings here?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't generally need them on methods that start with test_. I'll check if pylint/pep8 is complaining.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, no complaints about this.

create_all_course_groups(self.creator, self.location)
add_user_to_course_group(self.creator, self.staff, self.location, STAFF_ROLE_NAME)

class GrantInstructorsCreatorAccessTest(TestCase):
"""
Tests granting existing instructors course creator rights.
"""
def create_course(self, index):
"""
Creates a course with one instructor and one staff member.
"""
creator = User.objects.create_user('testcreator' + str(index), 'testcreator+courses@edx.org', 'foo')
staff = User.objects.create_user('teststaff' + str(index), 'teststaff+courses@edx.org', 'foo')
location = 'i4x', 'mitX', str(index), 'course', 'test'
create_all_course_groups(creator, location)
add_user_to_course_group(creator, staff, location, STAFF_ROLE_NAME)
return [creator, staff]

def test_grant_creator_access(self):
"""
Test for _grant_instructors_creator_access.
"""
[creator1, staff1] = self.create_course(1)
[creator2, staff2] = self.create_course(2)
with mock.patch.dict('django.conf.settings.MITX_FEATURES', {"ENABLE_CREATOR_GROUP": True}):
# Initially no creators.
self.assertFalse(is_user_in_creator_group(creator1))
self.assertFalse(is_user_in_creator_group(creator2))
self.assertFalse(is_user_in_creator_group(staff1))
self.assertFalse(is_user_in_creator_group(staff2))

admin = User.objects.create_user('populate_creators_command', 'grant+creator+access@edx.org', 'foo')
admin.is_staff = True
_grant_instructors_creator_access(admin)

# Now instructors only are creators.
self.assertTrue(is_user_in_creator_group(creator1))
self.assertTrue(is_user_in_creator_group(creator2))
self.assertFalse(is_user_in_creator_group(staff1))
self.assertFalse(is_user_in_creator_group(staff2))
location2 = 'i4x', 'mitX', '103', 'course2', 'test2'
staff2 = User.objects.create_user('teststaff2', 'teststaff2+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())

def test_get_instructor(self):
# Do this test with creators 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'
creator2 = User.objects.create_user('testcreator2', 'testcreator2+courses@edx.org', 'foo')
staff2 = User.objects.create_user('teststaff2', 'teststaff2+courses@edx.org', 'foo')
create_all_course_groups(creator2, location2)
add_user_to_course_group(creator2, staff2, location2, STAFF_ROLE_NAME)

self.assertSetEqual({self.creator, creator2}, get_users_with_instructor_role())
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@

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 get_users_with_instructor_role, 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
Expand Down Expand Up @@ -31,5 +32,17 @@ def handle(self, *args, **options):
# the admin user will already exist.
admin = User.objects.get(username=username, email=email)

_grant_instructors_creator_access(admin)
for user in get_users_with_instructor_role():
add_user_with_status_granted(admin, 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(admin, 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()
Empty file.
63 changes: 63 additions & 0 deletions cms/djangoapps/course_creators/admin.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
"""
django admin page for the course creators table
"""

from course_creators.models import CourseCreator, update_creator_state
from course_creators.views import update_course_creator_group

from django.contrib import admin
from django.dispatch import receiver


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 = ['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': ['user', 'state', 'state_changed', 'note']
}),
)
# Fields that filtering support
list_filter = ['state', 'state_changed']
# Fields that search supports.
search_fields = ['user__username', 'user__email', 'state', 'note']
# Turn off the action bar (we have no bulk actions)
actions = None

def has_add_permission(self, request):
return False

def has_delete_permission(self, request, obj=None):
return False

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)


@receiver(update_creator_state, sender=CourseCreator)
def update_creator_group_callback(sender, **kwargs):
"""
Callback for when the model's creator status has changed.
"""
update_course_creator_group(kwargs['caller'], kwargs['user'], kwargs['add'])
74 changes: 74 additions & 0 deletions cms/djangoapps/course_creators/migrations/0001_initial.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# -*- coding: utf-8 -*-
import datetime
from south.db import db
from south.v2 import SchemaMigration
from django.db import models


class Migration(SchemaMigration):

def forwards(self, orm):
# Adding model 'CourseCreator'
db.create_table('course_creators_coursecreator', (
('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='unrequested', max_length=24)),
('note', self.gf('django.db.models.fields.CharField')(max_length=512, blank=True)),
))
db.send_create_signal('course_creators', ['CourseCreator'])


def backwards(self, orm):
# Deleting model 'CourseCreator'
db.delete_table('course_creators_coursecreator')


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'},
'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': "'unrequested'", 'max_length': '24'}),
'state_changed': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'unique': 'True'})
}
}

complete_apps = ['course_creators']
Empty file.
71 changes: 71 additions & 0 deletions cms/djangoapps/course_creators/models.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
"""
Table for storing information about whether or not Studio users have course creation privileges.
"""
from django.db import models
from django.db.models.signals import post_init, post_save
from django.dispatch import receiver, Signal
from django.contrib.auth.models import User

from django.utils import timezone
from django.utils.translation import ugettext as _

# A signal that will be sent when users should be added or removed from the creator group
update_creator_state = Signal(providing_args=["caller", "user", "add"])


class CourseCreator(models.Model):
"""
Creates the database table model.
"""
UNREQUESTED = 'unrequested'
PENDING = 'pending'
GRANTED = 'granted'
DENIED = 'denied'

# Second value is the "human-readable" version.
STATES = (
(UNREQUESTED, _(u'unrequested')),
(PENDING, _(u'pending')),
(GRANTED, _(u'granted')),
(DENIED, _(u'denied')),
)

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=24, blank=False, choices=STATES, default=UNREQUESTED,
help_text=_("Current course creator state"))
note = models.CharField(max_length=512, blank=True, help_text=_("Optional notes about this user (for example, "
"why course creation access was denied)"))

def __unicode__(self):
return u'%str | %str [%str] | %str' % (self.user, self.state, self.state_changed, self.note)


@receiver(post_init, sender=CourseCreator)
def post_init_callback(sender, **kwargs):
"""
Extend to store previous state.
"""
instance = kwargs['instance']
instance.orig_state = instance.state


@receiver(post_save, sender=CourseCreator)
def post_save_callback(sender, **kwargs):
"""
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:
update_creator_state.send(
sender=sender,
caller=instance.admin,
user=instance.user,
add=instance.state == CourseCreator.GRANTED
)
instance.state_changed = timezone.now()
instance.orig_state = instance.state
instance.save()
Loading