diff --git a/common/djangoapps/util/tests/test_submit_feedback.py b/common/djangoapps/util/tests/test_submit_feedback.py
index 077765e1d7..35e80c83d5 100644
--- a/common/djangoapps/util/tests/test_submit_feedback.py
+++ b/common/djangoapps/util/tests/test_submit_feedback.py
@@ -11,8 +11,19 @@ from util import views
from zendesk import ZendeskError
import json
import mock
+from ddt import ddt, data, unpack
from student.tests.test_configuration_overrides import fake_get_value
+from student.tests.factories import CourseEnrollmentFactory
+
+TEST_SUPPORT_EMAIL = "support@example.com"
+TEST_ZENDESK_CUSTOM_FIELD_CONFIG = {"course_id": 1234, "enrollment_mode": 5678}
+TEST_REQUEST_HEADERS = {
+ "HTTP_REFERER": "test_referer",
+ "HTTP_USER_AGENT": "test_user_agent",
+ "REMOTE_ADDR": "1.2.3.4",
+ "SERVER_NAME": "test_server",
+}
def fake_support_backend_values(name, default=None): # pylint: disable=unused-argument
@@ -21,13 +32,20 @@ def fake_support_backend_values(name, default=None): # pylint: disable=unused-a
"""
config_dict = {
"CONTACT_FORM_SUBMISSION_BACKEND": "email",
- "email_from_address": "support_from@example.com",
+ "email_from_address": TEST_SUPPORT_EMAIL,
}
return config_dict[name]
+@ddt
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_FEEDBACK_SUBMISSION": True})
-@override_settings(ZENDESK_URL="dummy", ZENDESK_USER="dummy", ZENDESK_API_KEY="dummy")
+@override_settings(
+ DEFAULT_FROM_EMAIL=TEST_SUPPORT_EMAIL,
+ ZENDESK_URL="dummy",
+ ZENDESK_USER="dummy",
+ ZENDESK_API_KEY="dummy",
+ ZENDESK_CUSTOM_FIELDS={}
+)
@mock.patch("util.views.dog_stats_api")
@mock.patch("util.views._ZendeskApi", autospec=True)
class SubmitFeedbackTest(TestCase):
@@ -44,14 +62,12 @@ class SubmitFeedbackTest(TestCase):
username="test",
profile__name="Test User"
)
- # This contains issue_type and course_id to ensure that tags are submitted correctly
self._anon_fields = {
"email": "test@edx.org",
"name": "Test User",
"subject": "a subject",
"details": "some details",
- "issue_type": "test_issue",
- "course_id": "test_course"
+ "issue_type": "test_issue"
}
# This does not contain issue_type nor course_id to ensure that they are optional
self._auth_fields = {"subject": "a subject", "details": "some details"}
@@ -66,10 +82,10 @@ class SubmitFeedbackTest(TestCase):
req = self._request_factory.post(
"/submit_feedback",
data=fields,
- HTTP_REFERER="test_referer",
- HTTP_USER_AGENT="test_user_agent",
- REMOTE_ADDR="1.2.3.4",
- SERVER_NAME="test_server",
+ HTTP_REFERER=TEST_REQUEST_HEADERS["HTTP_REFERER"],
+ HTTP_USER_AGENT=TEST_REQUEST_HEADERS["HTTP_USER_AGENT"],
+ REMOTE_ADDR=TEST_REQUEST_HEADERS["REMOTE_ADDR"],
+ SERVER_NAME=TEST_REQUEST_HEADERS["SERVER_NAME"],
)
req.user = user
return views.submit_feedback(req)
@@ -130,13 +146,58 @@ class SubmitFeedbackTest(TestCase):
resp = self._build_and_run_request(user, fields)
self.assertEqual(resp.status_code, 200)
- def _assert_datadog_called(self, datadog_mock, with_tags):
- expected_datadog_calls = [
- mock.call.increment(
- views.DATADOG_FEEDBACK_METRIC,
- tags=(["course_id:test_course", "issue_type:test_issue"] if with_tags else [])
- )
+ def _build_zendesk_ticket(self, recipient, name, email, subject, details, tags, custom_fields=None):
+ """
+ Build a Zendesk ticket that can be used in assertions to verify that the correct
+ data was submitted to create a Zendesk ticket.
+ """
+ ticket = {
+ "ticket": {
+ "recipient": recipient,
+ "requester": {"name": name, "email": email},
+ "subject": subject,
+ "comment": {"body": details},
+ "tags": tags
+ }
+ }
+
+ if custom_fields is not None:
+ ticket["ticket"]["custom_fields"] = custom_fields
+
+ return ticket
+
+ def _build_zendesk_ticket_update(self, request_headers, username=None):
+ """
+ Build a Zendesk ticket update that can be used in assertions to verify that the correct
+ data was submitted to update a Zendesk ticket.
+ """
+ body = []
+ if username:
+ body.append("username: {}".format(username))
+
+ # FIXME the tests rely on the body string being built in this specific order, which doesn't seem
+ # reliable given that the view builds the string by iterating over a dictionary.
+ header_text_mapping = [
+ ("Client IP", "REMOTE_ADDR"),
+ ("Host", "SERVER_NAME"),
+ ("Page", "HTTP_REFERER"),
+ ("Browser", "HTTP_USER_AGENT")
]
+
+ for text, header in header_text_mapping:
+ body.append("{}: {}".format(text, request_headers[header]))
+
+ body = "Additional information:\n\n" + "\n".join(body)
+ return {"ticket": {"comment": {"public": False, "body": body}}}
+
+ def _assert_zendesk_called(self, zendesk_mock, ticket_id, ticket, ticket_update):
+ """Assert that Zendesk was called with the correct ticket and ticket_update."""
+ expected_zendesk_calls = [mock.call.create_ticket(ticket), mock.call.update_ticket(ticket_id, ticket_update)]
+ self.assertEqual(zendesk_mock.mock_calls, expected_zendesk_calls)
+
+ def _assert_datadog_called(self, datadog_mock, tags):
+ """Assert that datadog was called with the correct tags."""
+ expected_datadog_calls = [mock.call.increment(views.DATADOG_FEEDBACK_METRIC, tags=tags)]
self.assertEqual(datadog_mock.mock_calls, expected_datadog_calls)
def test_bad_request_anon_user_no_name(self, zendesk_mock_class, datadog_mock):
@@ -174,39 +235,26 @@ class SubmitFeedbackTest(TestCase):
the given information should have been submitted via the Zendesk API.
"""
zendesk_mock_instance = zendesk_mock_class.return_value
- zendesk_mock_instance.create_ticket.return_value = 42
- self._test_success(self._anon_user, self._anon_fields)
- expected_zendesk_calls = [
- mock.call.create_ticket(
- {
- "ticket": {
- "recipient": "registration@example.com",
- "requester": {"name": "Test User", "email": "test@edx.org"},
- "subject": "a subject",
- "comment": {"body": "some details"},
- "tags": ["test_course", "test_issue", "LMS"]
- }
- }
- ),
- mock.call.update_ticket(
- 42,
- {
- "ticket": {
- "comment": {
- "public": False,
- "body":
- "Additional information:\n\n"
- "Client IP: 1.2.3.4\n"
- "Host: test_server\n"
- "Page: test_referer\n"
- "Browser: test_user_agent"
- }
- }
- }
- )
- ]
- self.assertEqual(zendesk_mock_instance.mock_calls, expected_zendesk_calls)
- self._assert_datadog_called(datadog_mock, with_tags=True)
+ user = self._anon_user
+ fields = self._anon_fields
+
+ ticket_id = 42
+ zendesk_mock_instance.create_ticket.return_value = ticket_id
+
+ ticket = self._build_zendesk_ticket(
+ recipient=TEST_SUPPORT_EMAIL,
+ name=fields["name"],
+ email=fields["email"],
+ subject=fields["subject"],
+ details=fields["details"],
+ tags=[fields["issue_type"], "LMS"]
+ )
+
+ ticket_update = self._build_zendesk_ticket_update(TEST_REQUEST_HEADERS)
+
+ self._test_success(user, fields)
+ self._assert_zendesk_called(zendesk_mock_instance, ticket_id, ticket, ticket_update)
+ self._assert_datadog_called(datadog_mock, ["issue_type:{}".format(fields["issue_type"])])
@mock.patch("openedx.core.djangoapps.site_configuration.helpers.get_value", fake_get_value)
def test_valid_request_anon_user_configuration_override(self, zendesk_mock_class, datadog_mock):
@@ -218,39 +266,75 @@ class SubmitFeedbackTest(TestCase):
tag that will come from site configuration override.
"""
zendesk_mock_instance = zendesk_mock_class.return_value
- zendesk_mock_instance.create_ticket.return_value = 42
- self._test_success(self._anon_user, self._anon_fields)
- expected_zendesk_calls = [
- mock.call.create_ticket(
- {
- "ticket": {
- "recipient": "no-reply@fakeuniversity.com",
- "requester": {"name": "Test User", "email": "test@edx.org"},
- "subject": "a subject",
- "comment": {"body": "some details"},
- "tags": ["test_course", "test_issue", "LMS", "whitelabel_fakeorg"]
- }
- }
- ),
- mock.call.update_ticket(
- 42,
- {
- "ticket": {
- "comment": {
- "public": False,
- "body":
- "Additional information:\n\n"
- "Client IP: 1.2.3.4\n"
- "Host: test_server\n"
- "Page: test_referer\n"
- "Browser: test_user_agent"
- }
- }
- }
- )
- ]
- self.assertEqual(zendesk_mock_instance.mock_calls, expected_zendesk_calls)
- self._assert_datadog_called(datadog_mock, with_tags=True)
+ user = self._anon_user
+ fields = self._anon_fields
+
+ ticket_id = 42
+ zendesk_mock_instance.create_ticket.return_value = ticket_id
+
+ ticket = self._build_zendesk_ticket(
+ recipient=fake_get_value("email_from_address"),
+ name=fields["name"],
+ email=fields["email"],
+ subject=fields["subject"],
+ details=fields["details"],
+ tags=[fields["issue_type"], "LMS", "whitelabel_{}".format(fake_get_value("course_org_filter"))]
+ )
+
+ ticket_update = self._build_zendesk_ticket_update(TEST_REQUEST_HEADERS)
+
+ self._test_success(user, fields)
+ self._assert_zendesk_called(zendesk_mock_instance, ticket_id, ticket, ticket_update)
+ self._assert_datadog_called(datadog_mock, ["issue_type:{}".format(fields["issue_type"])])
+
+ @data("course-v1:testOrg+testCourseNumber+testCourseRun", "", None)
+ @override_settings(ZENDESK_CUSTOM_FIELDS=TEST_ZENDESK_CUSTOM_FIELD_CONFIG)
+ def test_valid_request_anon_user_with_custom_fields(self, course_id, zendesk_mock_class, datadog_mock):
+ """
+ Test a valid request from an anonymous user when configured to use Zendesk Custom Fields.
+
+ The response should have a 200 (success) status code, and a ticket with
+ the given information should have been submitted via the Zendesk API. When course_id is
+ present, it should be sent to Zendesk via a custom field. When course_id is blank or missing,
+ the request should still be processed successfully.
+ """
+ zendesk_mock_instance = zendesk_mock_class.return_value
+ user = self._anon_user
+
+ fields = self._anon_fields.copy()
+ if course_id is not None:
+ fields["course_id"] = course_id
+
+ ticket_id = 42
+ zendesk_mock_instance.create_ticket.return_value = ticket_id
+
+ zendesk_tags = [fields["issue_type"], "LMS"]
+ datadog_tags = ["issue_type:{}".format(fields["issue_type"])]
+ zendesk_custom_fields = None
+ if course_id:
+ # FIXME the tests rely on the tags being in this specific order, which doesn't seem
+ # reliable given that the view builds the list by iterating over a dictionary.
+ zendesk_tags.insert(0, course_id)
+ datadog_tags.insert(0, "course_id:{}".format(course_id))
+ zendesk_custom_fields = [
+ {"id": TEST_ZENDESK_CUSTOM_FIELD_CONFIG["course_id"], "value": course_id}
+ ]
+
+ ticket = self._build_zendesk_ticket(
+ recipient=TEST_SUPPORT_EMAIL,
+ name=fields["name"],
+ email=fields["email"],
+ subject=fields["subject"],
+ details=fields["details"],
+ tags=zendesk_tags,
+ custom_fields=zendesk_custom_fields
+ )
+
+ ticket_update = self._build_zendesk_ticket_update(TEST_REQUEST_HEADERS)
+
+ self._test_success(user, fields)
+ self._assert_zendesk_called(zendesk_mock_instance, ticket_id, ticket, ticket_update)
+ self._assert_datadog_called(datadog_mock, datadog_tags)
def test_bad_request_auth_user_no_subject(self, zendesk_mock_class, datadog_mock):
"""Test a request from an authenticated user not specifying `subject`."""
@@ -270,40 +354,92 @@ class SubmitFeedbackTest(TestCase):
the given information should have been submitted via the Zendesk API.
"""
zendesk_mock_instance = zendesk_mock_class.return_value
- zendesk_mock_instance.create_ticket.return_value = 42
- self._test_success(self._auth_user, self._auth_fields)
- expected_zendesk_calls = [
- mock.call.create_ticket(
- {
- "ticket": {
- "recipient": "registration@example.com",
- "requester": {"name": "Test User", "email": "test@edx.org"},
- "subject": "a subject",
- "comment": {"body": "some details"},
- "tags": ["LMS"]
- }
- }
- ),
- mock.call.update_ticket(
- 42,
- {
- "ticket": {
- "comment": {
- "public": False,
- "body":
- "Additional information:\n\n"
- "username: test\n"
- "Client IP: 1.2.3.4\n"
- "Host: test_server\n"
- "Page: test_referer\n"
- "Browser: test_user_agent"
- }
- }
- }
- )
- ]
- self.assertEqual(zendesk_mock_instance.mock_calls, expected_zendesk_calls)
- self._assert_datadog_called(datadog_mock, with_tags=False)
+ user = self._auth_user
+ fields = self._auth_fields
+
+ ticket_id = 42
+ zendesk_mock_instance.create_ticket.return_value = ticket_id
+
+ ticket = self._build_zendesk_ticket(
+ recipient=TEST_SUPPORT_EMAIL,
+ name=user.profile.name,
+ email=user.email,
+ subject=fields["subject"],
+ details=fields["details"],
+ tags=["LMS"]
+ )
+
+ ticket_update = self._build_zendesk_ticket_update(TEST_REQUEST_HEADERS, user.username)
+
+ self._test_success(user, fields)
+ self._assert_zendesk_called(zendesk_mock_instance, ticket_id, ticket, ticket_update)
+ self._assert_datadog_called(datadog_mock, [])
+
+ @data(
+ ("course-v1:testOrg+testCourseNumber+testCourseRun", True),
+ ("course-v1:testOrg+testCourseNumber+testCourseRun", False),
+ ("", None),
+ (None, None)
+ )
+ @unpack
+ @override_settings(ZENDESK_CUSTOM_FIELDS=TEST_ZENDESK_CUSTOM_FIELD_CONFIG)
+ def test_valid_request_auth_user_with_custom_fields(self, course_id, enrolled, zendesk_mock_class, datadog_mock):
+ """
+ Test a valid request from an authenticated user when configured to use Zendesk Custom Fields.
+
+ The response should have a 200 (success) status code, and a ticket with
+ the given information should have been submitted via the Zendesk API. When course_id is
+ present, it should be sent to Zendesk via a custom field, along with the enrollment mode
+ if the user has an active enrollment for that course. When course_id is blank or missing,
+ the request should still be processed successfully.
+ """
+ zendesk_mock_instance = zendesk_mock_class.return_value
+ user = self._auth_user
+
+ fields = self._auth_fields.copy()
+ if course_id is not None:
+ fields["course_id"] = course_id
+
+ ticket_id = 42
+ zendesk_mock_instance.create_ticket.return_value = ticket_id
+
+ zendesk_tags = ["LMS"]
+ datadog_tags = []
+ zendesk_custom_fields = None
+ if course_id:
+ # FIXME the tests rely on the tags being in this specific order, which doesn't seem
+ # reliable given that the view builds the list by iterating over a dictionary.
+ zendesk_tags.insert(0, course_id)
+ datadog_tags.insert(0, "course_id:{}".format(course_id))
+ zendesk_custom_fields = [
+ {"id": TEST_ZENDESK_CUSTOM_FIELD_CONFIG["course_id"], "value": course_id}
+ ]
+ if enrolled is not None:
+ enrollment = CourseEnrollmentFactory.create(
+ user=user,
+ course_id=course_id,
+ is_active=enrolled
+ )
+ if enrollment.is_active:
+ zendesk_custom_fields.append(
+ {"id": TEST_ZENDESK_CUSTOM_FIELD_CONFIG["enrollment_mode"], "value": enrollment.mode}
+ )
+
+ ticket = self._build_zendesk_ticket(
+ recipient=TEST_SUPPORT_EMAIL,
+ name=user.profile.name,
+ email=user.email,
+ subject=fields["subject"],
+ details=fields["details"],
+ tags=zendesk_tags,
+ custom_fields=zendesk_custom_fields
+ )
+
+ ticket_update = self._build_zendesk_ticket_update(TEST_REQUEST_HEADERS, user.username)
+
+ self._test_success(user, fields)
+ self._assert_zendesk_called(zendesk_mock_instance, ticket_id, ticket, ticket_update)
+ self._assert_datadog_called(datadog_mock, datadog_tags)
def test_get_request(self, zendesk_mock_class, datadog_mock):
"""Test that a GET results in a 405 even with all required fields"""
@@ -329,7 +465,7 @@ class SubmitFeedbackTest(TestCase):
resp = self._build_and_run_request(self._anon_user, self._anon_fields)
self.assertEqual(resp.status_code, 500)
self.assertFalse(resp.content)
- self._assert_datadog_called(datadog_mock, with_tags=True)
+ self._assert_datadog_called(datadog_mock, ["issue_type:{}".format(self._anon_fields["issue_type"])])
def test_zendesk_error_on_update(self, zendesk_mock_class, datadog_mock):
"""
@@ -344,7 +480,7 @@ class SubmitFeedbackTest(TestCase):
zendesk_mock_instance.update_ticket.side_effect = err
resp = self._build_and_run_request(self._anon_user, self._anon_fields)
self.assertEqual(resp.status_code, 200)
- self._assert_datadog_called(datadog_mock, with_tags=True)
+ self._assert_datadog_called(datadog_mock, ["issue_type:{}".format(self._anon_fields["issue_type"])])
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_FEEDBACK_SUBMISSION": False})
def test_not_enabled(self, zendesk_mock_class, datadog_mock):
diff --git a/common/djangoapps/util/views.py b/common/djangoapps/util/views.py
index ea886e7777..2c7045b540 100644
--- a/common/djangoapps/util/views.py
+++ b/common/djangoapps/util/views.py
@@ -25,6 +25,7 @@ from edxmako.shortcuts import render_to_response, render_to_string
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
import track.views
from student.roles import GlobalStaff
+from student.models import CourseEnrollment
log = logging.getLogger(__name__)
@@ -222,6 +223,40 @@ class _ZendeskApi(object):
return None
+def _get_zendesk_custom_field_context(request):
+ """
+ Construct a dictionary of data that can be stored in Zendesk custom fields.
+ """
+ context = {}
+
+ course_id = request.POST.get("course_id")
+ if not course_id:
+ return context
+
+ context["course_id"] = course_id
+ if not request.user.is_authenticated():
+ return context
+
+ enrollment = CourseEnrollment.get_enrollment(request.user, CourseKey.from_string(course_id))
+ if enrollment and enrollment.is_active:
+ context["enrollment_mode"] = enrollment.mode
+
+ return context
+
+
+def _format_zendesk_custom_fields(context):
+ """
+ Format the data in `context` for compatibility with the Zendesk API.
+ Ignore any keys that have not been configured in `ZENDESK_CUSTOM_FIELDS`.
+ """
+ custom_fields = []
+ for key, val, in settings.ZENDESK_CUSTOM_FIELDS.items():
+ if key in context:
+ custom_fields.append({"id": val, "value": context[key]})
+
+ return custom_fields
+
+
def _record_feedback_in_zendesk(
realname,
email,
@@ -231,7 +266,8 @@ def _record_feedback_in_zendesk(
additional_info,
group_name=None,
require_update=False,
- support_email=None
+ support_email=None,
+ custom_fields=None
):
"""
Create a new user-requested Zendesk ticket.
@@ -246,6 +282,8 @@ def _record_feedback_in_zendesk(
If `require_update` is provided, returns False when the update does not
succeed. This allows using the private comment to add necessary information
which the user will not see in followup emails from support.
+
+ If `custom_fields` is provided, submits data to those fields in Zendesk.
"""
zendesk_api = _ZendeskApi()
@@ -271,6 +309,10 @@ def _record_feedback_in_zendesk(
"tags": zendesk_tags
}
}
+
+ if custom_fields:
+ new_ticket["ticket"]["custom_fields"] = custom_fields
+
group = None
if group_name is not None:
group = zendesk_api.get_group(group_name)
@@ -322,7 +364,7 @@ def get_feedback_form_context(request):
context["subject"] = request.POST["subject"]
context["details"] = request.POST["details"]
context["tags"] = dict(
- [(tag, request.POST[tag]) for tag in ["issue_type", "course_id"] if tag in request.POST]
+ [(tag, request.POST[tag]) for tag in ["issue_type", "course_id"] if request.POST.get(tag)]
)
context["additional_info"] = {}
@@ -412,6 +454,11 @@ def submit_feedback(request):
if not settings.ZENDESK_URL or not settings.ZENDESK_USER or not settings.ZENDESK_API_KEY:
raise Exception("Zendesk enabled but not configured")
+ custom_fields = None
+ if settings.ZENDESK_CUSTOM_FIELDS:
+ custom_field_context = _get_zendesk_custom_field_context(request)
+ custom_fields = _format_zendesk_custom_fields(custom_field_context)
+
success = _record_feedback_in_zendesk(
context["realname"],
context["email"],
@@ -419,7 +466,8 @@ def submit_feedback(request):
context["details"],
context["tags"],
context["additional_info"],
- support_email=context["support_email"]
+ support_email=context["support_email"],
+ custom_fields=custom_fields
)
_record_feedback_in_datadog(context["tags"])
diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py
index b18e26fd0e..467629d7a1 100644
--- a/common/lib/capa/capa/inputtypes.py
+++ b/common/lib/capa/capa/inputtypes.py
@@ -328,9 +328,16 @@ class InputTypeBase(object):
}
# Generate the list of ids to be used with the aria-describedby field.
+ descriptions = list()
+
+ # If there is trailing text, add the id as the first element to the list before adding the status id
+ if 'trailing_text' in self.loaded_attributes and self.loaded_attributes['trailing_text']:
+ trailing_text_id = 'trailing_text_' + self.input_id
+ descriptions.append(trailing_text_id)
+
# Every list should contain the status id
status_id = 'status_' + self.input_id
- descriptions = list([status_id])
+ descriptions.append(status_id)
descriptions.extend(self.response_data.get('descriptions', {}).keys())
description_ids = ' '.join(descriptions)
context.update(
diff --git a/common/lib/capa/capa/templates/formulaequationinput.html b/common/lib/capa/capa/templates/formulaequationinput.html
index fe66dfa89a..798623f06e 100644
--- a/common/lib/capa/capa/templates/formulaequationinput.html
+++ b/common/lib/capa/capa/templates/formulaequationinput.html
@@ -16,7 +16,7 @@
size="${size}"
% endif
/>
-
${trailing_text}
+
${trailing_text}
<%include file="status_span.html" args="status=status, status_id=id"/>
diff --git a/common/lib/capa/capa/templates/textline.html b/common/lib/capa/capa/templates/textline.html
index 73452bacd8..632fb0f7da 100644
--- a/common/lib/capa/capa/templates/textline.html
+++ b/common/lib/capa/capa/templates/textline.html
@@ -34,7 +34,7 @@
style="display:none;"
% endif
/>
-
${trailing_text}
+
${trailing_text}
<%include file="status_span.html" args="status=status, status_id=id"/>
diff --git a/common/lib/capa/capa/tests/test_inputtypes.py b/common/lib/capa/capa/tests/test_inputtypes.py
index 851de494cb..2e8ae0f32c 100644
--- a/common/lib/capa/capa/tests/test_inputtypes.py
+++ b/common/lib/capa/capa/tests/test_inputtypes.py
@@ -37,6 +37,8 @@ lookup_tag = inputtypes.registry.get_class_for_tag
DESCRIBEDBY = HTML('aria-describedby="status_{status_id} desc-1 desc-2"')
+# Use TRAILING_TEXT_DESCRIBEDBY when trailing_text is not null
+TRAILING_TEXT_DESCRIBEDBY = HTML('aria-describedby="trailing_text_{trailing_text_id} status_{status_id} desc-1 desc-2"')
DESCRIPTIONS = OrderedDict([('desc-1', 'description text 1'), ('desc-2', 'description text 2')])
RESPONSE_DATA = {
'label': 'question text 101',
@@ -361,7 +363,7 @@ class TextLineTest(unittest.TestCase):
'trailing_text': expected_text,
'preprocessor': None,
'response_data': RESPONSE_DATA,
- 'describedby_html': DESCRIBEDBY.format(status_id=prob_id)
+ 'describedby_html': TRAILING_TEXT_DESCRIBEDBY.format(trailing_text_id=prob_id, status_id=prob_id)
}
self.assertEqual(context, expected)
@@ -1295,7 +1297,7 @@ class FormulaEquationTest(unittest.TestCase):
'inline': False,
'trailing_text': expected_text,
'response_data': RESPONSE_DATA,
- 'describedby_html': DESCRIBEDBY.format(status_id=prob_id)
+ 'describedby_html': TRAILING_TEXT_DESCRIBEDBY.format(trailing_text_id=prob_id, status_id=prob_id)
}
self.assertEqual(context, expected)
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py
index c7f6e91e68..da8af6cecf 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py
@@ -13,7 +13,7 @@ from contextlib import contextmanager
from uuid import uuid4
from factory import Factory, Sequence, lazy_attribute_sequence, lazy_attribute
-from factory.containers import CyclicDefinitionError
+from factory.errors import CyclicDefinitionError
from mock import patch
from nose.tools import assert_less_equal, assert_greater_equal
import dogstats_wrapper as dog_stats_api
diff --git a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py
index 22d6786e3b..f464a9d1db 100644
--- a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py
+++ b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py
@@ -296,11 +296,9 @@ def copy_or_rename_transcript(new_name, old_name, item, delete_old=False, user=N
def get_html5_ids(html5_sources):
"""
Helper method to parse out an HTML5 source into the ideas
- NOTE: This assumes that '/' are not in the filename.
- Slices each id by 150, restricting too long strings as video names.
+ NOTE: This assumes that '/' are not in the filename
"""
- html5_ids = [x.split('/')[-1].rsplit('.', 1)[0][:150] for x in html5_sources]
-
+ html5_ids = [x.split('/')[-1].rsplit('.', 1)[0] for x in html5_sources]
return html5_ids
diff --git a/common/static/common/js/discussion/views/discussion_thread_list_view.js b/common/static/common/js/discussion/views/discussion_thread_list_view.js
index ef9cf793a2..ac151c9493 100644
--- a/common/static/common/js/discussion/views/discussion_thread_list_view.js
+++ b/common/static/common/js/discussion/views/discussion_thread_list_view.js
@@ -93,6 +93,7 @@
this.courseSettings = options.courseSettings;
this.hideRefineBar = options.hideRefineBar;
this.supportsActiveThread = options.supportsActiveThread;
+ this.hideReadState = options.hideReadState || false;
this.displayedCollection = new Discussion(this.collection.models, {
pages: this.collection.pages
});
@@ -342,7 +343,8 @@
neverRead: neverRead,
threadUrl: thread.urlFor('retrieve'),
threadPreview: threadPreview,
- showThreadPreview: this.showThreadPreview
+ showThreadPreview: this.showThreadPreview,
+ hideReadState: this.hideReadState
},
thread.toJSON()
);
diff --git a/common/static/common/js/spec/discussion/view/discussion_thread_list_view_spec.js b/common/static/common/js/spec/discussion/view/discussion_thread_list_view_spec.js
index 50f5048ab8..713ffa801a 100644
--- a/common/static/common/js/spec/discussion/view/discussion_thread_list_view_spec.js
+++ b/common/static/common/js/spec/discussion/view/discussion_thread_list_view_spec.js
@@ -169,6 +169,7 @@
});
return this.view.render();
});
+
setupAjax = function(callback) {
return $.ajax.and.callFake(function(params) {
if (callback) {
@@ -185,19 +186,27 @@
};
});
};
+
renderSingleThreadWithProps = function(props) {
return makeView(new Discussion([new Thread(DiscussionViewSpecHelper.makeThreadWithProps(props))])).render();
};
- makeView = function(discussion) {
- return new DiscussionThreadListView({
- el: $('#fixture-element'),
- collection: discussion,
- showThreadPreview: true,
- courseSettings: new DiscussionCourseSettings({
- is_cohorted: true
- })
- });
+
+ makeView = function(discussion, props) {
+ return new DiscussionThreadListView(
+ _.extend(
+ {
+ el: $('#fixture-element'),
+ collection: discussion,
+ showThreadPreview: true,
+ courseSettings: new DiscussionCourseSettings({
+ is_cohorted: true
+ })
+ },
+ props
+ )
+ );
};
+
expectFilter = function(filterVal) {
return $.ajax.and.callFake(function(params) {
_.each(['unread', 'unanswered', 'flagged'], function(paramName) {
@@ -681,5 +690,45 @@
expect(view.$el.find('.thread-preview-body').length).toEqual(0);
});
});
+
+ describe('read/unread state', function() {
+ it('adds never-read class to unread threads', function() {
+ var unreads = this.threads.filter(function(thread) {
+ return !thread.read && thread.unread_comments_count === thread.comments_count;
+ }).length;
+
+ this.view = makeView(new Discussion(this.threads));
+ this.view.render();
+ expect(this.view.$('.never-read').length).toEqual(unreads);
+ });
+
+ it('shows a "x new" message for threads that are read, but have unread comments', function() {
+ var unreadThread = this.threads.filter(function(thread) {
+ return thread.read && thread.unread_comments_count !== thread.comments_count;
+ })[0],
+ newCommentsOnUnreadThread = unreadThread.unread_comments_count;
+
+ this.view = makeView(new Discussion(this.threads));
+ this.view.render();
+ expect(
+ this.view.$('.forum-nav-thread-unread-comments-count')
+ .first()
+ .text()
+ .trim()
+ ).toEqual(newCommentsOnUnreadThread + ' new');
+ });
+
+ it('should display every thread as read if hideReadState: true is passed to the constructor', function() {
+ this.view = makeView(new Discussion(this.threads), {hideReadState: true});
+ this.view.render();
+ expect(this.view.$('.never-read').length).toEqual(0);
+ });
+
+ it('does not show the "x new" indicator for any thread if hideReadState: true is passed', function() {
+ this.view = makeView(new Discussion(this.threads), {hideReadState: true});
+ this.view.render();
+ expect(this.view.$('.forum-nav-thread-unread-comments-count').length).toEqual(0);
+ });
+ });
});
}).call(this);
diff --git a/common/static/common/templates/discussion/thread-list-item.underscore b/common/static/common/templates/discussion/thread-list-item.underscore
index d57aec8630..6aa6e4070b 100644
--- a/common/static/common/templates/discussion/thread-list-item.underscore
+++ b/common/static/common/templates/discussion/thread-list-item.underscore
@@ -1,4 +1,4 @@
-
+
<%
@@ -75,7 +75,7 @@
%>
- <% if (!neverRead && unread_comments_count > 0) { %>
+ <% if (!hideReadState && !neverRead && unread_comments_count > 0) { %>