Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Adds settings.FEATURES.ENABLE_SEPARATE_ARCHIVED_COURSES

Browse Source 
Separates archived courses into a different tab on the Studio index page.

Data is rendered synchronously, but tests are added to ensure that no new sql
or mongo queries were required.
This commit is contained in:
Jillian Vogel
2017-06-28 19:06:24 +09:30
parent fd22467261
commit 8087a1d165
7 changed files with 225 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
cms/djangoapps/contentstore/views/course.py
View File

@@ -518,11 +518,13 @@ def course_listing(request):
u'can_edit': has_studio_write_access(request.user, library.location.library_key),
}
courses_iter = _remove_in_process_courses(courses_iter, in_process_course_actions)
split_archived = settings.FEATURES.get(u'ENABLE_SEPARATE_ARCHIVED_COURSES', False)
active_courses, archived_courses = _process_courses_list(courses_iter, in_process_course_actions, split_archived)
in_process_course_actions = [format_in_process_course_view(uca) for uca in in_process_course_actions]
return render_to_response(u'index.html', {
u'courses': list(courses_iter),
u'courses': active_courses,
u'archived_courses': archived_courses,
u'in_process_course_actions': in_process_course_actions,
u'libraries_enabled': LIBRARIES_ENABLED,
u'libraries': [format_library_for_view(lib) for lib in libraries],
@@ -639,7 +641,6 @@ def get_courses_accessible_to_user(request, org=None):
the courses returned. A value of None will have no effect (all courses
returned), an empty string will result in no courses, and otherwise only courses with the
specified org will be returned. The default value is None.
"""
if GlobalStaff().has_user(request.user):
# user has global access so no need to get courses from django groups
@@ -654,10 +655,15 @@ def get_courses_accessible_to_user(request, org=None):
return courses, in_process_course_actions
def _remove_in_process_courses(courses_iter, in_process_course_actions):
def _process_courses_list(courses_iter, in_process_course_actions, split_archived=False):
"""
removes any in-process courses in courses list. in-process actually refers to courses
that are in the process of being generated for re-run
Iterates over the list of courses to be displayed to the user, and:
* Removes any in-process courses from the courses list. "In-process" refers to courses
that are in the process of being generated for re-run.
* If split_archived=True, removes any archived courses and returns them in a separate list.
Archived courses have has_ended() == True.
* Formats the returned courses (in both lists) to prepare them for rendering to the view.
"""
def format_course_for_view(course):
"""
@@ -675,11 +681,20 @@ def _remove_in_process_courses(courses_iter, in_process_course_actions):
}
in_process_action_course_keys = {uca.course_key for uca in in_process_course_actions}
return (
format_course_for_view(course)
for course in courses_iter
if not isinstance(course, ErrorDescriptor) and (course.id not in in_process_action_course_keys)
)
active_courses = []
archived_courses = []
for course in courses_iter:
if isinstance(course, ErrorDescriptor) or (course.id in in_process_action_course_keys):
continue
formatted_course = format_course_for_view(course)
if split_archived and course.has_ended():
archived_courses.append(formatted_course)
else:
active_courses.append(formatted_course)
return active_courses, archived_courses
def course_outline_initial_state(locator_to_show, course_structure):
@@ -1035,7 +1050,7 @@ def settings_handler(request, course_key_string):
# exclude current course from the list of available courses
courses = (course for course in courses if course.id != course_key)
if courses:
courses = _remove_in_process_courses(courses, in_process_course_actions)
courses, __ = _process_courses_list(courses, in_process_course_actions)
settings_context.update({'possible_pre_requisite_courses': list(courses)})
if credit_eligibility_enabled:

135
cms/djangoapps/contentstore/views/tests/test_course_index.py
View File

@@ -10,6 +10,7 @@ import mock
import pytz
from django.conf import settings
from django.core.exceptions import PermissionDenied
from django.test.utils import override_settings
from django.utils.translation import ugettext as _
from opaque_keys.edx.locator import CourseLocator
from search.api import perform_search
@@ -26,13 +27,13 @@ from contentstore.views.item import VisibilityState, create_xblock_info
from course_action_state.managers import CourseRerunUIStateManager
from course_action_state.models import CourseRerunState
from student.auth import has_course_author_access
from student.roles import LibraryUserRole
from student.roles import CourseStaffRole, GlobalStaff, LibraryUserRole
from student.tests.factories import UserFactory
from util.date_utils import get_default_time_display
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, LibraryFactory
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, LibraryFactory, check_mongo_calls
class TestCourseIndex(CourseTestCase):
@@ -328,6 +329,136 @@ class TestCourseIndex(CourseTestCase):
self.assertIn('display_course_number: ""', response.content)
@ddt.ddt
class TestCourseIndexArchived(CourseTestCase):
"""
Unit tests for testing the course index list when there are archived courses.
"""
NOW = datetime.datetime.now(pytz.utc)
DAY = datetime.timedelta(days=1)
YESTERDAY = NOW - DAY
TOMORROW = NOW + DAY
ORG = 'MyOrg'
ENABLE_SEPARATE_ARCHIVED_COURSES = settings.FEATURES.copy()
ENABLE_SEPARATE_ARCHIVED_COURSES['ENABLE_SEPARATE_ARCHIVED_COURSES'] = True
DISABLE_SEPARATE_ARCHIVED_COURSES = settings.FEATURES.copy()
DISABLE_SEPARATE_ARCHIVED_COURSES['ENABLE_SEPARATE_ARCHIVED_COURSES'] = False
def setUp(self):
"""
Add courses with the end date set to various values
"""
super(TestCourseIndexArchived, self).setUp()
# Base course has no end date (so is active)
self.course.end = None
self.course.display_name = 'Active Course 1'
self.ORG = self.course.location.org
self.save_course()
# Active course has end date set to tomorrow
self.active_course = CourseFactory.create(
display_name='Active Course 2',
org=self.ORG,
end=self.TOMORROW,
)
# Archived course has end date set to yesterday
self.archived_course = CourseFactory.create(
display_name='Archived Course',
org=self.ORG,
end=self.YESTERDAY,
)
# Base user has global staff access
self.assertTrue(GlobalStaff().has_user(self.user))
# Staff user just has course staff access
self.staff, self.staff_password = self.create_non_staff_user()
for course in (self.course, self.active_course, self.archived_course):
CourseStaffRole(course.id).add_users(self.staff)
def check_index_page_with_query_count(self, separate_archived_courses, org, mongo_queries, sql_queries):
"""
Checks the index page, and ensures the number of database queries is as expected.
"""
with self.assertNumQueries(sql_queries):
with check_mongo_calls(mongo_queries):
self.check_index_page(separate_archived_courses=separate_archived_courses, org=org)
def check_index_page(self, separate_archived_courses, org):
"""
Ensure that the index page displays the archived courses as expected.
"""
index_url = '/home/'
index_params = {}
if org is not None:
index_params['org'] = org
index_response = self.client.get(index_url, index_params, HTTP_ACCEPT='text/html')
self.assertEquals(index_response.status_code, 200)
parsed_html = lxml.html.fromstring(index_response.content)
course_tab = parsed_html.find_class('courses')
self.assertEqual(len(course_tab), 1)
course_links = course_tab[0].find_class('course-link')
course_titles = course_tab[0].find_class('course-title')
archived_course_tab = parsed_html.find_class('archived-courses')
if separate_archived_courses:
# Archived courses should be separated from the main course list
self.assertEqual(len(archived_course_tab), 1)
archived_course_links = archived_course_tab[0].find_class('course-link')
archived_course_titles = archived_course_tab[0].find_class('course-title')
self.assertEqual(len(archived_course_links), 1)
self.assertEqual(len(archived_course_titles), 1)
self.assertEqual(archived_course_titles[0].text, 'Archived Course')
self.assertEqual(len(course_links), 2)
self.assertEqual(len(course_titles), 2)
self.assertEqual(course_titles[0].text, 'Active Course 1')
self.assertEqual(course_titles[1].text, 'Active Course 2')
else:
# Archived courses should be included in the main course list
self.assertEqual(len(archived_course_tab), 0)
self.assertEqual(len(course_links), 3)
self.assertEqual(len(course_titles), 3)
self.assertEqual(course_titles[0].text, 'Active Course 1')
self.assertEqual(course_titles[1].text, 'Active Course 2')
self.assertEqual(course_titles[2].text, 'Archived Course')
@ddt.data(
# Staff user has course staff access
(True, 'staff', None, 4, 20),
(False, 'staff', None, 4, 20),
# Base user has global staff access
(True, 'user', ORG, 3, 16),
(False, 'user', ORG, 3, 16),
(True, 'user', None, 3, 16),
(False, 'user', None, 3, 16),
)
@ddt.unpack
def test_separate_archived_courses(self, separate_archived_courses, username, org, mongo_queries, sql_queries):
"""
Ensure that archived courses are shown as expected for all user types, when the feature is enabled/disabled.
Also ensure that enabling the feature does not adversely affect the database query count.
"""
# Authenticate the requested user
user = getattr(self, username)
password = getattr(self, username + '_password')
self.client.login(username=user, password=password)
# Enable/disable the feature before viewing the index page.
features = settings.FEATURES.copy()
features['ENABLE_SEPARATE_ARCHIVED_COURSES'] = separate_archived_courses
with override_settings(FEATURES=features):
self.check_index_page_with_query_count(separate_archived_courses=separate_archived_courses,
org=org,
mongo_queries=mongo_queries,
sql_queries=sql_queries)
@ddt.ddt
class TestCourseOutline(CourseTestCase):
"""

2
cms/static/js/index.js
View File

@@ -161,6 +161,7 @@ define(['domReady', 'jquery', 'underscore', 'js/utils/cancel_on_escape', 'js/vie
return function(e) {
e.preventDefault();
$('.courses-tab').toggleClass('active', tab === 'courses');
$('.archived-courses-tab').toggleClass('active', tab === 'archived-courses');
$('.libraries-tab').toggleClass('active', tab === 'libraries');
// Also toggle this course-related notice shown below the course tab, if it is present:
@@ -179,6 +180,7 @@ define(['domReady', 'jquery', 'underscore', 'js/utils/cancel_on_escape', 'js/vie
$('.action-reload').bind('click', ViewUtils.reload);
$('#course-index-tabs .courses-tab').bind('click', showTab('courses'));
$('#course-index-tabs .archived-courses-tab').bind('click', showTab('archived-courses'));
$('#course-index-tabs .libraries-tab').bind('click', showTab('libraries'));
};

6
cms/static/sass/views/_dashboard.scss
View File

@@ -318,7 +318,7 @@
}
// ELEM: course listings
.courses-tab, .libraries-tab {
.courses-tab, .archived-courses-tab, .libraries-tab {
display: none;
&.active {
@@ -326,7 +326,7 @@
}
}
.courses, .libraries {
.courses, .libraries, .archived-courses {
.title {
@extend %t-title6;
margin-bottom: $baseline;
@@ -342,7 +342,7 @@
padding-bottom: ($baseline/2);
color: $gray-l2;
}
}
}
.list-courses {
border-radius: 3px;

45
cms/templates/index.html
View File

@@ -308,10 +308,15 @@ from openedx.core.djangolib.markup import HTML, Text
</div>
%endif
% if libraries_enabled:
% if libraries_enabled or archived_courses:
<ul id="course-index-tabs">
<li class="courses-tab active"><a>${_("Courses")}</a></li>
% if archived_courses:
<li class="archived-courses-tab"><a>${_("Archived Courses")}</a></li>
% endif
% if libraries_enabled:
<li class="libraries-tab"><a>${_("Libraries")}</a></li>
% endif
</ul>
% endif
@@ -467,6 +472,44 @@ from openedx.core.djangolib.markup import HTML, Text
</div>
% endif
%if archived_courses:
<div class="archived-courses archived-courses-tab">
<ul class="list-courses">
%for course_info in sorted(archived_courses, key=lambda s: s['display_name'].lower() if s['display_name'] is not None else ''):
<li class="course-item" data-course-key="${course_info['course_key']}">
<a class="course-link" href="${course_info['url']}">
<h3 class="course-title">${course_info['display_name']}</h3>
<div class="course-metadata">
<span class="course-org metadata-item">
<span class="label">${_("Organization:")}</span> <span class="value">${course_info['org']}</span>
</span>
<span class="course-num metadata-item">
<span class="label">${_("Course Number:")}</span>
<span class="value">${course_info['number']}</span>
</span>
<span class="course-run metadata-item">
<span class="label">${_("Course Run:")}</span> <span class="value">${course_info['run']}</span>
</span>
</div>
</a>
<ul class="item-actions course-actions">
% if allow_course_reruns and rerun_creator_status and course_creator_status=='granted':
<li class="action action-rerun">
<a href="${course_info['rerun_link']}" class="button rerun-button">${_("Re-run Course")}</a>
</li>
% endif
<li class="action action-view">
<a href="${course_info['lms_link']}" rel="external" class="button view-button">${_("View Live")}</a>
</li>
</ul>
</li>
%endfor
</ul>
</div>
%endif
%if len(libraries) > 0:
<div class="libraries libraries-tab">
<ul class="list-courses">

17
common/lib/xmodule/xmodule/course_module.py
View File

@@ -5,6 +5,7 @@ import json
import logging
from cStringIO import StringIO
from datetime import datetime, timedelta
import dateutil.parser
import requests
from lazy import lazy
@@ -1393,9 +1394,10 @@ class CourseSummary(object):
A lightweight course summary class, which constructs split/mongo course summary without loading
the course. It is used at cms for listing courses to global staff user.
"""
course_info_fields = ['display_name', 'display_coursenumber', 'display_organization']
course_info_fields = ['display_name', 'display_coursenumber', 'display_organization', 'end']
def __init__(self, course_locator, display_name=u"Empty", display_coursenumber=None, display_organization=None):
def __init__(self, course_locator, display_name=u"Empty", display_coursenumber=None, display_organization=None,
end=None):
"""
Initialize and construct course summary
@@ -1412,6 +1414,8 @@ class CourseSummary(object):
display_organization (unicode|None): Course organization that is specified & appears in the courseware
end (unicode|None): Course end date. Must contain timezone.
"""
self.display_coursenumber = display_coursenumber
self.display_organization = display_organization
@@ -1419,6 +1423,9 @@ class CourseSummary(object):
self.id = course_locator # pylint: disable=invalid-name
self.location = course_locator.make_usage_key('course', 'course')
self.end = end
if end is not None and not isinstance(end, datetime):
self.end = dateutil.parser.parse(end)
@property
def display_org_with_default(self):
@@ -1439,3 +1446,9 @@ class CourseSummary(object):
if self.display_coursenumber:
return self.display_coursenumber
return self.location.course
def has_ended(self):
"""
Returns whether the course has ended.
"""
return course_metadata_utils.has_course_ended(self.end)

2
common/lib/xmodule/xmodule/modulestore/tests/test_semantics.py
View File

@@ -211,7 +211,7 @@ class DirectOnlyCategorySemantics(PureModulestoreTestCase):
"""
def verify_course_summery_fields(course_summary):
""" Verify that every `course_summary` object has all the required fields """
expected_fields = CourseSummary.course_info_fields + ['id', 'location']
expected_fields = CourseSummary.course_info_fields + ['id', 'location', 'has_ended']
return all([hasattr(course_summary, field) for field in expected_fields])
self.assertTrue(all(verify_course_summery_fields(course_summary) for course_summary in course_summaries))