diff --git a/lms/djangoapps/instructor/access.py b/lms/djangoapps/instructor/access.py index 99bebdee4a..66ca5e25fb 100644 --- a/lms/djangoapps/instructor/access.py +++ b/lms/djangoapps/instructor/access.py @@ -9,11 +9,14 @@ TODO sync instructor and staff flags {instructor: true, staff: true} """ +import logging from django.contrib.auth.models import Group from courseware.access import (get_access_group_name, course_beta_test_group_name) from django_comment_common.models import Role +log = logging.getLogger(__name__) + def list_with_level(course, level): """ @@ -23,7 +26,7 @@ def list_with_level(course, level): There could be other levels specific to the course. If there is no Group for that course-level, returns an empty list """ - if level is 'beta': + if level == 'beta': grpname = course_beta_test_group_name(course.location) else: grpname = get_access_group_name(course, level) @@ -31,6 +34,7 @@ def list_with_level(course, level): try: return Group.objects.get(name=grpname).user_set.all() except Group.DoesNotExist: + log.info("list_with_level called with non-existant group named {}".format(grpname)) return [] @@ -59,10 +63,10 @@ def _change_access(course, user, level, action): level is one of ['instructor', 'staff', 'beta'] action is one of ['allow', 'revoke'] - NOTE: will NOT create a group that does not yet exist. + NOTE: will create a group if it does not yet exist. """ - if level is 'beta': + if level == 'beta': grpname = course_beta_test_group_name(course.location) elif level in ['instructor', 'staff']: grpname = get_access_group_name(course, level) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 45501efcfa..ac4a459a3d 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -17,6 +17,7 @@ from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseForbid from courseware.access import has_access from courseware.courses import get_course_with_access, get_course_by_id from django.contrib.auth.models import User +from django_comment_client.utils import has_forum_access from django_comment_common.models import (Role, FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, @@ -33,6 +34,7 @@ import analytics.csvs log = logging.getLogger(__name__) + def common_exceptions_400(func): """ Catches common exceptions and renders matching 400 errors. @@ -630,12 +632,33 @@ def list_forum_members(request, course_id): Lists forum members of a certain rolename. Limited to staff access. - Takes query parameter `rolename` + The requesting user must be at least staff. + Staff forum admins can access all roles EXCEPT for FORUM_ROLE_ADMINISTRATOR + which is limited to instructors. + + Takes query parameter `rolename`. """ + course = get_course_by_id(course_id) + has_instructor_access = has_access(request.user, course, 'instructor') + has_forum_admin = has_forum_access( + request.user, course_id, FORUM_ROLE_ADMINISTRATOR + ) + rolename = request.GET.get('rolename') + # default roles require either (staff & forum admin) or (instructor) + if not (has_forum_admin or has_instructor_access): + return HttpResponseBadRequest( + "Operation requires staff & forum admin or instructor access" + ) + + # EXCEPT FORUM_ROLE_ADMINISTRATOR requires (instructor) + if rolename == FORUM_ROLE_ADMINISTRATOR and not has_instructor_access: + return HttpResponseBadRequest("Operation requires instructor access.") + + # filter out unsupported for roles if not rolename in [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA]: - return HttpResponseBadRequest() + return HttpResponseBadRequest("Unrecognized rolename '{}'.".format(rolename)) try: role = Role.objects.get(name=rolename, course_id=course_id) @@ -664,7 +687,7 @@ def list_forum_members(request, course_id): @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) -@require_level('instructor') +@require_level('staff') @require_query_params( email="the target users email", rolename="the forum role", @@ -675,20 +698,46 @@ def update_forum_role_membership(request, course_id): """ Modify user's forum role. + The requesting user must be at least staff. + Staff forum admins can access all roles EXCEPT for FORUM_ROLE_ADMINISTRATOR + which is limited to instructors. + No one can revoke an instructors FORUM_ROLE_ADMINISTRATOR status. + Query parameters: - email is the target users email - rolename is one of [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA] - action is one of ['allow', 'revoke'] + - `email` is the target users email + - `rolename` is one of [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA] + - `action` is one of ['allow', 'revoke'] """ + course = get_course_by_id(course_id) + has_instructor_access = has_access(request.user, course, 'instructor') + has_forum_admin = has_forum_access( + request.user, course_id, FORUM_ROLE_ADMINISTRATOR + ) + email = request.GET.get('email') rolename = request.GET.get('rolename') action = request.GET.get('action') + # default roles require either (staff & forum admin) or (instructor) + if not (has_forum_admin or has_instructor_access): + return HttpResponseBadRequest( + "Operation requires staff & forum admin or instructor access" + ) + + # EXCEPT FORUM_ROLE_ADMINISTRATOR requires (instructor) + if rolename == FORUM_ROLE_ADMINISTRATOR and not has_instructor_access: + return HttpResponseBadRequest("Operation requires instructor access.") + if not rolename in [FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_MODERATOR, FORUM_ROLE_COMMUNITY_TA]: - return HttpResponseBadRequest() + return HttpResponseBadRequest("Unrecognized rolename '{}'.".format(rolename)) + + user = User.objects.get(email=email) + target_is_instructor = has_access(user, course, 'instructor') + # cannot revoke instructor + if target_is_instructor and action == 'revoke' and rolename == FORUM_ROLE_ADMINISTRATOR: + return HttpResponseBadRequest("Cannot revoke instructor forum admin privelages.") try: - user = User.objects.get(email=email) access.update_forum_role_membership(course_id, user, rolename, action) except Role.DoesNotExist: return HttpResponseBadRequest("Role does not exist.") diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index 3acc6fd17b..897e43fdfd 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -40,7 +40,7 @@ def instructor_dashboard_2(request, course_id): } if not access['staff']: - raise Http404 + raise Http404() sections = [ _section_course_info(course_id), diff --git a/lms/static/coffee/src/instructor_dashboard/instructor_dashboard.coffee b/lms/static/coffee/src/instructor_dashboard/instructor_dashboard.coffee index c87e1b8490..3e75358d52 100644 --- a/lms/static/coffee/src/instructor_dashboard/instructor_dashboard.coffee +++ b/lms/static/coffee/src/instructor_dashboard/instructor_dashboard.coffee @@ -74,18 +74,24 @@ setup_instructor_dashboard = (idash_content) => # plantTimeout 0, -> section.data('wrapper')?.onExit?() - # activate an initial section by programmatically clicking on it. + # activate an initial section by 'clicking' on it. # check for a deep-link, or click the first link. + click_first_link = -> + link = links.eq(0) + link.click() + link.data('wrapper')?.onClickTitle?() + if (new RegExp "^#{HASH_LINK_PREFIX}").test location.hash rmatch = (new RegExp "^#{HASH_LINK_PREFIX}(.*)").exec location.hash section_name = rmatch[1] link = links.filter "[data-section='#{section_name}']" - link.click() - link.data('wrapper')?.onClickTitle?() + if link.length == 1 + link.click() + link.data('wrapper')?.onClickTitle?() + else + click_first_link() else - link = links.eq(0) - link.click() - link.data('wrapper')?.onClickTitle?() + click_first_link() diff --git a/lms/templates/courseware/instructor_dashboard_2/membership.html b/lms/templates/courseware/instructor_dashboard_2/membership.html index 19ca7e2f89..d50e71f9fb 100644 --- a/lms/templates/courseware/instructor_dashboard_2/membership.html +++ b/lms/templates/courseware/instructor_dashboard_2/membership.html @@ -57,16 +57,18 @@ %endif - %if section_data['access']['forum_admin']: -
-
-
- - + %if section_data['access']['instructor']: +
+
+
+ + +
+
-
-
+ %endif + %if section_data['access']['instructor'] or section_data['access']['forum_admin']: