- Instructors can set whether a post in a cohorted topic is visible to all cohorts or only to a specific cohort.
+ Discussion admins, moderators, and TAs can make their posts visible to all students or specify a single cohort group.
<% } %>
diff --git a/common/test/acceptance/tests/discussion/test_cohort_management.py b/common/test/acceptance/tests/discussion/test_cohort_management.py
index f67af694dd..882fadd7f8 100644
--- a/common/test/acceptance/tests/discussion/test_cohort_management.py
+++ b/common/test/acceptance/tests/discussion/test_cohort_management.py
@@ -3,6 +3,10 @@
End-to-end tests related to the cohort management on the LMS Instructor Dashboard
"""
+from datetime import datetime
+
+from pymongo import MongoClient
+
from bok_choy.promise import EmptyPromise
from .helpers import CohortTestMixin
from ..helpers import UniqueCourseTest
@@ -25,6 +29,8 @@ class CohortConfigurationTest(UniqueCourseTest, CohortTestMixin):
"""
super(CohortConfigurationTest, self).setUp()
+ self.event_collection = MongoClient()["test"]["events"]
+
# create course with cohorts
self.manual_cohort_name = "ManualCohort1"
self.auto_cohort_name = "AutoCohort1"
@@ -106,7 +112,9 @@ class CohortConfigurationTest(UniqueCourseTest, CohortTestMixin):
And I get a notification that 2 users have been added to the cohort
And I get a notification that 1 user was moved from the other cohort
And the user input field is empty
+ And appropriate events have been emitted
"""
+ start_time = datetime.now()
self.membership_page.select_cohort(self.auto_cohort_name)
self.assertEqual(0, self.membership_page.get_selected_cohort_count())
self.membership_page.add_students_to_selected_cohort([self.student_name, self.instructor_name])
@@ -119,6 +127,44 @@ class CohortConfigurationTest(UniqueCourseTest, CohortTestMixin):
self.assertEqual("2 students have been added to this cohort group", confirmation_messages[0])
self.assertEqual("1 student was removed from " + self.manual_cohort_name, confirmation_messages[1])
self.assertEqual("", self.membership_page.get_cohort_student_input_field_value())
+ self.assertEqual(
+ self.event_collection.find({
+ "name": "edx.cohort.user_added",
+ "time": {"$gt": start_time},
+ "event.user_id": {"$in": [int(self.instructor_id), int(self.student_id)]},
+ "event.cohort_name": self.auto_cohort_name,
+ }).count(),
+ 2
+ )
+ self.assertEqual(
+ self.event_collection.find({
+ "name": "edx.cohort.user_removed",
+ "time": {"$gt": start_time},
+ "event.user_id": int(self.student_id),
+ "event.cohort_name": self.manual_cohort_name,
+ }).count(),
+ 1
+ )
+ self.assertEqual(
+ self.event_collection.find({
+ "name": "edx.cohort.user_add_requested",
+ "time": {"$gt": start_time},
+ "event.user_id": int(self.instructor_id),
+ "event.cohort_name": self.auto_cohort_name,
+ "event.previous_cohort_name": None,
+ }).count(),
+ 1
+ )
+ self.assertEqual(
+ self.event_collection.find({
+ "name": "edx.cohort.user_add_requested",
+ "time": {"$gt": start_time},
+ "event.user_id": int(self.student_id),
+ "event.cohort_name": self.auto_cohort_name,
+ "event.previous_cohort_name": self.manual_cohort_name,
+ }).count(),
+ 1
+ )
def test_add_students_to_cohort_failure(self):
"""
@@ -164,7 +210,9 @@ class CohortConfigurationTest(UniqueCourseTest, CohortTestMixin):
Then the new cohort is displayed and has no users in it
And when I add the user to the new cohort
Then the cohort has 1 user
+ And appropriate events have been emitted
"""
+ start_time = datetime.now()
new_cohort = str(uuid.uuid4().get_hex()[0:20])
self.assertFalse(new_cohort in self.membership_page.get_cohorts())
self.membership_page.add_cohort(new_cohort)
@@ -178,3 +226,19 @@ class CohortConfigurationTest(UniqueCourseTest, CohortTestMixin):
EmptyPromise(
lambda: 1 == self.membership_page.get_selected_cohort_count(), 'Waiting for student to be added'
).fulfill()
+ self.assertEqual(
+ self.event_collection.find({
+ "name": "edx.cohort.created",
+ "time": {"$gt": start_time},
+ "event.cohort_name": new_cohort,
+ }).count(),
+ 1
+ )
+ self.assertEqual(
+ self.event_collection.find({
+ "name": "edx.cohort.creation_requested",
+ "time": {"$gt": start_time},
+ "event.cohort_name": new_cohort,
+ }).count(),
+ 1
+ )
diff --git a/docs/en_us/data/source/internal_data_formats/event_list.rst b/docs/en_us/data/source/internal_data_formats/event_list.rst
index bc1d6c0499..7b742b72ed 100644
--- a/docs/en_us/data/source/internal_data_formats/event_list.rst
+++ b/docs/en_us/data/source/internal_data_formats/event_list.rst
@@ -42,6 +42,16 @@ Alphabetical Event List
- :ref:`Instructor_Event_Types`
* - ``dump-grades-raw``
- :ref:`Instructor_Event_Types`
+ * - ``edx.cohort.created``
+ - :ref:`student_cohort_events`
+ * - ``edx.cohort.creation_requested``
+ - :ref:`instructor_cohort_events`
+ * - ``edx.cohort.user_add_requested``
+ - :ref:`instructor_cohort_events`
+ * - ``edx.cohort.user_added``
+ - :ref:`student_cohort_events`
+ * - ``edx.cohort.user_removed``
+ - :ref:`student_cohort_events`
* - ``edx.course.enrollment.activated``
- :ref:`enrollment` and :ref:`instructor_enrollment`
* - ``edx.course.enrollment.deactivated``
@@ -207,4 +217,4 @@ Alphabetical Event List
.. * - ``problem_reset``
.. - :ref:`problem`
.. * - ``show_transcript``
-.. - :ref:`video`
\ No newline at end of file
+.. - :ref:`video`
diff --git a/docs/en_us/data/source/internal_data_formats/tracking_logs.rst b/docs/en_us/data/source/internal_data_formats/tracking_logs.rst
index 58a9bafaf5..b8331fde1a 100644
--- a/docs/en_us/data/source/internal_data_formats/tracking_logs.rst
+++ b/docs/en_us/data/source/internal_data_formats/tracking_logs.rst
@@ -301,6 +301,8 @@ outside the Instructor Dashboard.
* :ref:`AB_Event_Types`
+* :ref:`student_cohort_events`
+
* :ref:`ora`
The descriptions that follow include what each event represents, the system
@@ -1747,7 +1749,7 @@ After a user executes a text search in the navigation sidebar of the course
**Event Source**: Server
**History**: Added 16 May 2014. The ``corrected_text`` field was added 5
-Jun 2014.
+Jun 2014. The ``group_id`` field was added 7 October 2014.
``event`` **Fields**:
@@ -1761,6 +1763,15 @@ Jun 2014.
* - ``query``
- string
- The text entered into the search box by the user.
+ * - ``group_id``
+ - integer
+ - The numeric ID of the cohort group to which the user's search is
+ restricted, or ``null`` if the search is not restricted in this way. In a
+ course with cohorts enabled, a student's searches will always be
+ restricted to the student's cohort group. Discussion Admins, Moderators, and
+ Community TAs in such a course can search all discussions
+ without specifying a cohort group, which leaves this field
+ ``null`, or they can specify a single cohort group to search.
* - ``page``
- integer
- Results are returned in sets of 20 per page. Identifies the page of
@@ -2159,6 +2170,106 @@ the child module that was shown to the student.
- string
- ID of the module that displays to the student.
+.. _student_cohort_events:
+
+==========================
+Student Cohort Events
+==========================
+
+``edx.cohort.created``
+----------------------------------
+
+When a cohort group is created, the server emits an ``edx.cohort.created``
+event. A member of the course staff can create a cohort group manually via the
+Instructor Dashboard (see :ref:`instructor_cohort_events`). The system
+automatically creates the default cohort group and cohort groups included in the
+course's ``auto_cohort_groups`` setting as they are needed (e.g. when a student
+is assigned to one).
+
+**Event Source**: Server
+
+**History** Added 7 Oct 2014.
+
+``event`` **Fields**:
+
+.. list-table::
+ :widths: 15 15 60
+ :header-rows: 1
+
+ * - Field
+ - Type
+ - Details
+ * - ``cohort_id``
+ - integer
+ - The numeric ID of the cohort group.
+ * - ``cohort_name``
+ - string
+ - The display name of the cohort group.
+
+``edx.cohort.user_added``
+----------------------------------
+
+When a user is added to a cohort group, the server emits an
+``edx.cohort.user_added`` event. A member of the course staff can add a user to
+a cohort group manually via the Instructor Dashboard (see
+:ref:`instructor_cohort_events`). The system automatically adds a user to the default
+cohort group or a cohort group included in the course's ``auto_cohort_groups``
+setting if the user accesses a discussion but has not yet been assigned to a
+cohort group.
+
+**Event Source**: Server
+
+**History** Added 7 Oct 2014.
+
+``event`` **Fields**:
+
+.. list-table::
+ :widths: 15 15 60
+ :header-rows: 1
+
+ * - Field
+ - Type
+ - Details
+ * - ``user_id``
+ - integer
+ - The numeric ID (from auth_user.id) of the added user.
+ * - ``cohort_id``
+ - integer
+ - The numeric ID of the cohort group.
+ * - ``cohort_name``
+ - string
+ - The display name of the cohort group.
+
+``edx.cohort.user_removed``
+----------------------------------
+
+When a user is removed from a cohort group (by being assigned to a different
+cohort group via the Instructor Dashboard), the server emits an
+``edx.cohort.user_removed`` event.
+
+**Event Source**: Server
+
+**History** Added 7 Oct 2014.
+
+``event`` **Fields**:
+
+.. list-table::
+ :widths: 15 15 60
+ :header-rows: 1
+
+ * - Field
+ - Type
+ - Details
+ * - ``user_id``
+ - integer
+ - The numeric ID (from auth_user.id) of the removed user.
+ * - ``cohort_id``
+ - integer
+ - The numeric ID of the cohort group.
+ * - ``cohort_name``
+ - string
+ - The display name of the cohort group.
+
.. _ora:
============================================
@@ -2593,5 +2704,82 @@ members also generate enrollment events.
For details about the enrollment events, see :ref:`enrollment`.
+.. _instructor_cohort_events:
+
+=============================
+Instructor Cohort Events
+=============================
+
+In addition to the cohort events that are generated when students are assigned
+to cohort groups (which can happen automatically or manually via the Instructor
+Dashboard; see :ref:`student_cohort_events`), actions by instructors and course
+staff members generate additional events.
+
+``edx.cohort.creation_requested``
+----------------------------------
+
+When an instructor or course staff member manually creates a cohort group via
+the Instructor Dashboard, the server emits an ``edx.cohort.creation_requested``
+event.
+
+**Event Source**: Server
+
+**History** Added 7 Oct 2014.
+
+``event`` **Fields**:
+
+.. list-table::
+ :widths: 15 15 60
+ :header-rows: 1
+
+ * - Field
+ - Type
+ - Details
+ * - ``cohort_id``
+ - integer
+ - The numeric ID of the cohort group.
+ * - ``cohort_name``
+ - string
+ - The display name of the cohort group.
+
+``edx.cohort.user_add_requested``
+----------------------------------
+
+When an instructor or course staff member adds a student to a cohort group via
+the Instructor Dashboard, the server emits an ``edx.cohort.user_add_requested``
+event.
+
+**Event Source**: Server
+
+**History** Added 7 Oct 2014.
+
+``event`` **Fields**:
+
+.. list-table::
+ :widths: 15 15 60
+ :header-rows: 1
+
+ * - Field
+ - Type
+ - Details
+ * - ``user_id``
+ - integer
+ - The numeric ID (from auth_user.id) of the added user.
+ * - ``cohort_id``
+ - integer
+ - The numeric ID of the cohort group.
+ * - ``cohort_name``
+ - string
+ - The display name of the cohort group.
+ * - ``previous_cohort_id``
+ - integer
+ - The numeric ID of the cohort group that the user was previously assigned
+ to (or null if the user was not previously assigned to a cohort group).
+ * - ``previous_cohort_name``
+ - string
+ - The display name of the cohort group that the user was previously
+ assigned to (or null if the user was not previously assigned to a cohort
+ group).
+
.. _Creating a Peer Assessment: http://edx.readthedocs.org/projects/edx-open-response-assessments/en/latest/
diff --git a/lms/djangoapps/django_comment_client/base/tests.py b/lms/djangoapps/django_comment_client/base/tests.py
index 7f3f22a2bc..5c3d06b682 100644
--- a/lms/djangoapps/django_comment_client/base/tests.py
+++ b/lms/djangoapps/django_comment_client/base/tests.py
@@ -173,8 +173,11 @@ class ViewsTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSetupMixin):
super(ViewsTestCase, self).setUp(create_user=False)
# create a course
- self.course = CourseFactory.create(org='MITx', course='999',
- display_name='Robot Super Course')
+ self.course = CourseFactory.create(
+ org='MITx', course='999',
+ discussion_topics={"Some Topic": {"id": "some_topic"}},
+ display_name='Robot Super Course',
+ )
self.course_id = self.course.id
# seed the forums permissions and roles
call_command('seed_permissions_roles', self.course_id.to_deprecated_string())
@@ -368,7 +371,15 @@ class ViewsTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSetupMixin):
mock_request
)
- @patch('django_comment_client.base.views.get_discussion_id_map', return_value={"test_commentable": {}})
+ def test_update_thread_course_topic(self, mock_request):
+ self._setup_mock_request(mock_request)
+ response = self.client.post(
+ reverse("update_thread", kwargs={"thread_id": "dummy", "course_id": self.course_id.to_deprecated_string()}),
+ data={"body": "foo", "title": "foo", "commentable_id": "some_topic"}
+ )
+ self.assertEqual(response.status_code, 200)
+
+ @patch('django_comment_client.base.views.get_discussion_categories_ids', return_value=["test_commentable"])
def test_update_thread_wrong_commentable_id(self, mock_get_discussion_id_map, mock_request):
self._test_request_error(
"update_thread",
@@ -861,7 +872,7 @@ class UpdateThreadUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin, MockReq
self.student = UserFactory.create()
CourseEnrollmentFactory(user=self.student, course_id=self.course.id)
- @patch('django_comment_client.base.views.get_discussion_id_map', return_value={"test_commentable": {}})
+ @patch('django_comment_client.base.views.get_discussion_categories_ids', return_value=["test_commentable"])
@patch('lms.lib.comment_client.utils.requests.request')
def _test_unicode_data(self, text, mock_request, mock_get_discussion_id_map):
self._set_mock_request_data(mock_request, {
diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py
index 4b0bba874a..222754b9d7 100644
--- a/lms/djangoapps/django_comment_client/base/views.py
+++ b/lms/djangoapps/django_comment_client/base/views.py
@@ -27,7 +27,7 @@ from django_comment_client.utils import (
JsonResponse,
prepare_content,
get_group_id_for_comments_service,
- get_discussion_id_map,
+ get_discussion_categories_ids
)
from django_comment_client.permissions import check_permissions_by_view, cached_has_permission
import lms.lib.comment_client as cc
@@ -149,8 +149,8 @@ def update_thread(request, course_id, thread_id):
thread.thread_type = request.POST["thread_type"]
if "commentable_id" in request.POST:
course = get_course_with_access(request.user, 'load', course_key)
- id_map = get_discussion_id_map(course)
- if request.POST.get("commentable_id") in id_map:
+ commentable_ids = get_discussion_categories_ids(course)
+ if request.POST.get("commentable_id") in commentable_ids:
thread.commentable_id = request.POST["commentable_id"]
else:
return JsonError(_("Topic doesn't exist"))
diff --git a/lms/djangoapps/django_comment_client/tests/test_utils.py b/lms/djangoapps/django_comment_client/tests/test_utils.py
index 7052b17d2b..9fd36cfae5 100644
--- a/lms/djangoapps/django_comment_client/tests/test_utils.py
+++ b/lms/djangoapps/django_comment_client/tests/test_utils.py
@@ -148,7 +148,7 @@ class CategoryMapTestCase(ModuleStoreTestCase):
self.course.discussion_topics = {}
self.course.save()
self.discussion_num = 0
- self.maxDiff = None # pylint: disable=C0103
+ self.maxDiff = None # pylint: disable=C0103
def create_discussion(self, discussion_category, discussion_target, **kwargs):
self.discussion_num += 1
@@ -193,7 +193,7 @@ class CategoryMapTestCase(ModuleStoreTestCase):
}
)
- check_cohorted_topics([]) # default (empty) cohort config
+ check_cohorted_topics([]) # default (empty) cohort config
self.course.cohort_config = {"cohorted": False, "cohorted_discussions": []}
check_cohorted_topics([])
@@ -211,7 +211,6 @@ class CategoryMapTestCase(ModuleStoreTestCase):
self.course.cohort_config = {"cohorted": False, "cohorted_discussions": ["Topic_A"]}
check_cohorted_topics([])
-
def test_single_inline(self):
self.create_discussion("Chapter", "Discussion")
self.assertCategoryMapEquals(
@@ -337,7 +336,6 @@ class CategoryMapTestCase(ModuleStoreTestCase):
self.course.cohort_config = {"cohorted": True}
check_cohorted(True)
-
def test_start_date_filter(self):
now = datetime.now()
later = datetime.max
@@ -564,6 +562,46 @@ class CategoryMapTestCase(ModuleStoreTestCase):
}
)
+ def test_ids_empty(self):
+ self.assertEqual(utils.get_discussion_categories_ids(self.course), [])
+
+ def test_ids_configured_topics(self):
+ self.course.discussion_topics = {
+ "Topic A": {"id": "Topic_A"},
+ "Topic B": {"id": "Topic_B"},
+ "Topic C": {"id": "Topic_C"}
+ }
+ self.assertItemsEqual(
+ utils.get_discussion_categories_ids(self.course),
+ ["Topic_A", "Topic_B", "Topic_C"]
+ )
+
+ def test_ids_inline(self):
+ self.create_discussion("Chapter 1", "Discussion 1")
+ self.create_discussion("Chapter 1", "Discussion 2")
+ self.create_discussion("Chapter 2", "Discussion")
+ self.create_discussion("Chapter 2 / Section 1 / Subsection 1", "Discussion")
+ self.create_discussion("Chapter 2 / Section 1 / Subsection 2", "Discussion")
+ self.create_discussion("Chapter 3 / Section 1", "Discussion")
+ self.assertItemsEqual(
+ utils.get_discussion_categories_ids(self.course),
+ ["discussion1", "discussion2", "discussion3", "discussion4", "discussion5", "discussion6"]
+ )
+
+ def test_ids_mixed(self):
+ self.course.discussion_topics = {
+ "Topic A": {"id": "Topic_A"},
+ "Topic B": {"id": "Topic_B"},
+ "Topic C": {"id": "Topic_C"}
+ }
+ self.create_discussion("Chapter 1", "Discussion 1")
+ self.create_discussion("Chapter 2", "Discussion")
+ self.create_discussion("Chapter 2 / Section 1 / Subsection 1", "Discussion")
+ self.assertItemsEqual(
+ utils.get_discussion_categories_ids(self.course),
+ ["Topic_A", "Topic_B", "Topic_C", "discussion1", "discussion2", "discussion3"]
+ )
+
class JsonResponseTestCase(TestCase, UnicodeTestMixin):
def _test_unicode_data(self, text):
diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py
index 4500dfb875..f88fc3d82c 100644
--- a/lms/djangoapps/django_comment_client/utils.py
+++ b/lms/djangoapps/django_comment_client/utils.py
@@ -90,7 +90,7 @@ def _filter_unstarted_categories(category_map):
unfiltered_queue = [category_map]
filtered_queue = [result_map]
- while len(unfiltered_queue) > 0:
+ while unfiltered_queue:
unfiltered_map = unfiltered_queue.pop()
filtered_map = filtered_queue.pop()
@@ -202,6 +202,23 @@ def get_discussion_category_map(course):
return _filter_unstarted_categories(category_map)
+def get_discussion_categories_ids(course):
+ """
+ Returns a list of available ids of categories for the course.
+ """
+ ids = []
+ queue = [get_discussion_category_map(course)]
+ while queue:
+ category_map = queue.pop()
+ for child in category_map["children"]:
+ if child in category_map["entries"]:
+ ids.append(category_map["entries"][child]["id"])
+ else:
+ queue.append(category_map["subcategories"][child])
+
+ return ids
+
+
class JsonResponse(HttpResponse):
def __init__(self, data=None):
content = json.dumps(data, cls=i4xEncoder)
diff --git a/lms/djangoapps/instructor/features/common.py b/lms/djangoapps/instructor/features/common.py
index 386305b1fb..3d511b2b9b 100644
--- a/lms/djangoapps/instructor/features/common.py
+++ b/lms/djangoapps/instructor/features/common.py
@@ -91,9 +91,9 @@ def click_a_button(step, button): # pylint: disable=unused-argument
# Expect to see a message that grade report is being generated
expected_msg = "Your grade report is being generated! You can view the status of the generation task in the 'Pending Instructor Tasks' section."
- world.wait_for_visible('#grade-request-response')
+ world.wait_for_visible('#report-request-response')
assert_in(
- expected_msg, world.css_text('#grade-request-response'),
+ expected_msg, world.css_text('#report-request-response'),
msg="Could not find grade report generation success message."
)
@@ -112,7 +112,8 @@ def click_a_button(step, button): # pylint: disable=unused-argument
elif button == "Download profile information as a CSV":
# Go to the data download section of the instructor dash
go_to_section("data_download")
- # Don't do anything else, next step will handle clicking & downloading
+
+ world.css_click('input[name="list-profiles-csv"]')
else:
raise ValueError("Unrecognized button option " + button)
diff --git a/lms/djangoapps/instructor/features/data_download.feature b/lms/djangoapps/instructor/features/data_download.feature
index 6efe9c6397..f83f785226 100644
--- a/lms/djangoapps/instructor/features/data_download.feature
+++ b/lms/djangoapps/instructor/features/data_download.feature
@@ -44,3 +44,12 @@ Feature: LMS.Instructor Dash Data Download
| Role |
| instructor |
| staff |
+
+ Scenario: Generate & download a student profile report
+ Given I am "
" for a course
+ When I click "Download profile information as a CSV"
+ Then I see a student profile csv file in the reports table
+ Examples:
+ | Role |
+ | instructor |
+ | staff |
diff --git a/lms/djangoapps/instructor/features/data_download.py b/lms/djangoapps/instructor/features/data_download.py
index 6df3403e7e..9aefec5d47 100644
--- a/lms/djangoapps/instructor/features/data_download.py
+++ b/lms/djangoapps/instructor/features/data_download.py
@@ -68,15 +68,24 @@ length=0""".format(world.course_key)
assert_in(expected_config, world.css_text('#data-grade-config-text'))
-@step(u"I see a grade report csv file in the reports table")
-def find_grade_report_csv_link(step): # pylint: disable=unused-argument
- # Need to reload the page to see the grades download table
+def verify_report_is_generated(report_name_substring):
+ # Need to reload the page to see the reports table updated
reload_the_page(step)
world.wait_for_visible('#report-downloads-table')
# Find table and assert a .csv file is present
quoted_id = http.urlquote(world.course_key).replace('/', '_')
- expected_file_regexp = quoted_id + '_grade_report_\d{4}-\d{2}-\d{2}-\d{4}\.csv'
+ expected_file_regexp = quoted_id + '_' + report_name_substring + '\d{4}-\d{2}-\d{2}-\d{4}\.csv'
assert_regexp_matches(
world.css_html('#report-downloads-table'), expected_file_regexp,
- msg="Expected grade report filename was not found."
+ msg="Expected report filename was not found."
)
+
+
+@step(u"I see a grade report csv file in the reports table")
+def find_grade_report_csv_link(step): # pylint: disable=unused-argument
+ verify_report_is_generated('grade_report')
+
+
+@step(u"I see a student profile csv file in the reports table")
+def find_student_profile_report_csv_link(step): # pylint: disable=unused-argument
+ verify_report_is_generated('student_profile_info')
diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py
index 757adfd58d..480622672b 100644
--- a/lms/djangoapps/instructor/tests/test_api.py
+++ b/lms/djangoapps/instructor/tests/test_api.py
@@ -55,6 +55,22 @@ from ..views.tools import get_extended_due
EXPECTED_CSV_HEADER = '"code","course_id","company_name","created_by","redeemed_by","invoice_id","purchaser","customer_reference_number","internal_reference"'
EXPECTED_COUPON_CSV_HEADER = '"course_id","percentage_discount","code_redeemed_count","description"'
+# ddt data for test cases involving reports
+REPORTS_DATA = (
+ {
+ 'report_type': 'grade',
+ 'instructor_api_endpoint': 'calculate_grades_csv',
+ 'task_api_endpoint': 'instructor_task.api.submit_calculate_grades_csv',
+ 'extra_instructor_api_kwargs': {}
+ },
+ {
+ 'report_type': 'enrolled student profile',
+ 'instructor_api_endpoint': 'get_students_features',
+ 'task_api_endpoint': 'instructor_task.api.submit_calculate_students_features_csv',
+ 'extra_instructor_api_kwargs': {'csv': '/csv'}
+ }
+)
+
@common_exceptions_400
def view_success(request): # pylint: disable=W0613
@@ -156,6 +172,7 @@ class TestInstructorAPIDenyLevels(ModuleStoreTestCase, LoginEnrollmentTestCase):
('list_background_email_tasks', {}),
('list_report_downloads', {}),
('calculate_grades_csv', {}),
+ ('get_students_features', {}),
]
# Endpoints that only Instructors can access
self.instructor_level_endpoints = [
@@ -1343,6 +1360,7 @@ class TestInstructorAPILevelsAccess(ModuleStoreTestCase, LoginEnrollmentTestCase
self.assertNotIn(rolename, user_roles)
+@ddt.ddt
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
class TestInstructorAPILevelsDataDump(ModuleStoreTestCase, LoginEnrollmentTestCase):
"""
@@ -1350,6 +1368,7 @@ class TestInstructorAPILevelsDataDump(ModuleStoreTestCase, LoginEnrollmentTestCa
"""
def setUp(self):
+ super(TestInstructorAPILevelsDataDump, self).setUp()
self.course = CourseFactory.create()
self.course_mode = CourseMode(course_id=self.course.id,
mode_slug="honor",
@@ -1613,6 +1632,21 @@ class TestInstructorAPILevelsDataDump(ModuleStoreTestCase, LoginEnrollmentTestCa
self.assertEqual(student_json['username'], student.username)
self.assertEqual(student_json['email'], student.email)
+ @ddt.data(True, False)
+ def test_get_students_features_cohorted(self, is_cohorted):
+ """
+ Test that get_students_features includes cohort info when the course is
+ cohorted, and does not when the course is not cohorted.
+ """
+ url = reverse('get_students_features', kwargs={'course_id': unicode(self.course.id)})
+ self.course.cohort_config = {'cohorted': is_cohorted}
+ self.store.update_item(self.course, self.instructor.id)
+
+ response = self.client.get(url, {})
+ res_json = json.loads(response.content)
+
+ self.assertEqual('cohort' in res_json['feature_names'], is_cohorted)
+
@patch.object(instructor.views.api, 'anonymous_id_for_user', Mock(return_value='42'))
@patch.object(instructor.views.api, 'unique_id_for_user', Mock(return_value='41'))
def test_get_anon_ids(self):
@@ -1625,9 +1659,9 @@ class TestInstructorAPILevelsDataDump(ModuleStoreTestCase, LoginEnrollmentTestCa
body = response.content.replace('\r', '')
self.assertTrue(body.startswith(
'"User ID","Anonymized User ID","Course Specific Anonymized User ID"'
- '\n"2","41","42"\n'
+ '\n"3","41","42"\n'
))
- self.assertTrue(body.endswith('"7","41","42"\n'))
+ self.assertTrue(body.endswith('"8","41","42"\n'))
def test_list_report_downloads(self):
url = reverse('list_report_downloads', kwargs={'course_id': self.course.id.to_deprecated_string()})
@@ -1655,33 +1689,31 @@ class TestInstructorAPILevelsDataDump(ModuleStoreTestCase, LoginEnrollmentTestCa
res_json = json.loads(response.content)
self.assertEqual(res_json, expected_response)
- def test_calculate_grades_csv_success(self):
- url = reverse('calculate_grades_csv', kwargs={'course_id': self.course.id.to_deprecated_string()})
+ @ddt.data(*REPORTS_DATA)
+ @ddt.unpack
+ def test_calculate_report_csv_success(self, report_type, instructor_api_endpoint, task_api_endpoint, extra_instructor_api_kwargs):
+ kwargs = {'course_id': unicode(self.course.id)}
+ kwargs.update(extra_instructor_api_kwargs)
+ url = reverse(instructor_api_endpoint, kwargs=kwargs)
- with patch('instructor_task.api.submit_calculate_grades_csv') as mock_cal_grades:
- mock_cal_grades.return_value = True
+ with patch(task_api_endpoint):
response = self.client.get(url, {})
- success_status = "Your grade report is being generated! You can view the status of the generation task in the 'Pending Instructor Tasks' section."
+ success_status = "Your {report_type} report is being generated! You can view the status of the generation task in the 'Pending Instructor Tasks' section.".format(report_type=report_type)
self.assertIn(success_status, response.content)
- def test_calculate_grades_csv_already_running(self):
- url = reverse('calculate_grades_csv', kwargs={'course_id': self.course.id.to_deprecated_string()})
+ @ddt.data(*REPORTS_DATA)
+ @ddt.unpack
+ def test_calculate_report_csv_already_running(self, report_type, instructor_api_endpoint, task_api_endpoint, extra_instructor_api_kwargs):
+ kwargs = {'course_id': unicode(self.course.id)}
+ kwargs.update(extra_instructor_api_kwargs)
+ url = reverse(instructor_api_endpoint, kwargs=kwargs)
- with patch('instructor_task.api.submit_calculate_grades_csv') as mock_cal_grades:
- mock_cal_grades.side_effect = AlreadyRunningError()
+ with patch(task_api_endpoint) as mock:
+ mock.side_effect = AlreadyRunningError()
response = self.client.get(url, {})
- already_running_status = "A grade report generation task is already in progress. Check the 'Pending Instructor Tasks' table for the status of the task. When completed, the report will be available for download in the table below."
+ already_running_status = "{report_type} report generation task is already in progress. Check the 'Pending Instructor Tasks' table for the status of the task. When completed, the report will be available for download in the table below.".format(report_type=report_type)
self.assertIn(already_running_status, response.content)
- def test_get_students_features_csv(self):
- """
- Test that some minimum of information is formatted
- correctly in the response to get_students_features.
- """
- url = reverse('get_students_features', kwargs={'course_id': self.course.id.to_deprecated_string()})
- response = self.client.get(url + '/csv', {})
- self.assertEqual(response['Content-Type'], 'text/csv')
-
def test_get_distribution_no_feature(self):
"""
Test that get_distribution lists available features
diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py
index 931a3da703..f62119aa99 100644
--- a/lms/djangoapps/instructor/views/api.py
+++ b/lms/djangoapps/instructor/views/api.py
@@ -80,6 +80,7 @@ from .tools import (
bulk_email_is_enabled_for_course,
add_block_ids,
)
+from opaque_keys.edx.keys import CourseKey
from opaque_keys.edx.locations import SlashSeparatedCourseKey
from opaque_keys import InvalidKeyError
@@ -690,7 +691,8 @@ def get_students_features(request, course_id, csv=False): # pylint: disable=W06
TO DO accept requests for different attribute sets.
"""
- course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id)
+ course_key = CourseKey.from_string(course_id)
+ course = get_course_by_id(course_key)
available_features = instructor_analytics.basic.AVAILABLE_FEATURES
@@ -704,8 +706,6 @@ def get_students_features(request, course_id, csv=False): # pylint: disable=W06
'goals'
]
- student_data = instructor_analytics.basic.enrolled_students_features(course_id, query_features)
-
# Provide human-friendly and translatable names for these features. These names
# will be displayed in the table generated in data_download.coffee. It is not (yet)
# used as the header row in the CSV, but could be in the future.
@@ -723,9 +723,15 @@ def get_students_features(request, course_id, csv=False): # pylint: disable=W06
'goals': _('Goals'),
}
+ if course.is_cohorted:
+ # Translators: 'Cohort' refers to a group of students within a course.
+ query_features.append('cohort')
+ query_features_names['cohort'] = _('Cohort')
+
if not csv:
+ student_data = instructor_analytics.basic.enrolled_students_features(course_key, query_features)
response_payload = {
- 'course_id': course_id.to_deprecated_string(),
+ 'course_id': unicode(course_key),
'students': student_data,
'students_count': len(student_data),
'queried_features': query_features,
@@ -734,8 +740,13 @@ def get_students_features(request, course_id, csv=False): # pylint: disable=W06
}
return JsonResponse(response_payload)
else:
- header, datarows = instructor_analytics.csvs.format_dictlist(student_data, query_features)
- return instructor_analytics.csvs.create_csv_response("enrolled_profiles.csv", header, datarows)
+ try:
+ instructor_task.api.submit_calculate_students_features_csv(request, course_key, query_features)
+ success_status = _("Your enrolled student profile report is being generated! You can view the status of the generation task in the 'Pending Instructor Tasks' section.")
+ return JsonResponse({"status": success_status})
+ except AlreadyRunningError:
+ already_running_status = _("An enrolled student profile report generation task is already in progress. Check the 'Pending Instructor Tasks' table for the status of the task. When completed, the report will be available for download in the table below.")
+ return JsonResponse({"status": already_running_status})
@ensure_csrf_cookie
diff --git a/lms/djangoapps/instructor_analytics/basic.py b/lms/djangoapps/instructor_analytics/basic.py
index 3b394a6001..eecede860a 100644
--- a/lms/djangoapps/instructor_analytics/basic.py
+++ b/lms/djangoapps/instructor_analytics/basic.py
@@ -122,22 +122,27 @@ def purchase_transactions(course_id, features):
return [purchase_transactions_info(purchased_course, features) for purchased_course in purchased_courses]
-def enrolled_students_features(course_id, features):
+def enrolled_students_features(course_key, features):
"""
Return list of student features as dictionaries.
- enrolled_students_features(course_id, ['username, first_name'])
+ enrolled_students_features(course_key, ['username', 'first_name'])
would return [
{'username': 'username1', 'first_name': 'firstname1'}
{'username': 'username2', 'first_name': 'firstname2'}
{'username': 'username3', 'first_name': 'firstname3'}
]
"""
+ include_cohort_column = 'cohort' in features
+
students = User.objects.filter(
- courseenrollment__course_id=course_id,
+ courseenrollment__course_id=course_key,
courseenrollment__is_active=1,
).order_by('username').select_related('profile')
+ if include_cohort_column:
+ students = students.prefetch_related('course_groups')
+
def extract_student(student, features):
""" convert student to dictionary """
student_features = [x for x in STUDENT_FEATURES if x in features]
@@ -151,6 +156,14 @@ def enrolled_students_features(course_id, features):
for feature in profile_features)
student_dict.update(profile_dict)
+ if include_cohort_column:
+ # Note that we use student.course_groups.all() here instead of
+ # student.course_groups.filter(). The latter creates a fresh query,
+ # therefore negating the performance gain from prefetch_related().
+ student_dict['cohort'] = next(
+ (cohort.name for cohort in student.course_groups.all() if cohort.course_id == course_key),
+ "[unassigned]"
+ )
return student_dict
return [extract_student(student, features) for student in students]
diff --git a/lms/djangoapps/instructor_analytics/tests/test_basic.py b/lms/djangoapps/instructor_analytics/tests/test_basic.py
index 77c6e1dfdc..788c55fb12 100644
--- a/lms/djangoapps/instructor_analytics/tests/test_basic.py
+++ b/lms/djangoapps/instructor_analytics/tests/test_basic.py
@@ -13,15 +13,19 @@ from instructor_analytics.basic import (
sale_record_features, enrolled_students_features, course_registration_features, coupon_codes_features,
AVAILABLE_FEATURES, STUDENT_FEATURES, PROFILE_FEATURES
)
+from course_groups.tests.helpers import CohortFactory
+from course_groups.models import CourseUserGroup
from courseware.tests.factories import InstructorFactory
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
-class TestAnalyticsBasic(TestCase):
+class TestAnalyticsBasic(ModuleStoreTestCase):
""" Test basic analytics functions. """
def setUp(self):
+ super(TestAnalyticsBasic, self).setUp()
self.course_key = SlashSeparatedCourseKey('robot', 'course', 'id')
self.users = tuple(UserFactory() for _ in xrange(30))
self.ces = tuple(CourseEnrollment.enroll(user, self.course_key)
@@ -40,7 +44,8 @@ class TestAnalyticsBasic(TestCase):
query_features = ('username', 'name', 'email')
for feature in query_features:
self.assertIn(feature, AVAILABLE_FEATURES)
- userreports = enrolled_students_features(self.course_key, query_features)
+ with self.assertNumQueries(1):
+ userreports = enrolled_students_features(self.course_key, query_features)
self.assertEqual(len(userreports), len(self.users))
for userreport in userreports:
self.assertEqual(set(userreport.keys()), set(query_features))
@@ -48,6 +53,37 @@ class TestAnalyticsBasic(TestCase):
self.assertIn(userreport['email'], [user.email for user in self.users])
self.assertIn(userreport['name'], [user.profile.name for user in self.users])
+ def test_enrolled_students_features_keys_cohorted(self):
+ course = CourseFactory.create(course_key=self.course_key)
+ course.cohort_config = {'cohorted': True, 'auto_cohort': True, 'auto_cohort_groups': ['cohort']}
+ self.store.update_item(course, self.instructor.id)
+ cohort = CohortFactory.create(name='cohort', course_id=course.id)
+ cohorted_students = [UserFactory.create() for _ in xrange(10)]
+ cohorted_usernames = [student.username for student in cohorted_students]
+ non_cohorted_student = UserFactory.create()
+ for student in cohorted_students:
+ cohort.users.add(student)
+ CourseEnrollment.enroll(student, course.id)
+ CourseEnrollment.enroll(non_cohorted_student, course.id)
+ instructor = InstructorFactory(course_key=course.id)
+ self.client.login(username=instructor.username, password='test')
+
+ query_features = ('username', 'cohort')
+ # There should be a constant of 2 SQL queries when calling
+ # enrolled_students_features. The first query comes from the call to
+ # User.objects.filter(...), and the second comes from
+ # prefetch_related('course_groups').
+ with self.assertNumQueries(2):
+ userreports = enrolled_students_features(course.id, query_features)
+ self.assertEqual(len([r for r in userreports if r['username'] in cohorted_usernames]), len(cohorted_students))
+ self.assertEqual(len([r for r in userreports if r['username'] == non_cohorted_student.username]), 1)
+ for report in userreports:
+ self.assertEqual(set(report.keys()), set(query_features))
+ if report['username'] in cohorted_usernames:
+ self.assertEqual(report['cohort'], cohort.name)
+ else:
+ self.assertEqual(report['cohort'], '[unassigned]')
+
def test_available_features(self):
self.assertEqual(len(AVAILABLE_FEATURES), len(STUDENT_FEATURES + PROFILE_FEATURES))
self.assertEqual(set(AVAILABLE_FEATURES), set(STUDENT_FEATURES + PROFILE_FEATURES))
@@ -59,6 +95,7 @@ class TestCourseSaleRecordsAnalyticsBasic(ModuleStoreTestCase):
"""
Fixtures.
"""
+ super(TestCourseSaleRecordsAnalyticsBasic, self).setUp()
self.course = CourseFactory.create()
self.instructor = InstructorFactory(course_key=self.course.id)
self.client.login(username=self.instructor.username, password='test')
diff --git a/lms/djangoapps/instructor_task/api.py b/lms/djangoapps/instructor_task/api.py
index e91e58fa0f..219df82899 100644
--- a/lms/djangoapps/instructor_task/api.py
+++ b/lms/djangoapps/instructor_task/api.py
@@ -17,7 +17,8 @@ from instructor_task.tasks import (rescore_problem,
reset_problem_attempts,
delete_problem_state,
send_bulk_course_email,
- calculate_grades_csv)
+ calculate_grades_csv,
+ calculate_students_features_csv)
from instructor_task.api_helper import (check_arguments_for_rescoring,
encode_problem_and_student_input,
@@ -218,3 +219,17 @@ def submit_calculate_grades_csv(request, course_key):
task_key = ""
return submit_task(request, task_type, task_class, course_key, task_input, task_key)
+
+
+def submit_calculate_students_features_csv(request, course_key, features):
+ """
+ Submits a task to generate a CSV containing student profile info.
+
+ Raises AlreadyRunningError if said CSV is already being updated.
+ """
+ task_type = 'profile_info_csv'
+ task_class = calculate_students_features_csv
+ task_input = {'features': features}
+ task_key = ""
+
+ return submit_task(request, task_type, task_class, course_key, task_input, task_key)
diff --git a/lms/djangoapps/instructor_task/tasks.py b/lms/djangoapps/instructor_task/tasks.py
index 69e8ee1865..297c306637 100644
--- a/lms/djangoapps/instructor_task/tasks.py
+++ b/lms/djangoapps/instructor_task/tasks.py
@@ -31,6 +31,7 @@ from instructor_task.tasks_helper import (
reset_attempts_module_state,
delete_problem_module_state,
push_grades_to_s3,
+ push_students_csv_to_s3
)
from bulk_email.tasks import perform_delegate_email_batches
@@ -136,6 +137,19 @@ def calculate_grades_csv(entry_id, xmodule_instance_args):
"""
Grade a course and push the results to an S3 bucket for download.
"""
+ # Translators: This is a past-tense verb that is inserted into task progress messages as {action}.
action_name = ugettext_noop('graded')
task_fn = partial(push_grades_to_s3, xmodule_instance_args)
return run_main_task(entry_id, task_fn, action_name)
+
+
+@task(base=BaseInstructorTask, routing_key=settings.GRADES_DOWNLOAD_ROUTING_KEY) # pylint: disable=E1102
+def calculate_students_features_csv(entry_id, xmodule_instance_args):
+ """
+ Compute student profile information for a course and upload the
+ CSV to an S3 bucket for download.
+ """
+ # Translators: This is a past-tense verb that is inserted into task progress messages as {action}.
+ action_name = ugettext_noop('generated')
+ task_fn = partial(push_students_csv_to_s3, xmodule_instance_args)
+ return run_main_task(entry_id, task_fn, action_name)
diff --git a/lms/djangoapps/instructor_task/tasks_helper.py b/lms/djangoapps/instructor_task/tasks_helper.py
index c13d22271f..44beec3bb3 100644
--- a/lms/djangoapps/instructor_task/tasks_helper.py
+++ b/lms/djangoapps/instructor_task/tasks_helper.py
@@ -23,6 +23,8 @@ from courseware.grades import iterate_grades_for
from courseware.models import StudentModule
from courseware.model_data import FieldDataCache
from courseware.module_render import get_module_for_descriptor_internal
+from instructor_analytics.basic import enrolled_students_features
+from instructor_analytics.csvs import format_dictlist
from instructor_task.models import ReportStore, InstructorTask, PROGRESS
from student.models import CourseEnrollment
@@ -477,6 +479,32 @@ def delete_problem_module_state(xmodule_instance_args, _module_descriptor, stude
return UPDATE_STATUS_SUCCEEDED
+def upload_csv_to_report_store(rows, csv_name, course_id, timestamp):
+ """
+ Upload data as a CSV using ReportStore.
+
+ Arguments:
+ rows: CSV data in the following format (first column may be a
+ header):
+ [
+ [row1_colum1, row1_colum2, ...],
+ ...
+ ]
+ csv_name: Name of the resulting CSV
+ course_id: ID of the course
+ """
+ report_store = ReportStore.from_config()
+ report_store.store_rows(
+ course_id,
+ u"{course_prefix}_{csv_name}_{timestamp_str}.csv".format(
+ course_prefix=urllib.quote(unicode(course_id).replace("/", "_")),
+ csv_name=csv_name,
+ timestamp_str=timestamp.strftime("%Y-%m-%d-%H%M")
+ ),
+ rows
+ )
+
+
def push_grades_to_s3(_xmodule_instance_args, _entry_id, course_id, _task_input, action_name):
"""
For a given `course_id`, generate a grades CSV file for all students that
@@ -557,25 +585,30 @@ def push_grades_to_s3(_xmodule_instance_args, _entry_id, course_id, _task_input,
curr_step = "Uploading CSVs"
update_task_progress()
- # Generate parts of the file name
- timestamp_str = start_time.strftime("%Y-%m-%d-%H%M")
- course_id_prefix = urllib.quote(course_id.to_deprecated_string().replace("/", "_"))
-
# Perform the actual upload
- report_store = ReportStore.from_config()
- report_store.store_rows(
- course_id,
- u"{}_grade_report_{}.csv".format(course_id_prefix, timestamp_str),
- rows
- )
+ upload_csv_to_report_store(rows, 'grade_report', course_id, start_time)
# If there are any error rows (don't count the header), write them out as well
if len(err_rows) > 1:
- report_store.store_rows(
- course_id,
- u"{}_grade_report_{}_err.csv".format(course_id_prefix, timestamp_str),
- err_rows
- )
+ upload_csv_to_report_store(err_rows, 'grade_report_err', course_id, start_time)
# One last update before we close out...
return update_task_progress()
+
+
+def push_students_csv_to_s3(_xmodule_instance_args, _entry_id, course_id, task_input, _action_name):
+ """
+ For a given `course_id`, generate a CSV file containing profile
+ information for all students that are enrolled, and store using a
+ `ReportStore`.
+ """
+ # compute the student features table and format it
+ query_features = task_input.get('features')
+ student_data = enrolled_students_features(course_id, query_features)
+ header, rows = format_dictlist(student_data, query_features)
+ rows.insert(0, header)
+
+ # Perform the upload
+ upload_csv_to_report_store(rows, 'student_profile_info', course_id, datetime.now(UTC))
+
+ return UPDATE_STATUS_SUCCEEDED
diff --git a/lms/djangoapps/instructor_task/tests/test_api.py b/lms/djangoapps/instructor_task/tests/test_api.py
index dac66fdac1..ce2960307b 100644
--- a/lms/djangoapps/instructor_task/tests/test_api.py
+++ b/lms/djangoapps/instructor_task/tests/test_api.py
@@ -15,6 +15,7 @@ from instructor_task.api import (
submit_reset_problem_attempts_for_all_students,
submit_delete_problem_state_for_all_students,
submit_bulk_course_email,
+ submit_calculate_students_features_csv,
)
from instructor_task.api_helper import AlreadyRunningError
@@ -170,14 +171,34 @@ class InstructorTaskCourseSubmitTest(InstructorTaskCourseTestCase):
course_email = CourseEmail.create(self.course.id, self.instructor, SEND_TO_ALL, "Test Subject", "This is a test message
")
return course_email.id # pylint: disable=E1101
- def test_submit_bulk_email_all(self):
- email_id = self._define_course_email()
- instructor_task = submit_bulk_course_email(self.create_task_request(self.instructor), self.course.id, email_id)
-
- # test resubmitting, by updating the existing record:
+ def _test_resubmission(self, api_call):
+ """
+ Tests the resubmission of an instructor task through the API.
+ The call to the API is a lambda expression passed via
+ `api_call`. Expects that the API call returns the resulting
+ InstructorTask object, and that its resubmission raises
+ `AlreadyRunningError`.
+ """
+ instructor_task = api_call()
instructor_task = InstructorTask.objects.get(id=instructor_task.id) # pylint: disable=E1101
instructor_task.task_state = PROGRESS
instructor_task.save()
-
with self.assertRaises(AlreadyRunningError):
- instructor_task = submit_bulk_course_email(self.create_task_request(self.instructor), self.course.id, email_id)
+ api_call()
+
+ def test_submit_bulk_email_all(self):
+ email_id = self._define_course_email()
+ api_call = lambda: submit_bulk_course_email(
+ self.create_task_request(self.instructor),
+ self.course.id,
+ email_id
+ )
+ self._test_resubmission(api_call)
+
+ def test_submit_calculate_students_features(self):
+ api_call = lambda: submit_calculate_students_features_csv(
+ self.create_task_request(self.instructor),
+ self.course.id,
+ features=[]
+ )
+ self._test_resubmission(api_call)
diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
index 99bc997a18..976022e93c 100644
--- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
+++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py
@@ -13,32 +13,32 @@ from mock import Mock, patch
from django.conf import settings
from django.test.testcases import TestCase
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
from student.tests.factories import CourseEnrollmentFactory, UserFactory
-from instructor_task.tasks_helper import push_grades_to_s3
+from instructor_task.models import ReportStore
+from instructor_task.tasks_helper import push_grades_to_s3, push_students_csv_to_s3, UPDATE_STATUS_SUCCEEDED
-TEST_COURSE_ORG = 'edx'
-TEST_COURSE_NAME = 'test_course'
-TEST_COURSE_NUMBER = '1.23x'
-
-
-@ddt.ddt
-class TestInstructorGradeReport(TestCase):
+class TestReport(ModuleStoreTestCase):
"""
- Tests that CSV grade report generation works.
+ Base class for testing CSV download tasks.
"""
def setUp(self):
- self.course = CourseFactory.create(org=TEST_COURSE_ORG,
- number=TEST_COURSE_NUMBER,
- display_name=TEST_COURSE_NAME)
+ self.course = CourseFactory.create()
def tearDown(self):
if os.path.exists(settings.GRADES_DOWNLOAD['ROOT_PATH']):
shutil.rmtree(settings.GRADES_DOWNLOAD['ROOT_PATH'])
+
+@ddt.ddt
+class TestInstructorGradeReport(TestReport):
+ """
+ Tests that CSV grade report generation works.
+ """
def create_student(self, username, email):
student = UserFactory.create(username=username, email=email)
CourseEnrollmentFactory.create(user=student, course_id=self.course.id)
@@ -58,3 +58,18 @@ class TestInstructorGradeReport(TestCase):
result = push_grades_to_s3(None, None, self.course.id, None, 'graded')
#This assertion simply confirms that the generation completed with no errors
self.assertEquals(result['succeeded'], result['attempted'])
+
+
+class TestStudentReport(TestReport):
+ """
+ Tests that CSV student profile report generation works.
+ """
+ def test_success(self):
+ task_input = {'features': []}
+ with patch('instructor_task.tasks_helper._get_current_task'):
+ result = push_students_csv_to_s3(None, None, self.course.id, task_input, 'calculated')
+ report_store = ReportStore.from_config()
+ links = report_store.links_for(self.course.id)
+
+ self.assertEquals(len(links), 1)
+ self.assertEquals(result, UPDATE_STATUS_SUCCEEDED)
diff --git a/lms/djangoapps/notifier_api/serializers.py b/lms/djangoapps/notifier_api/serializers.py
index db145768bf..5164ad812f 100644
--- a/lms/djangoapps/notifier_api/serializers.py
+++ b/lms/djangoapps/notifier_api/serializers.py
@@ -1,4 +1,5 @@
from django.contrib.auth.models import User
+from django.http import Http404
from rest_framework import serializers
from course_groups.cohorts import is_course_cohorted
@@ -47,16 +48,20 @@ class NotifierUserSerializer(serializers.ModelSerializer):
for role in user.roles.all()
for perm in role.permissions.all() if perm.name == "see_all_cohorts"
}
- return {
- unicode(enrollment.course_id): {
- "cohort_id": cohort_id_map.get(enrollment.course_id),
- "see_all_cohorts": (
- enrollment.course_id in see_all_cohorts_set or
- not is_course_cohorted(enrollment.course_id)
- ),
- }
- for enrollment in user.courseenrollment_set.all() if enrollment.is_active
- }
+ ret = {}
+ for enrollment in user.courseenrollment_set.all():
+ if enrollment.is_active:
+ try:
+ ret[unicode(enrollment.course_id)] = {
+ "cohort_id": cohort_id_map.get(enrollment.course_id),
+ "see_all_cohorts": (
+ enrollment.course_id in see_all_cohorts_set or
+ not is_course_cohorted(enrollment.course_id)
+ ),
+ }
+ except Http404: # is_course_cohorted raises this if course does not exist
+ pass
+ return ret
class Meta:
model = User
diff --git a/lms/djangoapps/notifier_api/tests.py b/lms/djangoapps/notifier_api/tests.py
index d741a4f940..69733ae7ad 100644
--- a/lms/djangoapps/notifier_api/tests.py
+++ b/lms/djangoapps/notifier_api/tests.py
@@ -10,6 +10,7 @@ from django_comment_common.models import Role, Permission
from lang_pref import LANGUAGE_KEY
from notification_prefs import NOTIFICATION_PREF_KEY
from notifier_api.views import NotifierUsersViewSet
+from opaque_keys.edx.locator import CourseLocator
from student.models import CourseEnrollment
from student.tests.factories import UserFactory, CourseEnrollmentFactory
from user_api.models import UserPreference
@@ -120,6 +121,14 @@ class NotifierUsersViewSetTest(UrlResetMixin, ModuleStoreTestCase):
result = self._get_detail()
self.assertEqual(result["course_info"], {})
+ def test_course_info_non_existent_course_enrollment(self):
+ CourseEnrollmentFactory(
+ user=self.user,
+ course_id=CourseLocator(org="dummy", course="dummy", run="non_existent")
+ )
+ result = self._get_detail()
+ self.assertEqual(result["course_info"], {})
+
def test_preferences(self):
lang_pref = UserPreferenceFactory(
user=self.user,
diff --git a/lms/envs/bok_choy.auth.json b/lms/envs/bok_choy.auth.json
index 66ca730f5e..86d08d6029 100644
--- a/lms/envs/bok_choy.auth.json
+++ b/lms/envs/bok_choy.auth.json
@@ -46,6 +46,15 @@
"port": 27017,
"user": "edxapp"
},
+ "EVENT_TRACKING_BACKENDS": {
+ "mongo": {
+ "ENGINE": "eventtracking.backends.mongodb.MongoBackend",
+ "OPTIONS": {
+ "database": "test",
+ "collection": "events"
+ }
+ }
+ },
"MODULESTORE": {
"default": {
"ENGINE": "xmodule.modulestore.mixed.MixedModuleStore",
diff --git a/lms/lib/comment_client/thread.py b/lms/lib/comment_client/thread.py
index 255efd92a5..ceedbd3899 100644
--- a/lms/lib/comment_client/thread.py
+++ b/lms/lib/comment_client/thread.py
@@ -62,6 +62,7 @@ class Thread(models.Model):
if query_params.get('text'):
search_query = query_params['text']
course_id = query_params['course_id']
+ group_id = query_params['group_id'] if 'group_id' in query_params else None
requested_page = params['page']
total_results = response.get('total_results')
corrected_text = response.get('corrected_text')
@@ -72,15 +73,17 @@ class Thread(models.Model):
{
'query': search_query,
'corrected_text': corrected_text,
+ 'group_id': group_id,
'page': requested_page,
'total_results': total_results,
}
)
log.info(
- 'forum_text_search query="{search_query}" corrected_text="{corrected_text}" course_id={course_id} page={requested_page} total_results={total_results}'.format(
+ u'forum_text_search query="{search_query}" corrected_text="{corrected_text}" course_id={course_id} group_id={group_id} page={requested_page} total_results={total_results}'.format(
search_query=search_query,
corrected_text=corrected_text,
course_id=course_id,
+ group_id=group_id,
requested_page=requested_page,
total_results=total_results
)
diff --git a/lms/static/coffee/src/instructor_dashboard/data_download.coffee b/lms/static/coffee/src/instructor_dashboard/data_download.coffee
index 3ff456c871..4931e35c83 100644
--- a/lms/static/coffee/src/instructor_dashboard/data_download.coffee
+++ b/lms/static/coffee/src/instructor_dashboard/data_download.coffee
@@ -26,11 +26,11 @@ class DataDownload
# response areas
@$download = @$section.find '.data-download-container'
@$download_display_text = @$download.find '.data-display-text'
- @$download_display_table = @$download.find '.data-display-table'
@$download_request_response_error = @$download.find '.request-response-error'
- @$grades = @$section.find '.grades-download-container'
- @$grades_request_response = @$grades.find '.request-response'
- @$grades_request_response_error = @$grades.find '.request-response-error'
+ @$reports = @$section.find '.reports-download-container'
+ @$download_display_table = @$reports.find '.data-display-table'
+ @$reports_request_response = @$reports.find '.request-response'
+ @$reports_request_response_error = @$reports.find '.request-response-error'
@report_downloads = new ReportDownloads(@$section)
@instructor_tasks = new (PendingInstructorTasks()) @$section
@@ -45,11 +45,22 @@ class DataDownload
# this handler binds to both the download
# and the csv button
@$list_studs_csv_btn.click (e) =>
+ @clear_display()
+
url = @$list_studs_csv_btn.data 'endpoint'
# handle csv special case
# redirect the document to the csv file.
url += '/csv'
- location.href = url
+
+ $.ajax
+ dataType: 'json'
+ url: url
+ error: (std_ajax_err) =>
+ @$reports_request_response_error.text gettext("Error generating student profile information. Please try again.")
+ $(".msg-error").css({"display":"block"})
+ success: (data) =>
+ @$reports_request_response.text data['status']
+ $(".msg-confirm").css({"display":"block"})
@$list_studs_btn.click (e) =>
url = @$list_studs_btn.data 'endpoint'
@@ -62,7 +73,7 @@ class DataDownload
$.ajax
dataType: 'json'
url: url
- error: std_ajax_err =>
+ error: (std_ajax_err) =>
@clear_display()
@$download_request_response_error.text gettext("Error getting student list.")
success: (data) =>
@@ -89,7 +100,7 @@ class DataDownload
$.ajax
dataType: 'json'
url: url
- error: std_ajax_err =>
+ error: (std_ajax_err) =>
@clear_display()
@$download_request_response_error.text gettext("Error retrieving grading configuration.")
success: (data) =>
@@ -105,11 +116,11 @@ class DataDownload
$.ajax
dataType: 'json'
url: url
- error: std_ajax_err =>
- @$grades_request_response_error.text gettext("Error generating grades. Please try again.")
+ error: (std_ajax_err) =>
+ @$reports_request_response_error.text gettext("Error generating grades. Please try again.")
$(".msg-error").css({"display":"block"})
success: (data) =>
- @$grades_request_response.text data['status']
+ @$reports_request_response.text data['status']
$(".msg-confirm").css({"display":"block"})
# handler for when the section title is clicked.
@@ -129,8 +140,8 @@ class DataDownload
@$download_display_text.empty()
@$download_display_table.empty()
@$download_request_response_error.empty()
- @$grades_request_response.empty()
- @$grades_request_response_error.empty()
+ @$reports_request_response.empty()
+ @$reports_request_response_error.empty()
# Clear any CSS styling from the request-response areas
$(".msg-confirm").css({"display":"none"})
$(".msg-error").css({"display":"none"})
@@ -157,7 +168,7 @@ class ReportDownloads
@create_report_downloads_table data.downloads
else
console.log "No reports ready for download"
- error: std_ajax_err => console.error "Error finding report downloads"
+ error: (std_ajax_err) => console.error "Error finding report downloads"
create_report_downloads_table: (report_downloads_data) ->
@$report_downloads_table.empty()
diff --git a/lms/static/js/views/cohorts.js b/lms/static/js/views/cohorts.js
index c7cf01aeba..9f9131f316 100644
--- a/lms/static/js/views/cohorts.js
+++ b/lms/static/js/views/cohorts.js
@@ -7,7 +7,8 @@
'change .cohort-select': 'onCohortSelected',
'click .action-create': 'showAddCohortForm',
'click .action-cancel': 'cancelAddCohortForm',
- 'click .action-save': 'saveAddCohortForm'
+ 'click .action-save': 'saveAddCohortForm',
+ 'click .link-cross-reference': 'showSection'
},
initialize: function(options) {
@@ -170,6 +171,13 @@
event.preventDefault();
this.removeNotification();
this.onSync();
+ },
+
+ showSection: function(event) {
+ event.preventDefault();
+ var section = $(event.currentTarget).data("section");
+ $(".instructor-nav .nav-item a[data-section='" + section + "']").click();
+ $(window).scrollTop(0);
}
});
}).call(this, $, _, Backbone, gettext, interpolate_text, CohortEditorView, NotificationModel, NotificationView);
diff --git a/lms/static/sass/course/instructor/_instructor_2.scss b/lms/static/sass/course/instructor/_instructor_2.scss
index d530877f3f..7ae6a4fc6e 100644
--- a/lms/static/sass/course/instructor/_instructor_2.scss
+++ b/lms/static/sass/course/instructor/_instructor_2.scss
@@ -894,15 +894,12 @@ section.instructor-dashboard-content-2 {
line-height: 1.3em;
}
- .data-download-container {
+ .reports-download-container {
.data-display-table {
.slickgrid {
height: 400px;
}
}
- }
-
- .grades-download-container {
.report-downloads-table {
.slickgrid {
height: 300px;
diff --git a/lms/templates/dashboard.html b/lms/templates/dashboard.html
index 23dac72409..e061d6117d 100644
--- a/lms/templates/dashboard.html
+++ b/lms/templates/dashboard.html
@@ -77,7 +77,7 @@
location.href = "${reverse('dashboard')}";
} else if (xhr.status == 403) {
location.href = "${reverse('signin_user')}?course_id=" +
- encodeURIComponont($("#unenroll_course_id").val()) + "&enrollment_action=unenroll";
+ encodeURIComponent($("#unenroll_course_id").val()) + "&enrollment_action=unenroll";
} else {
$('#unenroll_error').html(
xhr.responseText ? xhr.responseText : "${_("An error occurred. Please try again later.")}"
diff --git a/lms/templates/discussion/_underscore_templates.html b/lms/templates/discussion/_underscore_templates.html
index 635ff9d7f0..1e36c0fc9b 100644
--- a/lms/templates/discussion/_underscore_templates.html
+++ b/lms/templates/discussion/_underscore_templates.html
@@ -397,7 +397,7 @@
${'<% }); %>'}
- ${_("Instructors can set whether a post in a cohorted topic is visible to all cohorts or only to a specific cohort.")}
+ ${_("Discussion admins, moderators, and TAs can make their posts visible to all students or specify a single cohort group.")}
${'<% } %>'}
diff --git a/lms/templates/instructor/instructor_dashboard_2/cohort-editor.underscore b/lms/templates/instructor/instructor_dashboard_2/cohort-editor.underscore
index 8b22401b70..9c7c37fad1 100644
--- a/lms/templates/instructor/instructor_dashboard_2/cohort-editor.underscore
+++ b/lms/templates/instructor/instructor_dashboard_2/cohort-editor.underscore
@@ -17,10 +17,11 @@